[Bug 42560] New: Add support for [% items.content %]
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42560 Bug ID: 42560 Summary: Add support for [% items.content %] Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Templates Assignee: oleonard@myacpl.org Reporter: dcook@prosentient.com.au QA Contact: testopia@bugs.koha-community.org If you're updating <<>> to [% %], it's easy to accidentally change <<items.content>> to [% items.content %] However, if you do that, you'll break your notices, because... "content" isn't a valid column in the items table: ERROR PROCESSING TEMPLATE: undef error - The method Koha::Items->content is not covered by tests! Trace begun at /usr/share/koha/lib/Koha/Objects.pm line 581 Koha::Objects::AUTOLOAD('Koha::Items=HASH(0x5e093f483010)') called at input text line 8 eval {...} at input text line 8 eval {...} at input text line 7 Template::Document::__ANON__('Template::Context=HASH(0x5e093f3d4a68)') called at /usr/lib/x86_64-linux-gnu/perl5/5.38/Template/Document.pm line 163 eval {...} at /usr/lib/x86_64-linux-gnu/perl5/5.38/Template/Document.pm line 161 Template::Document::process('Template::Document=HASH(0x5e093f7b2060)', 'Template::Context=HASH(0x5e093f3d4a68)') called at /usr/lib/x86_64-linux-gnu/perl5/5.38/Template/Context.pm line 351 eval {...} at /usr/lib/x86_64-linux-gnu/perl5/5.38/Template/Context.pm line 321 Template::Context::process('Template::Context=HASH(0x5e093f3d4a68)', 'Template::Document=HASH(0x5e093f7b2060)') called at /usr/lib/x86_64-linux-gnu/perl5/5.38/Template/Service.pm line 94 eval {...} at /usr/lib/x86_64-linux-gnu/perl5/5.38/Template/Service.pm line 91 Template::Service::process('Template::Service=HASH(0x5e093f3a8308)', 'SCALAR(0x5e093ec6ad90)', 'HASH(0x5e093f47fe98)') called at /usr/lib/x86_64-linux-gnu/perl5/5.38/Template.pm line 66 Template::process('Template=HASH(0x5e093f3a8350)', 'SCALAR(0x5e093ec6ad90)', 'HASH(0x5e093f47fe98)', 'SCALAR(0x5e093ec6b618)') called at /usr/share/koha/lib/C4/Letters.pm line 1764 C4::Letters::try {...} at /usr/share/perl5/Try/Tiny.pm line 100 eval {...} at /usr/share/perl5/Try/Tiny.pm line 93 Try::Tiny::try('CODE(0x5e093f508378)', 'Try::Tiny::Finally=REF(0x5e0935931a68)') called at /usr/share/koha/lib/C4/Letters.pm line 1768 C4::Letters::_process_tt('HASH(0x5e093f450930)') called at /usr/share/koha/lib/C4/Letters.pm line 651 C4::Letters::GetPreparedLetter('module', 'circulation', 'letter_code', 'PREDUEDGST', 'branchcode', 'BRANCH', 'lang', 'default', 'substitute', 'HASH(0x5e093f480198)', 'tables', 'HASH(0x5e093eca56b8)', 'loops', 'HASH(0x5e093f3d6bc0)', 'message_transport_type', 'email') called at /usr/share/koha/bin/cronjobs/advance_notices.pl line 505 main::parse_letter('HASH(0x5e093f480af8)') called at /usr/share/koha/bin/cronjobs/advance_notices.pl line 612 main::send_digests('HASH(0x5e093f46f920)') called at /usr/share/koha/bin/cronjobs/advance_notices.pl line 451 at /usr/share/koha/bin/cronjobs/advance_notices.pl line 505. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42560 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |29698 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29698 [Bug 29698] items are not available for TT syntax for PREDUEDGST -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42560 --- Comment #1 from David Cook <dcook@prosentient.com.au> --- In advance_notices.pl, "items.content" is set as a key in "substitute"... so the variable name would literally be "items.content"... but if you do [% items.content %] it's going to be calling the "content" method on the "items" variable. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42560 --- Comment #2 from David Cook <dcook@prosentient.com.au> --- Actually, this might just be impossible... Take the Template::Toolkit params in my advance notice. 'borrower' 'branch' 'branches.branchaddress1' 'branches.branchaddress2' 'branches.branchaddress3' 'branches.branchcity' 'branches.branchcode' 'branches.branchcountry' 'branches.branchemail' 'branches.branchfax' 'branches.branchillemail' 'branches.branchip' 'branches.branchname' 'branches.branchnotes' 'branches.branchphone' 'branches.branchreplyto' 'branches.branchreturnpath' 'branches.branchstate' 'branches.branchurl' 'branches.branchzip' 'branches.geolocation' 'branches.issuing' 'branches.marcorgcode' 'branches.opacusercss' 'branches.opacuserjs' 'branches.pickup_location' 'branches.public' 'checkouts' 'count' 'items' 'items.content' 'today' I remember there was a bug report about the order in which <<>> and [% and %] are processed. Bug 36244. Right... Not 100% sure where all those "branches.*" variables are coming from. But I can't afford to spend any more time on this today. Could be a good idea to delete any keys with a "." from _get_tt_params because I don't think they'd ever work. Of course, items.content would still be problematic with [% %] syntax. More reading: Bug 17971 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42560 --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- What items.content actually contains can only be configured on command line. It provides a list of values tab separated, which makes it not easy to work with especially in HTML notices. I feel like it should die with hungry alligators one day and be replaced by a FOREACH with the individual fields listed inside. I am trying to say: it served it's purpose, but the TT approach is so much better to use for libraries we should encourage that instead. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42560 --- Comment #4 from David Cook <dcook@prosentient.com.au> --- (In reply to Katrin Fischer from comment #3)
What items.content actually contains can only be configured on command line. It provides a list of values tab separated, which makes it not easy to work with especially in HTML notices. I feel like it should die with hungry alligators one day and be replaced by a FOREACH with the individual fields listed inside.
That's a good point! Maybe the solution is for us to update the default notice rather than support "[% items.content %]".
I am trying to say: it served it's purpose, but the TT approach is so much better to use for libraries we should encourage that instead.
The TT is powerful. I actually regret us adding it in though, as it presents a number of (security) problems too. But... I think there's no putting the genie back in the bottle now... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42560 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=9083 CC| |jonathan.druart@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42560 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Add support for [% |Using [% items.content %] |items.content %] |causes fatal errors | |preventing outbound notices -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42560 --- Comment #5 from David Cook <dcook@prosentient.com.au> --- All right maybe let's not support [% items.content %] after all, but... let's make it so that notices don't fatally error if they include it? I think we could probably add a warning and leave it at that. I think it might be better to have notices going out missing details rather than no notices at all? (Because at least that way the patron knows the library is trying to get in touch and then they'll either complain about the problem notice or they'll check their loans and go "oh crap I better return my items") -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42560 --- Comment #6 from David Cook <dcook@prosentient.com.au> --- (I've actually been thinking it could be interesting to somehow do notice previews with dummy data and make it more obvious earlier that the notices are going to have problems...) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42560 --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Currently it's quite easy to break notices using TT. Adding better validation would be great, but I am not sure if the item.content case should be treated separately or as part of "better validation" in general. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42560 --- Comment #8 from David Cook <dcook@prosentient.com.au> --- (In reply to Katrin Fischer from comment #7)
Currently it's quite easy to break notices using TT. Adding better validation would be great, but I am not sure if the item.content case should be treated separately or as part of "better validation" in general.
It's a good question. I'll have a think about it. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org