| Summary: | Warnings in overdue_notices.pl | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Katrin Fischer <katrin.fischer> |
| Component: | Command-line Utilities | Assignee: | Hayley Pelham <hayleypelham> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | minor | ||
| Priority: | P5 - low | CC: | hayleypelham, jonathan.druart, martin.renvoize, nick, robin, wizzyrea |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
19.05.00, 18.11.06
|
|
| Circulation function: | |||
| Attachments: |
Bug 20537: Added check to remove warning from overdue_notices.pl
Bug 20537: Added checks to remove warning from overdue_notices.pl Bug 20537: Added checks to remove warning from overdue_notices.pl Bug 20537: Added checks to remove warning from overdue_notices.pl |
||
(In reply to Katrin Fischer from comment #0) > Should we check here if firstname/surname are defined? yes Created attachment 86124 [details] [review] Bug 20537: Added check to remove warning from overdue_notices.pl When executing overdue_notices.pl on borrowers that lack a surname, we see this error: Use of uninitialized value in concatenation (.) or string at /usr/share/koha/bin/cronjobs/overdue_notices.pl line 575. This patch fixes this issue by setting the $borr variable based on the information that has been defined. To test: 1) Create a borrower and set its surname to null 2) Checkout an item to the borrower and set it to be overdue 3) Navigate to kohaclone/misc/cronjobs and enter a koha-shell 4) Run the script: ./overdue_notices.pl 5) Observe the error appears 6) Apply the patch 7) Repeat steps 3-4 8) Observe the error is gone 9) Sign off! Hi Hayley,
It will work, but I am wondering if something like that would not be more readable:
my $borr = sprintf( "%s%s%s (%s)",
$data->{surname} || '',
$data->{firstname} && $data->{surname} ? ', ' : '',
$data->{firstname} || '',
$borrowernumber );
What do you think?
(In reply to Jonathan Druart from comment #3) > Hi Hayley, > > It will work, but I am wondering if something like that would not be more > readable: > > my $borr = sprintf( "%s%s%s (%s)", > $data->{surname} || '', > $data->{firstname} && $data->{surname} ? ', ' : '', > $data->{firstname} || '', > $borrowernumber ); > > What do you think? Hi Jonathan, I'm happy to implement your solution, however it looks like maybe this error is not present on master anymore. Would you mind confirming this? The code is the same in master so I bet the warning is still there. Created attachment 89368 [details] [review] Bug 20537: Added checks to remove warning from overdue_notices.pl When executing overdue_notices.pl on borrowers that lack a surname, we see this error: Use of uninitialized value in concatenation (.) or string at /usr/share/koha/bin/cronjobs/overdue_notices.pl line 575. This patch fixes this issue by setting the $borr variable based on the information that has been defined. To test: 1) Create a borrower and set its surname to null 2) Checkout an item to the borrower and set it to be overdue 3) Navigate to kohaclone/misc/cronjobs and enter a koha-shell 4) Run the script: ./overdue_notices.pl 5) Observe the error appears 6) Apply the patch 7) Repeat steps 3-4 8) Observe the error is gone 9) Sign off! Sponsored-by: Catalyst IT Hayley is right, the behaviour doesn't seem to appear in current master, even though that code is the same. I think we can call this one fixed? Cheers, Liz Actually, I was wrong and we do see this - I didn't have a rule defined for overdue notices for the borrower I was looking at. Bum. So, having another look. :) Liz Created attachment 90085 [details] [review] Bug 20537: Added checks to remove warning from overdue_notices.pl When executing overdue_notices.pl on borrowers that lack a surname, we see this error: Use of uninitialized value in concatenation (.) or string at /usr/share/koha/bin/cronjobs/overdue_notices.pl line 575. This patch fixes this issue by setting the $borr variable based on the information that has been defined. To test: 1) Create a borrower and set its surname to null 2) Checkout an item to the borrower and set it to be overdue 3) Navigate to kohaclone/misc/cronjobs and enter a koha-shell 4) Run the script: ./overdue_notices.pl 5) Observe the error appears 6) Apply the patch 7) Repeat steps 3-4 8) Observe the error is gone 9) Sign off! Sponsored-by: Catalyst IT Signed-off-by: Liz Rea <wizzyrea@gmail.com> patch wouldn't apply on current master, so I fixed that. Work is the same. Created attachment 90086 [details] [review] Bug 20537: Added checks to remove warning from overdue_notices.pl When executing overdue_notices.pl on borrowers that lack a surname, we see this error: Use of uninitialized value in concatenation (.) or string at /usr/share/koha/bin/cronjobs/overdue_notices.pl line 575. This patch fixes this issue by setting the $borr variable based on the information that has been defined. To test: 1) Create a borrower and set its surname to null 2) Checkout an item to the borrower and set it to be overdue 3) Navigate to kohaclone/misc/cronjobs and enter a koha-shell 4) Run the script: ./overdue_notices.pl 5) Observe the error appears 6) Apply the patch 7) Repeat steps 3-4 8) Observe the error is gone 9) Sign off! Sponsored-by: Catalyst IT Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Awesome work all! Pushed to master for 19.05 Pushed to 18.11.x for 18.11.06 |
We've seen a warning from overdue_notices.pl: Use of uninitialized value in concatenation (.) or string at /usr/share/koha/bin/cronjobs/overdue_notices.pl line 575. I checked in our version and in master and think it happens for users withut a surname (organisations usually): 576 $borrowernumber = $data->{'borrowernumber'}; 577 my $borr = 578 $data->{'firstname'} . ', ' 579 . $data->{'surname'} . ' (' 580 . $borrowernumber . ')'; 581 $verbose Should we check here if firstname/surname are defined?