Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices
Summary: Allow use of Template Toolkit syntax for Phone Notices
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on: 14757 17981
Blocks:
  Show dependency treegraph
 
Reported: 2016-03-03 12:15 UTC by Kyle M Hall
Modified: 2021-03-13 16:03 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices (6.97 KB, patch)
2016-03-03 12:19 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices (7.03 KB, patch)
2016-03-08 19:35 UTC, Nicole C. Engard
Details | Diff | Splinter Review
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices (8.47 KB, patch)
2016-09-26 11:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices (8.99 KB, patch)
2016-09-26 11:13 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices (8.99 KB, patch)
2016-12-02 16:22 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Add unit test of substitution param (1.72 KB, patch)
2016-12-02 16:22 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Unit test use of __MESSAGE_ID__ (2.05 KB, patch)
2016-12-02 16:22 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Fetch the message instead of substituting (1.19 KB, patch)
2016-12-02 16:22 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices (8.99 KB, patch)
2017-02-06 13:10 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Add unit test of substitution param (1.74 KB, patch)
2017-02-06 13:10 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Unit test use of __MESSAGE_ID__ (2.05 KB, patch)
2017-02-06 13:10 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Fetch the message instead of substituting (1.19 KB, patch)
2017-02-06 13:10 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Fetch the message instead of substituting (1.91 KB, patch)
2017-02-06 15:36 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Unit test use of __MESSAGE_ID__ (2.05 KB, patch)
2017-02-06 19:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Fetch the message instead of substituting (1.91 KB, patch)
2017-02-06 19:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices (8.99 KB, patch)
2017-02-06 19:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Add unit test of substitution param (1.74 KB, patch)
2017-02-06 19:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Unit test use of __MESSAGE_ID__ (2.04 KB, patch)
2017-02-06 19:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Fetch the message instead of substituting (1.91 KB, patch)
2017-02-06 19:12 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices (9.04 KB, patch)
2017-02-07 00:57 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15969 - Add unit test of substitution param (1.79 KB, patch)
2017-02-07 00:57 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15969 - Unit test use of __MESSAGE_ID__ (2.10 KB, patch)
2017-02-07 00:57 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15969 - Fetch the message instead of substituting (1.97 KB, patch)
2017-02-07 00:57 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices (8.19 KB, patch)
2018-02-02 10:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Add unit test of substitution param (1.22 KB, patch)
2018-02-02 10:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Unit test use of __MESSAGE_ID__ (2.10 KB, patch)
2018-02-02 10:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Fetch the message instead of substituting (1.97 KB, patch)
2018-02-02 10:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices (8.54 KB, patch)
2020-04-17 18:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Add unit test of substitution param (1.25 KB, patch)
2020-04-17 18:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Unit test use of __MESSAGE_ID__ (2.22 KB, patch)
2020-04-17 18:58 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Fetch the message instead of substituting (1.91 KB, patch)
2020-04-17 18:58 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices (8.56 KB, patch)
2020-12-15 18:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Add unit test of substitution param (1.06 KB, patch)
2020-12-15 18:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Unit test use of __MESSAGE_ID__ (2.23 KB, patch)
2020-12-15 18:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969 - Fetch the message instead of substituting (1.91 KB, patch)
2020-12-15 18:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15969: Allow use of Template Toolkit syntax for Phone Notices (8.56 KB, patch)
2021-03-12 15:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15969: Add unit test of substitution param (1.05 KB, patch)
2021-03-12 15:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15969: Unit test use of __MESSAGE_ID__ (2.21 KB, patch)
2021-03-12 15:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15969: Fetch the message instead of substituting (1.89 KB, patch)
2021-03-12 15:14 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2016-03-03 12:15:31 UTC
By allowing the use of Template Toolkit for generating the outgoing Talking Tech CSV files, we can give libaries more control over the data they send to Itiva. This enhancement would also make it possible to use the script for other services since the output is no longer fixed to a single format.
Comment 1 Kyle M Hall 2016-03-03 12:19:59 UTC Comment hidden (obsolete)
Comment 2 Nicole C. Engard 2016-03-08 19:35:16 UTC
Created attachment 48811 [details] [review]
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices

