Editing an unpublished revision more than once causes any files removed in first draft edit to disappear from published revision

OK, its disappointing none of the module maintainers seem interested in this because it is definitely an issue.

I've identified the precise problem. Core drupal does not handle the concept of editing the same revision multiple times, this is something introduced by Revisioning. The consequences of this, as related to my particular problem can be seen in part of the file_field_update function in file.field.inc reproduced below:

/**
 * Implements hook_field_update().
 *
 * Checks for files that have been removed from the object.
 */
function file_field_update($entity_type, $entity, $field, $instance, $langcode, &$items) {
  list($id, $vid, $bundle) = entity_extract_ids($entity_type, $entity);
  // On new revisions, all files are considered to be a new usage and no
  // deletion of previous file usages are necessary.

  if (!empty($entity->revision)) {
    foreach ($items as $item) {
      $file = (object) $item;
      file_usage_add($file, 'file', $entity_type, $id);
    }
    return;
  }

The issue I encountered occurs because $entity->revision is set to indicate if there is a new revision or not (1 if new, empty if not new), clearly, on multiple unpublished revision edits there is *not* a new revision so this is empty. In core drupal this is fine, when using Revisioning it obviously is not.

I'm not really to sure how to proceed from here, obviously I could hack the file_field_update function in core and change the if statement to check, for example $entity->is_pending as well, but that isn't really solving the problem.

Does anyone have any ideas? I've wondered if it is possible to override file_field_update and rewrite it within the revisioning module but I haven't been able to find any suggestion of this being achievable thus far.

Hmmm, nice work Sean, I had been thinking about a way to handle this bug in revisioning-7.x and had been planning to do some work using hook_file_delete to effectively re-attach a file to a revision if it is in draft, but your method seems better. I'll have a go implementing something similar for 7.x and post it up to this issue when I've got something working.

Thanks,
Sam

OK, I've worked out a solution that I think is actually pretty nice and very simple for this bug. I've used a hook_node_update function to call file_add_usage and add an additional record to the file_usage table that uses the VID as the ID for the record when a file is attached to a node.

This prevents files being deleted by drupal during multiple updates on draft revisions because when file_delete is called there is still a file_usage record for that file in the table. At the moment I have this sitting in a custom module, however I wonder if Rik would consider a patch for Revisioning? I'm not sure how one would implement a check to see if there were files attached to the node, but if it could be done then it would be a simple matter to adapt the code below to work within Revisioning.

example_node_update($node) {
  if ((isset($node->field_example_file_field['und'][0]) && !empty($node->field_example_file_field['und'][0]))
      && $node->revision_moderation == TRUE){
    $file = (object) $node->field_example_file_field['und'][0];
    file_usage_add($file, 'example_module', 'revision', $node->vid);
  }
}

That's it! Works really nicely from the testing I have done. I'd be happy to have a go creating a patch against Revisioning-7.x-dev if it is something the maintainer would be interested in.

Thanks,
Sam

Hi Rik, good points! I'll have a think tonight and try and get a patch rolled that addresses these problems.

Thanks,
Sam

Finally attaching patch for this, sorry for the delay.

I've rewritten it to use field API as suggested, and handle multiple file upload fields.

I've tested this on a vanilla install for drupal 7.22 running php 5.3 and postgres 9.2 as well as a live site which has been using an older version of revisioning and has many revisions created using an older version of the module and it seems to be working fine.

I've also added backwards compatibility so that sites with nodes and revisions created under previous releases of revisioning will no longer experience this issue when creating new revisions (ie, will add file usage records to old files).

Thanks,
Sam

Oops, this one is better, changed so that the module name added to the file_usage record is revisioning.

Thanks Rik, you're entirely right, that was a silly thing for me to miss, my testing must not have caught that particular use case, sorry about that!

Tested latest patch with a content type with multiple file fields, now working correctly. Thanks for spotting that.

Sam

Thanks Rik, a check there makes a lot of sense, all seems to working well from my tests. Glad someone else is finding this useful :)