Sending an email with the contents of your cart is broken in non english templates. To reproduce: - Search OPAC - Add some titles to your cart - Open cart and try to send you the contents - log in, if you didn't before - Send cart to any email address - Check email text and attachements The mail sent from the translated templats has subject "no subject" and is empty. The marc files are attached. The mail sent from the english templates is all correct. This is an important OPAC feature for some libraries. Bug has been confirmed in 3.6.3 with German templates and in 3.6.4 with Arabic templates. Still has to be confirmed against master and 3.8.
Confirmed in master using fresh nb-NO templates.
Ok, I compared the English with the translated German templates. The translated file is missing all line breaks after line 7 <MESSAGE> and is broken because of that. I am not sure how to fix it as it seems to be a problem with the translation scripts not preserving the line breaks correctly.
Ok, the error is starting even before line 7. Still investigating.
After giving it some thought I think the main problem here is that our translation scripts are not made to parse a plain text file, but TT and HTML. I also tried adding the line breaks back in Poedit, but they get taken out again on saving the file. We can also not use \n or similar, because that will also cause problems (bug 4883) Without the line breaks the formatting is broken and the regex searching for the markers (<SUBJECT><END_SUBJECT> etc) are not working. I am a bit out of ideas right now on how to fix that. This is a big problem for every translated Koha installation. It's not only affecting the OPAC cart, but also staff cart and sending lists from both staff and cart.
I confirm the bug, its severity, and your diagnostic. There should be already get an issue before 3.8. Email sent by Koha comes from a template which isn't strictly a template. There is a pseudo-syntax allowing subject/message extraction. But this mini-templating system isn't compliant with our current translation system which doesn't preserve new lines: it doesn't matter in HTML; it's important in raw text, which is used in basket email. We must rewrite in HTML the email sent by OPAC Send your Basket functionality.
Created attachment 9551 [details] [review] Proposed patch Bug 8062 Cart email broken for non english templates Fix basket email sent from OPAC. The email is now in HTML. To test that it works in other language than English, you have to update your language .po file: perl translate update de-DE Then, translated strings coming from opac-sendbasket.tt, and install the translation: perl translate install de-DE
Hi Frederic, thank you for providing a patch! I have tried to find out when the feature broke and tracked it back to the switch to TT. I am not sure how it could go unnoticed for so long. I am a bit worried about switching to HTML, because normally Koha is sending text mails. Is there a way we could make it work as text mail still? We could perhaps trick the translation tools by using <br/> and a paragraph in the notice template, but remove those (HTML::Scrubber?) before sending the mail out.
> I am a bit worried about switching to HTML, because normally Koha is > sending text mails. Take a look a the resulting mail. It's not that bad. > Is there a way we could make it work as text mail still? We could fix the translator, and get it preserving new line characters. But it's not that easy. > We could perhaps trick the translation tools by using <br/> and a > paragraph in the notice template, but remove those (HTML::Scrubber?) > before sending the mail out. I fear that chunk of HTML finish in the .po file, and so disturb translators. We could also transform the HTML into Text just before sending the email.
Created attachment 9572 [details] [review] Proposed patch 2nd version Fix basket email sent from OPAC. The email body is now generated HTML (to be translatable), and converted into TEXT before being sent by email. To test that it works in other language than English, you have to update your language .po file: perl translate update de-DE Then, translated strings coming from opac-sendbasket.tt, and install the translation: perl translate install de-DE New dependency: Template::Plugin::HtmlToText
Created attachment 9875 [details] [review] Bug 8062 [SIGNED-OFF] Cart email broken for non english templates Fix basket email sent from OPAC. The email body is now generated HTML (to be translatable), and converted into TEXT before beeing sent by email. To test that it works in other language than English, you have to update your language .po file: perl translate update de-DE Then, translated strings coming from opac-sendbasket.tt, and install the translation: perl translate install de-DE New dependency: Template::Plugin::HtmlToText Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
It works fine :) Perharps a followup is required to specified the new dependency (in C4/Installer/PerlDependencies.pm ?)
> Perharps a followup is required to specified the new dependency (in > C4/Installer/PerlDependencies.pm ?) The template contains a 'USE HtmlToText' directive, which use a Template::Toolkit module. It's not a Perl module, and it's included by default with Template::Toolkit. Is it the dependency we were thinking about?
To test your patch, I had to install the Template::Plugin::HtmlToText module via cpan.
Created attachment 9908 [details] [review] Add Perl module dependency My fault... You're correct, this a new required Perl module. Here it is. Thanks.
Created attachment 9940 [details] [review] Bug 8062 [SIGNED-OFF] Perl module dependency Template::Plugin::HtmlToText Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
QA comment: * small patch, that uses a T::T plugin to fix an annoying problem * introduces a new dependency that could be usefull in other circumstances, so it's OK * remove some commented/dead code ++ * opac/opac-sendbasket.pl still passes perlcritic passed QA
critical problem in non-english languages, worth porting it to 3.8
pushed to 3.8.x will be in 3.8.2
The patch for this major bug will require a few things for inclusion in 3.6.x: 1. A new patch that does not conflict on koha-tmpl/opac-tmpl/prog/en/modules/opac-sendbasket.tt 2. A follow-up patch that adds instructions for installing the new Perl module via CPAN and/or the debian.koha-community.org apt repository 3. A packaged version of libtemplate-plugin-htmltotext-perl for the oldstable distribution on debian.koha-community.org 4. An explanation of what the new dependency is required for so that I can put it in the release notes.
I have no time now for your points 1-3. > 4. An explanation of what the new dependency is required for so that I > can put it in the release notes. With Template::Plugin::HtmlToText, it's possible to transform HTML into Text! as suggested by the module name. It allows Koha developers to use standard (for Koha) Template::Toolkit templating system to generate HTML, and then, if necessary to transform this HTML into raw text. This is required for example for email sent by Koha, which use to be in Text. Benefits: --email templates can have a rich HTML-base layout, but still being sent in Text ; --email templates are translatable like Koha web pages
I think this patch introduces a bug: +use HTML::FormatText; this isn't mentioned in the dependency list, and doesn't appear to be used in the code. Thus, opac-sendbasket.pl breaks unless you happen to have that module installed. Can someone confirm that this is the case?
OK, it is a dependency of Template::Plugin::HtmlToText, however that doesn't declare it as a dependency in the META.yml, so the packages don't either, and so it never gets installed. Guess I'll have to redo the packages some time soon to account for that.
(In reply to comment #21) > I think this patch introduces a bug: > > +use HTML::FormatText; > > this isn't mentioned in the dependency list, and doesn't appear to be used > in the code. Thus, opac-sendbasket.pl breaks unless you happen to have that > module installed. Can someone confirm that this is the case? Confirmed: Can't locate HTML/FormatText.pm in @INC Paul: When you passed this through QA, you mentioned the new dependency. But nothing was done in a followup for dependency list, etc. ?
I submitted a patch for the dependency under 7368. But Robin is right that the code in opac-basket does not directly use FormatText. So the use statement can be deleted. But note that the Template::Plugin::HtmlToText requires HTML::FormatText. So it is a core Koha dependency.
Created attachment 10969 [details] [review] Patch for HTML::FormatText Deletes use statement only in opac-sendbasket.pl Template::Plugin::HtmlToText does require it on its own.
QA Comments: Bug 8062 introduced this new dependency in opac-sendbasket. But it did not call a routine of this module. As no new call have been added since, it's useless. Marking as Passed QA.
Follow-up pushed (yikes : I wanted to remove the self-signoff & put mine, but pushed to quickly...)
Follow up pushed to 3.8.x, will be in 3.8.4