https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19578 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19578 --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #0)

Jenkins fails (run 287) on a test in t/db_dependent/Letters/TemplateToolkit.t:

With the historical syntax: # Your request for an article from tQYRS (c3Av58O0P5xkkIGu) has been canceled for the following reason:

With the TT syntax: # Your request for an article from tQYRS_ (c3Av58O0P5xkkIGu) has been canceled for the following reason:

The last character of the biblio's title has been removed because it's a punctuation character. It comes from: C4::Letters::_parseletter 893 $val =~ s/\p{P}$// if $val && $table=~/biblio/;

The same replacement is done for patron's attributes too.

With the TT syntax we retrieve the objects and send them to the template, without fetching the attributes. To restore this behaviour we could apply this regex on all the fields of the biblio objects, but it could be perf killer (we will process fields that we will certainly not used in the template). I thought about the TT filter we could apply on all the variables we display, but we will not know where come from the data, and we need to apply this change only for the biblio table.

Any other ideas?

I think there are two valid options: 1) Remove line 893 ( what is it's purpose? ) 2) Add a TT filter like you suggest. I think if we just apply it to any TT syntax that contains ".biblio" within TT tags ( something like /^\[%.*biblio\..*%\]$/ ) it would be safe. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19578 --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 68990 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=68990&action=edit Bug 19578: Remove MARC punctuation in notices (TT syntax) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19578 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |jonathan.druart@bugs.koha-c |ity.org |ommunity.org --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Marcel, what do you think about this approach? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19578 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Marcel de Rooy from comment #7)

(In reply to Jonathan Druart from comment #6)

...

Marcel, what do you think about this approach?

Looks quite good. Is it possible to further reduce the number of fields in biblio and biblioitems to run this Remove_MARC_punctuation on in some pragmatic way?

Yes, we could apply the regex only with a list of fields. But then the 2 syntaxes will no give the same results. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19578 --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I would not recommend to make them diverge and I would not recommend to keep maintaining the 2 syntaxes... So let's keep behaviour as it for now :) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19578 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |17961 CC| |katrin.fischer@bsz-bw.de --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I think it seems the safest bet to keep the current behavour and do the processing for all fields from biblio and biblioitems. Could you provide a test plan Jonathan? Not quite sure yet how to work with the TT syntax in notices. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17961 [Bug 17961] TT syntax for notices - Prove we have an equivalent for our historical custom syntax -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19578 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_18_05_candidate -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19578 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19578 --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Katrin, sorry I have forgotten to add the new file (TT plugin) for the patch. I have also fixed the POD issue. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19578 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #73585|0 |1 is obsolete| | --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 73596 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73596&action=edit Bug 19578: Remove MARC punctuation in notices (TT syntax) Jenkins fails (run 287) on a test in t/db_dependent/Letters/TemplateToolkit.t: With the historical syntax: # Your request for an article from tQYRS (c3Av58O0P5xkkIGu) has been canceled for the following reason: With the TT syntax: # Your request for an article from tQYRS_ (c3Av58O0P5xkkIGu) has been canceled for the following reason: The last character of the biblio's title has been removed because it's a punctuation character. It comes from: C4::Letters::_parseletter 893 $val =~ s/\p{P}$// if $val && $table=~/biblio/; The same replacement is done for patron's attributes too. Test plan: - Confirm that the new tests pass. That should be enough to confirm this change make sense. Test plan (manual): - Create a biblio with a title ending with a punctuation (like "with_punctuation_"), or any other fields of biblio/biblioitem - Generate a notice which will display this field (CHECKIN for instance) Use the historical syntax and the TT syntax, both should display the title without the punctuation character at the end CHECKIN historical: The following items have been checked in: ---- <<biblio.title>> ---- CHECKIN TT syntax: The following items have been checked in: ---- [% biblio.title %] ---- Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19578 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #73596|0 |1 is obsolete| | --- Comment #17 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 73602 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73602&action=edit Bug 19578: Remove MARC punctuation in notices (TT syntax) Jenkins fails (run 287) on a test in t/db_dependent/Letters/TemplateToolkit.t: With the historical syntax: # Your request for an article from tQYRS (c3Av58O0P5xkkIGu) has been canceled for the following reason: With the TT syntax: # Your request for an article from tQYRS_ (c3Av58O0P5xkkIGu) has been canceled for the following reason: The last character of the biblio's title has been removed because it's a punctuation character. It comes from: C4::Letters::_parseletter 893 $val =~ s/\p{P}$// if $val && $table=~/biblio/; The same replacement is done for patron's attributes too. Test plan: - Confirm that the new tests pass. That should be enough to confirm this change make sense. Test plan (manual): - Create a biblio with a title ending with a punctuation (like "with_punctuation_"), or any other fields of biblio/biblioitem - Generate a notice which will display this field (CHECKIN for instance) Use the historical syntax and the TT syntax, both should display the title without the punctuation character at the end CHECKIN historical: The following items have been checked in: ---- <<biblio.title>> ---- CHECKIN TT syntax: The following items have been checked in: ---- [% biblio.title %] ---- Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19578 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_18_05_candidate | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19578 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #20 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Pushed to 17.05.x for v17.05.12 -- You are receiving this mail because: You are watching all bug changes.