Bug 16624

Summary: Times are formatted incorrectly in slips ( AM PM ) due to double processing
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: NoticesAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: f.demians, jonathan.druart, julian.maurice, katrin.fischer, kholten, kyle
Version: Main   
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:
Bug Depends on: 13622, 16441    
Bug Blocks:    
Attachments: Bug 16624 - Times are formatted incorrectly in slips ( AM PM ) due to double processing
[SIGNED-OFF] Bug 16624 - Times are formatted incorrectly in slips ( AM PM ) due to double processing
Bug 16624: Add regression tests
Bug 16624: Fix regression displaying a letter with hours
Bug 16624: Add regression tests
Bug 16624: Fix regression displaying a letter with hours
Screenshot of patron page being affected
[PASSED QA] Bug 16624: Add regression tests
[PASSED QA] Bug 16624: Fix regression displaying a letter with hours

Description Nick Clemens (kidclamp) 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 (kidclamp) 2016-05-31 17:18:32 UTC Comment hidden (obsolete)
Comment 2 Nick Clemens (kidclamp) 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 (kidclamp) 2016-06-07 12:56:05 UTC Comment hidden (obsolete)
Comment 7 Nick Clemens (kidclamp) 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 (kidclamp) 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