Problem: I've enabled upload.module I then later assign a few roles to be able to upload files. All of a sudden this new, hidden setting appears in Settings > Upload .. max file size and total file size for *each* role that has this capability. Hmm, good thing I decided to check back there! But where did those defaults come from, hmm.
Fix: There should be a 'default' setting in Settings > Upload for max file size and total file size for *all* roles. Then, when I create a role and assign it upload permissions, it should inherit these defaults, which shouldn't be hardcode into Drupal anyways (as they are now). However, after I assign this permission, override settings should appear, as they currently do.
Benefits: If I assign uploads to 20 different roles I can have default setting for them all instead of having to worry about changing them all. Also, this fixes the obviously usability problem and makes this module make more sense which is always a +1 :)
Comment | File | Size | Author |
---|---|---|---|
#15 | upload.module_33.patch | 7.05 KB | m3avrck |
#12 | upload.module_31.patch | 5.19 KB | m3avrck |
#11 | drupal-head.upload-defaults.junyor.patch | 4.97 KB | Junyor |
#3 | upload.module_4.patch | 3.54 KB | m3avrck |
#2 | upload.module_3.patch | 3.29 KB | m3avrck |
Comments
Comment #1
m3avrck CreditAttribution: m3avrck commentedCorrection, there should also be a default setting for extensions as well.
Comment #2
m3avrck CreditAttribution: m3avrck commentedOk I've created a patch which fixes this problem. Thorougly test here, please test and let me know! This certainly does simplify the out-of-box experience with the upload.module.
Comment #3
m3avrck CreditAttribution: m3avrck commentedFixed a small bug caught by mozillman in IRC.
Comment #4
Souvent22 CreditAttribution: Souvent22 commentedTried patch. Seems to work ok. +1
Comment #5
walkah CreditAttribution: walkah commented+1 - perhaps i'm getting old and foggy - but isn't this how upload.module used to work??
Comment #6
Junyor CreditAttribution: Junyor commentedThere used to be a setting for max file size for the whole site, but I don't think these defaults were listed anywhere.
Comment #7
m3avrck CreditAttribution: m3avrck commentedI think originally there was a setting similiar to this however I'm not sure if it was setting a 'Default' value or a 'Global' value for all uploads. I think that is what the originaly problem was for taking it out. This patch, coupled with my other patch: http://drupal.org/node/30038 (rerolled if/when Form API gets in) makes the administration of the upload settings page *very* easy and super clean and intuitive as well.
Comment #8
m3avrck CreditAttribution: m3avrck commentedBump for this simple usability fix!
Comment #9
Junyor CreditAttribution: Junyor commentedThese changes have been merged into http://drupal.org/node/25756, though they can easily be separated if needed.
Comment #10
m3avrck CreditAttribution: m3avrck commentedPatch now part of: http://drupal.org/node/25756
Comment #11
Junyor CreditAttribution: Junyor commentedI'm resurrecting this patch. http://drupal.org/node/25756 will be released as a contrib module for Drupal 4.7, but this part of the patch cannot be in contrib.
Comment #12
m3avrck CreditAttribution: m3avrck commentedThe above patch wasn't saving per role settings. This new patch fixes that. Should be ready to go now, please test and set RTC!
Comment #13
Junyor CreditAttribution: Junyor commentedWhoops, good catch. Saving settings, defaults for new roles, and resetting all work great. RFC.
Comment #14
Gerhard Killesreiter CreditAttribution: Gerhard Killesreiter commenteddoesn't apply anymore
Comment #15
m3avrck CreditAttribution: m3avrck commentedUpdated patch.
Also, I thought about moving the 'list files by default per node' option to be role based as well, instead of globa. However, that seems a bit strange, because if a user is part of 2 roles, one that has show files by default and other not, it would be strange as to what the default behavior is since you can't merge them like the other settings. So I left that one off.
Patch works same as it was before. Should be ready to go, someone please confirm, thanks!
Comment #16
dopry CreditAttribution: dopry commentedIt don't break nothing and the code looks good.
+1.
didn't test extensively, but all settings fields are present and save properly.
Comment #17
dopry CreditAttribution: dopry commentedgoing a head and setting this commitable since it was before it needed to be rerolled.
Comment #18
moshe weitzman CreditAttribution: moshe weitzman commentedthis booby trap is a bug IMO
Comment #19
drummCommitted to HEAD.
This page still needs UI help. Permissions aren't settings (not that we have a place to put permissions that aren't checkboxes).
Comment #20
(not verified) CreditAttribution: commented