[Bug 40101] New: Add `Koha::Patron->can_place_holds`
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40101 Bug ID: 40101 Summary: Add `Koha::Patron->can_place_holds` Change sponsored?: --- Product: Koha Version: Main Hardware: PC OS: Mac OS Status: NEW Severity: enhancement Priority: P1 - high Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com Some checks for patron's ability to place holds are baked in * controllers (e.g. opac-reserve.pl, request.pl) * templates (e.g. opac-reserve.tt, request.tt) We should move that logic into a reusable sub, covered with tests. My initial motivation is bug 39657 so we use this on the API, but it is a good opportunity to start cleaning up the codebase in the area as well. -- You are receiving this mail because: You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40101 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- OS|Mac OS |All Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | CC| |andrew@bywatersolutions.com | |, | |jonathan.druart@gmail.com, | |martin.renvoize@openfifth.c | |o.uk, | |nick@bywatersolutions.com, | |tomascohen@gmail.com -- You are receiving this mail because: You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40101 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #183104|0 |1 is obsolete| | --- Comment #4 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 183197 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183197&action=edit Bug 40101: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=40101 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #183105|0 |1 is obsolete| | --- Comment #5 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 183198 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183198&action=edit Bug 40101: Add `Koha::Patron->can_place_holds()` This patch adds a new method to the `Koha::Patron` class. As described in the POD, this method makes checks on the patron's current situation and returns a boolean telling if it should be allowed to place holds. This doesn't check specific item/biblio holdability. It only covers patron specific conditions that could prevent holds to be placed. Both in `opac-reserve.pl` and `reserve/request.pl` this checks are all run, and all blocking situations are presented to the UI user. Feedback in community (so far) expressed this is a desirable situation, so that behavior can be retained with the use of the `no_short_circuit` parameter. If `no_short_circuit` is not passed, then the routine will return on the first blocking condition it finds. An `overrides` parameter is added, allowing to be passed a list of strings that match the possible error messages. This way, this method can be asked not to check for patron expiration, for example, and things like that. This is of course designed with the API overrides use case in mind. To test: 1. Apply this patches 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Patron.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=40101 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #183106|0 |1 is obsolete| | --- Comment #6 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 183199 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183199&action=edit Bug 40101: Make opac-reserve.pl use `$patron->can_place_holds()` Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=40101 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=40101 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=33086 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40101 --- Comment #7 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 183204 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183204&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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40101 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #183204|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40101 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Medium patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40101 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #183197|0 |1 is obsolete| | Attachment #183198|0 |1 is obsolete| | Attachment #183199|0 |1 is obsolete| | --- Comment #8 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 183242 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183242&action=edit Bug 40101: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=40101 --- Comment #9 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 183243 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183243&action=edit Bug 40101: Add `Koha::Patron->can_place_holds()` This patch adds a new method to the `Koha::Patron` class. As described in the POD, this method makes checks on the patron's current situation and returns a boolean telling if it should be allowed to place holds. This doesn't check specific item/biblio holdability. It only covers patron specific conditions that could prevent holds to be placed. Both in `opac-reserve.pl` and `reserve/request.pl` this checks are all run, and all blocking situations are presented to the UI user. Feedback in community (so far) expressed this is a desirable situation, so that behavior can be retained with the use of the `no_short_circuit` parameter. If `no_short_circuit` is not passed, then the routine will return on the first blocking condition it finds. An `overrides` parameter is added, allowing to be passed a list of strings that match the possible error messages. This way, this method can be asked not to check for patron expiration, for example, and things like that. This is of course designed with the API overrides use case in mind. To test: 1. Apply this patches 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Patron.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=40101 --- Comment #10 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 183244 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183244&action=edit Bug 40101: Make opac-reserve.pl use `$patron->can_place_holds()` Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=40101 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |29746 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29746 [Bug 29746] Add a handy Koha::Result::Boolean class -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40101 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |40237 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40237 [Bug 40237] Update reserves/request.pl to use Koha::Patron->can_place_holds -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40101 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #183242|0 |1 is obsolete| | --- Comment #11 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 183488 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183488&action=edit Bug 40101: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=40101 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #183243|0 |1 is obsolete| | --- Comment #12 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 183489 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183489&action=edit Bug 40101: Add `Koha::Patron->can_place_holds()` This patch adds a new method to the `Koha::Patron` class. As described in the POD, this method makes checks on the patron's current situation and returns a boolean telling if it should be allowed to place holds. This doesn't check specific item/biblio holdability. It only covers patron specific conditions that could prevent holds to be placed. Both in `opac-reserve.pl` and `reserve/request.pl` this checks are all run, and all blocking situations are presented to the UI user. Feedback in community (so far) expressed this is a desirable situation, so that behavior can be retained with the use of the `no_short_circuit` parameter. If `no_short_circuit` is not passed, then the routine will return on the first blocking condition it finds. An `overrides` parameter is added, allowing to be passed a list of strings that match the possible error messages. This way, this method can be asked not to check for patron expiration, for example, and things like that. This is of course designed with the API overrides use case in mind. To test: 1. Apply this patches 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Patron.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=40101 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #183244|0 |1 is obsolete| | --- Comment #13 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 183490 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=183490&action=edit Bug 40101: Make opac-reserve.pl use `$patron->can_place_holds()` Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=40101 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |40238 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40238 [Bug 40238] ILSDI needs updating to use Koha::Patron->can_place_hold method -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40101 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #14 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- I've added two further follow-up bugs that need to be attended to in due course for the missing refactoring of the codebase to utilise this new method in a wider context. Otherwise, however, I'm happy to PQA on this bug as is. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40101 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- URL|https://github.com/bywaters | |olutions/bywater-koha-devel | |/tree/bug_40101 | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40101 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |martin.renvoize@openfifth.c | |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40101 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |25.11.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40101 --- Comment #15 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 25.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40101 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |release-notes-needed CC| |lucas@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40101 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable Version(s)|25.11.00 |25.11.00,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=40101 --- Comment #16 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=40101 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Status|Pushed to stable |Needs documenting --- Comment #17 from Fridolin Somers <fridolin.somers@biblibre.com> --- Enhancement not pushed to 24.11.x Great stuff :D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40101 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Needs documenting |RESOLVED CC| |david@davidnind.com --- Comment #18 from David Nind <david@davidnind.com> --- 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=40101 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=40101 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |33086 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33086 [Bug 33086] Allow to prevent holds at OPAC for a patron with overdues outstanding -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org