[Bug 43126] New: REST API: add holdability endpoints for biblios, items and patrons
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43126 Bug ID: 43126 Summary: REST API: add holdability endpoints for biblios, items and patrons Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@openfifth.co.uk QA Contact: testopia@bugs.koha-community.org CC: tomascohen@gmail.com Depends on: 43124 Blocks: 43123 Target Milestone: --- Add new REST endpoints built on Koha::Biblio::Availability::Hold and Koha::Item::Availability::Hold (bug 43124). GET /biblios/{biblio_id}/holdability, a single call biblio level holdability verdict plus an item availability summary, replacing the current CanBookBeReserved plus per-item loop pattern. GET /items/{item_id}/holdability, a per-item holdability verdict. Pickup locations are only included when include_pickup_locations=true is passed. POST /biblios/{biblio_id}/holdability/batch, checks holdability for multiple patrons (club holds) or multiple items in a single call. GET /patrons/{patron_id}/hold_eligibility, patron level blockers (debt, restricted, expired, etc) independent of any specific biblio. This is a thin new controller action over the existing Koha::Patron->can_place_holds method, not new backend logic. Also enhance the existing GET /biblios/{biblio_id}/items endpoint with an optional holdability parameter that embeds a per-item holdability object when true and a patron_id is supplied, alongside pagination. Response bodies for the new holdability endpoints serialise the blockers, confirmations and warnings shape of Koha::Result::Availability rather than a bespoke status enum schema. All existing hold endpoints (POST /holds, PATCH and DELETE /holds/{hold_id}, pickup_locations endpoints, suspension_bulk, cancellation_bulk) are unchanged. Test plan 1. Add OpenAPI definitions for each new endpoint under api/v1/swagger. 2. Add controller tests under t/db_dependent/api/v1 for each endpoint, covering holdable, non-holdable and override-required cases. 3. Confirm GET /biblios/{biblio_id}/holdability for a 100 item biblio completes in well under 1 second and issues a small, roughly constant number of queries. 4. Confirm existing hold endpoints are unaffected. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43123 [Bug 43123] Hold placement workflow: Vue modernisation (epic) https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43124 [Bug 43124] Add Koha::Item::Availability::Hold and Koha::Biblio::Availability::Hold -- 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=43126 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |43127 Blocks| |43128 Blocks|43123 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43123 [Bug 43123] Hold placement workflow: Vue modernisation https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43127 [Bug 43127] Vue holds module: scaffolding and Express bib-level hold workflow https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43128 [Bug 43128] Vue holds: granular item-level, club and multi-biblio hold workflow -- 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=43126 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Sponsorship status|--- |Sponsored Status|NEW |ASSIGNED Initiative type|--- |Feature Assignee|koha-bugs@lists.koha-commun |martin.renvoize@openfifth.c |ity.org |o.uk -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43126 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |42386 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 [Bug 42386] Unify hold availability checks into Koha::Availability pattern -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43126 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|42386 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 [Bug 42386] Unify hold availability checks into Koha::Availability pattern -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43126 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |matt.blenkinsop@openfifth.c | |o.uk Assignee|martin.renvoize@openfifth.c |matt.blenkinsop@openfifth.c |o.uk |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43126 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43126 --- Comment #1 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Created attachment 204003 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204003&action=edit Bug 43126: Add a QueryCounter test utility Adds t::lib::QueryCounter, which counts the SQL statements a block of code issues and measures how long it takes. It swaps in a private DBIx::Class statistics object rather than setting a filehandle on the shared one, and restores the previous settings on the way out, including after an exception. Two tests already inlined this pattern. Both now call the helper. Assisted-by: Sonnet 5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43126 --- Comment #2 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Created attachment 204004 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204004&action=edit Bug 43126: [DO NOT PUSH] Add a benchmark for existing holds code Adds a performance harness for the hold availability checks. It records the query count and the run time of the legacy CanBookBeReserved call and CanItemBeReserved loop against records of 10, 50, 100, 200 and 500 items. These are the present-day figures that the p95 targets on this bug are measured against. Later patches add a subtest for each new endpoint, so that one table can be read against another. Assisted-by: Sonnet 5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43126 --- Comment #3 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Created attachment 204005 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204005&action=edit Bug 43126: Add API specs for availability reasons Adds an enum based definition for availability reasons to return via the API when checking hold availability Assisted-by: Sonnet 5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43126 --- Comment #4 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Created attachment 204006 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204006&action=edit Bug 43126: Add an endpoint to test patron hold eligibility Adds GET /patrons/{patron_id}/hold_eligibility, which reports the patron-level gates only: expired, debt_limit, bad_address, card_lost, restricted and hold_limit. None of them needs item context, so a screen can ask for them once rather than once for each item it lists. Reports every blocker rather than stopping at the first, and honours x-koha-override. Assisted-by: Sonnet 5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43126 --- Comment #5 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Created attachment 204007 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204007&action=edit Bug 43126: Add an endpoint to test item hold eligibility Adds GET /items/{item_id}/holdability, which covers the item's own state, the circulation rules and the patron's eligibility for a single item. Pass pickup_library_id to also check the pickup location rules, or include_pickup_locations to get the valid locations in the same response and save a second call. Moves the pickup location response building out of Items#pickup_locations into a helper that both methods share. Assisted-by: Sonnet 5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43126 --- Comment #6 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Created attachment 204008 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204008&action=edit Bug 43126: Add an endpoint to test biblio hold availability Adds GET /biblios/{biblio_id}/holdability, which replaces a CanBookBeReserved call followed by a loop over the record's items. The response carries the verdict and a count of how many items could fill the hold. Adds a summarise_items option to Koha::Biblio::Availability::Hold->check for that count. It is off by default, so a caller that only needs a yes or no keeps the early return. Koha::Item::Availability::Hold also fetched the reservesallowed rule object before the block that reads it, so a caller skipping the count checks paid for that query once per item and discarded the answer. It now runs where it is used. Assisted-by: Sonnet 5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43126 --- Comment #7 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Created attachment 204009 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204009&action=edit Bug 43126: Improve caching of circulation rules to reduce requests when checking hold availability get_effective_rule_value only returned a cached value when the value was true, so a rule that is unset, zero or empty missed the cache and queried again on every call. Those are common, and a caller checking one rule per item on a record paid a query for each. The value is now wrapped so a false one caches too. That exposed a second gap: the cache was cleared in set_rule alone, so a write by any other route left a value behind that no longer existed. It was harmless only because false values never cached. Koha::CirculationRule->store and ->delete now clear it themselves, and set_rule calls the same method. Assisted-by: Sonnet 5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43126 --- Comment #8 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Created attachment 204010 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204010&action=edit Bug 43126: Add an optional holdability parameter to biblio/{biblio_id}/items Adds holdability and patron_id parameters to GET /biblios/{biblio_id}/items. With them, each item in the response carries a holdability attribute. Only the items on the requested page are checked, so the cost follows the page size rather than the size of the record. Makes the per-record context reader on Koha::Biblio::Availability::Hold public, so the controller reuses it instead of repeating it and drifting from it. Assisted-by: Sonnet 5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43126 --- Comment #9 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Created attachment 204011 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204011&action=edit Bug 43126: Add an endpoint for determining batch holdability Adds POST /biblios/{biblio_id}/holdability/batch, which reports a verdict for each patron in one call, or for each patron and item pair when item_ids is given. The record's item list is read once for the whole request rather than once per patron, which is what the club hold case needs. An id that cannot be resolved becomes an entry carrying an error_code rather than failing the request, so one stale id in a club does not cost the caller every other verdict. Assisted-by: Sonnet 5 (Anthropic) Test plan: Preparation 1) Apply the patches. 2) Run: yarn api:bundle && restart_all Test suite 3) Run the new and changed tests. All should pass. These are what covers the five new endpoints, which have no interface yet: prove t/Koha/Result/Availability.t prove t/db_dependent/QueryCounter.t prove t/db_dependent/Koha/CirculationRules.t prove t/db_dependent/Koha/Biblio/Availability/Hold.t prove t/db_dependent/api/v1/patrons_hold_eligibility.t prove t/db_dependent/api/v1/items_holdability.t prove t/db_dependent/api/v1/biblios_holdability.t prove t/db_dependent/api/v1/biblios_holdability_batch.t prove t/db_dependent/api/v1/biblios.t 4) Run the tests that cover what the patches touch but do not add. All should pass: prove xt/api.t prove t/db_dependent/Circulation.t prove t/db_dependent/Circulation/TooMany.t prove t/db_dependent/Reserves.t prove t/db_dependent/Holds.t prove t/db_dependent/Koha/Items.t prove t/db_dependent/Koha/Item/Availability/Hold.t prove t/db_dependent/Koha/Patron/Availability/Hold.t prove t/db_dependent/Koha/Club/Hold.t prove t/db_dependent/Koha/Patron.t prove t/db_dependent/api/v1/holds.t prove t/db_dependent/api/v1/clubs_holds.t prove t/db_dependent/api/v1/items.t prove t/db_dependent/api/v1/patrons.t Holds must still work The patches change Koha::Biblio::Availability::Hold and Koha::Item::Availability::Hold, which sit behind every hold in Koha. 5) Place a hold on a record from the staff interface. It should succeed as before. 6) Place a hold from the OPAC. It should succeed as before. 7) Restrict a patron, then try to place a hold for them in the staff interface. It should still be refused, with the same message as before the patches. Circ rules must still take effect One patch changes how circulation rule values are cached within a request, so that a rule whose value is zero or blank is cached rather than re-read every time. Confirm that a rule change is still noticed at once. 8) Administration > Circulation and fines rules. Set "Holds allowed (total)" to 0 for a rule that applies to your test patron and item. 9) Try to place a hold. It should be refused. 10) Set "Holds allowed (total)" back to a number above 0 and save. 11) Try again. The hold should now be allowed, with no need to restart Plack or wait. This is the case the caching patch has to get right: the earlier value was false, and a false value now stays in the cache until a write clears it. 12) Delete that rule row entirely and repeat. The effective rule should fall back to the next matching one straight away. Performance 13) Run the benchmark and read the tables it prints: prove -v t/db_dependent/api/v1/holdability_benchmark.t The first table is the present-day cost of CanBookBeReserved plus a CanItemBeReserved loop. The rest are the new endpoints. The record-level endpoint should show the same query count for a record of 10 items and one of 500. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43126 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #204003|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=43126 --- Comment #10 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Created attachment 204168 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204168&action=edit Bug 43126: Add a QueryCounter test utility Adds t::lib::QueryCounter, which counts the SQL statements a block of code issues and measures how long it takes. It swaps in a private DBIx::Class statistics object rather than setting a filehandle on the shared one, and restores the previous settings on the way out, including after an exception. Two tests already inlined this pattern. Both now call the helper. Assisted-by: Sonnet 5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org