Summary: | Granular parameters permissions | ||
---|---|---|---|
Product: | Koha | Reporter: | Srdjan Jankovic <srdjan> |
Component: | System Administration | Assignee: | Srdjan Jankovic <srdjan> |
Status: | CLOSED FIXED | QA Contact: | |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | gmcharlt, katrin.fischer, kyle.m.hall, nengard, paul.poulain |
Version: | 3.10 | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Attachments: |
patch
patch Attachment to Bug 8083 - Granular parameters permissions [SIGNED-OFF] bug_8083: parameters subpermissions: manage_circ_rules and parameters_remaining_permissions bug_8083: Explanation of update query |
Description
Srdjan Jankovic
2012-05-14 06:38:36 UTC
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 ! |