By allowing the use of Template Toolkit for generating the outgoing
Talking Tech CSV files, we can give libaries more control over the data
they send to Itiva. This enhancement would also make it possible to use
the script for other services since the output is no longer fixed to a
single format.

Test Plan:
1) Apply this patch
2) Set up a waiting hold, and overdue checkout, and a non-overdue checkout
3) Generate the CSV file via the script
   misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl
4) Enable Template Toolkit for the notices and update the notice text to be:
[% FILTER remove("\r\n") %]
"[% format %]",
"[% language %]",
"[% type %]",
"[% level %]",
"[% borrower.cardnumber %]",
"[% borrower.title %]",
"[% borrower.firstname %]",
"[% borrower.surname %]",
"[% borrower.phone %]",
"[% borrower.email %]",
"[% library_code %]",
"[% branch.id %]",
"[% branch.branchname %]",
"[% item.barcode %]",
"[% due_date %]",
"[% biblio.title %]",
"__MESSAGE_ID__"
[% END %]
5) Regenerate the notices
6) Compare the originals, note the format is the same

Signed-off-by: Sean McGarvey <seanm@pascolibraries.org>
Comment 3 Jonathan Druart 2016-04-04 10:41:34 UTC
Blocked by bug 14757.
Comment 4 Jonathan Druart 2016-08-02 12:01:21 UTC
CONFLICT (content): Merge conflict in misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl
CONFLICT (content): Merge conflict in C4/Letters.pm
Comment 5 Kyle M Hall 2016-09-26 11:07:51 UTC
Created attachment 55819 [details] [review]
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices

By allowing the use of Template Toolkit for generating the outgoing
Talking Tech CSV files, we can give libaries more control over the data
they send to Itiva. This enhancement would also make it possible to use
the script for other services since the output is no longer fixed to a
single format.

Test Plan:
1) Apply this patch
2) Set up a waiting hold, and overdue checkout, and a non-overdue checkout
3) Generate the CSV file via the script
   misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl
4) Enable Template Toolkit for the notices and update the notice text to be:
[% FILTER remove("\r\n") %]
"[% format %]",
"[% language %]",
"[% type %]",
"[% level %]",
"[% borrower.cardnumber %]",
"[% borrower.title %]",
"[% borrower.firstname %]",
"[% borrower.surname %]",
"[% borrower.phone %]",
"[% borrower.email %]",
"[% library_code %]",
"[% branch.id %]",
"[% branch.branchname %]",
"[% item.barcode %]",
"[% due_date %]",
"[% biblio.title %]",
"__MESSAGE_ID__"
[% END %]
5) Regenerate the notices
6) Compare the originals, note the format is the same

Signed-off-by: Sean McGarvey <seanm@pascolibraries.org>
Comment 6 Kyle M Hall 2016-09-26 11:13:58 UTC
Created attachment 55820 [details] [review]
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices

By allowing the use of Template Toolkit for generating the outgoing
Talking Tech CSV files, we can give libaries more control over the data
they send to Itiva. This enhancement would also make it possible to use
the script for other services since the output is no longer fixed to a
single format.

Test Plan:
1) Apply this patch
2) Set up a waiting hold, and overdue checkout, and a non-overdue checkout
3) Generate the CSV file via the script
   misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl
4) Enable Template Toolkit for the notices and update the notice text to be:
[% FILTER remove("\r\n") %]
"[% format %]",
"[% language %]",
"[% type %]",
"[% level %]",
"[% borrower.cardnumber %]",
"[% borrower.title %]",
"[% borrower.firstname %]",
"[% borrower.surname %]",
"[% borrower.phone %]",
"[% borrower.email %]",
"[% library_code %]",
"[% branch.id %]",
"[% branch.branchname %]",
"[% item.barcode %]",
"[% due_date %]",
"[% biblio.title %]",
"__MESSAGE_ID__"
[% END %]
5) Regenerate the notices
6) Compare the originals, note the format is the same

