Staff cannot place a request for a patron club if the staff member does not have modify_holds_priority set.
That's no good. I've also found that staff can't see the button for the tools page if the clubs is the only tool they have permission too. But that is a different bug. This definitely needs to be fixed. I don't like loop holes in permissions.
Still valid in 21.11.11. Technically place_holds is also required to get that far so it may be that the overarching reserveforothers permission is what's truly required to successfully place a club hold. The ability to place club holds should really be its own separate user permission nested under the reserveforothers (or possibly clubs?) permission group. In our club usage scenario, a mistaken club hold can lead to hundreds of a holds on a record which puts many items in motion to patrons that don't actually want them. I wrote some JS to hide the "Clubs" tab at request.pl with exception for the select staff users in charge of placing club holds. It also changes the label wording from "Search patrons or clubs" to just "Search patrons". var loggedUser = $("#logged-in-info-full .loggedinusername").text(); var clubbers = ["staffusername1", "staffusername2"]; //show for these users if ($.inArray(loggedUser, clubbers) == -1) { $('#circ_request [href="#holds_clubsearch_pane"]').closest('li').hide(); $('#circ_request label:contains("Search patrons or clubs")').text("Search patrons"); }
This bizarre requirement is still around in 24.05. I had to ask around to even figure out this permission was related as the name and the function we're attempting to do (place club holds) aren't related on the surface. If it happens to be fixed in 24.11 I will come back and update this bug (I'll find out in June 2025)
Hi Trevor, commenting on the bug and confirming it#s still an issue in a current version is very helpful. I can't promise you a fix, but I'll highlight it in the list on the dashboard with my keyword.
As George suggested, the API route expects full reserveforothers api/v1/swagger/paths/clubs.yaml: 94 x-koha-authorization: 95 permissions: 96 reserveforothers: "1"
Created attachment 179633 [details] [review] Bug 25787: Update club holds to only require place_holds permission This moves the permission check from top level to the specific place_hold permission To test: 1 - Create a club 2 - Enroll some patrons 3 - Find or create a staff user with 'place_holds' permission and catalogue permission 4 - Log in as that staff 5 - Find a book and go to holds tab' 6 - Search for your club 7 - Try to place hold 8 - Nothing happens FIXME we should alert of the 403 error or other 9 - Apply patch, yanr build, restart_all 10 - Place hold 11 - Success!
Partial patch attached, needs unit tests
Created attachment 181831 [details] [review] Bug 25787: Add unit tests prove -v t/db_dependent/api/v1/clubs_holds.t
Created attachment 181841 [details] [review] Bug 25787: Update club holds to only require place_holds permission This moves the permission check from top level to the specific place_hold permission To test: 1 - Create a club 2 - Enroll some patrons 3 - Find or create a staff user with 'place_holds' permission and catalogue permission 4 - Log in as that staff 5 - Find a book and go to holds tab' 6 - Search for your club 7 - Try to place hold 8 - Nothing happens FIXME we should alert of the 403 error or other 9 - Apply patch, yanr build, restart_all 10 - Place hold 11 - Success! Signed-off-by: David Nind <david@davidnind.com>
Created attachment 181842 [details] [review] Bug 25787: Add unit tests prove -v t/db_dependent/api/v1/clubs_holds.t Signed-off-by: David Nind <david@davidnind.com>
One thing I noticed on the holds page (before and after the patch) is that the autocomplete for the club ID or partial name on the holds page doesn't work for me. Most likely, another bug that needs fixing! Testing notes (suing KTD): 1. To enroll patrons in a club: 1.1 Go to a patron's page in the staff interface 1.2 On either their check out or details page, select the "Clubs" tab 1.3 Click "Enroll" action to enroll them in the club
Created attachment 181985 [details] [review] Bug 25787: Update club holds to only require place_holds permission This moves the permission check from top level to the specific place_hold permission To test: 1 - Create a club 2 - Enroll some patrons 3 - Find or create a staff user with 'place_holds' permission and catalogue permission 4 - Log in as that staff 5 - Find a book and go to holds tab' 6 - Search for your club 7 - Try to place hold 8 - Nothing happens FIXME we should alert of the 403 error or other 9 - Apply patch, yanr build, restart_all 10 - Place hold 11 - Success! Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Created attachment 181986 [details] [review] Bug 25787: Add unit tests prove -v t/db_dependent/api/v1/clubs_holds.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Passing QA here. (In reply to Jason Robb from comment #2) > The ability to place club holds should really be its own separate user > permission nested under the reserveforothers (or possibly clubs?) permission > group. This is a valid point, and likely worth to open a follow-up bug for it (enhancement). (In reply to David Nind from comment #11) > One thing I noticed on the holds page (before and after the patch) is that > the autocomplete for the club ID or partial name on the holds page doesn't > work for me. > > Most likely, another bug that needs fixing! I also noticed this, but I'm not sure it's a bug. I think it's a feature that's missing! I may be wrong, but it appears this club input was not designed to have autocomplete in the first place!
(In reply to Pedro Amorim from comment #14) > Passing QA here. > > (In reply to Jason Robb from comment #2) > > The ability to place club holds should really be its own separate user > > permission nested under the reserveforothers (or possibly clubs?) permission > > group. > > This is a valid point, and likely worth to open a follow-up bug for it > (enhancement). > > (In reply to David Nind from comment #11) > > One thing I noticed on the holds page (before and after the patch) is that > > the autocomplete for the club ID or partial name on the holds page doesn't > > work for me. > > > > Most likely, another bug that needs fixing! > > I also noticed this, but I'm not sure it's a bug. I think it's a feature > that's missing! I may be wrong, but it appears this club input was not > designed to have autocomplete in the first place! Can you please open the bugs mentioned here? Thanks!
Pushed for 25.05! Well done everyone, thank you!
I believe this broke the test on D12: t_db_dependent_api_v1_clubs_holds_t Can you please double check and provide a fix?
(In reply to Katrin Fischer from comment #17) > I believe this broke the test on D12: > > t_db_dependent_api_v1_clubs_holds_t > > Can you please double check and provide a fix? I can't reproduce this failure locally.
Hm, this one here is another candidate: 15:52:20 koha_1 | # Looks like you failed 1 test of 3. 15:52:20 koha_1 | [13:50:45] t/db_dependent/api/v1/clubs_holds.t
Can't reproduce this failure locally either.
Fixed the club related tests now it looks like!
(In reply to Katrin Fischer from comment #15) > (In reply to Pedro Amorim from comment #14) > > Passing QA here. > > > > (In reply to Jason Robb from comment #2) > > > The ability to place club holds should really be its own separate user > > > permission nested under the reserveforothers (or possibly clubs?) permission > > > group. > > > > This is a valid point, and likely worth to open a follow-up bug for it > > (enhancement). > > > > (In reply to David Nind from comment #11) > > > One thing I noticed on the holds page (before and after the patch) is that > > > the autocomplete for the club ID or partial name on the holds page doesn't > > > work for me. > > > > > > Most likely, another bug that needs fixing! > > > > I also noticed this, but I'm not sure it's a bug. I think it's a feature > > that's missing! I may be wrong, but it appears this club input was not > > designed to have autocomplete in the first place! > > Can you please open the bugs mentioned here? Thanks! Bugs created: 1. Bug 39873 - Patron club holds - separate permission for ability to place club holds 2. Bug 39872 - Patron club holds - autocomplete does not work as expected (like patron name autocomplete)