Change gallery permission from 'administer images' to 'administer image galleries'
joachim - March 22, 2009 - 08:41
| Project: | Image |
| Version: | 6.x-1.x-dev |
| Component: | image_gallery |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
This permission is just weird and confusing. It has nothing to do with administering images. It only allows access to the image gallery admin pages at 'admin/content/image/...'
'administer image galleries' would be much clearer to site admins.
#1
Why didn't you use the original issue? (#409770: how to create new gallery )
I thought that users would just need to be able to add (or edit/manage) taxonomy terms to create new galleries?
#2
Well the original issue's initial post wasn't really explaining the problem. I thought best to start fresh.
Yes, if you can administer taxonomy, then you can change galleries via the regular taxonomy admin.
But image_gallery adds an admin section just for galleries, which is essentially a duplication of the taxonomy admin.
This allows some labelling changes ('galleries' instead of 'terms') and delegation of gallery management to users without site-wide taxo admin access.
#3
renamed the permission and created a patch.
regards
#4
Looks good so far, but I think we need to provide an update function to change that permission for existing sites.
#5
added install patch.
regards
#6
Regarding the update function: Drupal core does the string replacing in PHP rather than SQL.
I've no idea if there's a reason for this, but my gut feeling is that we should do what core does -- it's probably due to cross-db compatibility.
Eg, in system.install:
function system_update_6039() {$ret = array();
$result = db_query("SELECT rid, perm FROM {permission} ORDER BY rid");
while ($role = db_fetch_object($result)) {
$renamed_permission = preg_replace('/(?<=^|,\ )edit\ ([a-zA-Z0-9_\-]+)\ content(?=,|$)/', 'edit any $1 content', $role->perm);
$renamed_permission = preg_replace('/(?<=^|,\ )create\ polls(?=,|$)/', 'create poll content', $renamed_permission);
if ($renamed_permission != $role->perm) {
$ret[] = update_sql("UPDATE {permission} SET perm = '$renamed_permission' WHERE rid = $role->rid");
}
}
return $ret;
}
Attaching a combined patch, with the update function changed.
I've tested the updating on a fresh D6.
#7
Your modified patch works for me, too.
#8
Invoking the PCRE engine seems needless. A simple str_replace() or strtr() (latter preferred) is sufficient in the update function.
#9
update joachim's patch to use strtr
regards
#10
Updated to use array syntax for strtr.
#11
Same as in the other issue for image.module applies here - quickly looking at image_gallery.test uncovers further instances of the permissions. Please ensure all permissions are updated accordingly.
#12
Updated the core-style permission patch, which includes the change of gallery permission in test files: http://drupal.org/node/44057#comment-1730082
#13
Maybe I'm reading it wrong, but it doesn't look like http://uk3.php.net/strtr is the right thing to use...
EDIT: yeah I'm reading it wrong. Carry on :D
#14
See also this well-known function: http://api.drupal.org/api/function/t/6 ;)
#15
Patch #10 still need's review. For changes in test files read #12.
regards
#16
@sun, @joachim: I just saw your discussion on IRC...
You could easily form_alter the image gallery's taxa administration pages to return a 403. However, IMO, 'administer taxonomy' as the more general permission should outweigh 'administer image galleries', so don't do that. I'd say 'administer image galleries' should allow using the image gallery administration pages even if the user does NOT have 'administer taxonomy', so that we can have "gallery administrators," without giving them full access to all vocabularies.
As for
Image Gallery Access takes care of that nicely.
#17
@salvis: we were talking about a different issue -- #113629: 'Image galleries' menu item appears without access -- which more or less requests a permission to access /image/tid/$tid. It's this that you can still access at /taxonomy/term/$tid, albeit not formatted the same way.
Though -- interesting module.
I'll point the poster of that other issue to it.
Would you care to add a note about your module to the docs page here: http://drupal.org/handbook/modules/image ?
#18
Tested patch on an installation that had the old permission set. Permission was correctly changed for role that had it assigned. No sign of the old perm string in the module code.
All looks good, committing.
#409974 by pebosi: Changed gallery permission from 'administer images' to 'administer image galleries'.
Thank you pebosi for your hard work on this and other patches :D
Sorry it's taken a while to get it committed.
#19
Automatically closed -- issue fixed for 2 weeks with no activity.