Bug 38758 - Make formatting date and datetime fields in notices a bit shorter/easier
Summary: Make formatting date and datetime fields in notices a bit shorter/easier
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Marcel de Rooy
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords: additional_work_needed, release-notes-needed
Depends on:
Blocks:
 
Reported: 2024-12-20 10:13 UTC by Marcel de Rooy
Modified: 2025-01-06 03:17 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement adds an easier way to format dates in notices, and minimise potential errors - strftime. It can be used for both date and date time fields, and is locale friendly. Examples: - Date field: [% borrower.dateexpiry.strftime('%d-%m-%y') %] - Date and time field: [% borrower.lastseen.strftime("%d-%m-%y %H:%M") %] - Locale: [% borrower.dateexpiry.strftime("%d %B %Y", "nl_NL") %]
Version(s) released in:
25.05.00,24.11.01
Circulation function:

Attachments
Bug 38758: Add strftime as TT virtual method (1.87 KB, patch)
2024-12-20 10:25 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 38758: Add subtest to Letters.t (3.84 KB, patch)
2024-12-20 10:26 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 38758: Add strftime as TT virtual method (1.89 KB, patch)
2024-12-20 10:36 UTC, Paul Derscheid 		Details | Diff | Splinter Review
Bug 38758: Add subtest to Letters.t (3.86 KB, patch)
2024-12-20 10:36 UTC, Paul Derscheid 		Details | Diff | Splinter Review
Bug 38758: Add strftime as TT virtual method (1.96 KB, patch)
2024-12-20 12:02 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Bug 38758: Add subtest to Letters.t (3.93 KB, patch)
2024-12-20 12:02 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2024-12-20 10:13:48 UTC 
Currently, date formatting in notices with the TT Date plugin is a bit user unfriendly since the plugin has very simplistic parsing that only works on datetime or timestamp fields but not on date only fields (like commonly used fields as borrower.dateexpiry or reserves.reservedate etc.) Should a user of Reports know that? (No.)
So this works:
	[% date.format(borrower.updated_on, "%B %d, %Y") %]
But this does not:
	[% date.format(borrower.dateexpiry, "%B %d, %Y") %]
	ERROR PROCESSING TEMPLATE: date error - bad time/date string:  expects 'h:m:s d:m:y'

But we can overcome that by doing something like (for both date as well as datetime):
	[% KohaDates.datetime_from_string(mydate).strftime("%B %d, %Y") %] # where e.g. mydate == borrower.dateexpiry
Note that it is a bit long and somewhat complex (unfriendly). (And btw, datetime_from_string does not check the result of dt_from_string but just assumes to get a DateTime object.)

What about the shorter and more intuitive (for both date and datetime fields):
	[% mydate.strftime("%B %d, %Y") %]
Or giving yet another example with a locale different from the default en_US:
	[% KohaDates.datetime_from_string(mydate).set_locale('nl_NL').strftime("%d %B %Y") %]
Would this not read better:
	[% mydate.strftime("%d %B %Y", "nl_NL") %]

The proposed patch adds a TT virtual method strftime that can be run on scalars as in the examples above (for date and datetime). No mention of KohaDates needed, no chaining of set_locale. Wont crash on invalid dates, just passes the original value if date could not be parsed. And importantly, the former KohaDates constructs with strftime still work (going via DateTime->strftime), so backwards compatible.

Shorter and easier! What do you think?
Comment 1 Marcel de Rooy 2024-12-20 10:25:57 UTC 
Created attachment 175818 [details] [review]
Bug 38758: Add strftime as TT virtual method

Test plan:
Try things like [% borrower.dateexpiry.strftime('%d-%m-%y') %] in
a notice. Also test datetime fields like borrower.lastseen.
Add locale support with strftime('%d %B', 'nl_NL') etc.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Marcel de Rooy 2024-12-20 10:26:00 UTC 
Created attachment 175819 [details] [review]
Bug 38758: Add subtest to Letters.t

Test plan:
Run t/db_dependent/Letters.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 3 Paul Derscheid 2024-12-20 10:36:19 UTC 
Created attachment 175820 [details] [review]
Bug 38758: Add strftime as TT virtual method

Test plan:
Try things like [% borrower.dateexpiry.strftime('%d-%m-%y') %] in
a notice. Also test datetime fields like borrower.lastseen.
Add locale support with strftime('%d %B', 'nl_NL') etc.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 4 Paul Derscheid 2024-12-20 10:36:21 UTC 
Created attachment 175821 [details] [review]
Bug 38758: Add subtest to Letters.t

Test plan:
Run t/db_dependent/Letters.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 5 Martin Renvoize (ashimema) 2024-12-20 12:02:52 UTC 
Created attachment 175822 [details] [review]
Bug 38758: Add strftime as TT virtual method

Test plan:
Try things like [% borrower.dateexpiry.strftime('%d-%m-%y') %] in
a notice. Also test datetime fields like borrower.lastseen.
Add locale support with strftime('%d %B', 'nl_NL') etc.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize (ashimema) 2024-12-20 12:02:59 UTC 
Created attachment 175823 [details] [review]
Bug 38758: Add subtest to Letters.t

Test plan:
Run t/db_dependent/Letters.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize (ashimema) 2024-12-20 12:12:03 UTC 
I'd be tempted to add the option to the filter call too

I've confirmed it works as is.. though I'm not entirely sure I understand the innards of TT plugins to know why.

It might be nice to be able to call it as so

[% borrower.lastseen | $KohaDates with_format = '%d-%m' %]
Comment 8 Marcel de Rooy 2024-12-20 16:10:10 UTC 
(In reply to Martin Renvoize (ashimema) from comment #7)
> I'd be tempted to add the option to the filter call too
> 
> I've confirmed it works as is.. though I'm not entirely sure I understand
> the innards of TT plugins to know why.
> 
> It might be nice to be able to call it as so
> 
> [% borrower.lastseen | $KohaDates with_format = '%d-%m' %]

Thanks for QA. I would prefer to change the filter sub of KohaDates on another report though.
Comment 9 Katrin Fischer 2024-12-23 12:30:46 UTC 
We have a bit of documentation for formatting dates in the TT notices in the wiki, can you please update?

https://wiki.koha-community.org/wiki/Notices_with_Template_Toolkit#Example:_Format_dates
Comment 10 Katrin Fischer 2024-12-23 12:52:37 UTC 
Pushed for 25.05!

Well done everyone, thank you!
Comment 11 Paul Derscheid 2024-12-30 11:29:50 UTC 
Nice work everyone!

Pushed to 24.11.x for 24.11.01
Comment 12 David Nind 2024-12-30 18:00:07 UTC 
I had a go at a relase note - I'm not sure if I've got this right, particularly showing the date time field example.
Comment 13 Jonathan Druart 2025-01-01 19:25:23 UTC 
That's elegant but definitely not consistent with anything else.
IMO it should have been a method of KohaDates (matching the existing pattern: KohaDates.output_preference)
Comment 14 Wainui Witika-Park 2025-01-06 03:17:55 UTC 
Not backporting to 24.05 unless requested