Summary: | Extra transfer generated when transfer for hold cancelled due to checkin at incorrect library | ||
---|---|---|---|
Product: | Koha | Reporter: | Andrew Fuerste-Henry <andrew> |
Component: | Hold requests | Assignee: | Nick Clemens (kidclamp) <nick> |
Status: | Signed Off --- | QA Contact: | Testopia <testopia> |
Severity: | normal | ||
Priority: | P5 - low | CC: | david.roberts, gmcharlt, hebah, karie.straube, kebliss, lisette, martin.renvoize, michael.adamyk, mteal, nick, rcoert, Slodico, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | --- | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
Attachments: |
Bug 40331: Return existing transfer if requesting a new transfer that is identical
Bug 40331: Return existing transfer if requesting a new transfer that is identical |
Description
Andrew Fuerste-Henry
2025-07-08 18:46:28 UTC
This seems to be conflicting ideas in the code. At step 4, we get a WrongTransfer message and pop-up a modal, before displaying that modal we do: 553 # We need to ignore limits here. While we can't transfer from this branch, it is, wrongly, here right now 554 # and that fact must be recorded 555 my $new_transfer = $item->request_transfer( 556 { 557 to => $old_transfer->to_library, 558 reason => $old_transfer->reason, 559 replace => 'WrongTransfer', 560 ignore_limits => 1 561 } 562 ); So one transfer is closed and a new one created. Then when the 'Confirm hold and transfer' button is clicked we hit the cud-affect_reserve code: 156 # Add transfer, enqueue if one is already in the queue, and immediately set to in transit 157 my $transfer = $item->request_transfer( { to => $tobranch, reason => 'Reserve', enqueue => 1 } ); 158 $transfer->transit; So we are intentionally queuing a second transfer - I don't understand the case where the first transfer needs to be left in place - or perhaps we can simply check if the first transfer exists, is going to the correct library, and then simply mark the existing one as in transit from our current location? (In reply to Nick Clemens (kidclamp) from comment #1) > This seems to be conflicting ideas in the code. > > At step 4, we get a WrongTransfer message and pop-up a modal, before > displaying that modal we do: > 553 # We need to ignore limits here. While we can't transfer from this > branch, it is, wrongly, here right now > 554 # and that fact must be recorded > 555 my $new_transfer = $item->request_transfer( > 556 { > 557 to => $old_transfer->to_library, > 558 reason => $old_transfer->reason, > 559 replace => 'WrongTransfer', > 560 ignore_limits => 1 > 561 } > 562 ); So this step updates the original transfer to cancelled with wrongtransfer reason and creates a new transfer > So one transfer is closed and a new one created. Then when the 'Confirm hold > and transfer' button is clicked we hit the cud-affect_reserve code: > 156 # Add transfer, enqueue if one is already in the queue, and > immediately set to in transit > 157 my $transfer = $item->request_transfer( { to => $tobranch, > reason => 'Reserve', enqueue => 1 } ); > 158 $transfer->transit; > > > So we are intentionally queuing a second transfer - I don't understand the > case where the first transfer needs to be left in place - or perhaps we can > simply check if the first transfer exists, is going to the correct library, > and then simply mark the existing one as in transit from our current > location? Then here we have a new transfer that is created and sent. We should instead be updating the transfer from the previous code. Should we be showing the "wrong transfer" modal? Currently the "hold-modal" is what shows, but I believe previously "wrong transfer" showed and the transfer could be canceled if needed? Created attachment 184629 [details] [review] Bug 40331: Return existing transfer if requesting a new transfer that is identical Currently request_transfer checks for the existence of a current transfer on the item, and a new one can either be enqueued or replace this transfer. This patch adds a check to see if the requested transfer matches an existing transfer on the item. In the case that it does we simply return that transfer rather than create a new transfer. This has the effect of eliminating 'WrongTransfer' generation if an item is checked in twice at the same branch, which feels correct. To test: 1 - place a hold on an item that is currently at branch A, for pickup at branch B 2 - check your item in at Branch A, click to confirm the hold and transfer (print slip if you want to) 3 - sudo koha-mysql kohadev: SELECT * FROM branchtransfers -- See one transfer 4 - check your item in at Branch A again, get a message that the item is supposed to go to Branch B. Don't click on the modal yet 5 - repeat 3 -- See two transfers, one cancelled as WrongTransfer, one pending 6 - Click 'Confirm and transfer' on the modal 7 - repeat 3 -- See three tranfers - the two from before and a new active transfer 8 - DELETE FROM branchtransfers 9 - Apply patch, restart all 10 - repeat 3 - no transfers 11 - Check item in at branch A again and confirm 12 - repeat 3, one transfer 13 - Check item in, don't confirm the modal 14 - repeat 3, one transfer 15 - Confirm the modal 16 - repeat 3, one transfer 17 - Check the item in at branch C 18 - repeat 3, two transfers - one cancelled WrongTransfer, one active 19 - Confirm modal 20 - repeat 3, two transfers 21 - Check item in at branch B 22 - repeat 3, two transfers, one cancelled, one completed Created attachment 184653 [details] [review] Bug 40331: Return existing transfer if requesting a new transfer that is identical Currently request_transfer checks for the existence of a current transfer on the item, and a new one can either be enqueued or replace this transfer. This patch adds a check to see if the requested transfer matches an existing transfer on the item. In the case that it does we simply return that transfer rather than create a new transfer. This has the effect of eliminating 'WrongTransfer' generation if an item is checked in twice at the same branch, which feels correct. To test: 1 - place a hold on an item that is currently at branch A, for pickup at branch B 2 - check your item in at Branch A, click to confirm the hold and transfer (print slip if you want to) 3 - sudo koha-mysql kohadev: SELECT * FROM branchtransfers -- See one transfer 4 - check your item in at Branch A again, get a message that the item is supposed to go to Branch B. Don't click on the modal yet 5 - repeat 3 -- See two transfers, one cancelled as WrongTransfer, one pending 6 - Click 'Confirm and transfer' on the modal 7 - repeat 3 -- See three tranfers - the two from before and a new active transfer 8 - DELETE FROM branchtransfers 9 - Apply patch, restart all 10 - repeat 3 - no transfers 11 - Check item in at branch A again and confirm 12 - repeat 3, one transfer 13 - Check item in, don't confirm the modal 14 - repeat 3, one transfer 15 - Confirm the modal 16 - repeat 3, one transfer 17 - Check the item in at branch C 18 - repeat 3, two transfers - one cancelled WrongTransfer, one active 19 - Confirm modal 20 - repeat 3, two transfers 21 - Check item in at branch B 22 - repeat 3, two transfers, one cancelled, one completed Signed-off-by: Christine Lee <chlee@pascolibraries.org> |