Signed-off-by: Sean McGarvey <seanm@pascolibraries.org>
Comment 7 Jonathan Druart 2016-09-28 10:37:32 UTC
Hi Kyle,
This patch does a bit more than advertise :)
1/ __MESSAGE_ID__ could be used everywhere, not only for phone notice. I think this should be covered by tests and documented somewhere (eventually added to the notice editor)
2/ 
+        if ( $use_tt ) {
+            $letter->{content} =~ s/__MESSAGE_ID__/$message_id/g;
+            print $OUT $letter->{content};
+        }

Don't you think it would be better to retrieve the notice instead of duplicating the substitution? It would be less error prone.
Btw, why does the script output something different if tt syntax is used?

3/ C4::Letters::_process_tt takes a new parameter, it should be covered by tests.
Comment 8 Kyle M Hall 2016-12-02 16:22:23 UTC
Created attachment 57931 [details] [review]
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices

By allowing the use of Template Toolkit for generating the outgoing
Talking Tech CSV files, we can give libaries more control over the data
they send to Itiva. This enhancement would also make it possible to use
the script for other services since the output is no longer fixed to a
single format.

Test Plan:
1) Apply this patch
2) Set up a waiting hold, and overdue checkout, and a non-overdue checkout
3) Generate the CSV file via the script
   misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl
4) Enable Template Toolkit for the notices and update the notice text to be:
[% FILTER remove("\r\n") %]
"[% format %]",
"[% language %]",
"[% type %]",
"[% level %]",
"[% borrower.cardnumber %]",
"[% borrower.title %]",
"[% borrower.firstname %]",
"[% borrower.surname %]",
"[% borrower.phone %]",
"[% borrower.email %]",
"[% library_code %]",
"[% branch.id %]",
"[% branch.branchname %]",
"[% item.barcode %]",
"[% due_date %]",
"[% biblio.title %]",
"__MESSAGE_ID__"
[% END %]
5) Regenerate the notices
6) Compare the originals, note the format is the same

Signed-off-by: Sean McGarvey <seanm@pascolibraries.org>
Comment 9 Kyle M Hall 2016-12-02 16:22:50 UTC
Created attachment 57933 [details] [review]
Bug 15969 - Add unit test of substitution param
Comment 10 Kyle M Hall 2016-12-02 16:22:54 UTC
Created attachment 57934 [details] [review]
Bug 15969 - Unit test use of __MESSAGE_ID__
Comment 11 Kyle M Hall 2016-12-02 16:22:57 UTC
Created attachment 57935 [details] [review]
Bug 15969 - Fetch the message instead of substituting
Comment 12 Kyle M Hall 2017-02-06 13:10:38 UTC
Created attachment 59923 [details] [review]
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices

By allowing the use of Template Toolkit for generating the outgoing
Talking Tech CSV files, we can give libaries more control over the data
they send to Itiva. This enhancement would also make it possible to use
the script for other services since the output is no longer fixed to a
single format.

Test Plan:
1) Apply this patch
2) Set up a waiting hold, and overdue checkout, and a non-overdue checkout
3) Generate the CSV file via the script
   misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl
4) Enable Template Toolkit for the notices and update the notice text to be:
[% FILTER remove("\r\n") %]
"[% format %]",
"[% language %]",
"[% type %]",
"[% level %]",
"[% borrower.cardnumber %]",
"[% borrower.title %]",
"[% borrower.firstname %]",
"[% borrower.surname %]",
"[% borrower.phone %]",
"[% borrower.email %]",
"[% library_code %]",
"[% branch.id %]",
"[% branch.branchname %]",
"[% item.barcode %]",
"[% due_date %]",
"[% biblio.title %]",
"__MESSAGE_ID__"
[% END %]
5) Regenerate the notices
6) Compare the originals, note the format is the same

