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
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
Created attachment 82316 [details] [review] Bug 21824: Add KohaDates method 'output_preference' to make it easy to store formatted dates in TT variable
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>
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>
Hi Nick, Kyle, Can you give more info about the use cases? Which problems are you trying to solve with these patches?
(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
Kyle, Can you give an example please? How would you display table.column in iso format without the time part?
(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' %]
(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?
Created attachment 83188 [details] [review] Bug 21824: Add tests
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.
(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.
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
(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 :)
Created attachment 91354 [details] [review] Bug 21824: Add tests Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
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>
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>
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>
Created attachment 91358 [details] [review] Bug 21824: Add tests Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
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>
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>
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?
(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
(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 :)
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 :)
I've elaborated a bit further on the wiki now too, listing the available options for dateformat and giving examples of their output.
Docs team, this might be something very useful to add to the notices and slips area of the manual.
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
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
Back to this after a reset_all...
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.
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.
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>
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>
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>
Good work team, Passing QA
Nice work! Pushed to master for 19.11.00