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 ---------------------------------------------------------------------------- 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 #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| |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 --- 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 #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 #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 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 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 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 #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 #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 #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 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 --- 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 #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 #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 #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 ---------------------------------------------------------------------------- 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 ---------------------------------------------------------------------------- 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 #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 #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 #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 #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 #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 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 #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 #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 #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 #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 #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 #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 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=43108 -- 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| |43124 Referenced Bugs: 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 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 |Passed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #48 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 202386 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202386&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> 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=42386 --- Comment #50 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 202388 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202388&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> 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=42386 Bug 42386 depends on bug 42529, which changed state. Bug 42529 Summary: Move GetAgeRestriction to Koha namespace https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42529 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #202387|1 |0 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 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201609|1 |0 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 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201611|1 |0 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 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201613|1 |0 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 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201615|1 |0 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 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201617|1 |0 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 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201609|0 |1 is obsolete| | Attachment #201610|0 |1 is obsolete| | Attachment #201611|0 |1 is obsolete| | Attachment #201612|0 |1 is obsolete| | Attachment #201613|0 |1 is obsolete| | Attachment #201614|0 |1 is obsolete| | Attachment #201615|0 |1 is obsolete| | Attachment #201616|0 |1 is obsolete| | Attachment #201617|0 |1 is obsolete| | Attachment #201618|0 |1 is obsolete| | Attachment #202386|0 |1 is obsolete| | Attachment #202387|0 |1 is obsolete| | Attachment #202388|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 #52 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203511 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203511&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> 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=42386 --- Comment #54 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203513 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203513&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 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=42386 --- Comment #56 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203515 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203515&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. 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=42386 --- Comment #58 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203517 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203517&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> 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=42386 --- Comment #60 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203519 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203519&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 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=42386 --- Comment #62 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203521 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203521&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 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=42386 --- Comment #64 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203523 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203523&action=edit Bug 42386: (follow-up) Fix hold-availability regressions and reduce duplication Koha::Patron->can_place_holds duplicated the exact same six patron eligibility checks now implemented in Koha::Patron::Availability::Hold, and Koha::REST::V1::Holds ran both in sequence on every hold placement. can_place_holds is now a thin wrapper over Koha::Patron::Availability::Hold, passing the already-fetched patron object through and preserving the existing message/payload contract (debt_limit/hold_limit payloads). Koha::Item::Availability::Hold's item_already_on_hold check had also stopped respecting skip_patron_checks (the legacy ignore_hold_counts semantics), which C4::Circulation's decreaseLoanHighHoldsControl=dynamic logic and Koha::Hold's hold-move logic both rely on when checking whether an item can fill a hold the patron already placed on it - both were newly at risk of mis-blocking. Restored, while adding a distinct skip_patron_count_checks flag so Koha::Biblio::Availability::Hold's per-item loop (which still needs item_already_on_hold evaluated per item) doesn't lose that check. Finally, CanItemBeReserved's per-item display loops in reserve/request.pl and opac-reserve.pl were newly re-running the full patron eligibility and rule-based count checks for every single item on a record, where the old CanItemBeReserved never did any patron-level checks at all. Added skip_eligibility_checks (skip only the no-item-context gates, already shown once at the page level) and an opt-in cache_counts flag (memoizing the rule-based count queries per request, off by default so the test suite and any caller that places holds mid-loop always sees a fresh count), and wired both into the two affected loops. 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 \ t/db_dependent/api/v1/holds.t t/db_dependent/api/v1/clubs_holds.t \ t/db_dependent/Koha/Patron.t t/db_dependent/DecreaseLoanHighHolds.t => SUCCESS: All tests pass! 3. Sign off :-D 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=42386 --- Comment #66 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203525 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203525&action=edit Bug 42386: (follow-up) Document 409 vs 403 error codes for POST /holds The new 403 error_code list added for this bug included codes (expired, debt_limit, bad_address, card_lost, restricted, hold_limit) that Koha::REST::V1::Holds actually returns with HTTP 409 via can_place_holds, and omitted library_not_found and no_items from the 403 list entirely. Split the codes correctly across both response blocks and added the missing 409 response. Test plan: 1. Apply patch 2. yarn build; review the generated API docs for POST /holds 3. Sign off :-D 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=42386 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_26_11_candidate Target Milestone|--- |26.11 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 --- Comment #68 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205826 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205826&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> 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=42386 --- Comment #70 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205828 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205828&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> 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=42386 --- Comment #72 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205830 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205830&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 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=42386 --- Comment #74 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205832 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205832&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> 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=42386 --- Comment #76 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205834 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205834&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> 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=42386 --- Comment #78 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205836 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205836&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 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=42386 --- Comment #80 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205838 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205838&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 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=42386 --- Comment #82 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205840 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205840&action=edit Bug 42386: (follow-up) Fix stale camelCase status codes in hold templates opac-reserve.tt was never updated for the snake_case status rename and wasn't in the original patchset's file list, even though opac-reserve.pl sets its per-item hash key directly from CanBookBeReserved's status string. OPAC patrons blocked by age restriction, hold limits, or an existing hold on the item saw only the generic "This title cannot be requested" message instead of the specific reason. reserve/request.tt and opac-user.tt also missed two renames: alreadypossession -> already_possession, and branchNotInHoldGroup -> branch_not_in_hold_group. Test plan: 1. Apply patch 2. Place a hold that triggers each affected condition (age restriction, too many holds, already in possession, hold-group restriction) from both the OPAC search-results/place-hold screen and the staff reserve/request.pl page, and confirm the specific message is shown. 3. Sign off :-D 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=42386 --- Comment #84 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 205842 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=205842&action=edit Bug 42386: (follow-up) Update stale status-code fixtures in holds.t A few mocked CanItemBeReserved status fixtures still used camelCase strings left over from before the snake_case rename. Cosmetic only, since these mock the function directly and the value is otherwise arbitrary, but confusing to a future reader expecting real status codes. 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 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=42386 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA CC| |pedro.amorim@openfifth.co.u | |k --- Comment #85 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- #1 — Club-holds crash 1) Debar or expire an existing sample patron (Patron details > 'Edit' > set 'Restricted' or a past expiry date), and enroll them in a club (Tools > Patron clubs — create a club, enroll the patron). 2) From a biblio's normal 'Place a hold' page (Catalog > any record > Holds tab, or the 'Place hold' link), use the 'Search patrons or clubs' > 'Clubs' tab and search/select your club. This shows a "Club: <name> / Members" view listing each enrolled patron with any restriction warnings inline (e.g. "Patron has restrictions"). This is a real UI path — the 'Place hold' button submits POST /api/v1/clubs/{club_id}/holds directly via a JS-driven form (reserve/request.tt:385). 3) Click 'Place hold'. 4) Expected: hold placed or a clean error. Actual (confirmed): HTTP 500 in the browser console/network tab, and SELECT error_code FROM club_holds_to_patron_holds ORDER BY id DESC LIMIT 1; returns an empty set (no row inserted at all). #2 — Possession-policy bypass (confirmed) (staff user needs catalogue + reserveforothers > place_holds + borrowers > list_borrowers permissions) 1) Set syspref AllowHoldsOnPatronsPossessions to 'Don't allow'. 2) Find a record with 2+ items (using bibnumber 77 for this), check out item 39999000003239 to patron Henry. 3) In the staff interface, place a hold for patron Henry on the record and pick the item-level 'Hold a specific item' option for item B (a different copy of the same biblio). 4) Expected: blocked ('already possession'). Actual: hold succeeds — confirm on Patron X's 'Holds' tab. #3 — Reservesallowed bypass on title-level holds (REST API only — not reachable via any UI, live-confirmed) This is the exact concern Martin raised in Comment 23 (point 3): "we can't afford to lose the branch/itemtype level policy resolution." The follow-up patch that responded to it ("Restore branch/itemtype filtering on reservesallowed") only fixed the item-level path — CanBookBeReserved's biblio-level path, exploited below, was never touched. 1) Confirm syspref RESTBasicAuth = 'Enable'. 2) Set an itemtype-specific circulation rule: Administration > Circulation and fine rules, edit the row for patron category S, Item type = MU, 'Holds allowed (total)' = 1. Leave the 'All' itemtype row's holds-allowed blank for that category. 3) Place the first hold to reach the limit (note: use http://, not https:// — KTD's ports are plain HTTP): curl -sS -w '\nHTTP %{http_code}\n' -X POST 'http://localhost:8081/api/v1/holds' \ -u 'koha:koha' \ -H 'Content-Type: application/json' \ -d '{"patron_id": 51, "biblio_id": 76, "pickup_library_id": "CPL"}' 4) Now try a second title-level hold on the other MU record: curl -sS -w '\nHTTP %{http_code}\n' -X POST 'http://localhost:8081/api/v1/holds' \ -u 'koha:koha' \ -H 'Content-Type: application/json' \ -d '{"patron_id": 51, "biblio_id": 77, "pickup_library_id": "CPL"}' 5) Expected: second call returns 403 with error_code: too_many_reserves. Actual (confirmed): both return 201 — the limit is silently bypassed because CanBookBeReserved never resolves an itemtype for the rule lookup. 6) Note: only reachable via a direct API/class caller — both the staff interface and OPAC independently pre-check item availability correctly before ever trusting this call, so neither UI shows the bug. #4 — Itemtype-match semantics changed (item override) Introduced as a side effect of the same follow-up patch that responded to Martin's Comment 23 concern above — fixing #3's item-level case replaced the old COALESCE-priority itemtype match with a flat OR, creating this regression. 1) Enable syspref item-level_itypes = 'specific item', then edit an item on a Book biblio (I used bibnumber 437) and set that item's own itype field to MU (Music) directly — this is the override that makes item.itype diverge from biblioitem.itemtype. 2) Go to Administration > Circulation and fine rules, create a new rule for Item type = BK. Set that row's 'Holds allowed (total)' field (reservesallowed) to 1, and leave the Item type = All row's holds-allowed blank for that same category. Save. 3) Place an item-level hold on that MU-override item. 4) Patron then tries a hold on a different BK record (i.e. bibnumber 302). 5) Expected: succeeds (the held item's effective type is MU, shouldn't count against BK). Actual: blocked — biblioitem.itemtype (BK, from the bib record) also matches the OR condition even though the item's own itype is MU. #5 — Group-aware counting inconsistent 1) Enable DisplayAddHoldGroups and DisplayMultiItemHolds. 2) Set maxreserves (global syspref) to 3. 3) Set the patron's category (circulation rule) 'Holds allowed (total)' to 10+. Use a patron with zero existing holds, no restrictions. 4) Tick 3 records in search results, use batch 'Place hold'. 5) Select the patron, tick 'Treat as hold group'. 6) Click 'Place hold' — should succeed cleanly (3 reserves, same hold_group_id). 7) Attempt a 4th, separate single hold on a different record (pick an item, this must be an item level hold). It will error with 'hold_limit'. 8) Expected: succeeds (group should count as 1, like reservesallowed does). Actual: no new row — maxreserves counts all 3 individually and is already maxed. EXTRA: If in step 7 you instead do a title-level hold, it will fail silently (pre-existing bug, out of scope) #6 — SIP2 now silently blocks ineligible patrons (confirmed live, confirmed genuinely new behavior) 1) Expire or debar a patron (e.g. cardnumber 42, borrowernumber 51). 2) Run (from inside ktd --shell): perl /kohadevbox/koha/misc/sip_cli_emulator.pl \ -a localhost -p 6001 -su term1 -sp term1 -l CPL \ --patron 42 --item 39999000003154 -m hold 3) Expected (pre-42386 behavior): hold succeeds. Actual (confirmed): Hold Response 16 comes back with ok=0 (denied) and no AF screen-message field at all. The hold is silently denied with zero explanation for a self-checkout machine or patron to act on. #7 — Generic failure reason on title-level holds (confirmed, genuine regression) Martin asked for exactly this in Comment 23 (point 2): "could return both no_item_available and a 'reasons' array of itemnumber + failure code for future use?" The response patch ("Collect item-level failure reasons in biblio hold check") added that data internally (item_failures), but CanBookBeReserved never actually reads or exposes it — the generic message he flagged is still there. 1) Set syspref AgeRestrictionMarker to PG. Leave AgeRestrictionOverride at 'Don't allow'. 2) Administration > Koha to MARC mapping: map biblioitems.agerestriction to field 521 subfield a (521,a in the box). 3) Edit biblio 437, add field 521$a = PG 18. 4) Use patron cardnumber 23529000080862 (borrowernumber 7, category J, already 15 years old). 5) Attempt a title-level hold on biblio 437 for that patron. 6) Expected: specific reason ('Age restricted'). Actual (confirmed): generic 'no item available' — per-item table still shows the specific reason. #8 — Inconsistent ILS-DI casing (inspection) 1) Confirm ILS-DI = 'Enable', and ILS-DI:AuthorizedIPs = 127.0.0.1. 2) Set BlockExpiredPatronOpacActions to include 'Placing a hold on an item'. 3) Expired-patron case — patron cardnumber 23529001223636 (borrowernumber 50, expired since 2020-12-31): curl 'http://localhost:8080/cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=50&bib_id=76&request_location=127.0.0.1' Expect <code>PatronExpired</code>. 4) Bad-pickup-location case — patron cardnumber 23529000080862 (borrowernumber 7), same biblio 76, pickup location Ve5ejL3B: curl 'http://localhost:8080/cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=7&bib_id=76&request_location=127.0.0.1&pickup_location=Ve5ejL3B' Expect <code>library_not_pickup_location</code>. 5) PatronExpired/PatronNotFound is camelCase, library_not_pickup_location/cannot_be_transferred is snake_case. Previous to the patchset this would return LocationNotFound instead. #9 — Check order flipped 1) Set syspref ReservesControlBranch to PatronLibrary. 2) Patron cardnumber 42 (borrowernumber 51): cancel any existing holds and clear any restriction/expiry. 3) Scroll to the 'Default holds and bookings policies by item type' section (separate table). Use the blank row at the bottom — 'Item type' = MU, 'Hold policy' = 'From home library', leave the rest default, click Add. 4) Add a new circulation rule row for patron category S, Item type = MU, 'Holds allowed (total)' = 1. 5) Using this patron: place an item-level hold on item 39999000003253 (biblio 78, home branch CPL). 6) Same patron: attempt an item-level hold on item 39999000003130 (biblio 76, home branch MPL). 7) Expected (old order, before this patchset): hold-count reason shown (Too many holds). Actual (new order, after this patchset): branch/policy reason shown instead (Patron is from different library). -- You are receiving this mail because: You are watching all bug changes.