From 5f21c2efad94411f96832212b571b72c7b33515f Mon Sep 17 00:00:00 2001 From: Emily Lamancusa Date: Wed, 10 Jan 2024 10:44:39 -0500 Subject: [PATCH] Bug 34972: GetOtherReserves should not alter hold states GetOtherReserves attempts to set the waiting/transit status for the next hold on the list when applicable, but in practice it either leaves the hold state unchanged, or sets the itemnumber without setting the found status (erroneously converting bib-level holds to item-level holds). The latter situation only occurs when the user has been prompted to confirm, cancel, or revert the hold, and is able to ignore the prompt. In those situations, the hold's state should not change. GetOtherReserves does not need to change the hold's state, and attempting to do so produces no effect except for erroneously converting bib-level holds to item-level holds in certain situations, so this patch removes that code. To test: 1. Place 2 bib-level holds for 2 different patrons (Patron A and Patron B) on the same record, both for pickup at the logged-in library 2. Check in an item from that record to fill Patron A's hold 3. Set the hold's expiration date to yesterday by accessing the database in the command line: - In a ktd shell prompt, open the db client with koha-mysql kohadev - UPDATE reserves SET expirationdate = DATE_SUB(CURDATE(), INTERVAL 1 DAY) WHERE borrowernumber = 4. Go to Circulation > Holds Awaiting Pickup, and find the hold in the "holds waiting past their expiration date" tab 5. Click the "Cancel hold" button in the Actions column next to the hold 6. Return to the bib record and look at Patron B's hold --> Note that Patron B's hold is now an item-level hold and does not have a waiting status 7. Cancel Patron B's hold 8. Place 2 new holds on the record: one for Patron A at the logged-in library, and one for Patron B at a different library 9. Check in an item to fill Patron A's hold 10. Repeat steps 3-5 to expire and cancel Patron A's hold 11. Return to the bib record and look at Patron B's hold --> Note that Patron B's hold is now an item-level hold and does not have a transit status 12. Place 2 bib-level holds for 2 different patrons (Patron A and Patron B) on the same record, both for pickup at the logged-in library 13. Check in an item from that record to fill Patron A's hold 14. Check in the same item again. A modal will pop up, saying that the hold is already waiting 15. In the modal, choose a cancellation reason and click "Cancel hold" --> A new modal will pop up to fill Patron B's hold 16. Click "Ignore" on the modal for Patron B's hold 17. Return to the bib record and look at Patron B's hold --> Note that Patron B's hold is now an item-level hold and does not have a waiting status 18. Apply patch 19. Repeat steps 1-6 --> Note that Patron B's hold is still a bib-level/"next available" hold 20. Repeat steps 7-11 --> Note that Patron B's hold is still a bib-level/"next available" hold 21. Repeat steps 12-17 --> Note that Patron B's hold is still a bib-level/"next available" hold 22. Prove t/db_dependent/Circulation.t 23. Prove t/db_dependent/Koha/Holds.t --> Tests pass --- C4/Reserves.pm | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index c95a3d7ad4..7abf211e87 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -729,32 +729,15 @@ sub GetOtherReserves { my $item = Koha::Items->find($itemnumber); my ( undef, $checkreserves, undef ) = CheckReserves($item); if ($checkreserves) { + + #step 2a : case of a reservation at a different branch, return 'transfert' message if ( $item->holdingbranch ne $checkreserves->{'branchcode'} ) { $messages->{'transfert'} = $checkreserves->{'branchcode'}; - #minus priorities of others reservs - ModReserveMinusPriority( - $itemnumber, - $checkreserves->{'reserve_id'}, - ); - - #launch the subroutine dotransfer - C4::Items::ModItemTransfer( - $itemnumber, - $item->holdingbranch, - $checkreserves->{'branchcode'}, - 'Reserve' - ), - ; } - #step 2b : case of a reservation on the same branch, set the waiting status + #step 2b : case of a reservation on the same branch, return 'waiting' message else { $messages->{'waiting'} = 1; - ModReserveMinusPriority( - $itemnumber, - $checkreserves->{'reserve_id'}, - ); - ModReserveStatus($itemnumber,'W'); } $nextreservinfo = $checkreserves; -- 2.34.1