I have a use case where I want to add some days to an issue date in a notice. We either need a dt_from_string wrapper in our template plugin like bug 33029 proposes or allow passing options like add_days => X to KohaDates which looks nicer but is less generic. Perhaps we can do both btw ! These approaches each have their benefits and do not exclude each other. Compare the 33029 approach [% KohaDates.datetime_from_string(checkout.issuedate).add( days => 3) | $KohaDates %] with the approch of this report [% checkout.issuedate | $KohaDates add_days => 3 %] The latter notation is definitely shorter and prettier in my use case and does only need a few lines more.
Ack, I'm torn now. I can see the utility of a dt method allowing us full access to dt functions from the template.. but I also love the idea of the 'pretty' syntax we could introduce with this approach.. `[% checkout.issuesdate | $KohaDates add => "3 days" %]`.. I'm overthinking.
Created attachment 148256 [details] [review] Bug 33242: Allow passing add_{duration} options to KohaDates For example, if I want to add a few days to the issuedate in a TT template, I could do this: [% checkout.issuedate | $KohaDates add_days => 3 %] This development allows you to pass add/subtract years, months, weeks, days, hours, minutes and seconds. Test plan: Pick a notice like CHECKOUT. Add a line like: [% checkout.date_due | $KohaDates with_hours => 1, add_minutes => 15 %] Do a checkout. Verify that the notice generated contains a time that shifted 15 minutes. Undo your change. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 148257 [details] [review] Bug 33242: Add unit test Test plan: Run t/db_dependent/Letters/TemplateToolkit.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Martin Renvoize from comment #1) > Ack, I'm torn now. > > I can see the utility of a dt method allowing us full access to dt functions > from the template.. but I also love the idea of the 'pretty' syntax we > could introduce with this approach.. `[% checkout.issuesdate | $KohaDates > add => "3 days" %]`.. I'm overthinking. I think both this bug and bug 33029 can live in harmony. This would be the "easy" mode to bug 33029's "advanced" mode.
Created attachment 148261 [details] [review] Bug 33242: Allow passing add_{duration} options to KohaDates For example, if I want to add a few days to the issuedate in a TT template, I could do this: [% checkout.issuedate | $KohaDates add_days => 3 %] This development allows you to pass add/subtract years, months, weeks, days, hours, minutes and seconds. Test plan: Pick a notice like CHECKOUT. Add a line like: [% checkout.date_due | $KohaDates with_hours => 1, add_minutes => 15 %] Do a checkout. Verify that the notice generated contains a time that shifted 15 minutes. Undo your change. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 148262 [details] [review] Bug 33242: Add unit test Test plan: Run t/db_dependent/Letters/TemplateToolkit.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 148264 [details] [review] Bug 33242: Allow passing add_{duration} options to KohaDates For example, if I want to add a few days to the issuedate in a TT template, I could do this: [% checkout.issuedate | $KohaDates add_days => 3 %] This development allows you to pass add/subtract years, months, weeks, days, hours, minutes and seconds. Test plan: Pick a notice like CHECKOUT. Add a line like: [% checkout.date_due | $KohaDates with_hours => 1, add_minutes => 15 %] Do a checkout. Verify that the notice generated contains a time that shifted 15 minutes. Undo your change. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 148265 [details] [review] Bug 33242: Add unit test Test plan: Run t/db_dependent/Letters/TemplateToolkit.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Tested with 33029 (accidentally applied merged patches first time) They both work well, I think having both provides nice flexibility, easy way to add to dates here, and full access to dt object from 33029
I was going to PQA until I saw bug 33029, which gives us more flexibility. Not sure we need both personally.
(In reply to Jonathan Druart from comment #10) > I was going to PQA until I saw bug 33029, which gives us more flexibility. > Not sure we need both personally. Yes we looked at that. Kyle, Nick and I concluded that both go fine together. See description too, saying this: Compare the 33029 approach [% KohaDates.datetime_from_string(checkout.issuedate).add( days => 3) | $KohaDates %] with the approch of this report [% checkout.issuedate | $KohaDates add_days => 3 %] The latter notation is definitely shorter and prettier in my use case and does only need a few lines more.
(In reply to Marcel de Rooy from comment #11) > (In reply to Jonathan Druart from comment #10) > > I was going to PQA until I saw bug 33029, which gives us more flexibility. > > Not sure we need both personally. > > Yes we looked at that. Kyle, Nick and I concluded that both go fine together. > See description too, saying this: > > Compare the 33029 approach > [% KohaDates.datetime_from_string(checkout.issuedate).add( days => 3) | > $KohaDates %] > with the approch of this report > [% checkout.issuedate | $KohaDates add_days => 3 %] > > The latter notation is definitely shorter and prettier in my use case and > does only need a few lines more. Yes, more code to maintain, and the parsing here looks hacky to me, for less flexibility. Note that I didn't FQA but I am not the one who is going to PQA this ;)
(In reply to Jonathan Druart from comment #12) > Yes, more code to maintain, and the parsing here looks hacky to me, for less > flexibility. What is hacky in this parsing ? if( $entry =~ /^(add|subtract)_(years|months|weeks|days|hours|minutes|seconds)$/ ) {
(In reply to Marcel de Rooy from comment #13) > (In reply to Jonathan Druart from comment #12) > > Yes, more code to maintain, and the parsing here looks hacky to me, for less > > flexibility. > > What is hacky in this parsing ? > if( $entry =~ > /^(add|subtract)_(years|months|weeks|days|hours|minutes|seconds)$/ ) { Here you are providing an handy way to do date calculation, but without its flexibility. It's all what I am saying. For instance what about the end_of_month limit? You cannot change it. But really, if others find it useful, just let it go ;)
Created attachment 150390 [details] [review] Bug 33242: Allow passing add_{duration} options to KohaDates For example, if I want to add a few days to the issuedate in a TT template, I could do this: [% checkout.issuedate | $KohaDates add_days => 3 %] This development allows you to pass add/subtract years, months, weeks, days, hours, minutes and seconds. Test plan: Pick a notice like CHECKOUT. Add a line like: [% checkout.date_due | $KohaDates with_hours => 1, add_minutes => 15 %] Do a checkout. Verify that the notice generated contains a time that shifted 15 minutes. Undo your change. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 150391 [details] [review] Bug 33242: Allow passing add_{duration} options to KohaDates For example, if I want to add a few days to the issuedate in a TT template, I could do this: [% checkout.issuedate | $KohaDates add_days => 3 %] This development allows you to pass add/subtract years, months, weeks, days, hours, minutes and seconds. Test plan: Pick a notice like CHECKOUT. Add a line like: [% checkout.date_due | $KohaDates with_hours => 1, add_minutes => 15 %] Do a checkout. Verify that the notice generated contains a time that shifted 15 minutes. Undo your change. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 150392 [details] [review] Bug 33242: Add unit test Test plan: Run t/db_dependent/Letters/TemplateToolkit.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(In reply to Kyle M Hall from comment #17) > Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Thx !
Pushed to master for 23.11. Nice work everyone, thanks!
Enhancement won't be backported to stable release.