Signed-off-by: Sean McGarvey <seanm@pascolibraries.org>
Comment 13 Kyle M Hall 2017-02-06 13:10:45 UTC
Created attachment 59924 [details] [review]
Bug 15969 - Add unit test of substitution param
Comment 14 Kyle M Hall 2017-02-06 13:10:48 UTC
Created attachment 59925 [details] [review]
Bug 15969 - Unit test use of __MESSAGE_ID__
Comment 15 Kyle M Hall 2017-02-06 13:10:50 UTC
Created attachment 59926 [details] [review]
Bug 15969 - Fetch the message instead of substituting
Comment 16 Kyle M Hall 2017-02-06 15:36:43 UTC
Created attachment 59937 [details] [review]
Bug 15969 - Fetch the message instead of substituting
Comment 17 Kyle M Hall 2017-02-06 19:00:27 UTC
Created attachment 59940 [details] [review]
Bug 15969 - Unit test use of __MESSAGE_ID__
Comment 18 Kyle M Hall 2017-02-06 19:00:35 UTC
Created attachment 59941 [details] [review]
Bug 15969 - Fetch the message instead of substituting
Comment 19 Kyle M Hall 2017-02-06 19:11:48 UTC
Created attachment 59942 [details] [review]
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices

By allowing the use of Template Toolkit for generating the outgoing
Talking Tech CSV files, we can give libaries more control over the data
they send to Itiva. This enhancement would also make it possible to use
the script for other services since the output is no longer fixed to a
single format.

Test Plan:
1) Apply this patch
2) Set up a waiting hold, and overdue checkout, and a non-overdue checkout
3) Generate the CSV file via the script
   misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl
4) Enable Template Toolkit for the notices and update the notice text to be:
[% FILTER remove("\r\n") %]
"[% format %]",
"[% language %]",
"[% type %]",
"[% level %]",
"[% borrower.cardnumber %]",
"[% borrower.title %]",
"[% borrower.firstname %]",
"[% borrower.surname %]",
"[% borrower.phone %]",
"[% borrower.email %]",
"[% library_code %]",
"[% branch.id %]",
"[% branch.branchname %]",
"[% item.barcode %]",
"[% due_date %]",
"[% biblio.title %]",
"__MESSAGE_ID__"
[% END %]
5) Regenerate the notices
6) Compare the originals, note the format is the same

Signed-off-by: Sean McGarvey <seanm@pascolibraries.org>
Comment 20 Kyle M Hall 2017-02-06 19:11:55 UTC
Created attachment 59943 [details] [review]
Bug 15969 - Add unit test of substitution param
Comment 21 Kyle M Hall 2017-02-06 19:11:58 UTC
Created attachment 59944 [details] [review]
Bug 15969 - Unit test use of __MESSAGE_ID__
Comment 22 Kyle M Hall 2017-02-06 19:12:01 UTC
Created attachment 59945 [details] [review]
Bug 15969 - Fetch the message instead of substituting
Comment 23 Nick Clemens 2017-02-07 00:57:17 UTC
Created attachment 59962 [details] [review]
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices

By allowing the use of Template Toolkit for generating the outgoing
Talking Tech CSV files, we can give libaries more control over the data
they send to Itiva. This enhancement would also make it possible to use
the script for other services since the output is no longer fixed to a
single format.

Test Plan:
1) Apply this patch
2) Set up a waiting hold, and overdue checkout, and a non-overdue checkout
3) Generate the CSV file via the script
   misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl
4) Enable Template Toolkit for the notices and update the notice text to be:
[% FILTER remove("\r\n") %]
"[% format %]",
"[% language %]",
"[% type %]",
"[% level %]",
"[% borrower.cardnumber %]",
"[% borrower.title %]",
"[% borrower.firstname %]",
"[% borrower.surname %]",
"[% borrower.phone %]",
"[% borrower.email %]",
"[% library_code %]",
"[% branch.id %]",
"[% branch.branchname %]",
"[% item.barcode %]",
"[% due_date %]",
"[% biblio.title %]",
"__MESSAGE_ID__"
[% END %]
5) Regenerate the notices
6) Compare the originals, note the format is the same

