Bug 14464

Summary: Add ability to cancel waiting holds from checkin screen
Product: Koha Reporter: Jesse Weaver <jweaver>
Component: Hold requestsAssignee: Jesse Weaver <jweaver>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: gmcharlt, gwilliams, jonathan.druart, katrin.fischer, tomascohen
Version: unspecified   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14801
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 15670, 19746    
Attachments: Bug 14464 - Add ability to cancel waiting holds from checkin screen
Bug 14464 - Add ability to cancel waiting holds from checkin screen
Bug 14464 - (QA followup) add unit tests
Bug 14464 - Add ability to cancel waiting holds from checkin screen
Bug 14464 - (QA followup) add unit tests

Description Jesse Weaver 2015-06-26 03:31:17 UTC
It should be possible to manually cancel a waiting hold from the returns screen, and add the fine for an expired hold (if configured).
Comment 1 Jesse Weaver 2015-06-26 22:23:35 UTC Comment hidden (obsolete)
Comment 2 Jesse Weaver 2015-07-01 20:23:53 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2015-07-02 14:03:05 UTC
Jesse,
You have to provide tests for changes in subroutines.
Comment 4 Jesse Weaver 2015-07-07 19:03:52 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2015-07-09 10:20:34 UTC
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?
Comment 6 Jesse Weaver 2015-07-09 15:43:45 UTC
(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.
Comment 7 Jonathan Druart 2015-07-09 16:11:48 UTC
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.
Comment 8 Jonathan Druart 2015-07-09 16:11:51 UTC
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>
Comment 9 Tomás Cohen Arazi 2015-08-21 13:23:27 UTC
Patches pushed to master.

Thanks Jesse!
Comment 10 Katrin Fischer 2016-01-26 14:17:10 UTC
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).