While the core data on a file is revisioned, and with #2099949: Cannot reivision the actual file. the files are revisioned, it turns out that field api data is also revisioned incorrectly.

This is because file_entity_revisions creates the revision as part of hook_file_insert and hook_file_update. However, file_entity.module calls field_attach_update/insert also as part of hook_file_update/insert.

Unfortunately, file_entity preceeds file_entity_revisions alphabetically, so it gets called first. This means that when field api updates its data, a new revision hasn't been created. It writes data to whatever revision was current at the time, THEN the new revision is created and that revision has no field api data attached to it. This isn't entirely obvious because that data is set current, but trying to look at the previous revision will show you incorrect data.

The solution is to reduce file_entity_revisions' weight in the system table so that its hook_file_* hooks always come before file_entity.

Patch forthcoming.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

merlinofchaos’s picture

Status: Active » Needs review
FileSize
1.09 KB

Here is the patch. This assumes that #2099949: Cannot reivision the actual file. is already applied; otherwise there will be conflicts in the update hooks. Since Eric told me earlier that he will commit that patch when he gets a chance today, this should be a safe assumption.

Fabianx’s picture

Status: Needs review » Reviewed & tested by the community

Uh, nice catch. I usually like module_implements_alter more to set explicit weighting per hook, but as this is pretty much for all hooks, the weight solution should work great!

I tested saving a revision and going back and it works correctly.

Therefore, RTBC

merlinofchaos’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.03 KB

Ok, I found a bug in the approach in that it caused file_entity_revisions to form_alter before file_admin.

I decided to switch it to Fabianx's approach where it just module_implement_alters for the hooks we know it needs to run first.

Fabianx’s picture

Status: Needs review » Reviewed & tested by the community

Still works nicely for me here and if it works for file_admin, too even better. So again: => RTBC