Bug 21829 - Date displays as a datetime in notices
Summary: Date displays as a datetime in notices
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: 17.11
Hardware: All All
: P5 - low minor (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-13 17:51 UTC by Arturo
Modified: 2019-03-07 11:42 UTC (History)
9 users (show)

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


Attachments
Bug 21829: Add tests for dateexpiry format in notices (1.44 KB, patch)
2018-11-14 12:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21829: Correctly format dateexpiry in notices (date only) (1.05 KB, patch)
2018-11-14 12:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21829: Add tests for dateexpiry format in notices (1.51 KB, patch)
2019-02-11 12:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 21829: Correctly format dateexpiry in notices (date only) (1.14 KB, patch)
2019-02-11 12:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 21829: (QA Follow-up) - Update the number of unit tests (764 bytes, patch)
2019-02-11 12:15 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Arturo 2018-11-13 17:51:00 UTC
The borrowers.dateexpiry field is a date, not a datetime, yet when it is used in a notice like MEMBERSHIP_EXPIRY, it displays as a datetime by default with the "12:00 AM" time ("MM/DD/YYYY 12:00 AM").

Bug 13622 added the option to strip times from datetimes by using a "dateonly" placeholder like so: <<borrowers.dateexpiry | dateonly>>, but if you use this option on borrowers.dateexpiry, it fails -- probably because the field is not a datetime?

<<borrowers.dateexpiry>> outputs "MM/DD/YYYY 12:00 AM".
<<borrowers.dateexpiry | dateonly>> outputs "<>" (a left-angle bracket followed by right-angle bracket).
Comment 1 Arturo 2018-11-13 18:10:29 UTC
In looking at the HTML source of the generated e-mail notice, it's actually just spitting out "<<borrowers.dateexpiry | dateonly>>" rather than "<>". That's just how my browser and e-mail client displayed the output, assuming it was an (invalid) HTML element.
Comment 2 Jonathan Druart 2018-11-14 12:39:31 UTC
Created attachment 82329 [details] [review]
Bug 21829: Add tests for dateexpiry format in notices
Comment 3 Jonathan Druart 2018-11-14 12:39:34 UTC
Created attachment 82330 [details] [review]
Bug 21829: Correctly format dateexpiry in notices (date only)

dateexpiry is a date, we should not display the time
part in notice.

Test plan:
Create a notice with borrowers.dateexpiry, generate this notice.
The value should be displayed without the time part
Comment 4 Jonathan Druart 2018-11-14 12:40:37 UTC
Since 18.05 it is fixed by
  commit a236b684fdae8e0da83ca7263b948da971dfc849
  Bug 17981: Add a preview mode for notice templates
Comment 5 Arturo 2018-11-14 15:04:27 UTC
Thanks for submitting this patch, Jonathan! I misunderstood your message to the Koha mailing list and didn't realize this issue was fixed in 18.05.

Regardless, I hoped to test this patch and get it signed off, but since the Koha sandboxes are limited and do not yet allow testing features that require a cronjob, which the MEMBERSHIP_EXPIRY notice requires, I'm not able to.
Comment 6 Arturo 2018-11-19 14:22:54 UTC
My library was upgraded to Koha 18.05.05 a couple days ago, so I decided to try using the "dateonly" option for borrowers.dateexpiry in my MEMBERSHIP_EXPIRY notice, but I'm still seeing the same broken behavior. The generated e-mails are not interpreting <<borrowers.dateexpiry | dateonly>> correctly and are just spitting out that same string, which gets interpreted as an invalid HTML element, so from the patron's point of view, the e-mail says "Your account will expire on <>."
Comment 7 David Cook 2018-11-19 23:47:16 UTC
You can always try this instead: https://wiki.koha-community.org/wiki/Notices_with_Template_Toolkit#Example:_Format_dates
Comment 8 Sophie MEYNIEUX 2018-11-20 11:49:50 UTC
With this patch, only using <<borrowers.dateexpiry>> in your notices templates should be ok
Comment 9 Arturo 2018-11-20 14:38:24 UTC
Sophie,

Thanks for your message! I was misunderstanding the already submitted patch. You are correct -- I just confirmed that today's messages only use a date, not a datetime, and all I have in the template is <<borrowers.dateexpiry>>. Problem solved!
Comment 10 Jonathan Druart 2018-12-05 22:29:00 UTC
Arturo, Could you signoff?
Comment 11 Arturo 2018-12-07 20:34:01 UTC
No problem -- signing off!
Comment 12 Kyle M Hall 2019-02-11 12:15:35 UTC
Created attachment 84965 [details] [review]
Bug 21829: Add tests for dateexpiry format in notices

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 13 Kyle M Hall 2019-02-11 12:15:44 UTC
Created attachment 84966 [details] [review]
Bug 21829: Correctly format dateexpiry in notices (date only)

dateexpiry is a date, we should not display the time
part in notice.

Test plan:
Create a notice with borrowers.dateexpiry, generate this notice.
The value should be displayed without the time part

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 14 Kyle M Hall 2019-02-11 12:15:48 UTC
Created attachment 84967 [details] [review]
Bug 21829: (QA Follow-up) - Update the number of unit tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 15 Kyle M Hall 2019-02-11 12:17:26 UTC
QA note, this bug was corrected by 17981 so these patches are not strictly necessary. However, the addition of unit tests and a more 'standard' use of dt_from_string make it's inclusion in Koha sensible.
Comment 16 Nick Clemens 2019-02-11 18:38:30 UTC
Awesome work all!

Pushed to master for 19.05
Comment 17 Martin Renvoize 2019-02-12 13:17:51 UTC
Pushed to 18.11.x for 18.11.03
Comment 18 Lucas Gass 2019-02-14 23:07:59 UTC
backported to 18.05.x for 18.05.09
Comment 19 Fridolin SOMERS 2019-02-22 08:16:53 UTC
Pushed to 17.11.x for 17.11.15