To recreate: 1 - Set AutomaticItemReturn to Do 2 - Checkin an item from Library B at Library A 3 - Confirm item is in transfer (check the details page) 4 - Place a item level hold for pickup at library A 5 - Checkin the item at Library A 6 - Confirm the hold 7 - View the details page 8 - Note the item is in transit and waiting
Created attachment 98678 [details] [review] Bug 24620: Unit tests
Created attachment 98679 [details] [review] Bug 24620: Close existing transfers when setting item to waiting This patch adds a clause in ModReserveAffect to check if there are existing transfers and close them when setting a hold to waiting To test: 1 - Set AutomaticItemReturn to Do 2 - Checkin an item from Library B at Library A 3 - Confirm item is in transfer (check the details page) 4 - Place a item level hold for pickup at library A 5 - Checkin the item at Library A 6 - Confirm the hold 7 - View the details page 8 - Note the item is in transit and waiting 9 - Apply patch 10 - Delete hold and repeat 11 - Confirm that transfer is closed when hold marked waiting
Created attachment 98711 [details] [review] Bug 24620: Close existing transfers when setting item to waiting This patch adds a clause in ModReserveAffect to check if there are existing transfers and close them when setting a hold to waiting To test: 1 - Set AutomaticItemReturn to Do 2 - Checkin an item from Library B at Library A 3 - Confirm item is in transfer (check the details page) 4 - Place a item level hold for pickup at library A 5 - Checkin the item at Library A 6 - Confirm the hold 7 - View the details page 8 - Note the item is in transit and waiting 9 - Apply patch 10 - Delete hold and repeat 11 - Confirm that transfer is closed when hold marked waiting Signed-off-by: Sally <sally.healey@cheshirewestandchester.gov.uk>
Created attachment 98805 [details] [review] Bug 24620: Close existing transfers when setting item to waiting This patch adds a clause in ModReserveAffect to check if there are existing transfers and close them when setting a hold to waiting To test: 1 - Set AutomaticItemReturn to Do 2 - Checkin an item from Library B at Library A 3 - Confirm item is in transfer (check the details page) 4 - Place a item level hold for pickup at library A 5 - Checkin the item at Library A 6 - Confirm the hold 7 - View the details page 8 - Note the item is in transit and waiting 9 - Apply patch 10 - Delete hold and repeat 11 - Confirm that transfer is closed when hold marked waiting Signed-off-by: Sally <sally.healey@cheshirewestandchester.gov.uk> Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se>
Nick, did not you forget to remove the existing call to _koha_notify_reserve 2 lines before? now we have: 1140 _koha_notify_reserve( $hold->reserve_id ) 1141 if ( !$transferToDo && !$already_on_shelf ); 1142 1143 if( !$transferToDo ){ 1144 _koha_notify_reserve( $hold->reserve_id ) unless $already_on_shelf;
Also: 1. ->store not needed on ->build_object 2. DateTime->now should not be used directly (or add arguments) 3. Tests are failing: t/db_dependent//Circulation.t .. 45/47 # Looks like you planned 4 tests but ran 1. # Failed test 'Filling a hold should cancel existing transfer' # at t/db_dependent//Circulation.t line 3783. Can't use string ("ZSpzGDUl") as a HASH ref while "strict refs" in use at /kohadevbox/koha/C4/Reserves.pm line 175. # Looks like your test exited with 255 just after 47.
Created attachment 100448 [details] [review] Bug 24620: Remove original call to notify reserve
Created attachment 100451 [details] [review] Bug 24620: Fix test, remove replaced code, use dt_from_string
What if branchtransfers.tobranch is not the logged in user's library?
(In reply to Jonathan Druart from comment #9) > What if branchtransfers.tobranch is not the logged in user's library? Then $transferToDo should be true and the code should not be called
(In reply to Nick Clemens from comment #10) > (In reply to Jonathan Druart from comment #9) > > What if branchtransfers.tobranch is not the logged in user's library? > > Then $transferToDo should be true and the code should not be called circ/branchtransfers.pl 102 ModReserveAffect( $item, $borrowernumber );
(In reply to Jonathan Druart from comment #11) > (In reply to Nick Clemens from comment #10) > > (In reply to Jonathan Druart from comment #9) > > > What if branchtransfers.tobranch is not the logged in user's library? > > > > Then $transferToDo should be true and the code should not be called > > circ/branchtransfers.pl > 102 ModReserveAffect( $item, $borrowernumber ); see bug 24839 - that code is intended to allow the transfer of a waiting hold - so cancelling when setting to waiting doesn't affect that workflow (that workflow may be problematic on its own) - the hold is set waiting, active transfers closed, then a new transfer generated
Created attachment 102914 [details] [review] Bug 24620: Unit tests Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 102915 [details] [review] Bug 24620: Close existing transfers when setting item to waiting This patch adds a clause in ModReserveAffect to check if there are existing transfers and close them when setting a hold to waiting To test: 1 - Set AutomaticItemReturn to Do 2 - Checkin an item from Library B at Library A 3 - Confirm item is in transfer (check the details page) 4 - Place a item level hold for pickup at library A 5 - Checkin the item at Library A 6 - Confirm the hold 7 - View the details page 8 - Note the item is in transit and waiting 9 - Apply patch 10 - Delete hold and repeat 11 - Confirm that transfer is closed when hold marked waiting Signed-off-by: Sally <sally.healey@cheshirewestandchester.gov.uk> Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 102916 [details] [review] Bug 24620: Fix test, remove replaced code, use dt_from_string Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Nice work everyone! Pushed to master for 20.05
doesn't apply to 19.11.x not backported
Created attachment 104437 [details] [review] Bug 24620: [19.11.x] Close existing transfer when hold set to waiting (squashed) Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Bug 24620: Close existing transfers when setting item to waiting This patch adds a clause in ModReserveAffect to check if there are existing transfers and close them when setting a hold to waiting To test: 1 - Set AutomaticItemReturn to Do 2 - Checkin an item from Library B at Library A 3 - Confirm item is in transfer (check the details page) 4 - Place a item level hold for pickup at library A 5 - Checkin the item at Library A 6 - Confirm the hold 7 - View the details page 8 - Note the item is in transit and waiting 9 - Apply patch 10 - Delete hold and repeat 11 - Confirm that transfer is closed when hold marked waiting Signed-off-by: Sally <sally.healey@cheshirewestandchester.gov.uk> Signed-off-by: Stina Hallin <stina.hallin@ub.lu.se> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Bug 24620: Fix test, remove replaced code, use dt_from_string Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Thanks for the rebase - backported to 19.11.x for 19.11.06
missing dependencies for 19.05.x no backport
*** Bug 14170 has been marked as a duplicate of this bug. ***