Bug 16780 - Specify due date always sets time as AM when using 12 hour time format
Summary: Specify due date always sets time as AM when using 12 hour time format
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on: 14903
Blocks:
  Show dependency treegraph
 
Reported: 2016-06-20 14:44 UTC by Nick Clemens
Modified: 2020-04-20 09:00 UTC (History)
9 users (show)

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


Attachments
Bug 16780 - Specify due date always sets time as AM when using 12 hour time format (1.73 KB, patch)
2016-06-24 10:43 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 16780 - Specify due date always sets time as AM when using 12 hour time format (1.78 KB, patch)
2016-06-26 20:24 UTC, Claire Gravely
Details | Diff | Splinter Review
Bug 16780 - Specify due date always sets time as AM when using 12 hour time format (1.85 KB, patch)
2016-06-27 13:53 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 Nick Clemens 2016-06-20 14:44:46 UTC
To recreate:
1 - On the checkout screen for a patron specify a due date and a PM time
2 - Checkout an item
3 - Note the due date for the item and the time in the box have switched to AM
Comment 1 Jonathan Druart 2016-06-21 12:14:09 UTC
Nick,
I confirm the issue.
This is a tricky one, it's because the AM/PM breaks the parsing of the dates somewhere. Do you know if it's a regression from a previous stable version or if it has never worked?
Comment 2 Karl Holten 2016-06-21 14:08:31 UTC
It's a regression, it used to work for our institutions before our update to 3.22.
Comment 3 Jonathan Druart 2016-06-23 15:05:09 UTC
Comes from
  165 $duedatespec = eval { output_pref( { dt => dt_from_string( $duedatespec ), dateformat => 'iso' }); }
  166     if ( $duedatespec );

Introduced by
  commit 421fae24f1cdcbe0ccc18de784454d0ca07ebe90
    Bug 14903: Remove C4::Dates from circ/circulation.pl (and more)
Comment 4 Kyle M Hall 2016-06-24 10:43:03 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2016-06-24 13:55:11 UTC
Smart fix, I did not think about that!
Will QA it as soon as it's signed off.
Comment 6 Claire Gravely 2016-06-26 20:24:31 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2016-06-27 13:53:19 UTC
Created attachment 52896 [details] [review]
Bug 16780 - Specify due date always sets time as AM when using 12 hour time format

This regression is occurs because the time format for the checkout
date/time widget is only meant to be given in 24 hour format. The time
due should not be converted to 12 hr time even if the library has 12
hour set as their preferred time format.

Test Plan:
1) Performat a backdated checkout with a date due that has a PM time
2) Note the date/time picker on the confirmition screen shows the time
   in AM/PM
2) Note the time is not stored correctly
3) Apply this patch
4) Repeate step 1
5) Note the confirmation date/time picker now show the time in 24hr
   format
6) Note the time was stored correctly

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Kyle M Hall 2016-07-08 12:52:02 UTC
Pushed to master for 16.11!
Comment 9 J Schmidt 2016-07-27 17:53:56 UTC
Can we have this backported to 3.22? One of our branches has hourly equipment and it impacts a great portion of their checkout transactions.
Comment 10 Frédéric Demians 2016-08-01 13:23:56 UTC
Pushed in 16.05. Will be in 16.05.02.
Comment 11 Julian Maurice 2016-08-16 14:38:35 UTC
Patch pushed to 3.22.x, will be in 3.22.10