Signed-off-by: Sean McGarvey <seanm@pascolibraries.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 24 Nick Clemens 2017-02-07 00:57:26 UTC
Created attachment 59963 [details] [review]
Bug 15969 - Add unit test of substitution param

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 25 Nick Clemens 2017-02-07 00:57:32 UTC
Created attachment 59964 [details] [review]
Bug 15969 - Unit test use of __MESSAGE_ID__

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 26 Nick Clemens 2017-02-07 00:57:36 UTC
Created attachment 59965 [details] [review]
Bug 15969 - Fetch the message instead of substituting

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 27 Jonathan Druart 2017-02-07 08:02:41 UTC
I'd like to know what it the plan here, I have done a lot of work on bug 17981 to make the different notices work with the TT syntax.
This patch will conflict with it.

It would also be good to get an independent QA point of view.
Comment 28 Marcel de Rooy 2017-03-31 09:45:29 UTC
Applying: Bug 15969 - Add unit test of substitution param
fatal: sha1 information is lacking or useless (C4/Letters.pm).
Comment 29 Kyle M Hall 2018-02-02 10:49:08 UTC
Created attachment 71141 [details] [review]
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices

By allowing the use of Template Toolkit for generating the outgoing
Talking Tech CSV files, we can give libaries more control over the data
they send to Itiva. This enhancement would also make it possible to use
the script for other services since the output is no longer fixed to a
single format.

Test Plan:
1) Apply this patch
2) Set up a waiting hold, and overdue checkout, and a non-overdue checkout
3) Generate the CSV file via the script
   misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl
4) Enable Template Toolkit for the notices and update the notice text to be:
[% FILTER remove("\r\n") %]
"[% format %]",
"[% language %]",
"[% type %]",
"[% level %]",
"[% borrower.cardnumber %]",
"[% borrower.title %]",
"[% borrower.firstname %]",
"[% borrower.surname %]",
"[% borrower.phone %]",
"[% borrower.email %]",
"[% library_code %]",
"[% branch.id %]",
"[% branch.branchname %]",
"[% item.barcode %]",
"[% due_date %]",
"[% biblio.title %]",
"__MESSAGE_ID__"
[% END %]
5) Regenerate the notices
6) Compare the originals, note the format is the same

Signed-off-by: Sean McGarvey <seanm@pascolibraries.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 30 Kyle M Hall 2018-02-02 10:49:15 UTC
Created attachment 71142 [details] [review]
Bug 15969 - Add unit test of substitution param

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 31 Kyle M Hall 2018-02-02 10:49:19 UTC
Created attachment 71143 [details] [review]
Bug 15969 - Unit test use of __MESSAGE_ID__

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 32 Kyle M Hall 2018-02-02 10:49:22 UTC
Created attachment 71144 [details] [review]
Bug 15969 - Fetch the message instead of substituting

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 33 Kyle M Hall 2018-02-02 10:50:51 UTC
(In reply to Jonathan Druart from comment #27)
> I'd like to know what it the plan here, I have done a lot of work on bug
> 17981 to make the different notices work with the TT syntax.
> This patch will conflict with it.
> 
> It would also be good to get an independent QA point of view.

Let's hold this one off until your bug is pushed to master. At that point I'll revisit this and make whatever changes are necessary to integrate them.
Comment 34 Jonathan Druart 2018-02-02 13:30:49 UTC
It's not pushed to master yet, still in NQA.

(In reply to Jonathan Druart from comment #7)
> Hi Kyle,
> This patch does a bit more than advertise :)
> 1/ __MESSAGE_ID__ could be used everywhere, not only for phone notice. I
> think this should be covered by tests and documented somewhere (eventually
> added to the notice editor)

Still valid.

> 2/ 
> +        if ( $use_tt ) {
> +            $letter->{content} =~ s/__MESSAGE_ID__/$message_id/g;
> +            print $OUT $letter->{content};
> +        }
> 
> Don't you think it would be better to retrieve the notice instead of
> duplicating the substitution? It would be less error prone.
> Btw, why does the script output something different if tt syntax is used?

Still need an answer.
Does the --tt flag really makes sense?

(In reply to Jonathan Druart from comment #27)
> It would also be good to get an independent QA point of view.

Still needed. I will do it myself if no one else does.
Comment 35 Julian Maurice 2018-04-10 05:51:37 UTC
Changing status to BLOCKED as it depends on a "does not apply" bug
Comment 36 Katrin Fischer 2018-04-12 10:47:09 UTC
Unblocking as dependency is passed QA!
Comment 37 Katrin Fischer 2018-04-12 13:57:44 UTC
This sentence sounds a little odd to me:
+OPTIONAL. If set, processes the notice content as Template Toolkit syntax and sends the content as the CSV line instead
+of generated a fixed CSV line.

Kyle, can you please answer qustions on comment#34?

If you work on the patches, can you please also fix the commit messages (Bug XXXX:)
Comment 38 Liz Rea 2019-03-25 15:19:12 UTC
Patch does not apply anymore sorry :(
Comment 39 Kyle M Hall 2020-04-17 18:57:47 UTC
Created attachment 103181 [details] [review]
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices

By allowing the use of Template Toolkit for generating the outgoing
Talking Tech CSV files, we can give libaries more control over the data
they send to Itiva. This enhancement would also make it possible to use
the script for other services since the output is no longer fixed to a
single format.

Test Plan:
1) Apply this patch
2) Set up a waiting hold, and overdue checkout, and a non-overdue checkout
3) Generate the CSV file via the script
   misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl
