Description
Nick Clemens
2020-02-10 18:18:25 UTC
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. *** |