Bug 21824

Summary: Add ability to format dates in various formats in templates
Product: Koha Reporter: Nick Clemens <nick>
Component: TemplatesAssignee: Kyle M Hall <kyle>
Status: CLOSED FIXED QA Contact: Martin Renvoize <martin.renvoize>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, josef.moravec, katrin.fischer, kyle, martin.renvoize
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This patchset allows end-users to use advanced date formatting options within template toolkit based notices and slips.
Version(s) released in:
19.11.00
Attachments: Bug 21824: Add ability to format dates in various formats in templates
Bug 21824: Add KohaDates method 'output_preference' to make it easy to store formatted dates in TT variable
Bug 21824: Add ability to format dates in various formats in templates
Bug 21824: Add KohaDates method 'output_preference' to make it easy to store formatted dates in TT variable
Bug 21824: Add tests
Bug 21824: Add tests
Bug 21824: Add ability to format dates in various formats in templates
Bug 21824: Add ability to format dates in various formats in templates
Bug 21824: Add KohaDates method 'output_preference' to make it easy to store formatted dates in TT variable
Bug 21824: Add tests
Bug 22540: Add ability to place holds using SIP CLI emulator
Bug 22540: (QA follow-up) fixes
Bug 21824: Add ability to format dates in various formats in templates
Bug 21824: Add KohaDates method 'output_preference' to make it easy to store formatted dates in TT variable
Bug 21824: Add tests

Description Nick Clemens 2018-11-13 15:59:42 UTC
Since enabling template toolkit for notices we have found we are trying to do date comparisons and more advanced manipulations. We have been using 'manip' but this is complicated.

We should build in some utils to allow for more advanced formatting and comparison
Comment 1 Kyle M Hall 2018-11-13 16:10:44 UTC
Created attachment 82308 [details] [review]
Bug 21824: Add ability to format dates in various formats in templates

This patch adds the ability to set a date format for the Template Toolkit filter KohaDates

Test plan:
1) Apply this patch
2) In a template, use the filter param "dateformat => 'iso'"
3) Note the output is in ISO format
Comment 2 Kyle M Hall 2018-11-13 17:36:30 UTC
Created attachment 82316 [details] [review]
Bug 21824: Add KohaDates method 'output_preference' to make it easy to store formatted dates in TT variable
Comment 3 Owen Leonard 2018-12-05 16:54:48 UTC
Created attachment 82879 [details] [review]
Bug 21824: Add ability to format dates in various formats in templates

This patch adds the ability to set a date format for the Template Toolkit filter KohaDates

