Bug 25133 - Specify Due date changes from PM to AM if library has their TimeFormat set to 12hr
Summary: Specify Due date changes from PM to AM if library has their TimeFormat set to...
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
: 25365 (view as bug list)
Depends on:
Blocks: 25418 25617 25365
  Show dependency treegraph
 
Reported: 2020-04-13 16:42 UTC by Kelly McElligott
Modified: 2020-05-28 14:13 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.06, 19.05.11


Attachments
Bug 25133: Fix time part of due date for 12hr (2.07 KB, patch)
2020-04-17 10:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25133: Handle 12hr format for dt_from_string (3.70 KB, patch)
2020-04-20 09:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25133: Fix time part of due date for 12hr (2.13 KB, patch)
2020-04-21 12:08 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25133: Handle 12hr format for dt_from_string (3.76 KB, patch)
2020-04-21 12:08 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25133: (follow-up) Remove Names as they do no affect display (1.15 KB, patch)
2020-04-21 12:08 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25133: Fix time part of due date for 12hr (2.19 KB, patch)
2020-04-21 12:32 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 25133: Handle 12hr format for dt_from_string (3.82 KB, patch)
2020-04-21 12:32 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 25133: (follow-up) Remove Names as they do no affect display (1.21 KB, patch)
2020-04-21 12:32 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 25133: Fix time part of due date for 12hr (2.96 KB, patch)
2020-05-03 20:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25133: Handle 12hr format for dt_from_string (3.88 KB, patch)
2020-05-03 20:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25133: (follow-up) Remove Names as they do no affect display (1.27 KB, patch)
2020-05-03 20:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25133: (follow-up) Move changes to timepicker.inc (2.03 KB, patch)
2020-05-04 11:36 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25133: (follow-up) Move changes to timepicker.inc (2.10 KB, patch)
2020-05-04 12:44 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 Kelly McElligott 2020-04-13 16:42:54 UTC
Prior to 19.05.08, when due date was specified in the checkout, the time in the box would show in 24hr format (e.g. 15:59) while the due date in the patron's checkouts would show in 12hr format (e.g. 3:59 PM)
When using the specified due date in the same transaction, 
- check an item out and the box would remain with 15:59 while the due date would display 3:59 PM
- check a second item out and the box would continue to display 15:59 and the due date 3:59 PM

After 19.05.08, the format in the specify a due date box shows in 24hr format and the due date in 12hr format (in patron checkouts) as before.

However, on the second transaction, the box converts to 3:59 AM and that time will be 3:59 AM on the patron's checkout.


This process only occurs if the library is using the TimeFormat at 12 hours in the system preferences and not 24 hours in the TIme format.
Comment 1 Azucena Aguayo 2020-04-13 22:52:00 UTC
12 hour format is easier for our staff to read.

We would appreciate if this bug could be fixed to restore the functionality we had prior to the 19.05.08 update.
Comment 2 Nick Clemens 2020-04-14 18:44:20 UTC
Confirmed on master
Comment 3 Jonathan Druart 2020-04-17 10:12:15 UTC
Created attachment 103137 [details] [review]
Bug 25133: Fix time part of due date for 12hr
Comment 4 Jonathan Druart 2020-04-17 10:13:24 UTC
Restoring the previous behaviour would be a regression in my opinion.
I think this patch is the correct way to fix it (ie. always display 12hr format)

