Hi there,
I would like to report a bug of this module.
I used this module with media 2.x, media gallery 2.x and plupload integration.
With this modules activated was impossible to upload video file with extention mp4, mov... ecc....
Multimedia upload is deprecated.
So we need to use file entities.
But this module have this file extention allowed

define('FILE_ENTITY_DEFAULT_ALLOWED_EXTENSIONS', 'jpg jpeg gif png txt doc docx xls xlsx pdf ppt pptx pps ppsx odt ods odp');

declared in the beginning of the file_entity.module.
So I changed this line like:
define('FILE_DEFAULT_ALLOWED_EXTENSIONS', 'jpg jpeg gif png txt doc docx xls xlsx pdf ppt pptx pps ppsx odt ods odp mp3 mov mp4 m4a m4v mpeg avi ogg oga ogv wmv ico');

and my problem was solved.
Hope it can help someone!
And sorry for my bad english!
Files: 
CommentFileSizeAuthor
#35 file_entity_variables-1836266-35.patch944 bytesDevin Carlson
PASSED: [[SimpleTest]]: [MySQL] 874 pass(es).
[ View ]
#33 file_entity_variables-1836266-33.patch1.74 KBczigor
PASSED: [[SimpleTest]]: [MySQL] 823 pass(es).
[ View ]
#28 file_entity_variables-1836266-27.patch2.33 KBiva2k
PASSED: [[SimpleTest]]: [MySQL] 810 pass(es).
[ View ]
#26 file_entity_variables-1836266-26.patch2.33 KBiva2k
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch file_entity_variables-1836266-26.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#12 file_entity_variables-1836266-12.patch1.63 KBiva2k
PASSED: [[SimpleTest]]: [MySQL] 638 pass(es).
[ View ]
#4 836266.4-file_entity-allowed-extensions.patch1.21 KBmrfelton
PASSED: [[SimpleTest]]: [MySQL] 258 pass(es).
[ View ]
#3 836266.3-file_entity-allowed-extensions.patch1.22 KBmrfelton
PASSED: [[SimpleTest]]: [MySQL] 258 pass(es).
[ View ]
#2 836266-file_entity-allowed-extensions.patch1.2 KBmrfelton
PASSED: [[SimpleTest]]: [MySQL] 258 pass(es).
[ View ]

Comments

Title:media entity deprecated, but file entity do not knowMake allowed extensions configurable
Category:bug» feature
Status:Fixed» Active

We should make this configurable..maybe turn it to a variable?

Status:Active» Needs review
StatusFileSize
new1.2 KB
PASSED: [[SimpleTest]]: [MySQL] 258 pass(es).
[ View ]

Simple patch to switch the setting over to a variable.

StatusFileSize
new1.22 KB
PASSED: [[SimpleTest]]: [MySQL] 258 pass(es).
[ View ]

Oops, last patch blew away the array.

StatusFileSize
new1.21 KB
PASSED: [[SimpleTest]]: [MySQL] 258 pass(es).
[ View ]

Ok, lets try again. Should have been a space separated string.

Oops, last patch blew away the array.

Status:Needs review» Reviewed & tested by the community

thanks..it makes sense

Status:Reviewed & tested by the community» Fixed

As a side note, media.module references FILE_DEFAULT_ALLOWED_EXTENSIONS and also needs to be updated to use the new variable.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Status:Closed (fixed)» Active

Adding a variable makes it one step closer to being configurable, however, without any UI to actually change the value of this variable the fix is incomplete.

Status:Active» Needs review
StatusFileSize
new1.63 KB
PASSED: [[SimpleTest]]: [MySQL] 638 pass(es).
[ View ]

Here's a patch that adds variable_ui hooks for a "free UI".

+++ b/file_entity.variable.incundefined
@@ -0,0 +1,31 @@
+//    'path' => 'admin/config/?',

nah, just remove it..there is a ui in variable module, we dont need to provide another one

I agree that making UI for every silly variable is a waste of time. I think variable/variable_ui should be part of the core.

