Hello,

I've been working on expanding this module to limit by extensions as well as size per role.

It is nearly working. When I get it tested as clean, are you interested in incorporating it as a patch.

Just wanted to post this update here to start a conversation about expanding this module before I send the patch.

Attached is an image of the config page I have working. The limits are almost tested for files and image types.

Thanks!

CommentFileSizeAuthor
#8 filefield_role_limit_add_extensions_1695038_5_8.patch11.47 KBkongoji
#5 filefield_role_limit_add_extensions_1695038_5.patch11.49 KBkongoji
#3 filefield_role_limit.tar_.gz45.69 KBdarrell_ulm
#3 filefield_role_limit_add_extensions_1695038.patch10.99 KBdarrell_ulm
5009c8cc9d29c91d5b00006c.jpeg134.22 KBdarrell_ulm

Comments

kongoji’s picture

kongoji commented

Hi darrellulm,
your idea is brilliant, I look forward to see your patch!
The screenshot seems very promising and, as far as I understood, this is how your patch should work (please correct me if I'm wrong):

  • if a specific role has specific extensions setted, then it cannot upload other file extensions, even if the extensions are specified in general field settings
  • if a role has no specific extension setted, it will fallback on general field setting
  • if a user has more roles, it will be able to upload all extensions specified for all its roles

The extensions specified for a role have to be a sub-set of general field settings' extensions or they can be different?
To make an example, what happen if I set for a field the extensions .mp3, .avi and for the role "Administrator" I set totally different .mp4, .mpg?
Will the Administrator role see all (.mp3, .avi, .mp4, .mpg), or there'll be a settings validation error because .mp4, .mpg is not into .mp3, .avi, or Administrator will only see .mp4, .mpg?
I think this could be an interesting point to talk about, let me know.

Are you working also on the 6.x module version or (if not) are you interested to?

Thank you for having taken the time to work on expanding this module

darrell_ulm’s picture

darrell_ulm commented

Hello again,

I don't know about brilliant, in Drupal 6 the file field, I believe had some nice role based options, but it could be useful.

  1. if a specific role has specific extensions setted, then it cannot upload other file extensions, even if the extensions are specified in general field settings
  2. if a role has no specific extension setted, it will fallback on general field setting
  3. if a user has more roles, it will be able to upload all extensions specified for all its roles

Answers:

  1. Yes, and the patch + changes does this correctly.
  2. Yes, falls back to the general, also works.
  3. Yep, expands on all roles.

The only issue I'm having is with the widget, I need to fix it blocking on file size by role. I think this is because while developing some data got stuck in there. So I need to clean out my content fields and test it again.

The extensions specified for a role have to be a sub-set of general field settings' extensions or they can be different?
To make an example, what happen if I set for a field the extensions .mp3, .avi and for the role "Administrator" I set totally different .mp4, .mpg?

It goes for the more specific, in this case .mp4, .mpg.

Will the Administrator role see all (.mp3, .avi, .mp4, .mpg), or there'll be a settings validation error because .mp4, .mpg is not into .mp3, .avi, or Administrator will only see .mp4, .mpg?
I think this could be an interesting point to talk about, let me know.

In your example, the admin will only have .mp4, .mpg available for upload. The options are completely by role, so if the Admin needs the other extensions, they all can be listed like: .mp3, .avi, .mp4, .mpg.

Are you working also on the 6.x module version or (if not) are you interested to?

I am not right now, but I thought Drupal 6 filefield module (depreciated) had some of these options for roles.

darrell_ulm’s picture

darrell_ulm commented
Assigned: Unassigned » darrell_ulm
Status: Needs review » Active
StatusFileSize
new filefield_role_limit_add_extensions_1695038.patch10.99 KB
new filefield_role_limit.tar_.gz45.69 KB

Hi, thought I would upload these in case anyone wanted to try to start incorporating limiting by extensions.

I want to get back to it, but many projects, etc. so here is some code, both a patch which should be applied to the version before the last update, and also just a file of the module with all the extension stuff added in, may be easier to try to incorporate as there are many changes.

I was hoping I could get back to it, but there are many things on my plate(s), so if useful in any way, cool. Just want to upload what is here before it gets too stale.

:)

Thanks!

*note: just changed the status

darrell_ulm’s picture

darrell_ulm commented
Assigned: darrell_ulm » Unassigned
Status: Active » Needs review

Needed to change the status.

kongoji’s picture

kongoji commented
Assigned: darrell_ulm » Unassigned
Status: Active » Needs review
StatusFileSize
new filefield_role_limit_add_extensions_1695038_5.patch11.49 KB

