Bug 14464 - Add ability to cancel waiting holds from checkin screen
Summary: Add ability to cancel waiting holds from checkin screen
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jesse Weaver
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 15670
  Show dependency treegraph
 
Reported: 2015-06-26 03:31 UTC by Jesse Weaver
Modified: 2017-06-14 22:05 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 14464 - Add ability to cancel waiting holds from checkin screen (11.78 KB, patch)
2015-06-26 22:23 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 14464 - Add ability to cancel waiting holds from checkin screen (11.90 KB, patch)
2015-07-01 20:23 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 14464 - (QA followup) add unit tests (3.50 KB, patch)
2015-07-07 19:03 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 14464 - Add ability to cancel waiting holds from checkin screen (12.01 KB, patch)
2015-07-09 16:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14464 - (QA followup) add unit tests (3.57 KB, patch)
2015-07-09 16:11 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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).