Bug 21176 - decreaseLoanHighHolds does not properly calculate date when TimeFormat set to 12 hour
Summary: decreaseLoanHighHolds does not properly calculate date when TimeFormat set t...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-08-08 11:51 UTC by Nick Clemens (kidclamp)
Modified: 2019-10-14 19:57 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 21176: Fix due date calculation when high holds and TimeFormat is 12hr (3.25 KB, patch)
2018-08-13 16:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21176: Fix due date calculation when high holds and TimeFormat is 12hr (3.35 KB, patch)
2018-09-24 16:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21176: Fix due date calculation when high holds and TimeFormat is 12hr (3.41 KB, patch)
2018-10-01 22:11 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2018-08-08 11:51:24 UTC
To recreate:
1 - Enable decreaseLoanHighHolds, set to 1 day and more than 0 holds
2 - Set TimeFormat to 12 hour
3 - Find or create a record with two items
4 - Place a hold on one of them
5 - Checkout the other to a different patron
6 - Note the warning message display correct time
7 - Confirm the checkout
8 - Note the item is due at 11:59AM
Comment 1 Jonathan Druart 2018-08-13 16:25:17 UTC
Created attachment 77761 [details] [review]
Bug 21176: Fix due date calculation when high holds and TimeFormat is 12hr

What we were doing was wrong and is still wrong. The output_pref should not be done
in modules or script, only template side.
Much more work would be needed to clean the situation. This patch provides less changes
as possible to, hopefully, not introduce side-effects.

To recreate:
1 - Enable decreaseLoanHighHolds, set to 1 day and more than 0 holds
2 - Set TimeFormat to 12 hour
3 - Find or create a record with two items
4 - Place a hold on one of them
5 - Checkout the other to a different patron
6 - Note the warning message display correct time
7 - Confirm the checkout
8 - Note the item is due at 11:59AM
Comment 2 Martin Renvoize 2018-09-24 16:55:38 UTC
Created attachment 79326 [details] [review]
Bug 21176: Fix due date calculation when high holds and TimeFormat is 12hr

What we were doing was wrong and is still wrong. The output_pref should not be done
in modules or script, only template side.
Much more work would be needed to clean the situation. This patch provides less changes
as possible to, hopefully, not introduce side-effects.

To recreate:
1 - Enable decreaseLoanHighHolds, set to 1 day and more than 0 holds
2 - Set TimeFormat to 12 hour
3 - Find or create a record with two items
4 - Place a hold on one of them
5 - Checkout the other to a different patron
6 - Note the warning message display correct time
7 - Confirm the checkout
8 - Note the item is due at 11:59AM

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 3 Martin Renvoize 2018-09-24 16:57:04 UTC
I agree with Jonathan that it would be better to fix this more thoroughly but that the patch is pragmatic and it resolves the problem for now.

Signing off.
Comment 4 Chris Cormack 2018-10-01 22:11:05 UTC
Created attachment 79788 [details] [review]
Bug 21176: Fix due date calculation when high holds and TimeFormat is 12hr

What we were doing was wrong and is still wrong. The output_pref should not be done
in modules or script, only template side.
Much more work would be needed to clean the situation. This patch provides less changes
as possible to, hopefully, not introduce side-effects.

To recreate:
1 - Enable decreaseLoanHighHolds, set to 1 day and more than 0 holds
2 - Set TimeFormat to 12 hour
3 - Find or create a record with two items
4 - Place a hold on one of them
5 - Checkout the other to a different patron
6 - Note the warning message display correct time
7 - Confirm the checkout
8 - Note the item is due at 11:59AM

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 5 Nick Clemens (kidclamp) 2018-10-02 00:13:36 UTC
Awesome work all!

Pushed to master for 18.11
Comment 6 Martin Renvoize 2018-10-08 08:08:50 UTC
Pushed to 18.05.x for 18.05.05
Comment 7 Fridolin Somers 2018-10-11 13:18:23 UTC
Pushed to 17.11.x for 17.11.11