Introduce 'manage_circ_rules' and 'parameters_remaining_permissions' parameters subpermissions. Add "GranularParametersPermissions" boolean syspref, No default. If set to yes use aforementioned subpermissions in admin/* functions, otherwise just use parameters flag
Hi Srdjan, I think adding granular permissions for administration is a good idea and I can think of more to be added later :) But I am not sure the system preference is needed here. We already have granular permissions for other modules. All granular permissions used to be controlled by a system preference that got removed after some time, because granular got to be standard over permissions on module level only. I think it should also be ok on update if both granular permissions are checked by default.
I agree with Katrin :)
I totally agree. The suspref was suggested in order not to disrupt other people, but when I looked at the workflow it did not make any sense. I've already removed it, just did not update the bug. Assigning both permissions goes without saying :)
Created attachment 9578 [details] [review] patch
I have blocked "Circ and Fines rules" link if manage_circ_rules permission is missing, but did not do similar for all other links in case parameters_remaining_permissions is missing - I did not know how to do it and still have a decent looking screen. Any ideas welcome.
Hi Srdjan, the userpermissions.sql is one of the files that gets translated to provide a web installer in different languages. Can you please include the other languages too? Leaving the description in English should be ok. There is also a test that got updated very recently (bug 8022): xt/permissions.t
Created attachment 9686 [details] [review] patch
Created attachment 10638 [details] [review] Attachment to Bug 8083 - Granular parameters permissions [SIGNED-OFF] bug_8083: parameters subpermissions: manage_circ_rules and parameters_remaining_permissions Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 10639 [details] [review] [SIGNED-OFF] bug_8083: parameters subpermissions: manage_circ_rules and parameters_remaining_permissions Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
QA comment, I was about passing QA this patch, when I saw in the updatedatabase: + $dbh->do("INSERT INTO user_permissions (borrowernumber, module_bit, code) + SELECT borrowernumber, 3, 'parameters_remaining_permissions' + FROM borrowers WHERE flags & (1 << 3)"); I think it gives the permission to all patrons that already had the 'admin' permission. But: 1- please confirm 2- a comment just before the request would be usefull for a future reader. Site comment: we have to find a way to handle translated user permission : our libraries are complaining that new ones are not translated. That's because the updatedatabase is in english & in english only. But that's another topic...
Created attachment 10917 [details] [review] bug_8083: Explanation of update query
> I think it gives the permission to all patrons that already had the 'admin' > permission. But: > 1- please confirm Confirmed, except it is 'parameters' permission > 2- a comment just before the request would be usefull for a future reader. Done
QA comments: previous comments fixed, no more reason not to pass QA
I tried to access all admin pages with a superlibrarian account after applying the patch, no problem displaying those pages. Good job srdjan. @all = follow-ups welcomed (in separate bugs) for introducing other admin granular permissions !