4) Enable Template Toolkit for the notices and update the notice text to be:
[% FILTER remove("\r\n") %]
"[% format %]",
"[% language %]",
"[% type %]",
"[% level %]",
"[% borrower.cardnumber %]",
"[% borrower.title %]",
"[% borrower.firstname %]",
"[% borrower.surname %]",
"[% borrower.phone %]",
"[% borrower.email %]",
"[% library_code %]",
"[% branch.id %]",
"[% branch.branchname %]",
"[% item.barcode %]",
"[% due_date %]",
"[% biblio.title %]",
"__MESSAGE_ID__"
[% END %]
5) Regenerate the notices
6) Compare the originals, note the format is the same

Signed-off-by: Sean McGarvey <seanm@pascolibraries.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 40 Kyle M Hall 2020-04-17 18:57:57 UTC
Created attachment 103182 [details] [review]
Bug 15969 - Add unit test of substitution param

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 41 Kyle M Hall 2020-04-17 18:58:00 UTC
Created attachment 103183 [details] [review]
Bug 15969 - Unit test use of __MESSAGE_ID__

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 42 Kyle M Hall 2020-04-17 18:58:04 UTC
Created attachment 103184 [details] [review]
Bug 15969 - Fetch the message instead of substituting

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 43 Katrin Fischer 2020-04-18 02:06:56 UTC
Turns out, as phone notices are not a thing here at all, I am not sure how this works.

In which notice templates (letter codes) do I have to add the TT example code?
Comment 44 Katrin Fischer 2020-04-18 02:07:25 UTC
Oh, and please fix the commit lines if you want to make me happy :)
Comment 45 Katrin Fischer 2020-06-22 11:26:25 UTC
Hi Kyle,

I tihnk I need a more detailed test plan here - I have failed to get it working with the existing instructions.

How does the letter need to be set up in Koha? (letter code, HTML?, etc.)
Comment 46 Martin Renvoize 2020-10-12 08:32:59 UTC
I was looking to take over on QA here, but the patches no longer apply, unfortunately :(
Comment 47 Kyle M Hall 2020-12-15 18:15:11 UTC
Created attachment 114413 [details] [review]
Bug 15969 - Allow use of Template Toolkit syntax for Phone Notices

By allowing the use of Template Toolkit for generating the outgoing
Talking Tech CSV files, we can give libaries more control over the data
they send to Itiva. This enhancement would also make it possible to use
the script for other services since the output is no longer fixed to a
single format.

Test Plan:
1) Apply this patch
2) Set up a waiting hold, and overdue checkout, and a non-overdue checkout
3) Generate the CSV file via the script
   misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl
