[Bug 31373] New: Notice template validation is missing INCLUDE_PATH
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31373 Bug ID: 31373 Summary: Notice template validation is missing INCLUDE_PATH Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Tools Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org We allow the use of staff client includes in template toolkit notice templates however, the current validation processing is missing this code and thus errantly reports issues with valid notices. -- 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=31373 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |31211 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31211 [Bug 31211] Check slips and notices for valid Template Toolkit and report errors -- 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=31373 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.com -- 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=31373 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |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=31373 --- Comment #1 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 139210 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=139210&action=edit Bug 31373: Add INCLUDE_PATH to notice validation This is a follow-up to bug 31211. It adds the same INCLUDE_PATH variables as C4::Templates adds during processing to prevent false failures. Test plan 1) Add an INCLUDE to any valid notice, for example: [% INCLUDE 'patron-title.inc' patron => patron %] 2) Save and continue 3) Confirm you are faced with a validation error complaining that the include file cannot be found 4) Apply this patch 5) Confirm that the reported error has dissapeared -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31373 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31373 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31373 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31373 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #139210|0 |1 is obsolete| | --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 139211 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=139211&action=edit Bug 31373: Add INCLUDE_PATH to notice validation This is a follow-up to bug 31211. It adds the same INCLUDE_PATH variables as C4::Templates adds during processing to prevent false failures. Test plan 1) Add an INCLUDE to any valid notice, for example: [% INCLUDE 'patron-title.inc' patron => patron %] 2) Save and continue 3) Confirm you are faced with a validation error complaining that the include file cannot be found 4) Apply this patch 5) Confirm that the reported error has dissapeared 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=31373 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #3 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Why do we process html_helpers? Why are we duplicating C4::Letters::_process_tt? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31373 --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Jonathan Druart from comment #3)
Why do we process html_helpers?
Ha, this crept in from another bug I was working... I found that including biblio-title.inc requires a pre-process of html_helpers and it felt like a good one to add to the default process rather than requiring people to know they need to have it processed at the top in the template itself. It should probably be in it's own bug.
Why are we duplicating C4::Letters::_process_tt?
So, I did wonder this myself.. I think it's because we don't have any parameters to actually pass in at this point.. so we just want a compile check.. must admit I didn't dig into the depths of the original bug. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31373 --- Comment #5 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Martin Renvoize from comment #4)
(In reply to Jonathan Druart from comment #3)
Why are we duplicating C4::Letters::_process_tt?
So, I did wonder this myself.. I think it's because we don't have any parameters to actually pass in at this point.. so we just want a compile check.. must admit I didn't dig into the depths of the original bug.
IMO we should stop as soon as possible and fix the situation, instead of adding more code to the controller. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31373 --- Comment #6 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Martin Renvoize from comment #4)
(In reply to Jonathan Druart from comment #3)
Why do we process html_helpers?
Ha, this crept in from another bug I was working... I found that including biblio-title.inc requires a pre-process of html_helpers and it felt like a good one to add to the default process rather than requiring people to know they need to have it processed at the top in the template itself. It should probably be in it's own bug.
Preferable to include it explicitly when you need it instead of adding black magic. And it won't help plugins to be compatible between different versions if sometimes it needs to be included, sometimes not. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31373 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Jonathan Druart from comment #6)
(In reply to Martin Renvoize from comment #4)
(In reply to Jonathan Druart from comment #3)
Why do we process html_helpers?
Ha, this crept in from another bug I was working... I found that including biblio-title.inc requires a pre-process of html_helpers and it felt like a good one to add to the default process rather than requiring people to know they need to have it processed at the top in the template itself. It should probably be in it's own bug.
Preferable to include it explicitly when you need it instead of adding black magic. And it won't help plugins to be compatible between different versions if sometimes it needs to be included, sometimes not.
Fair enough.. it should be very clearly documented somewhere which includes require that helper then so notice authors aren't surprised by them not working. I'd also remove the KohaDates and Remove_MARC_punctuation additions too as they suffer the same issues. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31373 --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- If we remove KohaDates now, that might be a little late. Same for the other TT filter also probably not as widely used. What about notices not including the USE already? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31373 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 139406 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=139406&action=edit Bug 31373: DRY - Use try/catch around _process_tt This patch reduces the repetition in the tools/letter.pl controller replaceing it with a try/catch block wrapping a call to C4::Letters::_process_tt instead. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31373 --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- The point I was trying to make with the including html_helpers was that I don't see how it's different from automagically including the other two we include.. to me, this is a well hidden undocumented template thing so made sense to 'just make it easy'.. Anywho.. it should certainly be in it's own bug. Moving on.. please review my follow-up/alternative (Let's golf it then squash). I don't like the regex mangling of the croak output but was failing to work out how else to clean up the resulting output to screen... we could remove the croak and get more meaningful in our return from the internal C4::Letter function.. but that also felt a bit funky. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31373 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31373 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #139406|0 |1 is obsolete| | --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 139677 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=139677&action=edit Bug 31373: DRY - Use try/catch around _process_tt This patch reduces the repetition in the tools/letter.pl controller replaceing it with a try/catch block wrapping a call to C4::Letters::_process_tt instead. 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=31373 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=31028 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31373 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31373 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #139211|0 |1 is obsolete| | --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 140456 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=140456&action=edit Bug 31373: Add INCLUDE_PATH to notice validation This is a follow-up to bug 31211. It adds the same INCLUDE_PATH variables as C4::Templates adds during processing to prevent false failures. Test plan 1) Add an INCLUDE to any valid notice, for example: [% INCLUDE 'patron-title.inc' patron => patron %] 2) Save and continue 3) Confirm you are faced with a validation error complaining that the include file cannot be found 4) Apply this patch 5) Confirm that the reported error has dissapeared Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <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=31373 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #139677|0 |1 is obsolete| | --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 140457 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=140457&action=edit Bug 31373: DRY - Use try/catch around _process_tt This patch reduces the repetition in the tools/letter.pl controller replaceing it with a try/catch block wrapping a call to C4::Letters::_process_tt instead. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <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=31373 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31373 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |22.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31373 --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 22.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31373 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |32442 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32442 [Bug 32442] Invalid Template Toolkit in notices can cause errors -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org