It should be possible to manually cancel a waiting hold from the returns screen, and add the fine for an expired hold (if configured).
Created attachment 40697 [details] [review] Bug 14464 - Add ability to cancel waiting holds from checkin screen Test plan: 1. Ensure that ExpireReservesMaxPickUpDelayCharge is set to 0. 2. Place a hold (doesn't matter whether it's a bib/item-level hold), then confirm the hold by checking it in. 3. Check in the item again, and hit Cancel. 4. The reserve in question should be cancelled. 5. Repeat steps 2-4 twice, once after setting ExpireReservesMaxPickUpDelayCharge to a nonzero value and again after clicking the "Forgive fees for manually expired holds" checkbox. A fine should only be applied when the syspref is enabled and the checkbox is not checked. Also, the checkbox should only appear after enabling the syspref. And finally, the checkbox should remember whether it is checked across multiple checkins, same as the "Forgive overdue charges" and "Book drop mode" checkboxes.
Created attachment 40752 [details] [review] Bug 14464 - Add ability to cancel waiting holds from checkin screen Test plan: 1. Ensure that ExpireReservesMaxPickUpDelayCharge is set to 0. 2. Place a hold (doesn't matter whether it's a bib/item-level hold), then confirm the hold by checking it in. 3. Check in the item again, and hit Cancel. 4. The reserve in question should be cancelled. 5. Repeat steps 2-4 twice, once after setting ExpireReservesMaxPickUpDelayCharge to a nonzero value and again after clicking the "Forgive fees for manually expired holds" checkbox. A fine should only be applied when the syspref is enabled and the checkbox is not checked. Also, the checkbox should only appear after enabling the syspref. And finally, the checkbox should remember whether it is checked across multiple checkins, same as the "Forgive overdue charges" and "Book drop mode" checkboxes. Signed-off-by: Jason Burds <jburds@dubuque.lib.ia.us>
Jesse, You have to provide tests for changes in subroutines.
Created attachment 40828 [details] [review] Bug 14464 - (QA followup) add unit tests This followup adds several tests to t/db_dependent/Reserves.t.
Comment on attachment 40752 [details] [review] Bug 14464 - Add ability to cancel waiting holds from checkin screen Review of attachment 40752 [details] [review]: ----------------------------------------------------------------- ::: circ/returns.pl @@ +30,5 @@ > use strict; > use warnings; > > +use Carp 'verbose'; > +$SIG{ __DIE__ } = sub { Carp::confess( @_ ) }; Why do you need that?
(In reply to Jonathan Druart from comment #5) > Comment on attachment 40752 [details] [review] [review] > Bug 14464 - Add ability to cancel waiting holds from checkin screen > > Review of attachment 40752 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: circ/returns.pl > @@ +30,5 @@ > > use strict; > > use warnings; > > > > +use Carp 'verbose'; > > +$SIG{ __DIE__ } = sub { Carp::confess( @_ ) }; > > Why do you need that? ... because I forgot to remove some debugging code, apologies. That's safe to remove.
Created attachment 40896 [details] [review] Bug 14464 - Add ability to cancel waiting holds from checkin screen Test plan: 1. Ensure that ExpireReservesMaxPickUpDelayCharge is set to 0. 2. Place a hold (doesn't matter whether it's a bib/item-level hold), then confirm the hold by checking it in. 3. Check in the item again, and hit Cancel. 4. The reserve in question should be cancelled. 5. Repeat steps 2-4 twice, once after setting ExpireReservesMaxPickUpDelayCharge to a nonzero value and again after clicking the "Forgive fees for manually expired holds" checkbox. A fine should only be applied when the syspref is enabled and the checkbox is not checked. Also, the checkbox should only appear after enabling the syspref. And finally, the checkbox should remember whether it is checked across multiple checkins, same as the "Forgive overdue charges" and "Book drop mode" checkboxes. Signed-off-by: Jason Burds <jburds@dubuque.lib.ia.us> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Amended patch: Removed 2 debugging lines.
Created attachment 40897 [details] [review] Bug 14464 - (QA followup) add unit tests This followup adds several tests to t/db_dependent/Reserves.t. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Patches pushed to master. Thanks Jesse!
We just started testing the new features of 3.22 and didn't understand that "cancel" meant cancelling the hold, as cancel usually means "do nothing". I filed bug 15670 to suggest renaming the button to the more explicit "Cancel hold" - especially since it will ease translation (abbrechen vs. stornieren in German).