Bug 15571

Summary: reserveforothers permission does not remove Search to hold button from patron account
Product: Koha Reporter: Christopher Brannon <cbrannon>
Component: CirculationAssignee: Bugs List <koha-bugs>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: brendan, brianagreally, chris, f.demians, gmcharlt, jonathan.druart, julian.maurice, kyle.m.hall
Version: unspecifiedKeywords: Academy
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 15571 reserveforothers permission does not remove Search to hold button from patron account
[SIGNED-OFF] Bug 15571 reserveforothers permission does not remove Search to hold button from patron account
Bug 15571 reserveforothers permission does not remove Search to hold button from patron account
Bug 15571 reserveforothers permission does not remove Search to hold button from patron account
Bug 15571 [QA Followup] - Remove tabs

Description Christopher Brannon 2016-01-13 19:00:48 UTC
Deactivating the reserveforothers permission (or place_holds) effectively keeps staff from placing holds for patrons, but it should also remove the button from the patron screens (circulation.pl, moremember.pl, pay.pl, routing-lists.pl, notices.pl, statistics.pl, etc.).

Christopher
Comment 1 Katrin Fischer 2016-01-18 01:00:52 UTC
Adding a check for the permission in the template should work here.
Comment 2 Briana Greally 2016-01-18 22:39:04 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2016-01-20 13:07:22 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2016-01-20 16:40:15 UTC
Hi Briana,
There are 2 subpermissions in the reserveforothers permission:
- modify_holds_priority: Modify holds priority
- place_holds: Place holds for patrons

It would make more sense to test CAN_user_reserveforothers_place_holds instead.

Katrin, could you confirm? It seems that we currently test CAN_user_reserveforothers in other places, but it does not look good.
Comment 5 Briana Greally 2016-01-20 21:47:22 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2016-01-21 09:31:07 UTC
I think place_holds is a good pick!
Comment 7 Kyle M Hall (khall) 2016-01-22 17:41:22 UTC
Created attachment 47210 [details] [review]
Bug 15571 reserveforothers permission does not remove Search to hold button from patron account

To Test:
1. Create a user without 'reserveforothers' permission
2. Go onto user details
3. 'Search to hold' button should not be there
4. Create a user with 'reserveforothers' permission
5. Repeat steps
6. 'Search to Hold' button should be there

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Briana <brianagreally@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 8 Kyle M Hall (khall) 2016-01-22 17:41:30 UTC
Created attachment 47211 [details] [review]
Bug 15571 [QA Followup] - Remove tabs

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Jonathan Druart 2016-01-25 10:01:32 UTC
Kyle, did you read comments 4 and 6?
Comment 10 Katrin Fischer 2016-01-25 10:13:19 UTC
I think they might have accidentally uploaded the wrong patch or forgot to include the change mentioned in comment#5. 
Can we fix this in a little follow-up please?
Comment 11 Jonathan Druart 2016-01-25 10:22:42 UTC
This subpermission is never checked actually.
So either we only test for CAN_user_reserveforothers everywhere or we change all the appropriate occurrences with CAN_user_reserveforothers_place_holds
Comment 12 Katrin Fischer 2016-01-25 10:23:41 UTC
Hm, so maybe push this patch as is and then to a general clean-up separately?
Comment 13 Jonathan Druart 2016-01-25 11:02:13 UTC
yep, agreed.
Comment 14 Brendan Gallagher 2016-01-27 05:41:47 UTC
Pushed to Master - Should be in the May 2016 release. Thanks!
Comment 15 Julian Maurice 2016-02-19 08:46:13 UTC
Patches pushed to 3.22.x, will be in 3.22.4
Comment 16 Frédéric Demians 2016-02-20 08:55:01 UTC
This patch has been pushed to 3.20.x, will be in 3.20.9.