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.
Complementary issue to the removal of the same functionality in the media module #1552920: Move file/add from media to file..
Comments
Comment #1
Letharion CreditAttribution: Letharion commentedHave yet to port the variable_{del,get,set}.
Comment #2
Letharion CreditAttribution: Letharion commentedPatch now includes a new file_entity.variable.inc file that mimics the same functionality from the media module.
Comment #4
Letharion CreditAttribution: Letharion commentedNow actually includes the new file. ;)
Comment #5
Letharion CreditAttribution: Letharion commentedComment #6
fabsor CreditAttribution: fabsor commentedSince we are introducing file_entity.variable.inc here and functions for setting variables through file_entity_variable_*, we probably want to switch over to that pattern completely, or drop it altogether. There are several places were we still use the generic pattern, it's probably a good idea to not mix them.
Comment #7
Dave ReidI don't wish to continue to using the bad variable pattern from media. It's not what core does and we shouldn't start using it here if we're trying to merge this module into core.
Comment #8
Letharion CreditAttribution: Letharion commentedIn that case, the patch in #1 needs review, instead of #4 needing work.
Comment #9
Letharion CreditAttribution: Letharion commentedTwo questions that would be helpful to answer:
Should we simply copy or move the defaults from the media module into their respective, variable_get()'s? Or set them on file_entity install?
Do we support an upgrade path? That would mean renaming the variable prefix.
Comment #10
Dave ReidI'm wondering why we need the variables at all? Core already provides us with a default extensions ('jpg jpeg gif png txt doc xls pdf ppt pps odt ods odp' from file_save_upload()) and file size (file_upload_max_size()) that we can use, I don't see a need to add anything new here at all. Media can add it's own default in by using the $params parameter for the form.
Comment #11
Dave ReidAdding sprint tags.
Comment #12
dgorton CreditAttribution: dgorton commented#1 works for me (in conjunction w/ #6 from #1552920: Move file/add from media to file.
Comment #13
mtiftI mentioned it in the other issue as well, but #1 works for me (in conjunction w/ #6 from #1552920: Move file/add from media to file.) works for me, too.
Comment #14
Dave ReidA couple of quick updates then this is ready!
Let's remove this function. To change a file to permanent, it only requires two lines. :)
Let's replace this with:
else {
$validators['file_validate_extensions'] = array('jpg jpeg gif png txt doc xls pdf ppt pps odt ods odp');
}
Let's remove this condition.
Replace this function call with:
$file->status = FILE_STATUS_PERMANENT;
Change this to:
$file->status = FILE_STATUS_PERMANENT;
file_save($file);
Comment #15
dgorton CreditAttribution: dgorton commentedOnce the changes in #14 are complete, the patch in #16 from #1394826: Add upload progress to the Upload tab will need to be rerolled against file_entity.
Comment #16
mtiftI've incorporated Dave's suggestions and re-rolled the patch
Comment #17
mtiftComment #18
dgorton CreditAttribution: dgorton commented#16 works for me
Comment #19
Dave ReidThanks all! I've committed this to 7.x-2.x with http://drupalcode.org/project/file_entity.git/commit/f050bd7. Nice work! Going to commit #1552920: Move file/add from media to file. to Media now.
Comment #20
Dave ReidSome follow-up issues for this:
#1600624: Move the 'Add file' local action on admin/content/file to file_entity
#1600610: Media allowed file size not used on file/add/upload
#1599892: Plupload integration broken