Do you want me to roll another patch to remove this comment? Or just omit it from commit?

i will just ommit it;) but that said..cant remember if file_entity owns any other variable, so we register it...if we have more, lets put them in there since we are implementing the hook

Ok, lets register those too:

  • file_entity_admin_files_limit
  • file_entity_cron_last

they are both integers

What is the plan when we start porting the module to Drupal 8 and inclusion in Drupal core? This hook would not be appropriate to remain since it's not included in core. Would this implementation move to variable module? This should add a @todo for a plan. Or better yet, likely it should form-alter the 'File system' settings page to add this variable, which would be a better fit for core.

in d8 there is CMI, so i am not sure what happens to variable module..for core inclusion, this should be removed, like any other integration with non-core modules that are in file_entity

Priority:Normal» Major

Another thing that should be considered here is the fact that Media provides very similar variable and even UI for it's configuration. That is VERY confusing, since one easily assumes that this variable effects file_entity also, modifies it and nothing happens. I believe that this could be VERY frustrating.

I'd consider this a major UX flaw...

I think 'file_entity_default_allowed_extensions' should be set based on the file types declared by hook_file_default_types.

or allow everything..

Allowing all types is generally not a very good idea.

Status:Needs review» Needs work

I agree that allowing all types would not be good. Per #17 and the issues previous to that, I am setting this issue to "needs work". For #19, I think that that should be an issue for the Media module.

@aaron
"needs work" - can you clarify what needs to change in #12? Is it only #13 note or something else?

I would add the variables noted in #16 and move it to the file settings form of core mentioned in #17. Yes, I would also take care of #13.

Status:Needs work» Needs review
StatusFileSize
new2.33 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch file_entity_variables-1836266-26.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

I re-rolled patch #12 to include #13, #16. The only change to file_entity module is an addition of new file file_entity.variable.inc (and a pointer in .info file)

As to a request of a plan in #17, when time comes, file_entity.variable.inc file can be moved to variable/includes, next to other include files addressing core variables.

Personally I don't like CMI and think that variable module hits the nail on the head with 99% of module configuration needs. I'm considering to strip settings UI in all my modules and make variable module take care of all that. Declarative programming rocks!

Status:Needs review» Needs work

The last submitted patch, file_entity_variables-1836266-26.patch, failed testing.

StatusFileSize
new2.33 KB
PASSED: [[SimpleTest]]: [MySQL] 810 pass(es).
[ View ]

.info file has changed since #12... Updated patch from #26.

Status:Needs work» Needs review

Looks nice. I would add an entry in README.txt to inform people that they can use variable.module to edit configuration that is not exposed in UI otherwise. Would also add this to online docs after this gets committed.

Status:Needs review» Needs work

i dont think we should add anything to the .info file btw
needs work for a README entry as well

@mailfox
Can you post a screenshot of the variables page for the settings?

Status:Needs work» Needs review
StatusFileSize
new1.74 KB
PASSED: [[SimpleTest]]: [MySQL] 823 pass(es).
[ View ]

Info file entry is useless, I have removed it. Otherwise the patch is practically the same as #28.

There's no README in this module.

Status:Needs review» Reviewed & tested by the community

This looks good to me.

Assigned:Unassigned» Devin Carlson
Status:Reviewed & tested by the community» Needs review
StatusFileSize
new944 bytes
PASSED: [[SimpleTest]]: [MySQL] 874 pass(es).
[ View ]

Even as a big proponent of the Variable module, requiring a user to figure out that they have to download and install a submodule (Variable Admin) of a primarily API module (Variable) that has a significantly smaller user base than Media just to configure a basic setting (what file types users can upload) seems like a bit much to me.

Attached is a simple patch to add the default allowed file extensions to the top of the File system page per Dave Reid's suggestion in #17.

I think that this is ready to go and will commit in the next few days unless anyone has any objects/concerns/improvements. :)

Status:Needs review» Fixed

Committed #35 to 7.x-2.x.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue summary:View changes

I've changed the deprecated entity