Nick, can you give it a try before I request a signoff please?
Comment 5 Nick Clemens 2020-04-17 15:07:15 UTC
(In reply to Jonathan Druart from comment #4)
> Restoring the previous behaviour would be a regression in my opinion.
> I think this patch is the correct way to fix it (ie. always display 12hr
> format)
> 
> Nick, can you give it a try before I request a signoff please?

In testing, all of my times are now AM
Comment 6 Jonathan Druart 2020-04-20 08:19:39 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2020-04-20 08:20:14 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2020-04-20 09:00:21 UTC
Created attachment 103236 [details] [review]
Bug 25133: Handle 12hr format for dt_from_string
Comment 9 Nick Clemens 2020-04-21 12:08:15 UTC
Created attachment 103366 [details] [review]
Bug 25133: Fix time part of due date for 12hr

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 10 Nick Clemens 2020-04-21 12:08:18 UTC
Created attachment 103367 [details] [review]
Bug 25133: Handle 12hr format for dt_from_string

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Nick Clemens 2020-04-21 12:08:21 UTC
Created attachment 103368 [details] [review]
Bug 25133: (follow-up) Remove Names as they do no affect display

In testing I couldn't make any changes to the names have an effect
and everythign worked as expected without these lines

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 12 ByWater Sandboxes 2020-04-21 12:32:44 UTC
Created attachment 103369 [details] [review]
Bug 25133: Fix time part of due date for 12hr

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 13 ByWater Sandboxes 2020-04-21 12:32:46 UTC
Created attachment 103370 [details] [review]
Bug 25133: Handle 12hr format for dt_from_string

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 14 ByWater Sandboxes 2020-04-21 12:32:49 UTC
Created attachment 103371 [details] [review]
Bug 25133: (follow-up) Remove Names as they do no affect display

In testing I couldn't make any changes to the names have an effect
and everythign worked as expected without these lines

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 15 Jonathan Druart 2020-04-21 12:55:41 UTC
Nick, Kelly, could you detail what you have tried?

This patch will affect all date inputs so we need quite intensive testing.
Comment 16 Katrin Fischer 2020-05-03 20:31:04 UTC
This works but wondering - is it expected that we only allow to add tags with emojis from the detail page, but not from the result list?
Comment 17 Katrin Fischer 2020-05-03 20:31:19 UTC
(In reply to Katrin Fischer from comment #16)
> This works but wondering - is it expected that we only allow to add tags
> with emojis from the detail page, but not from the result list?

Oviously wrong bug.
Comment 18 Katrin Fischer 2020-05-03 20:35:56 UTC
I know this is a bad bug... but why is there no test plan in the commits and no proper descriptions? :(
Comment 19 Katrin Fischer 2020-05-03 20:51:05 UTC
Created attachment 104228 [details] [review]
Bug 25133: Fix time part of due date for 12hr

With Timeformat system preference set to 12 hours format:

Prior to 19.05.08, when due date was specified at checkout,
the time in the box would show in 24hr format (e.g. 15:59)
while the due date in the patron's checkouts would show
in 12hr format (e.g. 3:59 PM).

After 19.05.08, the format in the specify a due date box
shows in 24hr format and the due date in 12hr format
(in patron checkouts) as before.

But when checking out multiple items using the specify due
date option for the session, the time changes from PM to
AM.

To test:
- Check out an item with specified due date 15:50 PM
- Check out a second item in same transaction
- Verify date changed to AM
- Apply patch
- Repeat
- Verify this time the date is kept as set

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 20 Katrin Fischer 2020-05-03 20:51:11 UTC
Created attachment 104229 [details] [review]
Bug 25133: Handle 12hr format for dt_from_string

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 21 Katrin Fischer 2020-05-03 20:51:16 UTC
Created attachment 104230 [details] [review]
Bug 25133: (follow-up) Remove Names as they do no affect display

In testing I couldn't make any changes to the names have an effect
and everythign worked as expected without these lines

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 22 Martin Renvoize 2020-05-04 07:55:20 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 23 Jonathan Druart 2020-05-04 10:34:01 UTC
I personally did not test those patches enough to confirm it could be backported without risk.
Comment 24 Katrin Fischer 2020-05-04 10:54:01 UTC
Just a quick note here, as I can't test further right now, but this causes a JavaScript error when adding an order line (and possible on other pages):

TypeError: $.timepicker is undefined

Bad bug, fix needed urgently!
Comment 25 Nick Clemens 2020-05-04 11:08:21 UTC
(In reply to Katrin Fischer from comment #24)
> Just a quick note here, as I can't test further right now, but this causes a
> JavaScript error when adding an order line (and possible on other pages):
> 
> TypeError: $.timepicker is undefined
> 
> Bad bug, fix needed urgently!

Looking now, should the changes to calendar.inc should have gone into timepicker.inc instead?
Comment 26 Jonathan Druart 2020-05-04 11:25:38 UTC
What about including timepicker.inc from calendar.inc?
Comment 27 Jonathan Druart 2020-05-04 11:27:04 UTC
(In reply to Jonathan Druart from comment #26)
> What about including timepicker.inc from calendar.inc?

That's silly :)

Yes Nick, it should!
Comment 28 Jonathan Druart 2020-05-04 11:27:55 UTC
We actually just need 'timeFormat' to be defined in timepicker.inc
Comment 29 Nick Clemens 2020-05-04 11:36:00 UTC
Created attachment 104260 [details] [review]
Bug 25133: (follow-up) Move changes to timepicker.inc

Witht the changes in calendar.inc there can be a js error where the timepicker is not
included. Moving the changes to timepicker.inc should resolve this

Note: in previous patches the amnames/pmnames were removed. It is possible they had
no effect because they were declared in two files, so I have left them in timepicker.inc
Comment 30 Jonathan Druart 2020-05-04 12:44:24 UTC
Created attachment 104269 [details] [review]
Bug 25133: (follow-up) Move changes to timepicker.inc

Witht the changes in calendar.inc there can be a js error where the timepicker is not
included. Moving the changes to timepicker.inc should resolve this

Note: in previous patches the amnames/pmnames were removed. It is possible they had
no effect because they were declared in two files, so I have left them in timepicker.inc

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 31 Martin Renvoize 2020-05-04 12:45:53 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 32 Katrin Fischer 2020-05-05 06:52:10 UTC
*** Bug 25365 has been marked as a duplicate of this bug. ***
Comment 33 Joy Nelson 2020-05-11 20:35:39 UTC
Backported to 19.11.x for 19.11.06
Comment 34 Lucas Gass 2020-05-15 20:00:19 UTC
backported to 19.05.x for 19.05.11