[Bug 39657] New: Optionally block API actions for expired patrons
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Bug ID: 39657 Summary: Optionally block API actions for expired patrons Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: andrew@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: tomascohen@gmail.com When the API is used to perform actions on behalf of patrons, we should be able to block those actions when the patron is expired. This is similar to the BlockExpiredPatronOpacActions system preference, which gives the option to block holds, renewals, and/or ILL requests for expired patrons. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Catrina Berka <catrina@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |catrina@bywatersolutions.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Wally DesChamps <wdeschamps@prospertx.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wdeschamps@prospertx.gov --- Comment #1 from Wally DesChamps <wdeschamps@prospertx.gov> --- This is causing confusion and bad PR for us and needs to be corrected ASAP. This has been reported in Aspen (Jira). All restrictions and circ rules should be honored across the board. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 --- Comment #2 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I believe these are different things. A) Make the API another option in BlockExpiredPatronOpacActions. B) Make a BlockExpiredPatronOpacActions for API use. c) Update our APIs to work like the staff interface - enforcing what is configured in BlockExpiredPatronOpacActions. BlockExpiredPatronOpacActions = meaning the system preference and the patron category specific overrride. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | --- Comment #3 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Our API policy override schema is designed so we can add another checks that can block placing holds, and we can add individual overrides for them. I'll give this a shot. -- You are receiving this mail because: You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff Summary|Optionally block API |Block holds placed via the |actions for expired patrons |API when patron would be | |blocked from placing OPAC | |hold --- Comment #7 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- These patches pursue Katrin's option C, making the API behavior match what the patron would be able to do via the OPAC. Expanding this test plan a little for more detail: To test: 1. Apply this patches 2. Run: $ ktd --shell k$ yarn api:bundle k$ koha-plack --restart kohadev k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 3. Test the endpoint with the various scenarios using your favourite REST tool (Postman!) 4. Have an expired patron; confirm BlockExpiredPatronOpacActions syspref set to allow patrons to place hold via opac; confirm your patron can have holds placed for them via API 5a. Set your expired patron's category to block holds for expired patrons; confirm your patron cannot have a hold placed for them via the API 5b. Switch BlockExpiredPatronOpacActions syspref to block OPAC holds for all expired patrons; switch your expired patron's category back to following the syspref; confirm your patron still cannot have a hold placed for them via the API 6. Have a patron flagged "Gone no address." Confirm they cannot have a hold placed for them via the API 7. Repeat for a patron flagged "Lost card" 8. and a patron with debt greater than the maxoutstanding syspref 9. and a patron with a restriction 10. and a patron who already has as many holds as they're allowed by policy 11. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kkrueger@cuyahogalibrary.or | |g --- Comment #8 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- *** Bug 35703 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 --- Comment #9 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Andrew Fuerste-Henry from comment #7)
These patches pursue Katrin's option C, making the API behavior match what the patron would be able to do via the OPAC.
Expanding this test plan a little for more detail:
I would also add to the test plan, that all checks that block the hold from happening can be overridden with the `x-koha-override` header, and can be tested. The valid overrides are: * any * bad_address * card_lost * debt_limit * expired * hold_limit * restricted This is well documented on the spec. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #183108|0 |1 is obsolete| | --- Comment #10 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 183200 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183200&action=edit Bug 39657: Unit tests Signed-off-by: Kristi Krueger <kkrueger@cuyahogalibrary.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #183109|0 |1 is obsolete| | --- Comment #11 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 183201 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183201&action=edit Bug 39657: Spec changes Signed-off-by: Kristi Krueger <kkrueger@cuyahogalibrary.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #183110|0 |1 is obsolete| | --- Comment #12 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 183202 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183202&action=edit Bug 39657: Add more checks and overrides to hold creation endpoint This patch adds new behaviors to the `POST /holds` endpoint. It uses the framework we created in the past for overriding policy rules, and add new options: * expired * debt_limit * bad_address * card_lost * restricted * hold_limit Some status codes are changed from 403 to 409. This should be revisited accross the codebase, as I think we made a wrong choice. Happy to review in this bug. The feature makes use of the newly introduced `$patron->can_place_holds()` method, which accepts to be passed through the overrides. To test: 1. Apply this patches 2. Run: $ ktd --shell k$ yarn api:bundle k$ koha-plack --restart kohadev k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 3. Test the endpoint with the various scenarios using your favourite REST tool (Postman!) 4. Sign off :-D Signed-off-by: Kristi Krueger <kkrueger@cuyahogalibrary.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |40130 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40130 [Bug 40130] API should respect BlockExpiredPatronOpacActions for ILL requests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |40131 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40131 [Bug 40131] API should respect BlockExpiredPatronOpacActions for renewals -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #183200|0 |1 is obsolete| | --- Comment #13 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 183491 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183491&action=edit Bug 39657: Unit tests Signed-off-by: Kristi Krueger <kkrueger@cuyahogalibrary.org> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #183201|0 |1 is obsolete| | --- Comment #14 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 183492 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183492&action=edit Bug 39657: Spec changes Signed-off-by: Kristi Krueger <kkrueger@cuyahogalibrary.org> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #183202|0 |1 is obsolete| | --- Comment #15 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 183493 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183493&action=edit Bug 39657: Add more checks and overrides to hold creation endpoint This patch adds new behaviors to the `POST /holds` endpoint. It uses the framework we created in the past for overriding policy rules, and add new options: * expired * debt_limit * bad_address * card_lost * restricted * hold_limit Some status codes are changed from 403 to 409. This should be revisited accross the codebase, as I think we made a wrong choice. Happy to review in this bug. The feature makes use of the newly introduced `$patron->can_place_holds()` method, which accepts to be passed through the overrides. To test: 1. Apply this patches 2. Run: $ ktd --shell k$ yarn api:bundle k$ koha-plack --restart kohadev k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 3. Test the endpoint with the various scenarios using your favourite REST tool (Postman!) 4. Sign off :-D Signed-off-by: Kristi Krueger <kkrueger@cuyahogalibrary.org> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA QA Contact|testopia@bugs.koha-communit |martin.renvoize@openfifth.c |y.org |o.uk --- Comment #16 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- OK, I'm passing QA here.. but I do wonder a little about when we should be just allowing 'x-koha-override' headers and when we should actually require a confirmation as the checkouts endpoint does. With this case, the API consumer could just always opt to pass x-koha-override=any or list all the overrides with any request and as such, we're not actually blocking anything. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 --- Comment #17 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Martin Renvoize (ashimema) from comment #16)
OK, I'm passing QA here.. but I do wonder a little about when we should be just allowing 'x-koha-override' headers and when we should actually require a confirmation as the checkouts endpoint does.
With this case, the API consumer could just always opt to pass x-koha-override=any or list all the overrides with any request and as such, we're not actually blocking anything.
Just FTR, as we already chatted about this: I think in its current design this endpoint could be extended with the `confirmation flow` in a pretty straight-forward way. That said, the system we are calling the API from (Aspen) is asking Koha to actually *block* the requests. We are adding the individual overrides just so other API consumers can recreate the previous behavior if they find the need for that. Martin: if you think the confirmation flow is desirable for holds, please file a follow-up bug and I will be more than happy to give it a shot. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Status|Passed QA |Failed QA --- Comment #18 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Tomas, Tests are failing here ( t/db_dependent/api/v1/holds.t ), can you have a look? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 --- Comment #19 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- I don't reproduce the failures. Remember the API always returns 503 if there are pending upgrades on the DB, and as this patch changes the spec, it needs a `yarn api:bundle` and `koha-plack --restart kohadev` in KTD. If you share more details about the failure, I'll fix it ASAP. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main --- Comment #20 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Sorry Tomas, it was my mistake. Pushed to main for 25.11 Thanks everyone. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Keywords| |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 --- Comment #21 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- I noticed a bug when looking at the tests. Will file a follow-up bug as it will need regression tests and stuff. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |40254 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40254 [Bug 40254] Individual overrides not effective in POST /holds -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |40257 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40257 [Bug 40257] Add Confirmation Flow to POST /holds -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This development adds more release notes| |holdability checks to the | |`POST /holds` endpoint. | |Overrides are added for all | |of them: | | | |* bad_address | |* card_lost | |* debt_limit | |* expired | |* hold_limit | |* restricted | | | |Before this development, | |only `any` could be passed | |as an override. It will now | |have more granularity. Keywords|release-notes-needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |magnus@libriotech.no --- Comment #22 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- *** Bug 24841 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 carthur@slolibrary.org <carthur@slolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |carthur@slolibrary.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Chip Halvorsen <Chip.Halvorsen@WestlakeLibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |Chip.Halvorsen@WestlakeLibr | |ary.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 George Williams (NEKLS) <george@nekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |george@nekls.org --- Comment #23 from George Williams (NEKLS) <george@nekls.org> --- Could this be back-ported to 25.05 and maybe even 24.11 if possible? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 --- Comment #24 from Chip Halvorsen <Chip.Halvorsen@WestlakeLibrary.org> --- (In reply to George Williams (NEKLS) from comment #23)
Could this be back-ported to 25.05 and maybe even 24.11 if possible?
+1 for 24.11 back-port -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable Version(s)| |25.05.02 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 --- Comment #25 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Nice work everyone! Pushed to 25.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Status|Pushed to stable |Needs documenting --- Comment #26 from Fridolin Somers <fridolin.somers@biblibre.com> --- This needs Bug 40101 that impacts opac-reserve.pl I think we should not impact such an important feature in 24.11 LTS There may be side effects -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED CC| |david@davidnind.com --- Comment #27 from David Nind <david@davidnind.com> --- API-related change, no changes required to the manual. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39657 Bug 39657 depends on bug 40101, which changed state. Bug 40101 Summary: Add `Koha::Patron->can_place_holds` https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40101 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org