Revisions not supported

Revisions support is now available with the patch in this issue - which also contains AJAX uploading (both features coupled in one patch out of "lazyness"), which I considered a good enough reason to open a new issue than cluttering this one.

So I split the revisions support out of the kitchensink patch, for better testability. This patch thus is the rightful successor of the patch in #123532 (extract "api" into filefield.inc) minus the "split" part, which makes for:

• Making filefield_file_delete() and filefield_file_load() public so that other modules can depend on filefield's functions for database file entries.
• Support for node revisions, including hook_form_alter() magic to not display filefields as upload module attachments when editing the node form.

As opposed to the kitchensink patch, this one has most style fixes removed in order to make for a minimal patch. Hope you enjoy it now :D

Unfortunately, a few things from the database consistency patch are needed as well (short sections in filefield_field() and filefield_update()) so I guess the two patches will conflict. Notify when either of them is applied, and I'll reroll the other one.

dopry asked in #138167:
> whats going on with the files_count query at the beginning of filefield_load?

Must have been a leftover from debugging this thing, removed in this version of the patch.

New version, with small changes in in filefield_field(): eliminate the $output variable in favor of direct return statements, and use array_merge() instead of the "+=" operator for adding the $file to $node_field['delta']. Both are done this way in imagefield, and seem the better thing to do.

As the {file_revisions} table also includes a 'list' column, I thought it might be appropriate to actually use this and make filefield's "List" checkbox actually work. So this new version of the revisions patch includes 'list' support as well.

One last functionality addition for now: This new version of the patch adds a $session_name parameter to filefield_file_delete(), so that imagefield can finally make use of the file management functions (when this patch is also applied).

And yeah, it's still not the best thing to have both revisions support and making important filefield functions public in one single patch, but then those two features affect the same pieces of code and two different patches would probably conflict a lot.

As this patch shares one section with the database consistency patch, and that one got better code documentation, add it in this patch as well. No changes in functionality.

New version of the patch:
Use the 'list' property to determine whether the file should be displayed on the page or not. Also, use the 'descriptions' column of the {file_revisions} table instead of the one specified in the field itself. As we don't need to store 'list' and 'description' in the field itself anymore, this version of the patch drops those columns from the CCK table specification.

Also, pass $item instead of a newly loaded $file without a specified revision, because $item already has all the information that we need. It's now even possible to use the 'description' property for displaying the link text, instead of the filename.

I thought it might be better not to clutter the diff with the "description as link" bugfix/feature (which is only a few lines, but still), so it's split out to issue #140690. Attached is the previous patch, but without having descriptions as link texts included, which reduces the patch by a few lines.

Yeah, another update. Rename filefield_hide_upload_files() to filefield_hide_upload_files_form(), because the former is supposed to be added in the future for properly cleaning out those files, to be called in filefield_node_alter(). But as that one doesn't yet exist (though Eaton is working on it), just make sure it won't clash when it's going to be added.

Also, check on 'description' and 'list' being set in filefield_file_insert(), because we can't be sure that other modules that depend on filefield will have them set as well (and we want to have proper database entries nevertheless).

Having talked to dopry about this, it's now decided that filefield won't the {file_revisions} table until at least Drupal 7. (It's private to upload.module in Drupal 6, so it's not a good idea to store our data there.)

Nevertheless, there has to be another way for supporting revisions while keeping the information in the CCK tables. Let's see what can be done here.

Subscribe

I see there is a revision possibility with file field, shouldn't this issue be closed?

No, because that patch is not suitable for being committed to the official upstream version of filefield, and thus doesn't solve the issue for the vast majority of filefield users.

Ok, sorry I didn't read the issue properly.

it's now decided that filefield won't the {file_revisions} table until at least Drupal 7. (It's private to upload.module in Drupal 6, so it's not a good idea to store our data there.)

That's sounds as a long time from now... How about a 'half-way' solution, by creating a dependency on the upload module for using the {file_revisions}table. In my opinion the benefit of having a working revision system is higher then the disadvantage of the dependency. Furthermore, since we're dealing with a core module, we know for for sure that all users have it.

Once a better solution will be implemented, the dependency can be removed.
Does this make sense?

> How about a 'half-way' solution, by creating a dependency on the upload module for using the {file_revisions}table.

No, that doesn't work because we can't remove upload.module's file table from the page view because of the way nodes are rendered at the moment. (I tried it, and it's just not possible without bad hacks.) Also, if upload.module's table is private then we should respect that, it does have a reason that it works this way.

> In my opinion the benefit of having a working revision system is higher then the disadvantage of the dependency.

Yeah, maybe, but the real solution (for Drupal 6, at least) is to find out why CCK's auto-revisioning features don't automatically work with filefield, and fix it there. My patch from further up the issue was done when I thought it had to be done differently, but that's not the case. It's probably not a conceptual problem, we don't have to introduce tables or whatever, we just need to ensure that CCK gets the right data to do all that for us.

No additional tables or upload.module dependency, therefore. I'll make sure this bug is fixed in the Drupal 6 version at latest, but I'd love to take patches that explain why it doesn't already work like it should.

This should be done upstream in CCK. the field shouldn't need to worry about revisions.The data -> node association is content.module's job.

subscribing

subscribing

subscribing

For posterity, I think this may have been solved here: #370531: Properly Handle Revisions for document control.