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 :)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

m3avrck’s picture

Correction, there should also be a default setting for extensions as well.

m3avrck’s picture

Status: Active » Needs review
FileSize
3.29 KB

Ok 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.

m3avrck’s picture

FileSize
3.54 KB

Fixed a small bug caught by mozillman in IRC.

Souvent22’s picture

Tried patch. Seems to work ok. +1

walkah’s picture

+1 - perhaps i'm getting old and foggy - but isn't this how upload.module used to work??

Junyor’s picture

There used to be a setting for max file size for the whole site, but I don't think these defaults were listed anywhere.

m3avrck’s picture

I 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.

m3avrck’s picture

Status: Needs review » Reviewed & tested by the community

Bump for this simple usability fix!

Junyor’s picture

These changes have been merged into http://drupal.org/node/25756, though they can easily be separated if needed.

m3avrck’s picture

Status: Reviewed & tested by the community » Closed (duplicate)

Patch now part of: http://drupal.org/node/25756

Junyor’s picture

Status: Closed (duplicate) » Needs review
FileSize
4.97 KB

I'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.

m3avrck’s picture

FileSize
5.19 KB

The above patch wasn't saving per role settings. This new patch fixes that. Should be ready to go now, please test and set RTC!

Junyor’s picture

Status: Needs review » Reviewed & tested by the community

Whoops, good catch. Saving settings, defaults for new roles, and resetting all work great. RFC.

Gerhard Killesreiter’s picture

Status: Reviewed & tested by the community » Needs work

doesn't apply anymore

m3avrck’s picture

Status: Needs work » Needs review
FileSize
7.05 KB

Updated 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!

dopry’s picture

It don't break nothing and the code looks good.
+1.

didn't test extensively, but all settings fields are present and save properly.

dopry’s picture

Status: Needs review » Reviewed & tested by the community

going a head and setting this commitable since it was before it needed to be rerolled.

moshe weitzman’s picture

Category: task » bug

this booby trap is a bug IMO

drumm’s picture

Status: Reviewed & tested by the community » Fixed

Committed 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).

Anonymous’s picture

Status: Fixed » Closed (fixed)