https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33350
Adolfo Rodríguez Taboada <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |In Discussion --- Comment #5 from Adolfo Rodríguez Taboada <[email protected]> --- Thanks for the pointer re: notices policy. Before taking this to the dev meeting, I want to lay out why I think this specific case is different from "new development should use TT" — and ask for input on how you'd like to proceed. repeat / <item>...</item> is not a marginal legacy format — it's a documented, currently-supported parameter of GetPreparedLetter (C4/Letters.pm:559-565: "replaces <token>...</token> subtemplate for each @tables row"), and it's actively used by core Koha code today, not just by user-customized templates: * C4/Overdues.pm:738 — repeat => { item => \@item_tables } — this is the call that builds the stock ODUE (overdue) notice shipped with Koha. * C4/Members.pm:481 — same mechanism for member-related notices. * The PDF slip layouts in Koha/pdfformat/layout*.pm. So this isn't "please add more support for an old syntax" — it's a correctness bug in a mechanism the core codebase itself relies on. The actual bug is one line, in repeat_enclosing_tags inside GetPreparedLetter: if ( $letter->{content} =~ m!<$tag>(.*)</$tag>!s ) { That greedy (.*) matches from the first <$tag> to the very last </$tag> in the whole content, so when a template contains the same tag twice (e.g. an English <item> block and a Spanish <item> block in a bilingual notice — a legitimate use of the documented repeat hashref form), both blocks get merged into a single match and the text between them is lost. A template with a single <item> block, which is the overwhelming majority of installs, is unaffected either way. The fix (attachment 201637) only replaces that one match with a non-greedy m!<$tag>(.*?)</$tag>!sg loop that expands each block independently, and ships with a unit test. "Use TT instead" isn't a straightforward migration path for the libraries hitting this: notice templates are edited by library staff through the GUI, not by developers, and TT's [% FOREACH %] relies on the loops parameter, not repeat — so affected libraries would need to rewrite their customized notices from scratch, with no deprecation notice or migration guide for repeat/<item> anywhere in the manual. If the actual intent is to deprecate repeat/<item> entirely going forward, I think that's a bigger decision than a Failed QA on this bug — it would mean deprecating a documented, core-used parameter of GetPreparedLetter, including the Overdues.pm/Members.pm call sites, and probably deserves its own RFC/dev meeting discussion with a proper migration plan, rather than quietly blocking bug fixes to it piecemeal. Happy to bring this to the next dev meeting if that's the right venue — let me know if you still think this specific bug fix shouldn't move forward in the meantime. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list -- [email protected] To unsubscribe send an email to [email protected] website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
