Created attachment 3596 [details] [review] Proposed Patch
I tested a few HTML tags and it appears to work, it should probably be tested again and get a second sign off.
doing some tests, I get: 17:47 ~/koha.dev/koha-community (BZ6030 $%)$ misc/cronjobs/overdue_notices.pl --html=test-html Global symbol "$columns_def_hashref" requires explicit package name at misc/cronjobs/overdue_notices.pl line 486. Execution of misc/cronjobs/overdue_notices.pl aborted due to compilation errors. Digging into biblibre/master, I see this script has more lines : my $columns_def_hashref = C4::Reports::Guided::_get_column_defs(); foreach my $key ( keys %$columns_def_hashref ) { my $initkey = $key; $key =~ s/[^\.]*\.//; $columns_def_hashref->{$initkey}=NormalizeString($columns_def_hashref->{$initkey}); $columns_def_hashref->{$initkey}=~s# #<br/>#; $columns_def_hashref->{$key} = $columns_def_hashref->{$initkey}; } I think adding them will result in the script working, but without this, it can't work. Nicole = what kind of tests did you do ?
All my tests are purely cosmetic. The email generated with HTML in it. I did not send the emails (cause I don't have email sending turned on) and I don't read the server logs or anything. Nicole
Created attachment 5240 [details] [review] proposed patch
Created attachment 5241 [details] [review] proposed patch follow-up This follow-up must be applied after the other patch. Both are rebased against today MASTER. Tested, the only glitch is no query in themelanguage at /home/paul/koha.dev/koha-community//C4/Output.pm line 146. display when launching the command that can't be avoided because the script is not a CGI one, so no cookie/no themelanguage Please test & sign-off or comment
Created attachment 6251 [details] Alternative patch-set to send html emails Found this bug after just having patched our installation at library.pbc.wa.edu.au to send html notices. (The same issue, addressed in slightly different ways) If it's of any use, I've attached the diff of what we use to send html emails.... If this is simply noise, please let me know!
Created attachment 6252 [details] SQL required for the previous patch-set This SQL is required for the previous patch-set used at Perth Bible College.
Ok, I am confused here - perhaps someone can clarify: why is this failed qa and what is the relation to 5878 (that I spent a lot of time testing on)?
(In reply to comment #9) > Ok, I am confused here - perhaps someone can clarify: why is this failed qa and > what is the relation to 5878 (that I spent a lot of time testing on)? Reading the bug history, I can't explain why I've set failed QA. I feel I made the change without noticing it. Switching back to need signoff. Nicholas : could you confirm your alternative patch & SQL required are both an alternative option ? In this case i'll mark them as obsolete & continue with my initial patches
Hi Paul, Yes - they're an alternative option. (A quick solution I threw together so that our library could send nice-looking email reminders out! - I'm more than happy for an official solution - this was just a "hey - this is how we've solved it" when I noticed that the problem was being worked on....)
Follow up does not apply: [~/kohaclone (6030_htmloverdues)⚡]> git am -iu3 /tmp/proposed-patch-follow-up-jH3H5e.patch Commit Body is: -------------------------- Bug 6030 follow-up Fixing problem detected previously: * add column_def retrieval, to have header displaying the "explained" row name, not the table name. (the name is not expanded for CSV exports) * removed some warnings. * fixed some problems with csvfilename being defined/empty. Now, csvfilename is always defined, and set to '' by default. Some tests have been updated accordingly * added the charset=UTF8 on mail headers. This patch also adds some missing columns to columns.def -------------------------- Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y Applying: Bug 6030 follow-up fatal: sha1 information is lacking or useless (misc/cronjobs/overdue_notices.pl). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 6030 follow-up When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort"
Paul, can you clear up the relationship to bug 5878? - Can this bug be closed as features will be implemented incrementally by other bugs/patches?
*** Bug 5878 has been marked as a duplicate of this bug. ***
Created attachment 12110 [details] [review] Bug 6030 follow-up Fixing problem detected previously: * add column_def retrieval, to have header displaying the "explained" row name, not the table name. (the name is not expanded for CSV exports) * removed some warnings. * fixed some problems with csvfilename being defined/empty. Now, csvfilename is always defined, and set to '' by default. Some tests have been updated accordingly * added the charset=UTF8 on mail headers. This patch also adds some missing columns to columns.def Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com>
Created attachment 12111 [details] [review] Bug 6030 followup : add a condition to use getlanguage in bath mode
Created attachment 12112 [details] [review] Bug 6030 followup : Specify "biblio.title" as a column name to differentiate from borrowers.title
Created attachment 12113 [details] [review] Bug 6030 Allow html content in letters Squashed commit of the following: commit 8368e5de723ec7607402be341975014c8b7b0ea9 Author: Henri-Damien LAURENT <henridamien.laurent@biblibre.com> Date: Tue Sep 21 14:41:07 2010 +0200 Bug 6030 : overdue_notices :Removing warning Two extra warnings were issued when using html commit 5984fc92772af68c51f7f5b5450fd5b27d5b6d37 Author: Henri-Damien LAURENT <henridamien.laurent@biblibre.com> Date: Tue Sep 21 12:27:29 2010 +0200 Bug 6030 : Letters can be html in overdu_notices.pl Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com>
I have signed off the first 2 patches and added 2 small follow-up : * the first one is need because when $query is not defined, themelanguage returns an error * the seconf one because in french we need to differentiate borrowers.title and biblio.title (it's not the same translation) I don't know why the main path is the fourth file sended : in consequence, to test, you should apply 4th patch first and then 1, 2 and 3
Created attachment 12231 [details] [review] Bug 6030 followup : Specify "biblio.title" as a column name to differentiate from borrowers.title
Created attachment 12232 [details] [review] Bug 6030 Allow html content in letters Squashed commit of the following: commit 8368e5de723ec7607402be341975014c8b7b0ea9 Author: Henri-Damien LAURENT <henridamien.laurent@biblibre.com> Date: Tue Sep 21 14:41:07 2010 +0200 Bug 6030 : overdue_notices :Removing warning Two extra warnings were issued when using html commit 5984fc92772af68c51f7f5b5450fd5b27d5b6d37 Author: Henri-Damien LAURENT <henridamien.laurent@biblibre.com> Date: Tue Sep 21 12:27:29 2010 +0200 Bug 6030 : Letters can be html in overdu_notices.pl Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com>
Created attachment 12234 [details] [review] Bug 6030 followup : add a condition to use getlanguage in bath mode
Created attachment 12235 [details] [review] Bug 6030 follow-up Fixing problem detected previously: * add column_def retrieval, to have header displaying the "explained" row name, not the table name. (the name is not expanded for CSV exports) * removed some warnings. * fixed some problems with csvfilename being defined/empty. Now, csvfilename is always defined, and set to '' by default. Some tests have been updated accordingly * added the charset=UTF8 on mail headers. This patch also adds some missing columns to columns.def Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com>
> Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com> i rebased these 4 patches to apply to master patches looks good, passing QA… nice work Sophie! $ koha-qa.pl -c 4 testing 4 commit(s) (applied to commit 0acebb8) * e34fa8f Bug 6030 followup : add a condition to use getlanguage in bath mo C4/Templates.pm * cb90e51 Bug 6030 follow-up koha-tmpl/intranet-tmpl/prog/en/columns.def misc/cronjobs/overdue_notices.pl * a346838 Bug 6030 followup : Specify "biblio.title" as a column name to di misc/cronjobs/overdue_notices.pl * 8ff05be Bug 6030 Allow html content in letters misc/cronjobs/overdue_notices.pl * C4/Templates.pm OK * misc/cronjobs/overdue_notices.pl OK
Running the script says: DBD::mysql::st execute failed: Unknown column '$date' in 'field list' at ./overdue_notices.pl line 505. DBD::mysql::st fetchrow_hashref failed: fetch() without execute() at ./overdue_notices.pl line 518. Investigating a little, show that line 505 is running $sth2, that contains: my $sth2 = $dbh->prepare( <<'END_SQL' ); SELECT biblio.title as "biblio.title", biblio.*, items.*, issues.*, biblioitems.itemtype, TO_DAYS($date)-TO_DAYS(date_due) AS days_overdue FROM issues,items,biblio, biblioitems WHERE items.itemnumber=issues.itemnumber AND biblio.biblionumber = items.biblionumber AND biblio.biblionumber = biblioitems.biblionumber AND issues.borrowernumber = ? AND TO_DAYS($date)-TO_DAYS(date_due) BETWEEN ? and ? END_SQL the line: AND TO_DAYS($date)-TO_DAYS(date_due) BETWEEN ? and ? is heavily wrong ! The signoff was probably right when it was made, but time passed, things were updated, and now it's invalid !
Created attachment 12397 [details] [review] SIGNED-OFF-Bug-6030-followup-Specify-biblio.title-as.patch restored single quotes around END_SQL to be double quotes and $date error is gone. wajasu
The Bug 6030 follow-up needs to be adjusted so the patch will apply.
(In reply to comment #27) > The > Bug 6030 follow-up > needs to be adjusted so the patch will apply. I'm not sure I understand which follow-up needs to be adjusted?
The third one, is the one I believe I had problems applying. attachment 12235 [details] [review]
As far as I can tell, attachments 12232, 12235 and 12397 have signoffs, but not 12234. Having a unified signoff on all of them would probably be good. I just did "git bz apply 6030" on current master and all patches applied, so I'm resetting to "Needs signoff", but I wont have time to do a signoff now myself, so please feel free... :-)
Created attachment 12698 [details] [review] [SIGNED-OFF] Bug 6030 Allow html content in letters Squashed commit of the following: commit 8368e5de723ec7607402be341975014c8b7b0ea9 Author: Henri-Damien LAURENT <henridamien.laurent@biblibre.com> Date: Tue Sep 21 14:41:07 2010 +0200 Bug 6030 : overdue_notices :Removing warning Two extra warnings were issued when using html commit 5984fc92772af68c51f7f5b5450fd5b27d5b6d37 Author: Henri-Damien LAURENT <henridamien.laurent@biblibre.com> Date: Tue Sep 21 12:27:29 2010 +0200 Bug 6030 : Letters can be html in overdu_notices.pl Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 12699 [details] [review] [SIGNED-OFF] Bug 6030 followup : add a condition to use getlanguage in bath mode Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 12700 [details] [review] [SIGNED-OFF] Bug 6030 follow-up Fixing problem detected previously: * add column_def retrieval, to have header displaying the "explained" row name, not the table name. (the name is not expanded for CSV exports) * removed some warnings. * fixed some problems with csvfilename being defined/empty. Now, csvfilename is always defined, and set to '' by default. Some tests have been updated accordingly * added the charset=UTF8 on mail headers. This patch also adds some missing columns to columns.def Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 12701 [details] [review] [SIGNED-OFF] Bug 6030 followup : Specify "biblio.title" as a column name to differentiate from borrowers.title Signed-off-by: wajasu <matted-34813@mypacks.net> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(In reply to comment #34) > Created attachment 12701 [details] [review] > [SIGNED-OFF] Bug 6030 followup : Specify "biblio.title" as a column name to > differentiate from borrowers.title > > Signed-off-by: wajasu <matted-34813@mypacks.net> > Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> 4 patches look good, passing QA on this... $ koha-qa.pl -c 4 testing 4 commit(s) (applied to commit 37340e3) * 4403c59 Bug 6030 followup : Specify "biblio.title" as a column name to differentiate from borrowers.title misc/cronjobs/overdue_notices.pl * a9a93a7 Bug 6030 follow-up koha-tmpl/intranet-tmpl/prog/en/columns.def misc/cronjobs/overdue_notices.pl * 5fc93ff Bug 6030 followup : add a condition to use getlanguage in bath mode C4/Templates.pm * 437886a Bug 6030 Allow html content in letters misc/cronjobs/overdue_notices.pl * misc/cronjobs/overdue_notices.pl OK * C4/Templates.pm OK
Created attachment 12772 [details] [review] Bug 6030 follow-up fix SQL error The <<'END_SQL' doesn't expand variables. So $date stay as $date, resulting in a nasty error DBD::mysql::st execute failed: Unknown column '$date' in 'field list' at misc/cronjobs/overdue_notices.pl line 505.
I don't think this patch is just an ENH, sor RMaint, your choice, but this patch should be OK in 3.8 imo Examining possible side effect before pushing: * The patch contains: - my $titles = ""; + my $titles; => I think it's a bad idea. If you have the html parameter, title will be filled by if ($htmlfilename) { $titles = "<table id='itemscontent$borrowernumber'>"; but, if you don't have html, it will stay empty, and the line $titles .= join("\t", @item_info) . "\n"; will issue an error I've reintroduced the = ''; * the patch add: $letter->{'content-type'}="text/".($htmlfilename?"html":"plain")."; charset=UTF8"; => having a content-type is always ++ and it's the correct one I think if we don't have html * testing the script: - I get a bad DBD::mysql::st execute failed: Unknown column '$date' in 'field list' at misc/cronjobs/overdue_notices.pl line 505. I've attached a follow-up 2nd try = I can't get anything in the generated file. Running the script misc/cronjobs/overdue_notices.pl --html=test-html -v tells me that I've one patron late: returns 1 rows at misc/cronjobs/overdue_notices.pl line 490. borrower firstname, TEST (7852) has items triggering level 1. at misc/cronjobs/overdue_notices.pl line 496. STH2 PARAMS: borrowernumber = 7852, mindays = 1, maxdays = 90 returns 1 rows at misc/cronjobs/overdue_notices.pl line 506. but the generated HTML is empty: html> <head> <style type='text/css'> pre {page-break-after: always;} pre {white-space: pre-wrap;} pre {white-space: -moz-pre-wrap;} pre {white-space: -o-pre-wrap;} pre {word-wrap: break-work;} </style> </head> <body> </body> </html> The notice defined for this patron is ODUE, i've checked the "html" on tools/letters.pl so... any idea why I can't make it work welcomed. I also think a squashed patch (with an updated git patch comment) would be more than welcomed. Those 5 patches are looking poor !
Created attachment 13017 [details] [review] Bug 6030 Allow html content in letters Squashed commit of the following: commit 8368e5de723ec7607402be341975014c8b7b0ea9 Author: Henri-Damien LAURENT <henridamien.laurent@biblibre.com> Date: Tue Sep 21 14:41:07 2010 +0200 Bug 6030 : overdue_notices :Removing warning Two extra warnings were issued when using html commit 5984fc92772af68c51f7f5b5450fd5b27d5b6d37 Author: Henri-Damien LAURENT <henridamien.laurent@biblibre.com> Date: Tue Sep 21 12:27:29 2010 +0200 Bug 6030 : Letters can be html in overdu_notices.pl Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com> Bug 6030 follow-up Fixing problem detected previously: * add column_def retrieval, to have header displaying the "explained" row name, not the table name. (the name is not expanded for CSV exports) * removed some warnings. * fixed some problems with csvfilename being defined/empty. Now, csvfilename is always defined, and set to '' by default. Some tests have been updated accordingly * added the charset=UTF8 on mail headers. This patch also adds some missing columns to columns.def Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com> Bug 6030 followup : add a condition to use getlanguage in bath mode Bug 6030 followup : Specify "biblio.title" as a column name to differentiate from borrowers.title Conflicts: misc/cronjobs/overdue_notices.pl
Created attachment 13018 [details] [review] Bug 6030 [3.8.x] Allow html content in letters Squashed commit of the following: commit 8368e5de723ec7607402be341975014c8b7b0ea9 Author: Henri-Damien LAURENT <henridamien.laurent@biblibre.com> Date: Tue Sep 21 14:41:07 2010 +0200 Bug 6030 : overdue_notices :Removing warning Two extra warnings were issued when using html commit 5984fc92772af68c51f7f5b5450fd5b27d5b6d37 Author: Henri-Damien LAURENT <henridamien.laurent@biblibre.com> Date: Tue Sep 21 12:27:29 2010 +0200 Bug 6030 : Letters can be html in overdu_notices.pl Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com> Bug 6030 follow-up Fixing problem detected previously: * add column_def retrieval, to have header displaying the "explained" row name, not the table name. (the name is not expanded for CSV exports) * removed some warnings. * fixed some problems with csvfilename being defined/empty. Now, csvfilename is always defined, and set to '' by default. Some tests have been updated accordingly * added the charset=UTF8 on mail headers. This patch also adds some missing columns to columns.def Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com> Bug 6030 followup : add a condition to use getlanguage in bath mode Bug 6030 followup : Specify "biblio.title" as a column name to differentiate from borrowers.title Signed-off-by: wajasu <matted-34813@mypacks.net> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
I have squashed all the followup in one patch en send a specific pathc for 3.8.x Each patch was signed-off individually, but maybe the global patch should be validated again.
I have changed the status of this ticket. This is not an enhancement, but a bugfix. The feature to create html file exist in overdue_notices.pl but it does not work withour this patch.
Hi Sophie, can you provide a test plan for this? I am not sure what this patch includes as it seems to do a lot of things.
Hi Sophie, taking a look at your patch. First thing I notice is that it also seems to touch the CSV export option. So this needs to be retested as well? I am worried about using the guided reports .pref files for the items table as those are known to not be translatable with the scripts. Also wondering if we really want to output a table. I think this should be optional as it's a bigger change in behaviour. This seems to be the old output: $return = "<pre>\n"; $return .= "$params->{'letter'}->{'content'}\n"; $return .= "\n</pre>\n"; I am also wondering about the syntaxes in the overdue letter to be used. Does it work with both <<items.content>> and the <item><fine></fine></item> syntax? Hope I didn't misread.
Katrin, Concerning table headers, we have defined a french .pref file that we copy in french directory each time we execute a translation. I agree it's not so good, but we need to have french headers for our customers. Do you have any better suggestion ? Else for the test plan, without the patch, if you use overdue_notices.pl with -html parameter, html file generated doesn't contain any HTML tags between <pre> and </pre>, so the .pdf generated misc/cronjobs/printoverdues.sh contains only plain text without layout. If you apply the patch, then html file contains html tags and pdf file can then be generated with a nice layout. As far as csv parameter is concerned, I dedn't change anything from proposed patches that where signed off and that I squashed. But I asssume it should be better to test that nothing is broken.
Created attachment 14128 [details] [review] Bug 6030 Allow html content in letters Squashed commit of the following: commit 8368e5de723ec7607402be341975014c8b7b0ea9 Author: Henri-Damien LAURENT <henridamien.laurent@biblibre.com> Date: Tue Sep 21 14:41:07 2010 +0200 Bug 6030 : overdue_notices :Removing warning Two extra warnings were issued when using html commit 5984fc92772af68c51f7f5b5450fd5b27d5b6d37 Author: Henri-Damien LAURENT <henridamien.laurent@biblibre.com> Date: Tue Sep 21 12:27:29 2010 +0200 Bug 6030 : Letters can be html in overdu_notices.pl Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com> Bug 6030 follow-up Fixing problem detected previously: * add column_def retrieval, to have header displaying the "explained" row name, not the table name. (the name is not expanded for CSV exports) * removed some warnings. * fixed some problems with csvfilename being defined/empty. Now, csvfilename is always defined, and set to '' by default. Some tests have been updated accordingly * added the charset=UTF8 on mail headers. This patch also adds some missing columns to columns.def Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com> Bug 6030 followup : add a condition to use getlanguage in bath mode Bug 6030 followup : Specify "biblio.title" as a column name to differentiate from borrowers.title Conflicts: misc/cronjobs/overdue_notices.pl Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 14129 [details] [review] [SIGNED-OFF] Bug 6030 Allow html content in letters Squashed commit of the following: commit 8368e5de723ec7607402be341975014c8b7b0ea9 Author: Henri-Damien LAURENT <henridamien.laurent@biblibre.com> Date: Tue Sep 21 14:41:07 2010 +0200 Bug 6030 : overdue_notices :Removing warning Two extra warnings were issued when using html commit 5984fc92772af68c51f7f5b5450fd5b27d5b6d37 Author: Henri-Damien LAURENT <henridamien.laurent@biblibre.com> Date: Tue Sep 21 12:27:29 2010 +0200 Bug 6030 : Letters can be html in overdu_notices.pl Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com> Bug 6030 follow-up Fixing problem detected previously: * add column_def retrieval, to have header displaying the "explained" row name, not the table name. (the name is not expanded for CSV exports) * removed some warnings. * fixed some problems with csvfilename being defined/empty. Now, csvfilename is always defined, and set to '' by default. Some tests have been updated accordingly * added the charset=UTF8 on mail headers. This patch also adds some missing columns to columns.def Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com> Bug 6030 followup : add a condition to use getlanguage in bath mode Bug 6030 followup : Specify "biblio.title" as a column name to differentiate from borrowers.title Conflicts: misc/cronjobs/overdue_notices.pl Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
I think this needs to move forward to QA for the patch for master. There is little sense in having this held back by the lack of an SO for the 3.8.x versioned patch.
I am entirely perplexed about what this patch is supposed to do and why it is marked critical. Based on the information on this report this sounds like at most a "minor" bug. Also, while I have this bully pulpit, could one of the original authors address Katrin's question about the CSV export changes?
Hi Jared, Without this patch, overdues notices are not correctly generated in html, so pdf file could not be produced. It is critical for our clients because they used to print overdue letters, and they can't do it anymore when upgrading to 3.8. There were several patches and follow-up provided. I only provide correction for the remaining problems and sqashed the already signed off patches. So concerning Katrin question about csv, modifications where in those primitive patches. Tell me if I should begin again from scratch an propose a new patch only dealing with html. Our librairies really need this funcitonality.
(In reply to comment #49) > Without this patch, overdues notices are not correctly generated in html, so > pdf file could not be produced. > It is critical for our clients because they used to print overdue letters, > and they can't do it anymore when upgrading to 3.8. So this is a feature that used to work? > There were several patches and follow-up provided. I only provide correction > for the remaining problems and sqashed the already signed off patches. > So concerning Katrin question about csv, modifications where in those > primitive patches. I guess hdl is the original author, and therefore no longer available to comment? I looked at the code, and it looks like the CSV-related changes may have been simply to quiet warnings, in which case, since it is an old patch, I have no objection to it remaining in place, provided the CSV export is tested.
After I applied this patch on my 3.10.02 setup, my overdue notices go out as plain text, but still with the HTML markup applied. After opening the email and changing: Content-Type: text/plain; charset=UTF8 to Content-Type: text/html; charset=UTF8 It displays perfectly! Also, and I'm not sure if this is related, but overdue notices also seem not to respect the KohaAdminEmailAddress preference, instead using the default library email address.
Derrin, if you use -html option, you have to check html on you notice template in order your email get the right content type. And KohaAdminEmailAddress is only used if Library email address is not set. This was the previous way to send overdues and this patch does not change that.
(In reply to comment #52) > Derrin, if you use -html option, you have to check html on you notice > template in order your email get the right content type. > > And KohaAdminEmailAddress is only used if Library email address is not set. > This was the previous way to send overdues and this patch does not change > that. This is without the -html option. I'm not outputting them to a file, which the -html option is for; I'm making them to email.
I will try to test this, but a step by step test plan would be a great help in making this bug move.
Several conflicts in overdue_notices.pl
Some email clients doesn't support UTF-8 encoding, should the charset be a system preference ? (emailCharset or emailEncoding)
I'm a big lost. Is this bug for 3.8.x or for master ? Mathieu
Where is this up to? Its marked 'Does not apply'. If it can be re-based, I'm up for testing it.
As this patch seemed difficult to sign, I have made several small patches, each dealing with one problem (list in "See also") I will mark this patch as resolved when all those patches will be pushed
Comment on attachment 13018 [details] [review] Bug 6030 [3.8.x] Allow html content in letters Review of attachment 13018 [details] [review]: ----------------------------------------------------------------- ::: misc/cronjobs/overdue_notices.pl @@ +344,4 @@ > > our $csv; # the Text::CSV_XS object > our $csv_fh; # the filehandle to the CSV file. > +if ( $csvfilename ) { No, because the file name could be "0". Old code is better. @@ +563,4 @@ > email => $email, > itemcount => $itemcount, > titles => $titles, > + outputformat => $csvfilename ? 'csv' : defined $htmlfilename ? 'html' : '', This nested mess should be made clearer elsewhere. And no, defined $csvfilename should be used. @@ +591,4 @@ > email => $email, > itemcount => $itemcount, > titles => $titles, > + outputformat => $csvfilename ? 'csv' : defined $htmlfilename ? 'html' : '', This nested mess should be made clearer elsewhere. And no, defined $csvfilename should be used. @@ +602,4 @@ > } > > if (@output_chunks) { > + if ( $csvfilename ) { This nested mess should be made clearer elsewhere. And no, defined $csvfilename should be used.
Looking at critical bugs I noticed this one - does the problem still exist and can we get the patch back into the queue?
As I said in comment #59 (http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6030#c59), this bug was splitted in many small one. It seems that all attached bugs are pushed except Bug 11173 (that won't apparently). So I think it could be closed.
Hi Sophie, thx for clarifying! I am closing this then.