In our installation 20.11.17 it's these lines: if (%$substitute) { while ( my ($token, $val) = each %$substitute ) { if ( $token eq 'items.content' ) { $val =~ s|\n|<br/>|g if $letter->{is_html}; } --> $letter->{title} =~ s/<<$token>>/$val/g; $letter->{content} =~ s/<<$token>>/$val/g; } } Itemscontent is used as: koha-foreach --chdir --enabled --email /usr/share/koha/bin/cronjobs/overdue_notices.pl -t -itemscontent date_due,title,subtitle,part_number,part_name,author,itemcallnumber,barcode koha-foreach --chdir --enabled --email /usr/share/koha/bin/cronjobs/advance_notices.pl -c --itemscontent date_due,title,subtitle,part_number,part_name,author,itemcallnumber,barcode This creates quite a bit of noise from the cronjobs, so it would be nice to get a hint on how to fix this or a patch. In master the code in question is unchanged.
That could be fixed with the following change. diff --git a/C4/Letters.pm b/C4/Letters.pm index 9ed1de0e44f..8ab0bb727e0 100644 --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -599,6 +599,7 @@ sub GetPreparedLetter { if (%$substitute) { while ( my ($token, $val) = each %$substitute ) { + $val //= q{}; if ( $token eq 'items.content' ) { $val =~ s|\n|<br/>|g if $letter->{is_html}; }
Created attachment 135177 [details] [review] Bug 30781: Add unit test for substitute with undefined value Add unit test that generates warn. This may be usefull as no regression test. Run prove t/db_dependent/Letters.t You see warn : t/db_dependent/Letters.t .. 1/84 Use of uninitialized value $val in substitution iterator at /kohadevbox/koha/C4/Letters.pm line 607.
Created attachment 135178 [details] [review] Bug 30781: Fix warning in GetPreparedLetter Test by running : prove t/db_dependent/Letters.t Check you dont see warning : Use of uninitialized value $val in substitution iterator
To test that a subroutine call does not produce a warning you should use warning_is.
(In reply to Jonathan Druart from comment #4) > To test that a subroutine call does not produce a warning you should use > warning_is. Ah OK. I keep the test on the build of letter content and add a new for the warning.
Created attachment 135192 [details] [review] Bug 30781: Add unit test for substitute with undefined value Add unit test that generates warn. This may be usefull as no regression test. Run prove t/db_dependent/Letters.t You see warn : t/db_dependent/Letters.t .. 1/84 Use of uninitialized value $val in substitution iterator at /kohadevbox/koha/C4/Letters.pm line 607.
Created attachment 135193 [details] [review] Bug 30781: Fix warning in GetPreparedLetter Test by running : prove t/db_dependent/Letters.t Check you dont see warning : Use of uninitialized value $val in substitution iterator
Created attachment 135219 [details] [review] Bug 30781: Add unit test for substitute with undefined value Add unit test that generates warn. This may be usefull as no regression test. Run prove t/db_dependent/Letters.t You see warn : t/db_dependent/Letters.t .. 1/84 Use of uninitialized value $val in substitution iterator at /kohadevbox/koha/C4/Letters.pm line 607. Signed-off-by: David Nind <david@davidnind.com>
Created attachment 135220 [details] [review] Bug 30781: Fix warning in GetPreparedLetter Test by running : prove t/db_dependent/Letters.t Check you dont see warning : Use of uninitialized value $val in substitution iterator Signed-off-by: David Nind <david@davidnind.com>
Created attachment 135222 [details] [review] Bug 30781: Fix warning in GetPreparedLetter Test by running : prove t/db_dependent/Letters.t Check you dont see warning : Use of uninitialized value $val in substitution iterator Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
QAing. Not for this release
(In reply to Marcel de Rooy from comment #11) > QAing. Not for this release Too late
I wanted to add this remark: We could have a theoretical debate if you should allow the substitution if you pass an undefined value. What we do now, is formalize what already happened but silence the warning. But we could also choose to skip this substitution since you should not pass undefined substitution parameters.
Pushed to master for 22.11. Nice work everyone, thanks!
Pushed to 22.05.x for 22.05.01
thx, pushed to 21.11.x for 21.11.07
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed. Nothing to document, marking resolved.
I've ported this to 20.11 as it creates a lot of noise, might be nice in 21.05 as well.
Backported: Pushed to 21.05.x branch for 21.05.16