When a borrower has those flags: -circulate -catalog -reserveforothers -borrow -updatecharges He can not make and check fines on a other borrower. He can make check in and check out but he has not a access to the fines tab. See video. http://communaute.koha.ccsr.qc.ca/wp-content/uploads/2013/09/flags.swf Thanks Marjorie
It appears you also need the borrowers permission - I am not sure if this is intentional: pay.pl: 53 flagsrequired => { borrowers => 1, updatecharges => $updatecharges_permissions },
(In reply to Katrin Fischer from comment #1) > It appears you also need the borrowers permission - I am not sure if this is > intentional: > > pay.pl: > 53 flagsrequired => { borrowers => 1, updatecharges => > $updatecharges_permissions }, Thanks Katrin. It works with borrowers permission. Marjorie
I think we should think about changing this - or make all checks consistent, like not showing the fines tab when the user doesn't have all needed permissions.
I' working on this.
Created attachment 35261 [details] [review] Bug 10943 - Impossible to manage fines without borrowers permission Added check for borrowers permission as that is also required when accessing fines. Test Plan: -Search for a patron from an account without the 'borrowers' permission and with the 'updatecharges' permission. *The fines tab should not be present. -Update the account's permissions to include the 'borrowers' permission *The fines tab should now be present. -Update the account's permissions so it has neither 'borrowers' or 'updatecharges' *The fines tab should not be present
Created attachment 35355 [details] [review] [SIGNED OFF] Bug 10943 - Impossible to manage fines without borrowers permission Added check for borrowers permission as that is also required when accessing fines. Test Plan: -Search for a patron from an account without the 'borrowers' permission and with the 'updatecharges' permission. *The fines tab should not be present. -Update the account's permissions to include the 'borrowers' permission *The fines tab should now be present. -Update the account's permissions so it has neither 'borrowers' or 'updatecharges' *The fines tab should not be present Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Works as described.
Hm, wondering if the other way around would make more sense: Allow paying fines without the permission to manage borrowers. I think the permission page doesn't suggest that you'd need to combine both. What do people think?
(In reply to Katrin Fischer from comment #7) > Hm, wondering if the other way around would make more sense: > Allow paying fines without the permission to manage borrowers. > > I think the permission page doesn't suggest that you'd need to combine both. > > What do people think? I would agree with Katrin. It should be possible to deal with fines and charges while not having the ability to edit the patron's details. That gives more flexibility for permissions, whereas this implementation gives less flexibility.
I agree and will get the students this year to switch it around
Just wanted to add another vote to this! Being able to assign permission to manage fines without having to also allow permission to add and modify patron information would be helpful.
Created attachment 70597 [details] [review] Bug 10943: Manage files without borrowers permission Test Plan: - If the patch is correctly applied then the fines tab should be present without the 'borrowers' permission as long as it has the update charges permission
(In reply to Grace from comment #11) > Created attachment 70597 [details] [review] [review] > Bug 10943: Manage files without borrowers permission > > Test Plan: > - If the patch is correctly applied then the fines tab should be present > without the 'borrowers' permission as long as it has the update charges > permission Hi Grace, I tested on sandbox and i do not see tab Fines if borrower permission is not checked. See attached screenshots. Do I understand your test plan?
Created attachment 70617 [details] permission borrowers not checked
Created attachment 70618 [details] Tab Fines not displayed
~/d/koha ❯❯❯ git bz apply 10943 Bug 10943 - Impossible to manage fines without borrowers permission 35355 - [SIGNED OFF] Bug 10943 - Impossible to manage fines without borrowers permission Added check for borrowers permission as that is also required when accessing fines. 70597 - Bug 10943: Manage files without borrowers permission Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 10943 - Impossible to manage fines without borrowers permission Added check for borrowers permission as that is also required when accessing fines. Using index info to reconstruct a base tree... M koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc error: Failed to merge in the changes. Patch failed at 0001 Bug 10943 - Impossible to manage fines without borrowers permission Added check for borrowers permission as that is also required when accessing fines. The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/SIGNED-OFF-Bug-10943---Impossible-to-manage-fines--VbHqCV.patch
There is a typo in message : Bug 10943: Manage files without borrowers permission "files" is "fines" ^^
From comments I understand that first patch " [SIGNED OFF] Bug 10943 - Impossible to manage fines without borrowers permission Added check for borrowers permission as that is also required when accessing fines." is obsolete
Bug 18403 changed borrowers => 1 to borrowers => 'edit_borrowers'
I set to in discussion because all pages about patron accounting need permission "edit_borrowers". So we should reconsider dropping it in pay.pl. See : $ git grep -B1 updatecharges members/ members/accountline-details.pl- borrowers => 'edit_borrowers', members/accountline-details.pl: updatecharges => 'remaining_permissions' -- members/boraccount.pl- flagsrequired => { borrowers => 'edit_borrowers', members/boraccount.pl: updatecharges => 'remaining_permissions'}, -- members/mancredit.pl- borrowers => 'edit_borrowers', members/mancredit.pl: updatecharges => 'remaining_permissions' -- members/maninvoice.pl- borrowers => 'edit_borrowers', members/maninvoice.pl: updatecharges => 'remaining_permissions' -- members/pay.pl- members/pay.pl:my $updatecharges_permissions = $input->param('woall') ? 'writeoff' : 'remaining_permissions'; -- members/pay.pl- type => 'intranet', members/pay.pl: flagsrequired => { borrowers => 'edit_borrowers', updatecharges => $updatecharges_permissions }, -- members/paycollect.pl- members/paycollect.pl:my $updatecharges_permissions = ($writeoff_individual || $type eq 'WRITEOFF') ? 'writeoff' : 'remaining_permissions'; -- members/paycollect.pl- type => 'intranet', members/paycollect.pl: flagsrequired => { borrowers => 'edit_borrowers', updatecharges => $updatecharges_permissions }, -- members/printfeercpt.pl- type => "intranet", members/printfeercpt.pl: flagsrequired => {borrowers => 'edit_borrowers', updatecharges => 'remaining_permissions'}, -- members/printinvoice.pl- type => "intranet", members/printinvoice.pl: flagsrequired => { borrowers => 'edit_borrowers', updatecharges => 'remaining_permissions' },