4) Enable Template Toolkit for the notices and update the notice text to be:
[% FILTER remove("\r\n") %]
"[% format %]",
"[% language %]",
"[% type %]",
"[% level %]",
"[% borrower.cardnumber %]",
"[% borrower.title %]",
"[% borrower.firstname %]",
"[% borrower.surname %]",
"[% borrower.phone %]",
"[% borrower.email %]",
"[% library_code %]",
"[% branch.id %]",
"[% branch.branchname %]",
"[% item.barcode %]",
"[% due_date %]",
"[% biblio.title %]",
"__MESSAGE_ID__"
[% END %]
5) Regenerate the notices
6) Compare the originals, note the format is the same

Signed-off-by: Sean McGarvey <seanm@pascolibraries.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 48 Kyle M Hall 2020-12-15 18:15:22 UTC
Created attachment 114414 [details] [review]
Bug 15969 - Add unit test of substitution param

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 49 Kyle M Hall 2020-12-15 18:15:25 UTC
Created attachment 114415 [details] [review]
Bug 15969 - Unit test use of __MESSAGE_ID__

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 50 Kyle M Hall 2020-12-15 18:15:28 UTC
Created attachment 114416 [details] [review]
Bug 15969 - Fetch the message instead of substituting

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 51 Marcel de Rooy 2021-03-05 07:39:53 UTC
Processing additional checks

        * Commit title does not start with 'Bug XXXXX: ' - 28c2325c47
        * Commit title does not start with 'Bug XXXXX: ' - 787cf5cda9
        * Commit title does not start with 'Bug XXXXX: ' - 01aa2c4bb8
        * Commit title does not start with 'Bug XXXXX: ' - ce321c3ad3
Comment 52 Nick Clemens 2021-03-12 15:14:15 UTC
Created attachment 118207 [details] [review]
Bug 15969: Allow use of Template Toolkit syntax for Phone Notices

By allowing the use of Template Toolkit for generating the outgoing
Talking Tech CSV files, we can give libaries more control over the data
they send to Itiva. This enhancement would also make it possible to use
the script for other services since the output is no longer fixed to a
single format.

Test Plan:
1) Apply this patch
2) Set up a waiting hold, and overdue checkout, and a non-overdue checkout
3) Generate the CSV file via the script
   misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl
4) Enable Template Toolkit for the notices and update the notice text to be:
[% FILTER remove("\r\n") %]
"[% format %]",
"[% language %]",
"[% type %]",
"[% level %]",
"[% borrower.cardnumber %]",
"[% borrower.title %]",
"[% borrower.firstname %]",
"[% borrower.surname %]",
"[% borrower.phone %]",
"[% borrower.email %]",
"[% library_code %]",
"[% branch.id %]",
"[% branch.branchname %]",
"[% item.barcode %]",
"[% due_date %]",
"[% biblio.title %]",
"__MESSAGE_ID__"
[% END %]
5) Regenerate the notices
6) Compare the originals, note the format is the same

Signed-off-by: Sean McGarvey <seanm@pascolibraries.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 53 Nick Clemens 2021-03-12 15:14:18 UTC
Created attachment 118208 [details] [review]
Bug 15969: Add unit test of substitution param

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 54 Nick Clemens 2021-03-12 15:14:22 UTC
Created attachment 118209 [details] [review]
Bug 15969: Unit test use of __MESSAGE_ID__

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 55 Nick Clemens 2021-03-12 15:14:25 UTC
Created attachment 118210 [details] [review]
Bug 15969: Fetch the message instead of substituting

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 56 Katrin Fischer 2021-03-13 16:03:59 UTC
(In reply to Katrin Fischer from comment #43)
> Turns out, as phone notices are not a thing here at all, I am not sure how
> this works.
> 
> In which notice templates (letter codes) do I have to add the TT example
> code?

Still applies...

Also:

Test Summary Report
-------------------
t/db_dependent/Letters/TemplateToolkit.t (Wstat: 512 Tests: 24 Failed: 1)
  Failed test:  24
  Non-zero exit status: 2
  Parse errors: Bad plan.  You planned 27 tests but ran 24.
Files=1, Tests=24,  6 wallclock secs ( 0.01 usr  0.02 sys +  4.27 cusr  1.13 csys =  5.43 CPU)
Result: FAIL

Possibly just a reabase error (wrong number)