Test plan:
1) Apply this patch
2) In a template, use the filter param "dateformat => 'iso'"
3) Note the output is in ISO format

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 4 Owen Leonard 2018-12-05 16:54:51 UTC
Created attachment 82880 [details] [review]
Bug 21824: Add KohaDates method 'output_preference' to make it easy to store formatted dates in TT variable

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 5 Jonathan Druart 2018-12-13 19:06:12 UTC
Hi Nick, Kyle,
Can you give more info about the use cases? Which problems are you trying to solve with these patches?
Comment 6 Nick Clemens 2018-12-13 20:05:26 UTC
(In reply to Jonathan Druart from comment #5)
> Hi Nick, Kyle,
> Can you give more info about the use cases? Which problems are you trying to
> solve with these patches?

When libraries want to format the date in various ways in slip/notices it is currently difficult, the goal is to make it easy to use Koha tools for date formatting
Comment 7 Jonathan Druart 2018-12-13 21:12:20 UTC
Kyle, Can you give an example please? How would you display table.column in iso format without the time part?
Comment 8 Jonathan Druart 2018-12-13 21:14:35 UTC
(In reply to Jonathan Druart from comment #7)
> Kyle, Can you give an example please? How would you display table.column in
> iso format without the time part?

[% table.column | $KohaDates dateformat => 'iso' %]
Comment 9 Jonathan Druart 2018-12-13 21:21:13 UTC
(In reply to Jonathan Druart from comment #8)
> (In reply to Jonathan Druart from comment #7)
> > Kyle, Can you give an example please? How would you display table.column in
> > iso format without the time part?
> 
> [% table.column | $KohaDates dateformat => 'iso' %]

or [% KohaDate.output_preference( str => table.column, dateformat => 'iso', dateonly => 1 %]

Is that what you had in mind?
Comment 10 Jonathan Druart 2018-12-13 21:24:56 UTC
Created attachment 83188 [details] [review]
Bug 21824: Add tests
Comment 11 Jonathan Druart 2018-12-13 21:32:00 UTC
I am not convinced we really need this additional parameter (and even less the new subroutine).

The following is much more readable IMO:
  [% USE date %]
  [% date.format(biblio.timestamp, '%F') %]
  [% date.format(biblio.timestamp, '%Y-%m-%d') %]


The only advantage would be to take into account the timezone if defined in the config.
Comment 12 Katrin Fischer 2019-02-17 10:15:16 UTC
(In reply to Jonathan Druart from comment #11)
> I am not convinced we really need this additional parameter (and even less
> the new subroutine).
> 
> The following is much more readable IMO:
>   [% USE date %]
>   [% date.format(biblio.timestamp, '%F') %]
>   [% date.format(biblio.timestamp, '%Y-%m-%d') %]
> 
> 
> The only advantage would be to take into account the timezone if defined in
> the config.

Just some thoughts here:

So without the patch the above constructs only display the server time, but not the configured time? If that's correct a syntax taking the timezone into account might prove really helpful over time.

The advantage I see with using standard template toolkit operations is better documentation and flexibility. At the moment the TT notices remain rather unhelpful to libraries, because we lack documentation and examples (Josef++ for his work on documentation this week)

What other formats would the new parameter support? In what kind of situations would it be useful? I miss a 'real life example' in the test plan to make things more clear.
Comment 13 Josef Moravec 2019-02-18 08:53:05 UTC
What is the use real use case? When librarians want to show date in various formats? I prefer to show date and time in human readable format for patrons...

Just my 2 cents
Comment 14 Kyle M Hall 2019-02-21 11:42:52 UTC
(In reply to Josef Moravec from comment #13)
> What is the use real use case? When librarians want to show date in various
> formats? I prefer to show date and time in human readable format for
> patrons...
> 
> Just my 2 cents

Sometimes librarians want or need specific formats. It's as simple as that :)
Comment 15 Bouzid Fergani 2019-07-05 18:26:49 UTC Comment hidden (obsolete)
Comment 16 Bouzid Fergani 2019-07-05 18:30:02 UTC
Created attachment 91355 [details] [review]
Bug 21824: Add ability to format dates in various formats in templates

This patch adds the ability to set a date format for the Template Toolkit filter KohaDates

Test plan:
1) Apply this patch
2) In a template, use the filter param "dateformat => 'iso'"
3) Note the output is in ISO format

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 17 Bouzid Fergani 2019-07-05 18:33:11 UTC
Created attachment 91356 [details] [review]
Bug 21824: Add ability to format dates in various formats in templates

This patch adds the ability to set a date format for the Template Toolkit filter KohaDates

Test plan:
1) Apply this patch
2) In a template, use the filter param "dateformat => 'iso'"
3) Note the output is in ISO format

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 18 Bouzid Fergani 2019-07-05 18:33:21 UTC
Created attachment 91357 [details] [review]
Bug 21824: Add KohaDates method 'output_preference' to make it easy to store formatted dates in TT variable

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 19 Bouzid Fergani 2019-07-05 18:33:48 UTC
Created attachment 91358 [details] [review]
Bug 21824: Add tests

Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 20 Bouzid Fergani 2019-07-05 18:59:03 UTC
Created attachment 91363 [details] [review]
Bug 22540: Add ability to place holds using SIP CLI emulator

Test Plan:
1) Place a hold using the following arguments:
   sip_cli_emulator.pl -a <address> -p <port> -su <sip_user> -sp <sip_password> -l <location_code> --patron <cardnumber> --item <barcode> -m hold
2) Note the hold was placed in Koha

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

https://bugs.koha-community.org/show_bug.cgi?id=22330
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 21 Bouzid Fergani 2019-07-05 18:59:09 UTC
Created attachment 91364 [details] [review]
Bug 22540: (QA follow-up) fixes

borrowernumber is not an accessor for C4::SIP::ILS::Patron, access is
via hashref

Add ability to specify the pickup location using a command line
parameter

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

https://bugs.koha-community.org/show_bug.cgi?id=22330
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 22 Katrin Fischer 2019-10-08 11:43:55 UTC
We've discussed the two approaches at the hackfest and came to the decision that the timezone aware approach seems the better one right now. 

The main issue with Kyle's approach is the lack of documentation, as there is a real danger this could turn into a secret feature not being used. So I make it a condition for QA that we get some documentation and examples on the wiki:

https://wiki.koha-community.org/wiki/Notices_with_Template_Toolkit

