Bug 17965 - TT syntax for notices - Prove that DUEDGST is compatible
Summary: TT syntax for notices - Prove that DUEDGST is compatible
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 17964 17970
Blocks: 17961 17975
  Show dependency treegraph
 
Reported: 2017-01-23 13:12 UTC by Jonathan Druart
Modified: 2017-06-20 08:32 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 17965: TT syntax for notices - Prove that DUEDGST is compatible (3.69 KB, patch)
2017-01-23 16:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17965: TT syntax for notices - Prove that DUEDGST is compatible (3.83 KB, patch)
2017-02-23 22:17 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 17965: TT syntax for notices - Prove that DUEDGST is compatible (4.06 KB, patch)
2017-06-05 16:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17965: TT syntax for notices - Prove that DUEDGST is compatible (4.05 KB, patch)
2017-06-05 16:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17965: TT syntax for notices - Prove that DUEDGST is compatible (4.05 KB, patch)
2017-06-05 16:38 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17965: TT syntax for notices - Prove that DUEDGST is compatible (4.09 KB, patch)
2017-06-15 14:48 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 Jonathan Druart 2017-01-23 13:12:27 UTC

    
Comment 1 Jonathan Druart 2017-01-23 16:32:00 UTC
Created attachment 59439 [details] [review]
Bug 17965: TT syntax for notices - Prove that DUEDGST is compatible

This notice template have the particular feature of using <<count>>.
This value is substitued during the process of the notice template.
For the TT syntax, all what we need is to send the values to substitute to the
template.

Note that items.content can also be used in these template, you can have
a look at bug 17967 to see a better alternative to this marker.