Hi darrellulm,
first of all, I want to thank you for this patch, it introduces some great features and you did a wonderful job with it.

The patch had a couple of @todos at the beginning, I took the freedom to fix them.
For extension validation I preferred using drupal _file_generic_settings_extensions function, so that also commas and leading dots can be handled.
I also added a few check_plain functions to better escape field settings.

There is still one /** @todo this!!! */ comment on _filefield_role_limit_field_widget_file_form_alter function, but I'm not very sure what is referred to.
In that function I uncommented line
//$widget[$key]['#upload_validators']['file_validate_size'][0] = $max_upload_size_plain;,
otherwise file size limit wouldn't work.

Thank you again, I hope you find the time to reply me soon!

darrell_ulm’s picture

darrell_ulm commented

@kongoji Cool! The check_plain's are the way to go, glad you caught those, wish I had more time to do more with it before I submitted it, but was hoping it would be of use. Also about _file_generic_settings_extensions, nice!

You can ignore the "/** @todo this!!! */" that is some comment I put it probably to remind myself to put in check_plains ;)

Not sure why I commented out 

//$widget[$key]['#upload_validators']['file_validate_size'][0] = $max_upload_size_plain;,

Probably to test the extension stuff on it's own, and that was why file size was not limiting correctly for me, makes sense. I did this a little on the rushed side, but hope it can be of use.

Did not test it yet, hope either I (or someone else) can soon,

Thank you!

darrell_ulm’s picture

darrell_ulm commented

Tested this a little bit (patch applied fine). Seems to be behaving w/ same issues, in that it does not always report when the file is too large to upload, and in some cases will upload the file even if exceeds the limit.

Notes:

  • Administrator seems to be able to override the file size, but not the extension limits. Some more testing there needed.
  • The allowed size or extension warning appears to happen once, and then after the first warning, cannot reset for the next upload attempt, and the same warning remains.
  • However if a good file, both size / extension is loaded, it will load and save, but the original warning never goes away and the user does not have any feedback for what is going on either with a valid or invalid file after the first upload warning.

This seemed to be the same behavior I was getting before, even w/o the extension enhancement w/ the limit by size only.

Do you get the same issues when trying to add different files? I picked a file field w/ unlimited files so I could add many to a content type to test this. Do you know any reasons this might occur?

Thank you!

kongoji’s picture

kongoji commented
StatusFileSize
new filefield_role_limit_add_extensions_1695038_5_8.patch11.47 KB

Hi darrellulm,
I removed the last @todo from the patch, thank you to have found the time to check my notes.

I try to reply to your notes here below:

  • Administrator is allowed to upload any file, no matter the size. This is the way core's File module works, the code is inside file_validate_size function, that is the function also FileField Role Limit module uses for validate the file.
  • You are right about warning messages, but based on my test, that happens also when FileField Role Limit module it's not enabled. It does seem to be a wrong ajax behavior of Drupal core's File module.

I will investigate better on warning messages on file upload, but at the moment I don't see any easy way to fix them, since it seems to me to be a core problem.

darrell_ulm’s picture

darrell_ulm commented
Status: Needs review » Reviewed & tested by the community

@kongoji

Yes, you are correct on both points, the 1st point about admin upload is limitless which is good, and also on the point of the Drupal 7's ajax upload not working correctly the second time after an incorrect 1st attempt.

Not sure if we're going to get a second reviewer, so I'll say:

Looks good to me.

and Thank you! :)

kongoji’s picture

kongoji commented

Hi darrellulm,
I committed the patch on 7.x-1.x-dev version (2fdd21f).
If no issue will be opened in a week from now, I'm going to release a 7.x-1.2 module version.

darrell_ulm’s picture

darrell_ulm commented

Hi kongoji,

Thanks that is great! I hope I can test on the weekend, but feel free to release when you feel like doing it.

Thanks again.

kongoji’s picture

kongoji commented
Status: Reviewed & tested by the community » Fixed

Hi darrellulm,
since 7.x-1.2 module version is out from 2 weeks and no issues were opened in the meantime, I set this issue as fixed.
Thank you again for the amazing job you did!

Status: Fixed » Closed (fixed)

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

darrell_ulm’s picture

darrell_ulm commented

Hi @kongoji, looks good! I'm still interested in tracking down the issue of after an error is flagged, the next upload does not always work. I suppose this is another ticket. I'm not 100% this is just an issue with core.

If so is there a core issue we can point people to look at?

It looks like this may be the issue:
http://drupal.org/node/1020916