Bug 16624 - Times are formatted incorrectly in slips ( AM PM ) due to double processing
Summary: Times are formatted incorrectly in slips ( AM PM ) due to double processing
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 13622 16441
Blocks:
  Show dependency treegraph
 
Reported: 2016-05-31 17:15 UTC by Nick Clemens
Modified: 2017-12-07 22:16 UTC (History)
6 users (show)

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


Attachments
Bug 16624 - Times are formatted incorrectly in slips ( AM PM ) due to double processing (1.70 KB, patch)
2016-05-31 17:18 UTC, Nick Clemens
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16624 - Times are formatted incorrectly in slips ( AM PM ) due to double processing (1.76 KB, patch)
2016-05-31 21:01 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16624: Add regression tests (2.75 KB, patch)
2016-06-07 11:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16624: Fix regression displaying a letter with hours (1.72 KB, patch)
2016-06-07 11:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16624: Add regression tests (2.81 KB, patch)
2016-06-07 12:56 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16624: Fix regression displaying a letter with hours (1.77 KB, patch)
2016-06-07 12:56 UTC, Nick Clemens
Details | Diff | Splinter Review
Screenshot of patron page being affected (43.47 KB, image/png)
2016-06-15 21:03 UTC, Karl Holten
Details
[PASSED QA] Bug 16624: Add regression tests (2.88 KB, patch)
2016-06-19 14:45 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 16624: Fix regression displaying a letter with hours (1.84 KB, patch)
2016-06-19 14:45 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 2016-05-31 17:15:12 UTC
Letters.pm reformats the time once for title, once for content, this causes improper formatting of timestamps and makes all times appear as AM
Comment 1 Nick Clemens 2016-05-31 17:18:32 UTC Comment hidden (obsolete)
Comment 2 Nick Clemens 2016-05-31 21:01:08 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2016-06-07 11:06:48 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2016-06-07 11:06:57 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2016-06-07 11:08:33 UTC
Nick,
Looking at the tests, your patch is wrong.
Have a look at this counter-patch.
Note that we will need bug 16441 to fix the problem in the existing tests (see second patch of bug 16441).
Comment 6 Nick Clemens 2016-06-07 12:56:05 UTC Comment hidden (obsolete)
Comment 7 Nick Clemens 2016-06-07 12:56:09 UTC Comment hidden (obsolete)
Comment 8 Karl Holten 2016-06-15 21:03:47 UTC
Created attachment 52453 [details]
Screenshot of patron page being affected

A note on this particular bug: I don't think it just affects the slips. It seems to affect the actual due date in Koha. 

When we check out an item and set the due date manually to the end of the day, it instantly becomes overdue. It also displays on the screen as being due in the morning. I am not sure if this bug-fix addresses this particular issue or no. It might need to be its own bug.
Comment 9 Jonathan Druart 2016-06-18 13:19:57 UTC
(In reply to Karl Holten from comment #8)
> Created attachment 52453 [details]
> Screenshot of patron page being affected
> 
> A note on this particular bug: I don't think it just affects the slips. It
> seems to affect the actual due date in Koha. 
> 
> When we check out an item and set the due date manually to the end of the
> day, it instantly becomes overdue. It also displays on the screen as being
> due in the morning. I am not sure if this bug-fix addresses this particular
> issue or no. It might need to be its own bug.

It's not related, please open a new bug report (and link it to this one using "see also")
Comment 10 Katrin Fischer 2016-06-19 14:42:04 UTC
I can reproduce the problem for the slip printing, but not for the checkouts table. The latter displays correctly for me.
Comment 11 Katrin Fischer 2016-06-19 14:45:19 UTC
Created attachment 52546 [details] [review]
[PASSED QA] Bug 16624: Add regression tests

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Katrin Fischer 2016-06-19 14:45:49 UTC
Created attachment 52547 [details] [review]
[PASSED QA] Bug 16624: Fix regression displaying a letter with hours

Bug 13622 has introduced a bug, if pref TimeFormat is 12hr and a date is
displayed in both title and content of the letter.

Test plan:
1 - Checkout an item (with default time 11:59:00 PM)
2 - Generate a quickslip
3 - Notice the time is 'AM'
4 - Apply patch
5 - Generate quickslip
6 - Note time is correct

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Nick Clemens 2016-06-20 14:46:42 UTC
(In reply to Jonathan Druart from comment #9)
> (In reply to Karl Holten from comment #8)
> > Created attachment 52453 [details]
> > Screenshot of patron page being affected
> > 
> > A note on this particular bug: I don't think it just affects the slips. It
> > seems to affect the actual due date in Koha. 
> > 
> > When we check out an item and set the due date manually to the end of the
> > day, it instantly becomes overdue. It also displays on the screen as being
> > due in the morning. I am not sure if this bug-fix addresses this particular
> > issue or no. It might need to be its own bug.
> 
> It's not related, please open a new bug report (and link it to this one
> using "see also")

See Bug 16780
Comment 14 Kyle M Hall 2016-06-24 11:57:14 UTC
Pushed to master for Koha 16.11, thanks Jonathan!
Comment 15 Frédéric Demians 2016-08-01 11:50:34 UTC
Pushed in 16.05. Will be in 16.05.02.
Comment 16 Julian Maurice 2016-08-16 12:40:04 UTC
Patches pushed to 3.22.x, will be in 3.22.10