Bug 25133

Summary: Specify Due date changes from PM to AM if library has their TimeFormat set to 12hr
Product: Koha Reporter: Kelly McElligott <kelly>
Component: CirculationAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: 1joynelson, azucena.aguayo, gmcharlt, jonathan.druart, kyle.m.hall, lucas, martin.renvoize, nick, oleonard
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16780
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.06, 19.05.11
Bug Depends on:    
Bug Blocks: 25365, 25418, 25617    
Attachments: Bug 25133: Fix time part of due date for 12hr
Bug 25133: Handle 12hr format for dt_from_string
Bug 25133: Fix time part of due date for 12hr
Bug 25133: Handle 12hr format for dt_from_string
Bug 25133: (follow-up) Remove Names as they do no affect display
Bug 25133: Fix time part of due date for 12hr
Bug 25133: Handle 12hr format for dt_from_string
Bug 25133: (follow-up) Remove Names as they do no affect display
Bug 25133: Fix time part of due date for 12hr
Bug 25133: Handle 12hr format for dt_from_string
Bug 25133: (follow-up) Remove Names as they do no affect display
Bug 25133: (follow-up) Move changes to timepicker.inc
Bug 25133: (follow-up) Move changes to timepicker.inc

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