[Bug 43844] New: Checked-out item with a biblio-level hold can be put as waiting
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43844 Bug ID: 43844 Summary: Checked-out item with a biblio-level hold can be put as waiting Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: arthur.suzuki@biblibre.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle@bywatersolutions.com Target Milestone: --- In Koha, an item that is checked out cannot at the same time be "waiting for pickup" for a hold: the item has to be physically back at the library before it can be put aside for the patron. However, a librarian can put an item that is already issued to another patron — and that has a hold — into the "waiting for pickup" state. This is not a race condition: the item was already issued when the librarian confirmed the hold. Steps to reproduce ================== 1. Create a biblio with one item. Check the item out to patron A. 2. Place an item-level hold on that item for patron B (pickup at the same library). 3. Go to Circulation, scan patron B's card, then scan the item's barcode. 4. The checkout screen shows that the item is issued to another patron and, because of the hold, also offers "Don't check out, confirm hold, and print slip". 5. Click "Confirm hold". 6. Open patron B's account and look at the Holds tab. -- 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=43844 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Sponsorship status|--- |Sponsored Patch complexity|--- |Small patch Comma delimited| |BibLibre list of Sponsors| | Status|NEW |Needs Signoff Assignee|koha-bugs@lists.koha-commun |arthur.suzuki@biblibre.com |ity.org | -- 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=43844 --- Comment #1 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 208415 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208415&action=edit Bug 43844: Add regression test: an on-loan item cannot be set waiting for a hold A librarian could put an item that is already issued (and has a hold) into the waiting-for-pickup state via circ/circulation.pl (op=cud-confirm_hold -> C4::Reserves::ModReserveAffect), because nothing validated the item's loan status when setting found='W'. Add a subtest to t/db_dependent/Reserves.t reproducing the scenario: item checked out to patron A, hold placed by patron B, then assert that both Koha::Hold->set_waiting and ModReserveAffect refuse to set the hold waiting while the item is on loan, and that the hold stays pending with no item attached. This test fails without the accompanying fix in Koha::Hold. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43844 --- Comment #2 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 208416 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208416&action=edit Bug 43844: Prevent setting a hold waiting when its item is on loan Koha::Hold::set_waiting set found='W' unconditionally. Combined with C4::Reserves::ModReserveAffect (which attaches any item to the hold before calling set_waiting), this allowed a librarian to put an already-issued item into the waiting-for-pickup state from the circulation screen (op=cud-confirm_hold), even though an item on loan cannot be picked up for a hold. set_waiting now throws Koha::Exceptions::Hold::ItemOnLoan when the item attached to the hold is on loan. This is the single choke point where found='W' is set, so all callers are protected (staff UI, SIP, ILS-DI, automatic checkin, hold autofill). The check uses the same items.onloan condition as the existing AddReserve guard. All legitimate callers operate on items that are not on loan (returns/SIP/automatic checkin run after checkin, branch transfers use the transfer path, autofill runs on a shelf item), so they are unaffected. Test plan: 1. Add a biblio with one item. Check the item out to patron A. 2. Place an item-level hold on that item for patron B. 3. Circulation: scan patron B's card, then scan the item's barcode. The screen shows the item is issued to another patron and offers "Don't check out, confirm hold, and print slip". 4. Click "Confirm hold". => FAIL before the patch: the hold is set waiting (found='W') even though the item is still on loan to patron A. => SUCCESS after the patch: the hold is NOT set waiting; an error is displayed instead, and on patron B's account the hold is still "Pending" (not "Waiting") while the item stays on loan to patron A. 5. Check the item back in (Circulation > Check in, scan the barcode). => SUCCESS: the hold is set waiting (found='W') for patron B as usual; the item shows up in "Holds to pull" and can be picked up. 6. Run the unit tests: $ prove t/db_dependent/Reserves.t => SUCCESS: 73 tests pass, including the new regression subtest 'ModReserveAffect() does not set an item on loan waiting'. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43844 --- Comment #3 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Created attachment 208417 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208417&action=edit Bug 43844: Fix test data: holds used with set_waiting need a non-loan item The new Koha::Hold::set_waiting guard throws when the hold's item is on loan. The generic test builder fills items.onloan with a random datetime for FK-auto-created items, so holds built with build_object({ class => 'Koha::Holds' }) looked on loan and the set_waiting calls in these subtests started throwing. Give those holds a real available item (build_sample_item) instead. In Holds_hooks.t the item creation hooks are mocked out following the pattern already used by the other subtests in the file, to keep the 'no warnings' test happy. No behavior change; these subtests only exercise waitingdate, expirationdate, suspend and plugin hook logic. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org