https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386
Pedro Amorim (ammopt) <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA CC| |[email protected] | |k --- Comment #85 from Pedro Amorim (ammopt) <[email protected]> --- #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. _______________________________________________ Koha-bugs mailing list -- [email protected] To unsubscribe send an email to [email protected] website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
