[Bug 42386] New: Unify hold availability checks into Koha::Availability pattern
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 Bug ID: 42386 Summary: Unify hold availability checks into Koha::Availability pattern Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org Depends on: 41728, 42385 Hold availability is currently split across three different interfaces with different return shapes: - Koha::Patron->can_place_holds â patron eligibility gate Returns Koha::Result::Boolean with messages - C4::Reserves::CanItemBeReserved â item-level hold policy Returns hashref with {status} - C4::Reserves::CanBookBeReserved â biblio-level hold policy Returns hashref with {status} This bug proposes unifying these into the Koha::Availability pattern introduced in bug 41728: - Koha::Patron::Hold::Availability â patron eligibility checks (expired, debt, restricted, card lost, global hold limit) - Koha::Item::Hold::Availability â item-level hold policy checks (hold policy, circ rules, on-shelf holds, pickup location, etc.) Both would return Koha::Availability::Result and support no_short_circuit and overrides, consistent with the checkin availability class. This would also require untangling patron-level checks that leaked into CanItemBeReserved (e.g. maxreserves in certain code paths) and pushing them back to the patron-level orchestrator. Note: This is a significant refactoring effort. CanItemBeReserved has ~15 different checks with complex interactions. A multi-patch approach is recommended. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41728 [Bug 41728] Add `Koha::Item::Checkin::Availability` to centralize logic https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42385 [Bug 42385] Extract _GetCircControlBranch into Koha::Policy::Circulation -- 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=42386 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |42387 Depends on| |42529 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42387 [Bug 42387] [UMBRELLA] Extend Koha::Availability pattern to checkout and hold operations https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42529 [Bug 42529] Move GetAgeRestriction to Koha namespace -- 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=42386 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | CC| |andrew@bywatersolutions.com | |, | |jonathan.druart@gmail.com, | |kyle@bywatersolutions.com, | |lisette@bywatersolutions.co | |m, | |martin.renvoize@openfifth.c | |o.uk, | |nick@bywatersolutions.com, | |tomascohen@gmail.com Status|NEW |ASSIGNED -- 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=42386 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Medium patch 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=42386 --- Comment #1 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198769 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198769&action=edit Bug 42386: Add Koha::Item::Availability::Hold Adds item-level hold availability checks following the Koha::Result::Availability pattern established in Bug 41728. Uses a hashref signature: check({ item => ..., patron => ..., ... }) Checks performed: - IndependentBranches restriction - Damaged item (AllowHoldsOnDamagedItems) - Age restriction (via Koha::Patron->meets_age_restriction) - Item already on hold by patron - Patron has item checked out (AllowHoldsOnPatronsPossessions) - Active recall on item - Hold policy (holdallowed: not_allowed, from_home_library, from_local_hold_group) - Pickup location validation (libraryNotPickupLocation, cannotBeTransferred, pickupNotInHoldGroup) Supports no_short_circuit for collecting all blockers. Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Item/Availability/Hold.t => SUCCESS: Tests pass! 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=42386 --- Comment #2 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198770 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198770&action=edit Bug 42386: Add Koha::Patron::Availability::Hold, chain and wire Adds patron-level hold availability checks and chains them into the item-level class. CanItemBeReserved becomes a thin wrapper. Koha::Patron::Availability::Hold checks: - Eligibility (no item context): expired, debt_limit, bad_address, card_lost, restricted, global maxreserves - Count limits (with item_type_id + library_id): holds_per_record, holds_per_day, reservesallowed, max_holds Koha::Item::Availability::Hold now calls patron-level first (unless skip_patron_checks is set), then adds item-specific blockers to the same Koha::Result::Availability object. No result merging needed. CanItemBeReserved is reduced to a single call to Koha::Item::Availability::Hold->check, mapping ignore_hold_counts to skip_patron_checks. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Reserves.t \ t/db_dependent/Koha/Item/Availability/Hold.t \ t/db_dependent/Koha/Patron/Availability/Hold.t => SUCCESS: Tests pass! 3. Sign off :-D Assisted-by: Sonnet 4.6 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 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=42386 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |42554 Attachment #198769|0 |1 is obsolete| | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42554 [Bug 42554] Deprecate CanItemBeReserved and CanBookBeReserved in favor of Koha::Availability -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198770|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=42386 --- Comment #3 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198828 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198828&action=edit Bug 42386: Add Koha::Patron::Availability::Hold Patron-level hold availability checks using the Koha::Result::Availability pattern. All blocker keys use snake_case consistently. Checks performed: - Eligibility (no item context needed): expired, debt_limit, bad_address, card_lost, restricted, hold_limit (maxreserves syspref) - Count limits (with item_type_id + library_id context): no_reserves_allowed, too_many_holds_for_this_record, too_many_reserves_today, too_many_reserves Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Patron/Availability/Hold.t => SUCCESS: Tests pass! 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=42386 --- Comment #4 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198829 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198829&action=edit Bug 42386: Add Koha::Item::Availability::Hold Item-level hold availability checks. Chains patron-level checks after item checks (unless skip_patron_checks), matching the legacy CanItemBeReserved check order. Checks performed (in order): - cannot_reserve_from_other_branches (IndependentBranches) - damaged - age_restricted - item_already_on_hold - already_possession - recall - no_reserves_allowed (reservesallowed == 0, policy not count) - not_reservable, from_home_library, from_local_hold_group (holdallowed) - Patron-level counts (chained, skippable) - Pickup location: library_not_pickup_location, cannot_be_transferred, pickup_not_in_hold_group (last, matching legacy order) Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Item/Availability/Hold.t \ t/db_dependent/Koha/Patron/Availability/Hold.t => SUCCESS: Tests pass! 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=42386 --- Comment #5 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198830 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198830&action=edit Bug 42386: Add Koha::Biblio::Availability::Hold Biblio-level hold availability. Runs patron eligibility once, then iterates items looking for at least one that is holdable (using Koha::Item::Availability::Hold with skip_patron_checks). Returns the first available item in context for callers that need it. Blockers: - Patron-level blockers (from chained patron check) - no_item_available (items exist but none holdable) - no_items (biblio has no items) Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Biblio/Availability/Hold.t => SUCCESS: Tests pass! 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=42386 --- Comment #6 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198831 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198831&action=edit Bug 42386: Wire CanItemBeReserved to use availability classes CanItemBeReserved is now a thin wrapper around Koha::Item::Availability::Hold->check. All callers updated to expect snake_case status strings. Changes: - C4::Reserves::CanItemBeReserved delegates to the new classes - library_not_found check stays in the wrapper (branchcode → object) - limit field extracted for too_many_* blockers - club_holds_to_patron_holds.error_code ENUM migrated to snake_case - All caller files updated (ILSDI, OPAC, staff, tests) Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ updatedatabase => SUCCESS: ENUM migrated k$ prove t/db_dependent/Holds.t \ t/db_dependent/Reserves.t \ t/db_dependent/Reserves/MultiplePerRecord.t \ t/db_dependent/api/v1/clubs_holds.t \ t/db_dependent/Koha/Item/Availability/Hold.t \ t/db_dependent/Koha/Patron/Availability/Hold.t \ t/db_dependent/Koha/Biblio/Availability/Hold.t => SUCCESS: Tests pass! 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=42386 --- Comment #7 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198832 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198832&action=edit Bug 42386: Wire CanBookBeReserved to use Koha::Biblio::Availability::Hold CanBookBeReserved is now a thin wrapper around Koha::Biblio::Availability::Hold->check. Behavior changes: - Returns 'no_items' instead of '' when biblio has no items - Returns 'no_item_available' instead of last item's blocker when all items are blocked - Returns 'too_many_reserves' instead of '' when itemtype limit reached These are more descriptive statuses. All callers check eq 'OK' so non-OK values are equivalent. Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Reserves.t \ t/db_dependent/Holds.t \ t/db_dependent/Koha/Biblio/Availability/Hold.t => SUCCESS: Tests pass! 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=42386 --- Comment #8 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198833 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198833&action=edit Bug 42386: Pass overrides through Can*BeReserved wrappers The API controller already sends overrides via x-koha-override header. Wire them through CanItemBeReserved and CanBookBeReserved into the availability classes so patron-level checks (expired, debt_limit, etc.) can be skipped when overridden. Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: All 20 tests pass (including override tests) 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=42386 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |42563 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42563 [Bug 42563] Add Koha::Biblio->host_items using MetadataExtractor -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|42563 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42563 [Bug 42563] Implement Koha::Biblio->host_items using MetadataExtractor -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|41728 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41728 [Bug 41728] Add `Koha::Item::Availability::Checkin` to centralize logic -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> 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=42386 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198828|0 |1 is obsolete| | Attachment #198829|0 |1 is obsolete| | Attachment #198830|0 |1 is obsolete| | Attachment #198831|0 |1 is obsolete| | Attachment #198832|0 |1 is obsolete| | Attachment #198833|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=42386 --- Comment #9 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 200779 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200779&action=edit Bug 42386: Add Koha::Patron::Availability::Hold Patron-level hold availability checks using the Koha::Result::Availability pattern. All blocker keys use snake_case consistently. Checks performed: - Eligibility (no item context needed): expired, debt_limit, bad_address, card_lost, restricted, hold_limit (maxreserves syspref) - Count limits (with item_type_id + library_id context): no_reserves_allowed, too_many_holds_for_this_record, too_many_reserves_today, too_many_reserves Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Patron/Availability/Hold.t => SUCCESS: Tests pass! 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=42386 --- Comment #10 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 200780 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200780&action=edit Bug 42386: Add Koha::Item::Availability::Hold Item-level hold availability checks. Chains patron-level checks after item checks (unless skip_patron_checks), matching the legacy CanItemBeReserved check order. Checks performed (in order): - cannot_reserve_from_other_branches (IndependentBranches) - damaged - age_restricted - item_already_on_hold - already_possession - recall - no_reserves_allowed (reservesallowed == 0, policy not count) - not_reservable, from_home_library, from_local_hold_group (holdallowed) - Patron-level counts (chained, skippable) - Pickup location: library_not_pickup_location, cannot_be_transferred, pickup_not_in_hold_group (last, matching legacy order) Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Item/Availability/Hold.t \ t/db_dependent/Koha/Patron/Availability/Hold.t => SUCCESS: Tests pass! 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=42386 --- Comment #11 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 200781 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200781&action=edit Bug 42386: Add Koha::Biblio::Availability::Hold Biblio-level hold availability. Runs patron eligibility once, then iterates items looking for at least one that is holdable (using Koha::Item::Availability::Hold with skip_patron_checks). Returns the first available item in context for callers that need it. Blockers: - Patron-level blockers (from chained patron check) - no_item_available (items exist but none holdable) - no_items (biblio has no items) Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Biblio/Availability/Hold.t => SUCCESS: Tests pass! 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=42386 --- Comment #12 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 200782 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200782&action=edit Bug 42386: Wire CanItemBeReserved to use availability classes CanItemBeReserved is now a thin wrapper around Koha::Item::Availability::Hold->check. All callers updated to expect snake_case status strings. Changes: - C4::Reserves::CanItemBeReserved delegates to the new classes - library_not_found check stays in the wrapper (branchcode → object) - limit field extracted for too_many_* blockers - club_holds_to_patron_holds.error_code ENUM migrated to snake_case - All caller files updated (ILSDI, OPAC, staff, tests) Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ updatedatabase => SUCCESS: ENUM migrated k$ prove t/db_dependent/Holds.t \ t/db_dependent/Reserves.t \ t/db_dependent/Reserves/MultiplePerRecord.t \ t/db_dependent/api/v1/clubs_holds.t \ t/db_dependent/Koha/Item/Availability/Hold.t \ t/db_dependent/Koha/Patron/Availability/Hold.t \ t/db_dependent/Koha/Biblio/Availability/Hold.t => SUCCESS: Tests pass! 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=42386 --- Comment #13 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 200783 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200783&action=edit Bug 42386: Wire CanBookBeReserved to use Koha::Biblio::Availability::Hold CanBookBeReserved is now a thin wrapper around Koha::Biblio::Availability::Hold->check. Behavior changes: - Returns 'no_items' instead of '' when biblio has no items - Returns 'no_item_available' instead of last item's blocker when all items are blocked - Returns 'too_many_reserves' instead of '' when itemtype limit reached These are more descriptive statuses. All callers check eq 'OK' so non-OK values are equivalent. Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Reserves.t \ t/db_dependent/Holds.t \ t/db_dependent/Koha/Biblio/Availability/Hold.t => SUCCESS: Tests pass! 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=42386 --- Comment #14 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 200784 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200784&action=edit Bug 42386: Pass overrides through Can*BeReserved wrappers The API controller already sends overrides via x-koha-override header. Wire them through CanItemBeReserved and CanBookBeReserved into the availability classes so patron-level checks (expired, debt_limit, etc.) can be skipped when overridden. Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: All 20 tests pass (including override tests) 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=42386 --- Comment #15 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 200785 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200785&action=edit Bug 42386: (follow-up) Use group-aware hold counting in availability class Koha::Patron::Availability::Hold used $patron->holds->count which does not account for hold groups. This patch switches to $patron->holds->count_holds which counts each hold group as a single unit. Also updates t/db_dependent/Reserves.t to use the new snake_case status names (too_many_reserves) instead of the old camelCase (tooManyReserves). Test plan: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Reserves.t => SUCCESS: Tests pass (except unrelated hold slip test) 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=42386 David Nind <david@davidnind.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=42386 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #200779|0 |1 is obsolete| | Attachment #200780|0 |1 is obsolete| | Attachment #200781|0 |1 is obsolete| | Attachment #200782|0 |1 is obsolete| | Attachment #200783|0 |1 is obsolete| | Attachment #200784|0 |1 is obsolete| | Attachment #200785|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=42386 --- Comment #16 from David Nind <david@davidnind.com> --- Created attachment 200795 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200795&action=edit Bug 42386: Add Koha::Patron::Availability::Hold Patron-level hold availability checks using the Koha::Result::Availability pattern. All blocker keys use snake_case consistently. Checks performed: - Eligibility (no item context needed): expired, debt_limit, bad_address, card_lost, restricted, hold_limit (maxreserves syspref) - Count limits (with item_type_id + library_id context): no_reserves_allowed, too_many_holds_for_this_record, too_many_reserves_today, too_many_reserves Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Patron/Availability/Hold.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #17 from David Nind <david@davidnind.com> --- Created attachment 200796 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200796&action=edit Bug 42386: Add Koha::Item::Availability::Hold Item-level hold availability checks. Chains patron-level checks after item checks (unless skip_patron_checks), matching the legacy CanItemBeReserved check order. Checks performed (in order): - cannot_reserve_from_other_branches (IndependentBranches) - damaged - age_restricted - item_already_on_hold - already_possession - recall - no_reserves_allowed (reservesallowed == 0, policy not count) - not_reservable, from_home_library, from_local_hold_group (holdallowed) - Patron-level counts (chained, skippable) - Pickup location: library_not_pickup_location, cannot_be_transferred, pickup_not_in_hold_group (last, matching legacy order) Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Item/Availability/Hold.t \ t/db_dependent/Koha/Patron/Availability/Hold.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #18 from David Nind <david@davidnind.com> --- Created attachment 200797 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200797&action=edit Bug 42386: Add Koha::Biblio::Availability::Hold Biblio-level hold availability. Runs patron eligibility once, then iterates items looking for at least one that is holdable (using Koha::Item::Availability::Hold with skip_patron_checks). Returns the first available item in context for callers that need it. Blockers: - Patron-level blockers (from chained patron check) - no_item_available (items exist but none holdable) - no_items (biblio has no items) Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Biblio/Availability/Hold.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #19 from David Nind <david@davidnind.com> --- Created attachment 200798 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200798&action=edit Bug 42386: Wire CanItemBeReserved to use availability classes CanItemBeReserved is now a thin wrapper around Koha::Item::Availability::Hold->check. All callers updated to expect snake_case status strings. Changes: - C4::Reserves::CanItemBeReserved delegates to the new classes - library_not_found check stays in the wrapper (branchcode → object) - limit field extracted for too_many_* blockers - club_holds_to_patron_holds.error_code ENUM migrated to snake_case - All caller files updated (ILSDI, OPAC, staff, tests) Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ updatedatabase => SUCCESS: ENUM migrated k$ prove t/db_dependent/Holds.t \ t/db_dependent/Reserves.t \ t/db_dependent/Reserves/MultiplePerRecord.t \ t/db_dependent/api/v1/clubs_holds.t \ t/db_dependent/Koha/Item/Availability/Hold.t \ t/db_dependent/Koha/Patron/Availability/Hold.t \ t/db_dependent/Koha/Biblio/Availability/Hold.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #20 from David Nind <david@davidnind.com> --- Created attachment 200799 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200799&action=edit Bug 42386: Wire CanBookBeReserved to use Koha::Biblio::Availability::Hold CanBookBeReserved is now a thin wrapper around Koha::Biblio::Availability::Hold->check. Behavior changes: - Returns 'no_items' instead of '' when biblio has no items - Returns 'no_item_available' instead of last item's blocker when all items are blocked - Returns 'too_many_reserves' instead of '' when itemtype limit reached These are more descriptive statuses. All callers check eq 'OK' so non-OK values are equivalent. Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Reserves.t \ t/db_dependent/Holds.t \ t/db_dependent/Koha/Biblio/Availability/Hold.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #21 from David Nind <david@davidnind.com> --- Created attachment 200800 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200800&action=edit Bug 42386: Pass overrides through Can*BeReserved wrappers The API controller already sends overrides via x-koha-override header. Wire them through CanItemBeReserved and CanBookBeReserved into the availability classes so patron-level checks (expired, debt_limit, etc.) can be skipped when overridden. Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: All 20 tests pass (including override tests) 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #22 from David Nind <david@davidnind.com> --- Created attachment 200801 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200801&action=edit Bug 42386: (follow-up) Use group-aware hold counting in availability class Koha::Patron::Availability::Hold used $patron->holds->count which does not account for hold groups. This patch switches to $patron->holds->count_holds which counts each hold group as a single unit. Also updates t/db_dependent/Reserves.t to use the new snake_case status names (too_many_reserves) instead of the old camelCase (tooManyReserves). Test plan: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Reserves.t => SUCCESS: Tests pass (except unrelated hold slip test) 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@openfifth.c |y.org |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #23 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- I like the direction of travel here.. but there's a fair few issues that need resolving and considering first. 1.Staff and OPAC hold-error templates were never updated for the snake_case rename — users now see generic/wrong messages instead of specific reasons. This patchset renames CanItemBeReserved/CanBookBeReserved status codes (e.g. ageRestricted → age_restricted) and updates every .pl caller, but reserve/request.tt and koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt are not in this diff at all and still switch on the old camelCase literals: - koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt:208-267 (two SWITCH/CASE blocks for failed_holds), :537 (IF (... || ageRestricted || ...)), :582 (biblioloo.tooManyHoldsForThisRecord), :976-998 (itemloo.not_holdable == 'ageRestricted' etc.), :1615-1616 (JS translation map keys). - koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt:83-105 (SWITCH fail / CASE 'ageRestricted' etc.) 2. You collapsed per-item hold-ability failures in the biblios hold check into a simpler no_item_available (no_items) response.. I agree this makes sense at the Biblio level given our past handling basically just returned the last item checked failure.. but I wonder if it might make sense to return both no_item_available and a 'reasons' array of itemnumber + failure code for future use? 3. Koha::Patron::Availability::Hold->check lost branch/itemtype filtering on the reservesallowed count, silently breaking per-branch/per-itemtype hold limits. We've had loads of bugs around this over the years.. we can't afford to lose the branch/itemtype level policy resolution 4. Your DBIC changes are inlined in another patch.. they should be split out to make sure the RM doesn't loose your other changes when regenerating 5. REST API error message format is a silent breaking change for third-party consumers. I like this change, but could we perhaps include an error code as well as the stringified error.. it looks like we're missing that here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #200795|0 |1 is obsolete| | Attachment #200796|0 |1 is obsolete| | Attachment #200797|0 |1 is obsolete| | Attachment #200798|0 |1 is obsolete| | Attachment #200799|0 |1 is obsolete| | Attachment #200800|0 |1 is obsolete| | Attachment #200801|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=42386 --- Comment #24 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201595 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201595&action=edit Bug 42386: Add Koha::Patron::Availability::Hold Patron-level hold availability checks using the Koha::Result::Availability pattern. All blocker keys use snake_case consistently. Checks performed: - Eligibility (no item context needed): expired, debt_limit, bad_address, card_lost, restricted, hold_limit (maxreserves syspref) - Count limits (with item_type_id + library_id context): no_reserves_allowed, too_many_holds_for_this_record, too_many_reserves_today, too_many_reserves Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Patron/Availability/Hold.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #25 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201596 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201596&action=edit Bug 42386: Add Koha::Item::Availability::Hold Item-level hold availability checks. Chains patron-level checks after item checks (unless skip_patron_checks), matching the legacy CanItemBeReserved check order. Checks performed (in order): - cannot_reserve_from_other_branches (IndependentBranches) - damaged - age_restricted - item_already_on_hold - already_possession - recall - no_reserves_allowed (reservesallowed == 0, policy not count) - not_reservable, from_home_library, from_local_hold_group (holdallowed) - Patron-level counts (chained, skippable) - Pickup location: library_not_pickup_location, cannot_be_transferred, pickup_not_in_hold_group (last, matching legacy order) Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Item/Availability/Hold.t \ t/db_dependent/Koha/Patron/Availability/Hold.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #26 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201597 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201597&action=edit Bug 42386: Add Koha::Biblio::Availability::Hold Biblio-level hold availability. Runs patron eligibility once, then iterates items looking for at least one that is holdable (using Koha::Item::Availability::Hold with skip_patron_checks). Returns the first available item in context for callers that need it. Blockers: - Patron-level blockers (from chained patron check) - no_item_available (items exist but none holdable) - no_items (biblio has no items) Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Biblio/Availability/Hold.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #27 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201598 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201598&action=edit Bug 42386: Wire CanItemBeReserved to use availability classes CanItemBeReserved is now a thin wrapper around Koha::Item::Availability::Hold->check. All callers updated to expect snake_case status strings. Changes: - C4::Reserves::CanItemBeReserved delegates to the new classes - library_not_found check stays in the wrapper (branchcode → object) - limit field extracted for too_many_* blockers - club_holds_to_patron_holds.error_code ENUM migrated to snake_case - All caller files updated (ILSDI, OPAC, staff, tests) Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ updatedatabase => SUCCESS: ENUM migrated k$ prove t/db_dependent/Holds.t \ t/db_dependent/Reserves.t \ t/db_dependent/Reserves/MultiplePerRecord.t \ t/db_dependent/api/v1/clubs_holds.t \ t/db_dependent/Koha/Item/Availability/Hold.t \ t/db_dependent/Koha/Patron/Availability/Hold.t \ t/db_dependent/Koha/Biblio/Availability/Hold.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #28 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201599 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201599&action=edit Bug 42386: Wire CanBookBeReserved to use Koha::Biblio::Availability::Hold CanBookBeReserved is now a thin wrapper around Koha::Biblio::Availability::Hold->check. Behavior changes: - Returns 'no_items' instead of '' when biblio has no items - Returns 'no_item_available' instead of last item's blocker when all items are blocked - Returns 'too_many_reserves' instead of '' when itemtype limit reached These are more descriptive statuses. All callers check eq 'OK' so non-OK values are equivalent. Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Reserves.t \ t/db_dependent/Holds.t \ t/db_dependent/Koha/Biblio/Availability/Hold.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #29 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201600 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201600&action=edit Bug 42386: Pass overrides through Can*BeReserved wrappers The API controller already sends overrides via x-koha-override header. Wire them through CanItemBeReserved and CanBookBeReserved into the availability classes so patron-level checks (expired, debt_limit, etc.) can be skipped when overridden. Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: All 20 tests pass (including override tests) 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #30 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201601 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201601&action=edit Bug 42386: (follow-up) Use group-aware hold counting in availability class Koha::Patron::Availability::Hold used $patron->holds->count which does not account for hold groups. This patch switches to $patron->holds->count_holds which counts each hold group as a single unit. Also updates t/db_dependent/Reserves.t to use the new snake_case status names (too_many_reserves) instead of the old camelCase (tooManyReserves). Test plan: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Reserves.t => SUCCESS: Tests pass (except unrelated hold slip test) 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #31 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201602 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201602&action=edit Bug 42386: (follow-up) Restore branch/itemtype filtering on reservesallowed The reservesallowed count was checking ALL patron holds regardless of branch or itemtype, breaking per-branch and per-itemtype hold limits. The old C4::Reserves code filtered the count by: - Branch (items.homebranch or borrowers.branchcode per ReservesControlBranch) - Itemtype (when the matching rule was itemtype-specific) This patch restores that behavior: - Item::Availability::Hold retrieves the rule object to get its itemtype - Passes rule_itemtype to Patron::Availability::Hold - The reservesallowed count filters by control branch (when ItemHomeLibrary) and by rule itemtype (when the rule is itemtype-specific) 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/Holds.t \ t/db_dependent/Reserves.t => SUCCESS: Tests pass! 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=42386 --- Comment #32 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201603 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201603&action=edit Bug 42386: (follow-up) Update templates to use snake_case status codes The hold availability classes return snake_case status codes but the templates were never updated from the old camelCase convention, causing users to see generic/wrong messages instead of specific hold failure reasons. This patch updates: - reserve/request.tt (SWITCH/CASE blocks, IF conditions, JS map) - opac-user.tt (SWITCH/CASE block for failed holds) Test plan: 1. Apply patch 2. Place a hold that triggers any failure reason (e.g. age restriction, too many holds, item already on hold) 3. Verify the specific error message is shown instead of a generic one 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=42386 --- Comment #33 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201604 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201604&action=edit Bug 42386: (follow-up) Add error_code to hold placement API responses Several hold-related error responses in the REST API returned an error message string but no machine-readable error_code, making it impossible for third-party consumers to programmatically handle specific failures. This patch adds error_code to: - Hold date in future (hold_date_in_future) - Item/biblio mismatch (item_biblio_mismatch) - Missing biblio_id/item_id (missing_biblio_or_item) - Patron not found (patron_not_found) - Invalid pickup location (invalid_pickup_location) - CanItemBeReserved/CanBookBeReserved failure (dynamic, uses the status code from the availability check, e.g. too_many_reserves, damaged) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 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=42386 --- Comment #34 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201605 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201605&action=edit Bug 42386: (follow-up) Collect item-level failure reasons in biblio hold check When no items are holdable, Koha::Biblio::Availability::Hold now collects per-item failure reasons in the result context as item_failures, an arrayref of { itemnumber, blockers } hashes. This allows callers (and future API responses) to report WHY each item failed, rather than just 'no item available'. The item_failures context is only set when no items pass (i.e. when no_item_available blocker is added). On success, the behavior is unchanged (short-circuits on first available item). Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Biblio/Availability/Hold.t => SUCCESS: Tests pass! 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=42386 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201595|0 |1 is obsolete| | Attachment #201596|0 |1 is obsolete| | Attachment #201597|0 |1 is obsolete| | Attachment #201598|0 |1 is obsolete| | Attachment #201599|0 |1 is obsolete| | Attachment #201600|0 |1 is obsolete| | Attachment #201601|0 |1 is obsolete| | Attachment #201602|0 |1 is obsolete| | Attachment #201603|0 |1 is obsolete| | Attachment #201604|0 |1 is obsolete| | Attachment #201605|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=42386 --- Comment #35 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201606 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201606&action=edit Bug 42386: Add Koha::Patron::Availability::Hold Patron-level hold availability checks using the Koha::Result::Availability pattern. All blocker keys use snake_case consistently. Checks performed: - Eligibility (no item context needed): expired, debt_limit, bad_address, card_lost, restricted, hold_limit (maxreserves syspref) - Count limits (with item_type_id + library_id context): no_reserves_allowed, too_many_holds_for_this_record, too_many_reserves_today, too_many_reserves Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Patron/Availability/Hold.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #36 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201607 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201607&action=edit Bug 42386: Add Koha::Item::Availability::Hold Item-level hold availability checks. Chains patron-level checks after item checks (unless skip_patron_checks), matching the legacy CanItemBeReserved check order. Checks performed (in order): - cannot_reserve_from_other_branches (IndependentBranches) - damaged - age_restricted - item_already_on_hold - already_possession - recall - no_reserves_allowed (reservesallowed == 0, policy not count) - not_reservable, from_home_library, from_local_hold_group (holdallowed) - Patron-level counts (chained, skippable) - Pickup location: library_not_pickup_location, cannot_be_transferred, pickup_not_in_hold_group (last, matching legacy order) Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Item/Availability/Hold.t \ t/db_dependent/Koha/Patron/Availability/Hold.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #37 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201608 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201608&action=edit Bug 42386: Add Koha::Biblio::Availability::Hold Biblio-level hold availability. Runs patron eligibility once, then iterates items looking for at least one that is holdable (using Koha::Item::Availability::Hold with skip_patron_checks). Returns the first available item in context for callers that need it. Blockers: - Patron-level blockers (from chained patron check) - no_item_available (items exist but none holdable) - no_items (biblio has no items) Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Biblio/Availability/Hold.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #38 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201609 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201609&action=edit Bug 42386: Wire CanItemBeReserved to use availability classes CanItemBeReserved is now a thin wrapper around Koha::Item::Availability::Hold->check. All callers updated to expect snake_case status strings. Changes: - C4::Reserves::CanItemBeReserved delegates to the new classes - library_not_found check stays in the wrapper (branchcode -> object) - limit field extracted for too_many_* blockers - All caller files updated (ILSDI, OPAC, staff, tests) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Holds.t \ t/db_dependent/Reserves.t \ t/db_dependent/Reserves/MultiplePerRecord.t \ t/db_dependent/api/v1/clubs_holds.t \ t/db_dependent/Koha/Item/Availability/Hold.t \ t/db_dependent/Koha/Patron/Availability/Hold.t \ t/db_dependent/Koha/Biblio/Availability/Hold.t => SUCCESS: Tests pass! 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=42386 --- Comment #39 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201610 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201610&action=edit Bug 42386: DB update - Migrate error_code ENUM to snake_case Migrates the club_holds_to_patron_holds.error_code ENUM column from camelCase values to snake_case, matching the new availability class status codes. The atomicupdate uses a 3-step approach: 1. Expand ENUM to accept both old and new values 2. UPDATE existing rows from old to new 3. Shrink ENUM to only snake_case values Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ updatedatabase => SUCCESS: ENUM migrated 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=42386 --- Comment #40 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201611 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201611&action=edit Bug 42386: Update DBIC schema [DO NOT PUSH] Generated DBIC schema update for club_holds_to_patron_holds.error_code ENUM migration to snake_case. This commit will be regenerated by the RM. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #41 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201612 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201612&action=edit Bug 42386: Wire CanBookBeReserved to use Koha::Biblio::Availability::Hold CanBookBeReserved is now a thin wrapper around Koha::Biblio::Availability::Hold->check. Behavior changes: - Returns 'no_items' instead of '' when biblio has no items - Returns 'no_item_available' instead of last item's blocker when all items are blocked - Returns 'too_many_reserves' instead of '' when itemtype limit reached These are more descriptive statuses. All callers check eq 'OK' so non-OK values are equivalent. Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Reserves.t \ t/db_dependent/Holds.t \ t/db_dependent/Koha/Biblio/Availability/Hold.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #42 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201613 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201613&action=edit Bug 42386: Pass overrides through Can*BeReserved wrappers The API controller already sends overrides via x-koha-override header. Wire them through CanItemBeReserved and CanBookBeReserved into the availability classes so patron-level checks (expired, debt_limit, etc.) can be skipped when overridden. Assisted-by: Sonnet 4.6 (Anthropic) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: All 20 tests pass (including override tests) 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #43 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201614 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201614&action=edit Bug 42386: (follow-up) Use group-aware hold counting in availability class Koha::Patron::Availability::Hold used $patron->holds->count which does not account for hold groups. This patch switches to $patron->holds->count_holds which counts each hold group as a single unit. Also updates t/db_dependent/Reserves.t to use the new snake_case status names (too_many_reserves) instead of the old camelCase (tooManyReserves). Test plan: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Reserves.t => SUCCESS: Tests pass (except unrelated hold slip test) 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #44 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201615 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201615&action=edit Bug 42386: (follow-up) Restore branch/itemtype filtering on reservesallowed The reservesallowed count was checking ALL patron holds regardless of branch or itemtype, breaking per-branch and per-itemtype hold limits. The old C4::Reserves code filtered the count by: - Branch (items.homebranch or borrowers.branchcode per ReservesControlBranch) - Itemtype (when the matching rule was itemtype-specific) This patch restores that behavior: - Item::Availability::Hold retrieves the rule object to get its itemtype - Passes rule_itemtype to Patron::Availability::Hold - The reservesallowed count filters by control branch (when ItemHomeLibrary) and by rule itemtype (when the rule is itemtype-specific) 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/Holds.t \ t/db_dependent/Reserves.t => SUCCESS: Tests pass! 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=42386 --- Comment #45 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201616 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201616&action=edit Bug 42386: (follow-up) Update templates to use snake_case status codes The hold availability classes return snake_case status codes but the templates were never updated from the old camelCase convention, causing users to see generic/wrong messages instead of specific hold failure reasons. This patch updates: - reserve/request.tt (SWITCH/CASE blocks, IF conditions, JS map) - opac-user.tt (SWITCH/CASE block for failed holds) Test plan: 1. Apply patch 2. Place a hold that triggers any failure reason (e.g. age restriction, too many holds, item already on hold) 3. Verify the specific error message is shown instead of a generic one 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=42386 --- Comment #46 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201617 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201617&action=edit Bug 42386: (follow-up) Add error_code to hold placement API responses Several hold-related error responses in the REST API returned an error message string but no machine-readable error_code, making it impossible for third-party consumers to programmatically handle specific failures. This patch adds error_code to: - Hold date in future (hold_date_in_future) - Item/biblio mismatch (item_biblio_mismatch) - Missing biblio_id/item_id (missing_biblio_or_item) - Patron not found (patron_not_found) - Invalid pickup location (invalid_pickup_location) - CanItemBeReserved/CanBookBeReserved failure (dynamic, uses the status code from the availability check, e.g. too_many_reserves, damaged) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 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=42386 --- Comment #47 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 201618 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201618&action=edit Bug 42386: (follow-up) Collect item-level failure reasons in biblio hold check When no items are holdable, Koha::Biblio::Availability::Hold now collects per-item failure reasons in the result context as item_failures, an arrayref of { itemnumber, blockers } hashes. This allows callers (and future API responses) to report WHY each item failed, rather than just 'no item available'. The item_failures context is only set when no items pass (i.e. when no_item_available blocker is added). On success, the behavior is unchanged (short-circuits on first available item). Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Biblio/Availability/Hold.t => SUCCESS: Tests pass! 3. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org