Bug 17765 - Hide 'Holds' link in staff results list
Summary: Hide 'Holds' link in staff results list
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low minor (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords: Academy
Depends on:
Blocks:
 
Reported: 2016-12-13 06:50 UTC by Katrin Fischer
Modified: 2020-01-06 20:14 UTC (History)
4 users (show)

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


Attachments
Bug 17765: Hid 'Holds' link on the 'Details' and 'Checkout' pages when the user does not have the permission to 'reserveforothers'. (6.78 KB, patch)
2018-01-16 23:22 UTC, Zoe Bennett
Details | Diff | Splinter Review
Bug 17765: Changed variable $flags to $userflags to eliminate the issue of masking declaration in the same scope. Hides 'Holds' link on the 'Details' and 'Checkout' pages when the user does not have the permission to 'reserveforothers'. (2.22 KB, patch)
2018-01-18 01:50 UTC, Zoe Bennett
Details | Diff | Splinter Review
Screenshot of results page (35.14 KB, image/png)
2019-01-17 21:26 UTC, Jack
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2016-12-13 06:50:05 UTC
The "Holds" link on the staff result list is still visible, even if the  user has no "reserveforothers" permission. 

At the same time it's not possible to place a hold from the detail page and you get a permission error when trying to use the link.
Comment 1 Zoe Bennett 2018-01-16 23:22:29 UTC
Created attachment 70584 [details] [review]
Bug 17765: Hid 'Holds' link on the 'Details' and 'Checkout' pages when the user does not have the permission to 'reserveforothers'.

Test Plan:
1) With a user who has the permission to 'reserveforothers' check that the 'Holds' link is on the 'Details' and 'Check out' pages
2) Then with a user who does not have the permission to 'reserveforothers' check that on the 'Details' and 'Check out' pages there is no longer the 'Holds' link next to the 'Checkouts' and 'Fines & Charges' tabs.
Comment 2 Owen Leonard 2018-01-17 13:27:54 UTC
I'm not sure if this is ready for testing or not, but in it's current state it doesn't work for me:

"my" variable $flags masks earlier declaration in same scope at /home/vagrant/kohaclone/circ/circulation.pl line 464. Can't call method "flags" on an undefined value at /home/vagrant/kohaclone/circ/circulation.pl line 179.
Comment 3 Zoe Bennett 2018-01-18 01:50:10 UTC
Created attachment 70662 [details] [review]
Bug 17765: Changed variable $flags to $userflags to eliminate the issue of masking declaration in the same scope. Hides 'Holds' link on the 'Details' and 'Checkout' pages when the user does not have the permission to 'reserveforothers'.

Test Plan:
1) With a user who has the permission to 'reserveforothers' check that the 'Holds' link is on the 'Details' and 'Check out' pages
2) Then with a user who does not have the permission to 'reserveforothers' check that on the 'Details' and 'Check out' pages there is no longer the 'Holds' link next to the 'Checkouts' and 'Fines & Charges' tabs.
Comment 4 Katrin Fischer 2018-01-18 06:02:45 UTC
Hi Zoe, small thing: I filed this for the result page - can you change the title of the bug to match your changes and maybe file a new one for the result list page if the problem still shows there? The titles of bugs are used for the release notes, so it's always good to give a good description.
Comment 5 Katrin Fischer 2018-05-20 00:25:56 UTC
I feel like the patches proposed here don't do the right thing. A staff patron without place_holds should still be able to see information about existing holds on the circ desk. 
If people disagree, we should discuss how to achieve this without causing a disruption for libraries, maybe using a new separate pref? Marking 'In discussion' for now.
Comment 6 Aleisha Amohia 2019-01-17 02:54:39 UTC
The 'Holds(In reply to Katrin Fischer from comment #5)
> I feel like the patches proposed here don't do the right thing. A staff
> patron without place_holds should still be able to see information about
> existing holds on the circ desk. 
> If people disagree, we should discuss how to achieve this without causing a
> disruption for libraries, maybe using a new separate pref? Marking 'In
> discussion' for now.

I agree with this. We must have misunderstood the original bug.

However, the original bug still stands. I'd like to change this bug description back to the original and have a 2019 academy student write the correct fix for that, because the current patch isn't desired behaviour.
Comment 7 Jack 2019-01-17 21:26:29 UTC
Created attachment 84165 [details]
Screenshot of results page

In master, when I log in with a user that does not have reserveforothers permissions, the place hold button no longer appears, and the holds text is no longer a link, so I think this may have been fixed in a different patch.