Posted by Letharion on April 29, 2012 at 12:21pm
10 followers
| Project: | File entity (fieldable files) |
| Version: | 7.x-2.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | Media Initiative, sprint |
Issue Summary
Complementary issue to the removal of the same functionality in the media module #1552920: Move file/add from media to file..
Comments
#1
Have yet to port the variable_{del,get,set}.
#2
Patch now includes a new file_entity.variable.inc file that mimics the same functionality from the media module.
#3
The last submitted patch, 1552988_2_add_file_add_to_file_entity_module.patch, failed testing.
#4
Now actually includes the new file. ;)
#5
#6
Since 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.
#7
I 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.
#8
In that case, the patch in #1 needs review, instead of #4 needing work.
#9
Two 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.
#10
I'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.
#11
Adding sprint tags.
#12
#1 works for me (in conjunction w/ #6 from #1552920: Move file/add from media to file.
#13
I 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.
#14
A couple of quick updates then this is ready!
+++ b/file_entity.moduleundefined@@ -867,3 +882,16 @@ function file_entity_file_is_local($file) {
+/**
+ * Sets the status to FILE_STATUS_PERMANENT.
+ *
+ * @param $file
+ * A file object.
+ */
+function _file_entity_save_file_permenently(&$file) {
+ if ($file->status < FILE_STATUS_PERMANENT) {
+ $file->status = FILE_STATUS_PERMANENT;
+ file_save($file);
+ }
+}
Let's remove this function. To change a file to permanent, it only requires two lines. :)
+++ b/file_entity.pages.incundefined@@ -31,6 +31,190 @@ function file_entity_view_page($file) {
+ elseif ($tmp = variable_get('file_extensions')) {
+ $validators['file_validate_extensions'] = array($tmp);
+ }
Let's replace this with:
else {
$validators['file_validate_extensions'] = array('jpg jpeg gif png txt doc xls pdf ppt pps odt ods odp');
}
+++ b/file_entity.pages.incundefined@@ -31,6 +31,190 @@ function file_entity_view_page($file) {
+ elseif (($tmp = variable_get('max_filesize')) && $tmp < $max_filesize) {
+ $validators['file_validate_size'] = array(parse_size($tmp));
+ }
Let's remove this condition.
+++ b/file_entity.pages.incundefined@@ -31,6 +31,190 @@ function file_entity_view_page($file) {
+ // Change the file status from temporary to permanent.
+ _file_entity_save_file_permenently($file);
Replace this function call with:
$file->status = FILE_STATUS_PERMANENT;
+++ b/file_entity.pages.incundefined@@ -31,6 +31,190 @@ function file_entity_view_page($file) {
+ file_save($file);
+ _file_entity_save_file_permenently($file);
Change this to:
$file->status = FILE_STATUS_PERMANENT;
file_save($file);
#15
Once 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.
#16
I've incorporated Dave's suggestions and re-rolled the patch
#17
#18
#16 works for me
#19
Thanks 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.
#20
Some 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
#21
Automatically closed -- issue fixed for 2 weeks with no activity.