Bug 14713 - updatecharges permissions also require borrowers permission
Summary: updatecharges permissions also require borrowers permission
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low trivial (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-08-24 16:47 UTC by Amy Schapiro
Modified: 2023-10-16 08:44 UTC (History)
9 users (show)

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


Attachments
Bug 14713: Add hint that updatecharges permissions require ability to view patron details (1.93 KB, patch)
2020-04-18 14:59 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14713: Add hint that updatecharges permissions require ability to view patron details (1.98 KB, patch)
2020-04-20 09:27 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Amy Schapiro 2015-08-24 16:47:04 UTC
Partner reports that:

One of our institutions would like students to be able to collect fines, but not waive fines. You would think selecting the "remaining_permissions" permission under the "updatecharges" permission would allow you to collect fines and fees without waiving them because "writeoff" fines and fees is a separate permission, but it is not.

They can't write off any fines- which is what we want. The problem is, they can't collect fines either, and we do want them to be able to do that.

We tested this and it seems to be true that even using the correct settings under this permission the student cannot collect fines, though they are prevented from waiving them as anticipated. 

On the student account permissions the remaining permissions is checked and write off is not selected.
Comment 1 Theodoros Theodoropoulos 2015-09-30 09:57:40 UTC
We had the same issue and found out that in order for fine collection to work, 

 (borrowers) Add or modify patrons 

permission must also be checked!

Can you please verify it? 
If so, this finding should could help the person who deals with the case identify the problem. If this is by design (although these permissions should work independently!), a note should be added in the relevant docs.
Comment 2 Karl Holten 2015-10-12 18:34:04 UTC
(In reply to Theodoros Theodoropoulos from comment #1)
> We had the same issue and found out that in order for fine collection to
> work, 
> 
>  (borrowers) Add or modify patrons 
> 
> permission must also be checked!
> 
> Can you please verify it? 
> If so, this finding should could help the person who deals with the case
> identify the problem. If this is by design (although these permissions
> should work independently!), a note should be added in the relevant docs.

We can confirm that what Theodoros is saying about this bug is true. Enabling the ability to add or modify patrons allowed our staff members to write-off fines.

I agree with Theodoros that these permissions should indeed be made wholly separate ideally, or documented if making them separate is a major project.
Comment 3 Julian Maurice 2016-02-09 14:24:39 UTC
Which version of Koha do you use ?
Tested on current master with a borrower that have only permissions: 'catalogue', 'borrowers' and 'updatecharges' > 'remaining_permissions'.
I can't write off any fine, but I can collect fine.

Also, I think the requirement for borrowers permission is by design, because how can you access borrower's fines without it ?
By the way, 'borrowers' permission's description has been reworded to "Add, modify and view patron information".
Comment 4 Karl Holten 2016-03-01 18:47:20 UTC
(In reply to Julian Maurice from comment #3)
> Which version of Koha do you use ?

We use 3.20.

> Tested on current master with a borrower that have only permissions:
> 'catalogue', 'borrowers' and 'updatecharges' > 'remaining_permissions'.
> I can't write off any fine, but I can collect fine.
>
> Also, I think the requirement for borrowers permission is by design, because
> how can you access borrower's fines without it ?

I've tested an account with the same permissions on our server and it is the same- you can't waive fines but you can collect them. So, the initial problem reported is no longer the issue... but I would say there's still an issue here, which is that the relationship between the borrowers permission and the ability to collect charges isn't really documented anywhere or made obvious by making the fines subpermissions of the borrowers permission.

I'd also argue that the ability to edit patron records should probably be made a sub-permission of borrowers as well, so that view and edit are different, but likely a separate ticket.
Comment 5 Fridolin Somers 2017-05-23 09:59:56 UTC
I see that pages :
members/pay.pl
members/paycollect.pl
Need permissions from "updatecharges" but also full permissions from "borrowers" :
  flagsrequired   => { borrowers => 1, updatecharges => $updatecharges_permissions },
Comment 6 Katrin Fischer 2020-04-18 14:59:58 UTC
Created attachment 103205 [details] [review]
Bug 14713: Add hint that updatecharges permissions require ability to view patron details

A staff usre with only updatecharges permissions or sub permissions but without
the ability to view patron details (borrowers permissions and sub permissions)
will not be able to access the fines and charges functionalities.

This patch adds a textual hint about the dependency to the permissions screen.

To test:
- Verify behaviour: Add a staff user with
  - updatecharges permissions
  - no borrowers permissions
- Try to access a patron fines record in the GUI
- Try by direct URL: cgi-bin/koha/members/pay.pl?borrowernumber=17
- Add borrowers permissions - everything should be accessible now
- Apply patch
- View the permissions of a staff user
- Verify there is now a note stating this fact
Comment 7 David Nind 2020-04-20 09:27:42 UTC
Created attachment 103238 [details] [review]
Bug 14713: Add hint that updatecharges permissions require ability to view patron details

A staff user with only updatecharges permissions or sub permissions but without
the ability to view patron details (borrowers permissions and sub permissions)
will not be able to access the fines and charges features.

This patch adds a textual hint about the dependency to the permissions screen.

To test:
- Verify behaviour: Add a staff user with
  - updatecharges permissions
  - no borrowers permissions
- Try to access a patron fines record in the GUI
- Try by direct URL: cgi-bin/koha/members/pay.pl?borrowernumber=17
- Add borrowers permissions - everything should be accessible now
- Apply patch
- View the permissions of a staff user
- Verify there is now a note stating this fact

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 Jonathan Druart 2020-04-20 11:44:34 UTC
Katrin, I think it's not correct for all the subperms.

I found, at least:

koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
 750                                     [% IF ( CAN_user_updatecharges_writeoff && overduecharges ) %]

to display "Forgive overdue charges"
Comment 9 Katrin Fischer 2020-05-23 12:38:28 UTC
(In reply to Jonathan Druart from comment #8)
> Katrin, I think it's not correct for all the subperms.
> 
> I found, at least:
> 
> koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
>  750                                     [% IF (
> CAN_user_updatecharges_writeoff && overduecharges ) %]
> 
> to display "Forgive overdue charges"

Hm, I think this is a somewhat special case that will also require a different permission (circulate). I could amend the note maybe:

From:
Requires permissions to view patron details.

To:

Requires permissions to view patron details or circulation.
Comment 10 Katrin Fischer 2020-08-14 13:22:59 UTC
How should we handle this? 

"Requires permission to view patron details in some cases"

?
Comment 11 Katrin Fischer 2021-08-24 10:03:14 UTC
I am not sure how to resolve this. I've tried to rephrase the note, but it gets complicated fast and doesn't look nice in the template with too much additional text. 

I think if we want to do this, we need to take a much closer look at permission dependencies in general and figure out a nice way to display these.