Right now, Koha has a single all or nothing permission for viewing/altering fees. It would be good to have a separate sub-permission for write-offs and a new sub-permission for all remaining fee related activities ( which could also be split up in the future ).
Created attachment 14757 [details] [review] Bug 9448 - Add separate permission for writing off fees Adds new sub-permissions to updatecharges, 'writeoff' for writing off fees and 'remaining_permissions' for all other fee related activites. Test plan: 1) Apply patch 2) Run updatedatabase.pl 3) Remove the 'writeoff' permission from your privileges 4) Attempt to write off a fee, you should not be able to 5) Re-add the 'writeoff' permission 6) Attempt to write off a fee, you should now be able to do so
Doesn't apply on master
Created attachment 16606 [details] [review] Kyle M Hall <kyle@bywatersolutions.com> Bug 9448 - Add separate permission for writing off fees Adds new sub-permissions to updatecharges, 'writeoff' for writing off fees and 'remaining_permissions' for all other fee related activites. Test plan: 1) Apply patch 2) Run updatedatabase.pl 3) Remove the 'writeoff' permission from your privileges 4) Attempt to write off a fee, you should not be able to 5) Re-add the 'writeoff' permission 6) Attempt to write off a fee, you should now be able to do so
Comment on attachment 16606 [details] [review] Kyle M Hall <kyle@bywatersolutions.com> Mistake with the subject name when rebasing. My mistake.
Applying: Bug 9448 - Add separate permission for writing off fees Using index info to reconstruct a base tree... M installer/data/mysql/de-DE/mandatory/userpermissions.sql M installer/data/mysql/en/mandatory/userpermissions.sql M installer/data/mysql/es-ES/mandatory/userpermissions.sql M installer/data/mysql/fr-FR/1-Obligatoire/userpermissions.sql M installer/data/mysql/it-IT/necessari/userpermissions.sql M installer/data/mysql/nb-NO/1-Obligatorisk/userpermissions.sql M installer/data/mysql/pl-PL/mandatory/userpermissions.sql M installer/data/mysql/ru-RU/mandatory/permissions_and_user_flags.sql M installer/data/mysql/uk-UA/mandatory/permissions_and_user_flags.sql M installer/data/mysql/updatedatabase.pl M koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt M members/boraccount.pl M members/mancredit.pl M members/maninvoice.pl M members/pay.pl M members/paycollect.pl M members/printinvoice.pl Falling back to patching base and 3-way merge... Auto-merging members/printinvoice.pl CONFLICT (content): Merge conflict in members/printinvoice.pl Auto-merging members/paycollect.pl Auto-merging members/pay.pl Auto-merging members/maninvoice.pl Auto-merging members/mancredit.pl CONFLICT (content): Merge conflict in members/mancredit.pl Auto-merging members/boraccount.pl Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Auto-merging installer/data/mysql/uk-UA/mandatory/permissions_and_user_flags.sql Auto-merging installer/data/mysql/ru-RU/mandatory/permissions_and_user_flags.sql Auto-merging installer/data/mysql/pl-PL/mandatory/userpermissions.sql Auto-merging installer/data/mysql/nb-NO/1-Obligatorisk/userpermissions.sql Auto-merging installer/data/mysql/it-IT/necessari/userpermissions.sql Auto-merging installer/data/mysql/fr-FR/1-Obligatoire/userpermissions.sql Auto-merging installer/data/mysql/es-ES/mandatory/userpermissions.sql Auto-merging installer/data/mysql/en/mandatory/userpermissions.sql Auto-merging installer/data/mysql/de-DE/mandatory/userpermissions.sql Patch failed at 0001 Bug 9448 - Add separate permission for writing off fees The copy of the patch that failed is found in: /home/christopher/git/koha/.git/rebase-apply/patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort".
Created attachment 21732 [details] [review] Bug 9448 - Add separate permission for writing off fees Adds new sub-permissions to updatecharges, 'writeoff' for writing off fees and 'remaining_permissions' for all other fee related activites. Test plan: 1) Apply patch 2) Run updatedatabase.pl 3) Remove the 'writeoff' permission from your privileges 4) Attempt to write off a fee, you should not be able to 5) Re-add the 'writeoff' permission 6) Attempt to write off a fee, you should now be able to do so
Patch applied cleanly, go forth and signoff
Created attachment 23515 [details] [review] Bug 9448 - Add separate permission for writing off fees Adds new sub-permissions to updatecharges, 'writeoff' for writing off fees and 'remaining_permissions' for all other fee related activites. Test plan: 1) Apply patch 2) Run updatedatabase.pl 3) Remove the 'writeoff' permission from your privileges 4) Attempt to write off a fee, you should not be able to 5) Re-add the 'writeoff' permission 6) Attempt to write off a fee, you should now be able to do so
Patch tested with a sandbox, by Joel Sasse <jsasse@plumcreeklibrary.net>
Created attachment 23519 [details] [review] Bug 9448 - Add separate permission for writing off fees Adds new sub-permissions to updatecharges, 'writeoff' for writing off fees and 'remaining_permissions' for all other fee related activites. Test plan: 1) Apply patch 2) Run updatedatabase.pl 3) Remove the 'writeoff' permission from your privileges 4) Attempt to write off a fee, you should not be able to 5) Re-add the 'writeoff' permission 6) Attempt to write off a fee, you should now be able to do so Signed-off-by: Joel Sasse <jsasse@plumcreeklibrary.net>
Kyle, Since bug 6427 deletes all the files that this patch changes, is it still valid?
Yes, it's still valid unless 6427 manages to get pushed first. In either case I will have to add a followup to 6427 for this same behavior. This patch will at the very least implement the permission itself. Adding the limit to the new accounting system will be a fairly simple process. (In reply to Jonathan Druart from comment #12) > Kyle, > Since bug 6427 deletes all the files that this patch changes, is it still > valid?
QA comment: It works well but could be improved. The permissions in the .pl are not checked in the template files. So it could happen a link is displayed but the user is rejected if he clicks. Example: set circulate, catalogue and updatecharges>remaining_permissions. Go on a patron page, the 'Fines' tab is displayed on the left. Click on it, you are redirected to the loggin page with an error "Error: You do not have permission to access this page." Marked as Failed QA.
Jonathan, can you point out which template files you believe this is an issue for? I'm not seeing it. Kyle (In reply to Jonathan Druart from comment #14) > QA comment: > > It works well but could be improved. > The permissions in the .pl are not checked in the template files. So it > could happen a link is displayed but the user is rejected if he clicks. > Example: set circulate, catalogue and updatecharges>remaining_permissions. > Go on a patron page, the 'Fines' tab is displayed on the left. Click on it, > you are redirected to the loggin page with an error "Error: You do not have > permission to access this page." > > Marked as Failed QA.
(In reply to Kyle M Hall from comment #15) > Jonathan, can you point out which template files you believe this is an > issue for? I'm not seeing it. Did you try to reproduce my example?
Kyle: ping!
Created attachment 27235 [details] [review] Bug 9448 - Add separate permission for writing off fees Adds new sub-permissions to updatecharges, 'writeoff' for writing off fees and 'remaining_permissions' for all other fee related activites. Test plan: 1) Apply patch 2) Run updatedatabase.pl 3) Remove the 'writeoff' permission from your privileges 4) Attempt to write off a fee, you should not be able to 5) Re-add the 'writeoff' permission 6) Attempt to write off a fee, you should now be able to do so Signed-off-by: Joel Sasse <jsasse@plumcreeklibrary.net>
Created attachment 27236 [details] [review] Bug 9448 [QA Followup]
Created attachment 27237 [details] [review] [PASSED QA] Bug 9448 - Add separate permission for writing off fees Adds new sub-permissions to updatecharges, 'writeoff' for writing off fees and 'remaining_permissions' for all other fee related activites. Test plan: 1) Apply patch 2) Run updatedatabase.pl 3) Remove the 'writeoff' permission from your privileges 4) Attempt to write off a fee, you should not be able to 5) Re-add the 'writeoff' permission 6) Attempt to write off a fee, you should now be able to do so Signed-off-by: Joel Sasse <jsasse@plumcreeklibrary.net> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Comments on second patch.
Created attachment 27238 [details] [review] [PASSED QA] Bug 9448 [QA Followup] Rephrases permission description a bit to make it clearer and easier to translate. Adds German descriptions as requested by katrin. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes QA script and all tests. Tested with various permission combinations: - superlibrarian - updatecharges (full permission) - only 'remaining permissions for updating charges' It works as expected. Note: I think we need a follow up to include the 'Forgive overdue charges' checkbox on the check in page in the new permission.
Created attachment 27239 [details] [review] Bug 9448 [QA Followup] - Hide 'Forgive overdue charges' for returns if user does not have writeoff permission
Created attachment 27240 [details] [review] [PASSED QA] Bug 9448 [QA Followup] - Hide 'Forgive overdue charges' for returns if user does not have writeoff permission Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Works as described, template change only.
Pushed to master, along with a couple follow-ups to tweak wording and keep circ staff honest. Thanks, Kyle!
Hm, looking at permissions again today, I wonder if (10, 'remaining_permissions',... should be (10, 'updatecharge_remaining_permissions',... To match with ( 1, 'circulate_remaining_permissions', 'Remaining circulation permissions'), ( 3, 'parameters_remaining_permissions', 'Remaining system parameters permissions'),