Test plan:
Generate DUEDGST and DUE notice messages.
You should be able to generate the same messages with the TT syntax.
Comment 2 Jonathan Druart 2017-01-23 17:42:28 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2017-01-23 17:59:10 UTC Comment hidden (obsolete)
Comment 4 Marc Véron 2017-02-22 08:03:07 UTC
I tested this patch on top of 17979, 17963 and 17964 ( https://bugs.koha-community.org/bugzilla3/showdependencygraph.cgi?id=17965 )

DUEDGST notice is defined as:

You have <<count>> items due, with TT syntax: [% count %]

The message created (for a patron with 2 items due) is:
You have 2 items due, with TT syntax: [% count %] 

Expected:
You have 2 items due, with TT syntax: 2

Did I forget a dependency or is something wrong with the definition?
Comment 5 Jonathan Druart 2017-02-22 09:40:31 UTC
(In reply to Marc Véron from comment #4)
> I tested this patch on top of 17979, 17963 and 17964 (
> https://bugs.koha-community.org/bugzilla3/showdependencygraph.cgi?id=17965 )

It works for me, could you confirm you apply 17970 and not 17979?
You should use `yes|git bz apply 17965` to apply the correct dependencies (with the patched git-bz to apply dependencies)
Comment 6 Marc Véron 2017-02-22 13:23:29 UTC
Re-tested.
Applied 17970, 17963, 17964, 17965.
Changed DUEDGST to

You have <<count>> items due, 
with TT syntax: [% count %] items due 

Run:
koha-foreach --enabled --email /home/marc/koha/misc/cronjobs/advance_notices.pl -c

Verify message(including time created...) in a patron's "Sent notices"

Get:

You have 2 items due,
with TT syntax: [% count %] items due 

Running TemplateToolkit.t fails the very first time, if I run it again it passes OK.
Comment 7 Marc Véron 2017-02-22 13:27:20 UTC
...and that are the two outputs of TemplateToolkit.t:

--First run ---------------------------

marc@debian8vm:~/koha$ prove t/db_dependent/Letters/TemplateToolkit.t
t/db_dependent/Letters/TemplateToolkit.t .. 1/16 DBD::mysql::st execute failed: Duplicate entry '37' for key 'PRIMARY' [for Statement "INSERT INTO old_issues SELECT * FROM issues
                                  WHERE borrowernumber = ?
                                  AND itemnumber = ?" with ParamValues: 0='654336', 1='25'] at /home/marc/koha/C4/Circulation.pm line 2172.
DBD::mysql::st execute failed: Duplicate entry '37' for key 'PRIMARY' [for Statement "INSERT INTO old_issues SELECT * FROM issues
                                  WHERE borrowernumber = ?
                                  AND itemnumber = ?" with ParamValues: 0='654336', 1='25'] at /home/marc/koha/C4/Circulation.pm line 2172.
 at t/db_dependent/Letters/TemplateToolkit.t line 463.

        #   Failed test at t/db_dependent/Letters/TemplateToolkit.t line 470.
        #          got: '
        # The following items have been checked out:
        # Test Biblio 1
        # Test Biblio 2
        # Thank you for visiting YCIAZz.
        # '
        #     expected: '
        # The following items have been checkin out:
        # Test Biblio 1 was due on 01.03.2017
        # Thank you for visiting YCIAZz.
        # '

        #   Failed test at t/db_dependent/Letters/TemplateToolkit.t line 471.
        #          got: '
        # The following items have been checkin out:
        # Test Biblio 2 was due on 01.03.2017
        # Thank you for visiting YCIAZz.
        # '
        #     expected: '
        # The following items have been checkin out:
        # Test Biblio 1 was due on 01.03.2017
        # Test Biblio 2 was due on 01.03.2017
        # Thank you for visiting YCIAZz.
        # '
        # Looks like you failed 2 tests of 4.

    #   Failed test 'CHECKOUT+CHECKIN'
    #   at t/db_dependent/Letters/TemplateToolkit.t line 473.
    # Looks like you failed 1 test of 4.
t/db_dependent/Letters/TemplateToolkit.t .. 16/16 
#   Failed test 'regression tests'
#   at t/db_dependent/Letters/TemplateToolkit.t line 503.
# Looks like you failed 1 test of 16.
t/db_dependent/Letters/TemplateToolkit.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/16 subtests 

Test Summary Report
-------------------
t/db_dependent/Letters/TemplateToolkit.t (Wstat: 256 Tests: 16 Failed: 1)
  Failed test:  16
  Non-zero exit status: 1
Files=1, Tests=16,  3 wallclock secs ( 0.02 usr  0.02 sys +  2.28 cusr  0.36 csys =  2.68 CPU)
Result: FAIL

-Second run, immediately after first run-----------------

marc@debian8vm:~/koha$ prove t/db_dependent/Letters/TemplateToolkit.t
t/db_dependent/Letters/TemplateToolkit.t .. ok     
All tests successful.
Files=1, Tests=16,  2 wallclock secs ( 0.03 usr  0.00 sys +  1.63 cusr  0.87 csys =  2.53 CPU)
Result: PASS
Comment 8 Jonathan Druart 2017-02-22 13:57:50 UTC
(In reply to Marc Véron from comment #6)
> koha-foreach --enabled --email
> /home/marc/koha/misc/cronjobs/advance_notices.pl -c

Prefer koha-shell then execute the advance_notices.pl script.
To make sure it will use the correct modules.

(In reply to Marc Véron from comment #7)
> ...and that are the two outputs of TemplateToolkit.t:
> 
> --First run ---------------------------
> 
> marc@debian8vm:~/koha$ prove t/db_dependent/Letters/TemplateToolkit.t
> t/db_dependent/Letters/TemplateToolkit.t .. 1/16 DBD::mysql::st execute
> failed: Duplicate entry '37' for key 'PRIMARY' [for Statement "INSERT INTO
> old_issues SELECT * FROM issues

This happens if AI of old_issues is >= AI of issues, it does not come from this patch set.
Comment 9 Marc Véron 2017-02-23 22:15:04 UTC
(In reply to Jonathan Druart from comment #8)
> (In reply to Marc Véron from comment #6)
> > koha-foreach --enabled --email
> > /home/marc/koha/misc/cronjobs/advance_notices.pl -c
> 
> Prefer koha-shell then execute the advance_notices.pl script.
> To make sure it will use the correct modules.
> (...)

Thanks for the hint regarding koha-shell. The problem was related to PERL5LIB pointing to a wrong directory.
Comment 10 Marc Véron 2017-02-23 22:17:12 UTC
Created attachment 60625 [details] [review]
Bug 17965: TT syntax for notices - Prove that DUEDGST is compatible

This notice template have the particular feature of using <<count>>.
This value is substitued during the process of the notice template.
For the TT syntax, all what we need is to send the values to substitute to the
template.

Note that items.content can also be used in these template, you can have
a look at bug 17967 to see a better alternative to this marker.

Test plan:
Generate DUEDGST and DUE notice messages.
You should be able to generate the same messages with the TT syntax.

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 11 Jonathan Druart 2017-02-24 07:18:08 UTC
(In reply to Marc Véron from comment #9)
> (In reply to Jonathan Druart from comment #8)
> > (In reply to Marc Véron from comment #6)
> > > koha-foreach --enabled --email
> > > /home/marc/koha/misc/cronjobs/advance_notices.pl -c
> > 
> > Prefer koha-shell then execute the advance_notices.pl script.
> > To make sure it will use the correct modules.
> > (...)
> 
> Thanks for the hint regarding koha-shell. The problem was related to
> PERL5LIB pointing to a wrong directory.

FYI this is fixed in master, you should copy the debian files from the git repo to the one used by the system.
Comment 12 Marc Véron 2017-02-25 18:56:48 UTC
Forgot to switch status (see comment #10)
Comment 13 Jonathan Druart 2017-06-05 16:28:50 UTC
Created attachment 63967 [details] [review]
Bug 17965: TT syntax for notices - Prove that DUEDGST is compatible

This notice template have the particular feature of using <<count>>.
This value is substitued during the process of the notice template.
For the TT syntax, all what we need is to send the values to substitute to the
template.

Note that items.content can also be used in these template, you can have
a look at bug 17967 to see a better alternative to this marker.

Test plan:
Generate DUEDGST and DUE notice messages.
You should be able to generate the same messages with the TT syntax.

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 14 Jonathan Druart 2017-06-05 16:34:48 UTC
Created attachment 63971 [details] [review]
Bug 17965: TT syntax for notices - Prove that DUEDGST is compatible

This notice template have the particular feature of using <<count>>.
This value is substitued during the process of the notice template.
For the TT syntax, all what we need is to send the values to substitute to the
template.

Note that items.content can also be used in these template, you can have
a look at bug 17967 to see a better alternative to this marker.

Test plan:
Generate DUEDGST and DUE notice messages.
You should be able to generate the same messages with the TT syntax.

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 15 Jonathan Druart 2017-06-05 16:35:43 UTC
Patch rebased!
Comment 16 Jonathan Druart 2017-06-05 16:38:36 UTC
Created attachment 63972 [details] [review]
Bug 17965: TT syntax for notices - Prove that DUEDGST is compatible

This notice template have the particular feature of using <<count>>.
This value is substitued during the process of the notice template.
For the TT syntax, all what we need is to send the values to substitute to the
template.

Note that items.content can also be used in these template, you can have
a look at bug 17967 to see a better alternative to this marker.

Test plan:
Generate DUEDGST and DUE notice messages.
You should be able to generate the same messages with the TT syntax.

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 17 Kyle M Hall 2017-06-15 14:48:49 UTC
Created attachment 64333 [details] [review]
Bug 17965: TT syntax for notices - Prove that DUEDGST is compatible

This notice template have the particular feature of using <<count>>.
This value is substitued during the process of the notice template.
For the TT syntax, all what we need is to send the values to substitute to the
template.

Note that items.content can also be used in these template, you can have
a look at bug 17967 to see a better alternative to this marker.

Test plan:
Generate DUEDGST and DUE notice messages.
You should be able to generate the same messages with the TT syntax.

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 18 Jonathan Druart 2017-06-15 19:08:06 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 19 Fridolin SOMERS 2017-06-20 08:32:45 UTC
Enhancement not pushed to 17.05.x