Bug 21075 - AutoUnsuspendHolds should unsuspend holds <= today
Summary: AutoUnsuspendHolds should unsuspend holds <= today
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-07-13 14:44 UTC by Nick Clemens
Modified: 2018-09-21 10:25 UTC (History)
6 users (show)

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


Attachments
Bug 21075: AutoUnsuspendHolds should unsuspend holds <= today (1.38 KB, patch)
2018-07-13 15:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 21075: Unit tests (2.22 KB, patch)
2018-07-13 15:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 21075: Unit tests (2.29 KB, patch)
2018-07-13 19:08 UTC, Pierre-Luc Lapointe
Details | Diff | Splinter Review
Bug 21075: AutoUnsuspendHolds should unsuspend holds <= today (1.44 KB, patch)
2018-08-02 11:27 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 21075: Unit tests (2.34 KB, patch)
2018-08-02 11:28 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2018-07-13 14:44:13 UTC
This is an issue because of the datepicker on the holds page in intranet - it doesn't allow you to select today's date, so it sets the date to tomorrow, if you then update another hold you have just extended the suspension - and it can keep happening.  

To recreate:
1 - Place a hold in your system
2 - Set that hold (or all holds) to expire today
    update reserves set suspend_until=CONCAT( CURDATE()," 00:00:00");
3 - Run misc/cronjobs/holds/auto_unsuspend_holds.p
4 - Note the hold is sitll suspended
5 - Visit /cgi-bin/koha/reserve/request.pl for the book with the hold
6 - Note the suspend date is tomorrow (and cannot be set to today]
7 - Click update holds - the date in db is now set to tomorrow
Comment 1 Nick Clemens 2018-07-13 15:15:04 UTC
Created attachment 76953 [details] [review]
Bug 21075: AutoUnsuspendHolds should unsuspend holds <= today

To recreate:
 1 - Place a hold in your system
 2 - Set that hold (or all holds) to expire today
    update reserves set suspend_until=CONCAT( CURDATE()," 00:00:00");
 3 - Run misc/cronjobs/holds/auto_unsuspend_holds.p
 4 - Note the hold is sitll suspended
 5 - Visit /cgi-bin/koha/reserve/request.pl for the book with the hold
 6 - Note the suspend date is tomorrow (and cannot be set to today]
 7 - Click update holds - the date in db is now set to tomorrow
 8 - Reset to today
 9 - Apply patch
10 - Run the cron again
11 - Visit the page and note hold is unsuspended
Comment 2 Nick Clemens 2018-07-13 15:15:09 UTC
Created attachment 76954 [details] [review]
Bug 21075: Unit tests

prove -v t/db_dependent/Reserves/AutoUnsuspendReserves.t
Comment 3 Pierre-Luc Lapointe 2018-07-13 19:08:19 UTC
Created attachment 76960 [details] [review]
Bug 21075: Unit tests

prove -v t/db_dependent/Reserves/AutoUnsuspendReserves.t

Signed-off-by: Pierre-Luc Lapointe <pierreluc.lapointe@inLibro.com>
Comment 4 Jonathan Druart 2018-07-23 15:30:04 UTC
(In reply to Nick Clemens from comment #0)
> This is an issue because of the datepicker on the holds page in intranet -
> it doesn't allow you to select today's date, so it sets the date to
> tomorrow

Why do not we fix the UI instead?
Comment 5 Nick Clemens 2018-07-23 15:51:04 UTC
(In reply to Jonathan Druart from comment #4)
> (In reply to Nick Clemens from comment #0)
> > This is an issue because of the datepicker on the holds page in intranet -
> > it doesn't allow you to select today's date, so it sets the date to
> > tomorrow
> 
> Why do not we fix the UI instead?

Because the script is wrong, and fixing it means the UI works right :-)

The description in staff side when suspending holds:
 Specify date on which to resume (MM/DD/YYYY) : 

If we resume on that date, then  we unsuspend before that date (today)
Comment 6 Katrin Fischer 2018-08-02 11:25:59 UTC
I think the patch is correct, but there is still a bug in the GUI. I think we should show the actual suspension date accurately somehow. This still fixes a bug, so passing.
Comment 7 Katrin Fischer 2018-08-02 11:27:56 UTC
Created attachment 77423 [details] [review]
Bug 21075: AutoUnsuspendHolds should unsuspend holds <= today

To recreate:
 1 - Place a hold in your system
 2 - Set that hold (or all holds) to expire today
    update reserves set suspend_until=CONCAT( CURDATE()," 00:00:00");
 3 - Run misc/cronjobs/holds/auto_unsuspend_holds.p
 4 - Note the hold is sitll suspended
 5 - Visit /cgi-bin/koha/reserve/request.pl for the book with the hold
 6 - Note the suspend date is tomorrow (and cannot be set to today]
 7 - Click update holds - the date in db is now set to tomorrow
 8 - Reset to today
 9 - Apply patch
10 - Run the cron again
11 - Visit the page and note hold is unsuspended

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Katrin Fischer 2018-08-02 11:28:00 UTC
Created attachment 77424 [details] [review]
Bug 21075: Unit tests

prove -v t/db_dependent/Reserves/AutoUnsuspendReserves.t

Signed-off-by: Pierre-Luc Lapointe <pierreluc.lapointe@inLibro.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Katrin Fischer 2018-08-02 11:30:30 UTC
Filed bug 21147 for the calendar oddness.
Comment 10 Nick Clemens 2018-08-08 21:33:48 UTC
Awesome work all!

Pushed to master for 18.11
Comment 11 Martin Renvoize 2018-08-09 10:23:25 UTC
Pushed to 18.05.x for 18.05.03
Comment 12 Fridolin SOMERS 2018-09-21 10:25:43 UTC
Pushed to 17.11.x for 17.11.10