[Bug 43124] New: Add Koha::Item::Availability::Hold and Koha::Biblio::Availability::Hold
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43124 Bug ID: 43124 Summary: Add Koha::Item::Availability::Hold and Koha::Biblio::Availability::Hold Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@openfifth.co.uk QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, lisette@bywatersolutions.com, tomascohen@gmail.com Depends on: 41728 Blocks: 43123 Target Milestone: --- Add two new classes for evaluating hold eligibility, replacing the ad hoc per-item query pattern in CanItemBeReserved. Koha::Item::Availability::Hold->check($item, $patron, $params) evaluates the existing 13 gate holdability check for a single item and returns a Koha::Item::Availability::Hold::Result, a subclass of Koha::Result::Availability (blockers, confirmations, warnings, context). This mirrors Koha::Item::Availability::Checkin from bug 41728 exactly. Koha::Biblio::Availability::Hold is the batching layer that fetches a biblio's items once, with prefetch on checkout, transfer, current hold and biblioitem relationships, then calls Koha::Item::Availability::Hold->check per item against that warm data, aggregating the results into a summary. Multi-patron calls reuse the same item fetch across every patron in the batch rather than repeating it per patron. C4::Reserves::CanItemBeReserved is refactored to become a thin wrapper around Koha::Item::Availability::Hold, so existing callers are unaffected and behaviour is unchanged. No REST or UI change is included in this bug. This bug depends on bug 41728, which introduces the shared Koha::Result::Availability base class these new classes are built on. Test plan 1. Run the existing C4::Reserves and Koha::Item test suites and confirm no regressions. 2. Add unit tests for Koha::Item::Availability::Hold covering the standard holdability gates (already held, too many reserves, age restricted, not for loan, item level hold policy, etc). 3. Add unit tests for Koha::Biblio::Availability::Hold confirming it issues a small, roughly constant number of queries regardless of item count (benchmark with 10, 50 and 100 item biblios). 4. Confirm CanItemBeReserved returns identical results to the pre-patch version for a representative set of items and patrons. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41728 [Bug 41728] Add `Koha::Item::Availability::Checkin` to centralize logic https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43123 [Bug 43123] Hold placement workflow: Vue modernisation (epic) -- 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=43124 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |43126 Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |martin.renvoize@openfifth.c |ity.org |o.uk Sponsorship status|--- |Sponsored Initiative type|--- |Feature Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43126 [Bug 43126] REST API: add holdability endpoints for biblios, items and 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=43124 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|41728 |42386 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41728 [Bug 41728] Add `Koha::Item::Availability::Checkin` to centralize logic 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=43124 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43124 --- Comment #1 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 202393 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202393&action=edit Bug 43124: Prefetch patron/biblio context to avoid per-item queries Koha::Item::Availability::Hold->check still issued its own DB query per item for item_already_on_hold, already_possession, and recall (all patron-scoped, not item-scoped - the same patron's held/checked-out/ recalled item sets don't vary across items on the same record), and recomputed age restriction (a biblio-scoped result) via a fresh $item->biblio fetch on every item. Added optional held_itemnumbers/checked_out_itemnumbers/recalled_itemnumbers (hashref) and age_restriction_ok (boolean) params to check(), falling back to the existing per-item queries when not supplied so every other caller of this class is unaffected. Koha::Biblio::Availability::Hold now computes these once before its item loop and passes them into every per-item check, collapsing what were 3N+ queries down to a handful regardless of item count. can_be_transferred (genuinely item-homebranch-specific) is instead memoized per request via Koha::Cache::Memory::Lite, keyed on the branch/ itemtype-or-ccode combination that determines it, opt-in via a new cache_transfers flag (off by default, matching the existing cache_counts pattern, since a caller that creates/deletes transfer limits between calls must see a fresh result). Added a query-count-at-scale test (10 vs 100 items, patron holding every item) asserting the query delta stays small rather than growing with item count - the concrete acceptance criterion from the original architecture proposal for this bug. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Patron/Availability/Hold.t \ t/db_dependent/Koha/Item/Availability/Hold.t \ t/db_dependent/Koha/Biblio/Availability/Hold.t \ t/db_dependent/Reserves.t t/db_dependent/Holds.t \ t/db_dependent/Reserves/MultiplePerRecord.t \ t/db_dependent/ILSDI_Services.t => SUCCESS: All tests pass, including the new query-count assertion! 3. 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=43124 --- Comment #2 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 202394 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202394&action=edit Bug 43124: Reuse the item fetch across club-hold members Koha::Club::Hold::add called CanBookBeReserved once (or twice, with default_patron_home) per enrolled member, and each call independently re-fetched the biblio's full item list (plus host/analytics items) via Koha::Biblio::Availability::Hold - the same list every time, since it doesn't depend on which member is being checked. Added an optional 'items' param to Koha::Biblio::Availability::Hold->check (and threaded through CanBookBeReserved) that skips its own item/host-item fetch when a pre-fetched list is supplied. Exposed the fetch itself as a new Koha::Biblio::Availability::Hold->fetch_items class method so Koha::Club::Hold::add can call it once before the member loop and pass the result to every CanBookBeReserved call. Added a query-count test confirming the item-table query issued by the availability check doesn't scale with club member count. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Club/Hold.t t/db_dependent/api/v1/clubs_holds.t => SUCCESS: All tests pass, including the new query-count assertion! 3. 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=43124 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Add |Improve performance of |Koha::Item::Availability::H |Koha::Item::Availability::H |old and |old and |Koha::Biblio::Availability: |Koha::Biblio::Availability: |:Hold |:Hold --- Comment #3 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- I didn't spot Tomas's work on bug 42386 before I posted this but.. I'm re-using the bug to add some further performance facing improvements to his submission. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43124 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com --- Comment #4 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Testing here is currently blocked by a merge error when applying Bug 42385 (PQA). -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org