Bug 10943 - Impossible to manage fines without borrowers permission
Summary: Impossible to manage fines without borrowers permission
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: Main
Hardware: PC Windows
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords: Academy
Depends on:
Blocks:
 
Reported: 2013-09-24 19:31 UTC by Marjorie Barry-Vila
Modified: 2023-10-16 08:44 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 10943 - Impossible to manage fines without borrowers permission Added check for borrowers permission as that is also required when accessing fines. (1.93 KB, patch)
2015-01-14 20:55 UTC, Thomas Wright
Details | Diff | Splinter 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. (2.01 KB, patch)
2015-01-18 22:21 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 10943: Manage files without borrowers permission (994 bytes, patch)
2018-01-17 04:01 UTC, Grace
Details | Diff | Splinter Review
permission borrowers not checked (133.04 KB, image/png)
2018-01-17 14:07 UTC, Marjorie Barry-Vila
Details
Tab Fines not displayed (53.14 KB, image/png)
2018-01-17 14:08 UTC, Marjorie Barry-Vila
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Marjorie Barry-Vila 2013-09-24 19:31:32 UTC
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
Comment 1 Katrin Fischer 2015-01-06 21:18:00 UTC
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 },
Comment 2 Marjorie Barry-Vila 2015-01-07 14:45:18 UTC
(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
Comment 3 Katrin Fischer 2015-01-08 06:54:53 UTC
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.
Comment 4 Thomas Wright 2015-01-14 01:38:05 UTC
I' working on this.
Comment 5 Thomas Wright 2015-01-14 20:55:27 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2015-01-18 22:21:02 UTC
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.
Comment 7 Katrin Fischer 2015-01-18 22:21:08 UTC
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?
Comment 8 Kyle M Hall 2015-01-30 12:51:20 UTC
(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.
Comment 9 Chris Cormack 2016-01-19 04:46:10 UTC
I agree and will get the students this year to switch it around
Comment 10 Kate Wenger 2017-07-26 22:11:46 UTC
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.
Comment 11 Grace 2018-01-17 04:01:40 UTC
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
Comment 12 Marjorie Barry-Vila 2018-01-17 14:06:29 UTC
(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?
Comment 13 Marjorie Barry-Vila 2018-01-17 14:07:29 UTC
Created attachment 70617 [details]
permission borrowers not checked
Comment 14 Marjorie Barry-Vila 2018-01-17 14:08:05 UTC
Created attachment 70618 [details]
Tab Fines not displayed
Comment 15 Victor Grousset/tuxayo 2018-03-12 14:06:32 UTC
~/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
Comment 16 Fridolin Somers 2020-11-10 10:04:36 UTC
There is a typo in message :
  Bug 10943: Manage files without borrowers permission
"files" is "fines" ^^
Comment 17 Fridolin Somers 2020-11-10 10:08:00 UTC
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
Comment 18 Fridolin Somers 2020-11-10 10:11:07 UTC
Bug 18403 changed 
  borrowers => 1
to
  borrowers => 'edit_borrowers'
Comment 19 Fridolin Somers 2020-11-10 10:16:06 UTC
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' },