Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.This patch extracts the file handling functions from filefiled.module into filefield.inc. The idea is to commit this and then start porting features from imagefield and multimediafile.inc (from mediafield module), then let those module depend on filefield ... is this ok?
| Comment | File | Size | Author |
|---|---|---|---|
| #19 | filefield-split-v7.patch | 17.98 KB | jpetso |
| #17 | filefield-split-v6.patch | 15.63 KB | Egon Bianchet |
| #12 | filefield-split-v5.patch | 13.23 KB | jpetso |
| #10 | filefield-split-v4.patch | 10.8 KB | jpetso |
| #8 | filefield-split-v3.patch | 10.05 KB | jpetso |
Comments
Comment #1
azote CreditAttribution: azote commentedI must agree with Egon... I believe this is the way to go... around having the same functions all over the cck modules..
Comment #2
azote CreditAttribution: azote commentedpatch works ok... let hope we can get this on head after further testing ....
about the return NULL; on the filefield_file_update (filefield.inc) ... i have no clue....
Comment #3
drewish CreditAttribution: drewish commentedbefore this turns into any kind of API, it really needs to start writing records to the {file_revisions} table.
Comment #4
jpetso CreditAttribution: jpetso commentedAdding file_revision entries, an include_once statement, and making file deletions work again (the first patch forgot to rename a _filefield_file_insert() call from private to public). Please review again.
Comment #5
drewish CreditAttribution: drewish commentedcool, i'll take a look at that. the only thing i'd suggest just by looking at it is to add an filefield_update to add missing file _revisions records.
Comment #6
drewish CreditAttribution: drewish commentedthe patch won't apply. it looks like your patch is against an older version.
Comment #7
jpetso CreditAttribution: jpetso commentedJust noticed that there have been a couple of changes to HEAD, while the original patch obviously had the DRUPAL-5 version as a base.
So it seems that it's a bad idea to apply the patch as is, I'm going to reroll it from HEAD.
Comment #8
jpetso CreditAttribution: jpetso commentedRerolled from HEAD. Please review once again :D
Comment #9
jpetso CreditAttribution: jpetso commentedEr, damn, it doesn't work. Let me debug a bit. Sorry for not testing before uploading.
Comment #10
jpetso CreditAttribution: jpetso commentedOk, so it might actually be a good idea to include the patch with the files that one actually modifies. It should work better now :-P
Comment #11
drewish CreditAttribution: drewish commentedseems to work fine. the only thing is, as we discussed on IRC, adding revisions support causes the files to appear in the upload module's form items. it might be worth doing some form alter to pull out our rows...
Comment #12
jpetso CreditAttribution: jpetso commentedYeah, I did it! form_alter magic, here we come. Ready to be reviewed.
Comment #13
Egon Bianchet CreditAttribution: Egon Bianchet commentedI'm not sure it's a good idea to use form_alter, shouldn't filefield be an alternative to upload? In that case users should just disable upload for that content type
Comment #14
jpetso CreditAttribution: jpetso commentedWell, it works, it hardly affects performance, and it provides the possibility to use upload nevertheless. What filefield is used for should be up to the user, not predefined by us.
Comment #15
hbfkf CreditAttribution: hbfkf commentedSubscribing.
Comment #16
Egon Bianchet CreditAttribution: Egon Bianchet commentedmoved filefield_check_directory and filefield_form_check_directory to the inc file
Comment #17
Egon Bianchet CreditAttribution: Egon Bianchet commentedThe file :-P
Comment #18
Egon Bianchet CreditAttribution: Egon Bianchet commentedUpdates and revisions are broken
Comment #19
jpetso CreditAttribution: jpetso commentedThis patch should make revisions work like they're supposed to. Yay. Please review again, and have a look if you can catch dopry so that we actually have a chance to get this applied :-P
Comment #20
Egon Bianchet CreditAttribution: Egon Bianchet commentedWith a single-file field, if I upload a file, edit the node again, upload a new file to replace the previous, and then revert the old revision, when I edit the node again no file is shown in the form.
Also, does anybody know if it's better to include filefield.inc with a hook_init like content.module does?
I tried to contact dopry but he didn't answer yet
Comment #21
Egon Bianchet CreditAttribution: Egon Bianchet commenteddopry said
so I think we could just limit the patch to the revisions support and attach it in this issue http://drupal.org/node/130269
Comment #22
jpetso CreditAttribution: jpetso commented> With a single-file field, if I upload a file, edit the node again, upload a new file
> to replace the previous, and then revert the old revision, when I edit the node again
> no file is shown in the form.
I actually tested that use case, and it worked here. Maybe we've been doing it slightly differently, but in any case, I can't reproduce what you described.
In other news, I merged the reworked functions back to filefield.module, and posted the related patch here. Unfortunately, I've also got AJAX support lying around in my local filefield checkout, and I don't really feel like decoupling the patch just for patch's sake, so it's kind of a kitchensink patch now. (Which is also why I haven't attached it to #130269.) Sorry for not keeping them apart.
Respecting dopry's opinion on file separation, I'll close this issue now. Further comments please go to the appropriate one of the two issues mentioned above.