[Bug 40331] New: Extra transfer generated when transfer for hold cancelled due to checkin at incorrect library
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Bug ID: 40331 Summary: Extra transfer generated when transfer for hold cancelled due to checkin at incorrect library Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: andrew@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, lisette@bywatersolutions.com, tomascohen@gmail.com To recreate: 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 - in reports: SELECT * FROM branchtransfers WHERE itemnumber=YOUR_ITEM AND date(daterequested)=curdate() -- See your transfer 4 - check your item in at Branch A again, get a message that the item is supposed to go to Branch B. Confirm and transfer. 5 - Run your report again, you now see 3 transfers: the original transfer has been cancelled and Koha has generated two new transfers. One of these new transfers has a datesent value, the other does not. The third transfer, the one that doesn't get Sent right away, ends up orphaned in the system. -- 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=40331 hebah@bywatersolutions.com changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hebah@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@openfifth.c | |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com --- Comment #1 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- 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? -- 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=40331 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |lisette@bywatersolutions.co |ity.org |m -- 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=40331 Sally Lodico <Slodico@knoxlib.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |Slodico@knoxlib.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 mteal@greenvillelibrary.org changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mteal@greenvillelibrary.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Michael Adamyk <michael.adamyk@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |michael.adamyk@bywatersolut | |ions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 --- Comment #2 from Lisette Scheer <lisette@bywatersolutions.com> --- (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? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 David Roberts <david.roberts@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david.roberts@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=40331 Rebecca Coert <rcoert@arlingtonva.us> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |rcoert@arlingtonva.us -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Katie Bliss <kebliss@dmpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kebliss@dmpl.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=40331 --- Comment #3 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 184629 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184629&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|lisette@bywatersolutions.co |nick@bywatersolutions.com |m | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Andrew Fuerste-Henry <andrew@bywatersolutions.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=40331 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #184629|0 |1 is obsolete| | --- Comment #4 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 184653 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184653&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 karie.straube@yavapaiaz.gov changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |karie.straube@yavapaiaz.gov -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Emily Lamancusa (emlam) <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |emily.lamancusa@montgomeryc |y.org |ountymd.gov CC| |emily.lamancusa@montgomeryc | |ountymd.gov -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #184653|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 185814 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185814&action=edit 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> 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=40331 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA QA Contact|emily.lamancusa@montgomeryc |martin.renvoize@openfifth.c |ountymd.gov |o.uk --- Comment #6 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- My apologies, I thought I'd QA'd this one before I went on leave. This make a lot of sense, is well tested and causes no regressions, Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA CC| |lucas@bywatersolutions.com --- Comment #7 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- prove -v t/db_dependent/Circulation/transfers.t # Failed test 'Date cancelled is set when new transfer is initiated' # at t/db_dependent/Circulation/transfers.t line 263. # Failed test 'Cancellation reason is set correctly when new transfer is initiated' # at t/db_dependent/Circulation/transfers.t line 264. # got: undef # expected: 'Manual' ok 20 - no warnings # Looks like you failed 2 tests of 20. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 --- Comment #8 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 185944 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185944&action=edit Bug 40331: (follow-up) Adjust t/db_dependent/Circulation/transfers.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Steve, OSLRI, USA <sspohn@oslri.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bywater@oslri.net, | |sspohn@oslri.net -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |25.11.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 --- Comment #9 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 25.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 --- Comment #10 from mteal@greenvillelibrary.org --- Glad to see this has been fixed! Would love to see it backported to 25.05! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_25_05_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Laura O'Neil <laura@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |ahernandez@ckls.org, | |laura@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 --- Comment #11 from Laura O'Neil <laura@bywatersolutions.com> --- +1 for a backport to 25.05, if possible. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Christine Lee <chlee@pascolibraries.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chlee@pascolibraries.org --- Comment #12 from Christine Lee <chlee@pascolibraries.org> --- +1 for a backport to 25.05 if possible! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Michelle Spinney <mspinney@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mspinney@clamsnet.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 --- Comment #13 from David Roberts <david.roberts@openfifth.co.uk> --- Would be great if this could also be backported to 24.11 - thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable Version(s)|25.11.00 |25.11.00,25.05.04 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 --- Comment #14 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Nice work everyone! Pushed to 25.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_25_05_candidate |rel_24_11_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|25.11.00,25.05.04 |25.11.00,25.05.04,24.11.10 released in| | CC| |fridolin.somers@biblibre.co | |m Status|Pushed to stable |Pushed to oldstable --- Comment #15 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 24.11.x for 24.11.10 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40331 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |Needs documenting --- Comment #16 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Not backporting to 22.11.x as it wasn't backported to 24.05.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org