Kyle, are you ok with that?
Comment 23 Kyle M Hall 2019-10-08 13:07:31 UTC
(In reply to Katrin Fischer from comment #22)
> We've discussed the two approaches at the hackfest and came to the decision
> that the timezone aware approach seems the better one right now. 
> 
> The main issue with Kyle's approach is the lack of documentation, as there
> is a real danger this could turn into a secret feature not being used. So I
> make it a condition for QA that we get some documentation and examples on
> the wiki:
> 
> https://wiki.koha-community.org/wiki/Notices_with_Template_Toolkit
> 
> Kyle, are you ok with that?

Yes! Did someone beat me to it, or would you like more examples?
https://wiki.koha-community.org/wiki/Notices_with_Template_Toolkit#Example:_Format_dates
Comment 24 Katrin Fischer 2019-10-08 15:45:41 UTC
(In reply to Kyle M Hall from comment #23)
> (In reply to Katrin Fischer from comment #22)
> > We've discussed the two approaches at the hackfest and came to the decision
> > that the timezone aware approach seems the better one right now. 
> > 
> > The main issue with Kyle's approach is the lack of documentation, as there
> > is a real danger this could turn into a secret feature not being used. So I
> > make it a condition for QA that we get some documentation and examples on
> > the wiki:
> > 
> > https://wiki.koha-community.org/wiki/Notices_with_Template_Toolkit
> > 
> > Kyle, are you ok with that?
> 
> Yes! Did someone beat me to it, or would you like more examples?
> https://wiki.koha-community.org/wiki/Notices_with_Template_Toolkit#Example:
> _Format_dates

I think some more complex examples would be nice that show why this patch is needed :)
Comment 25 Kyle M Hall 2019-10-09 15:18:08 UTC
I've added a few more examples. I the moment I can't find the actual use-case that made me write this patch :\

(In reply to Katrin Fischer from comment #24) 
> I think some more complex examples would be nice that show why this patch is
> needed :)
Comment 26 Martin Renvoize 2019-10-15 16:55:05 UTC
I've elaborated a bit further on the wiki now too, listing the available options for dateformat and giving examples of their output.
Comment 27 Martin Renvoize 2019-10-15 16:56:04 UTC
Docs team, this might be something very useful to add to the notices and slips area of the manual.
Comment 28 Katrin Fischer 2019-10-17 06:34:26 UTC
I am running out of time this morning to look deeper into it, but there is a problem with the new test file (probably file permissions):

kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ t/db_dependent/Letters/TemplateToolkit.t
bash: t/db_dependent/Letters/TemplateToolkit.t: Permission denied
Comment 29 Katrin Fischer 2019-10-17 06:39:10 UTC
Actually it helps to actually type 'prove' in front of it ;)

But... it still fails for me with and without the patch (MySQL strict is off...). Still will have to continue here later, if someone beats me to it - on problem!

kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/Letters/TemplateToolkit.t
t/db_dependent/Letters/TemplateToolkit.t .. DBD::mysql::st execute failed: Field 'changed_fields' doesn't have a default value [for Statement "INSERT INTO `borrower_modifications` ( `verification_token`) VALUES ( ? )" with ParamValues: 0='TEST'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836.
Can't locate object method "rethrow" via package "DBD::mysql::st execute failed: Field 'changed_fields' doesn't have a default value [for Statement "INSERT INTO `borrower_modifications` ( `verification_token`) VALUES ( ? )" with ParamValues: 0='TEST'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836.
" (perhaps you forgot to load "DBD::mysql::st execute failed: Field 'changed_fields' doesn't have a default value [for Statement "INSERT INTO `borrower_modifications` ( `verification_token`) VALUES ( ? )" with ParamValues: 0='TEST'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836.
"?) at /home/vagrant/kohaclone/Koha/Object.pm line 186.
# Looks like your test exited with 255 before it could output anything.
t/db_dependent/Letters/TemplateToolkit.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 19/19 subtests 

Test Summary Report
-------------------
t/db_dependent/Letters/TemplateToolkit.t (Wstat: 65280 Tests: 0 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 19 tests but ran 0.
Files=1, Test
Comment 30 Katrin Fischer 2019-10-18 05:58:34 UTC
Back to this after a reset_all...
Comment 31 Katrin Fischer 2019-10-18 06:03:47 UTC
No change - I am sorry, but the failing tests block a PQA from me right now. Maybe they need to be fixed separately, not sure.

Resetting QA contact so someone else might continue before I can.
Comment 32 Martin Renvoize 2019-10-31 16:38:58 UTC
The tests failing was actually in master and is now fixed.. Tests all pass for me.. going to go for a QA round on this.
Comment 33 Martin Renvoize 2019-10-31 16:40:53 UTC
Created attachment 94920 [details] [review]
Bug 21824: Add ability to format dates in various formats in templates

This patch adds the ability to set a date format for the Template Toolkit filter KohaDates

Test plan:
1) Apply this patch
2) In a template, use the filter param "dateformat => 'iso'"
3) Note the output is in ISO format

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 34 Martin Renvoize 2019-10-31 16:40:57 UTC
Created attachment 94921 [details] [review]
Bug 21824: Add KohaDates method 'output_preference' to make it easy to store formatted dates in TT variable

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 35 Martin Renvoize 2019-10-31 16:41:03 UTC
Created attachment 94922 [details] [review]
Bug 21824: Add tests

Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 36 Martin Renvoize 2019-10-31 16:41:24 UTC
Good work team, Passing QA
Comment 37 Martin Renvoize 2019-11-01 08:53:16 UTC
Nice work!

Pushed to master for 19.11.00