When you check-in some books you get the following warnings to error logs: > [WARN] Use of uninitialized value in concatenation (.) or string at /usr/share/koha/intranet/cgi-bin/circ/returns.pl line 164. > [WARN] Use of uninitialized value in concatenation (.) or string at /usr/share/koha/intranet/cgi-bin/circ/returns.pl line 402. This seems to be due to treating the patron fields as non-null values: > 164 my $name = $patron ? $patron->surname . ", " . $patron->title . " " . $patron->firstname : ''; The DB schema allows NULL values (and I think that is okay) so we should check for every one of these whether the value is defined or not before using it.
The variable is no longer used in the template, it has been removed by: commit 76eb3a0549efe4c1f1557071a6cb4b33d4bd3e4d Bug 18789: (follow-up) Pass a Koha::Patron object from returns.pl - <td>[% name %]</td> + <td>[% INCLUDE 'patron-title.inc' patron=patron %]</td> We can simply remove the $name variable from the controller script.
Created attachment 115291 [details] [review] Bug 27011 removed $name variable Test Plan 1. Check out a book 2. Check in a book 3. Note what happens 4. Apply patch 5. Repeat step 1-2 6. Note that it doesn't break and everything still works
Created attachment 115301 [details] [review] Bug 27011: Remove unused $name variable The variable is no longer used in the template, it has been removed by: commit 76eb3a0549efe4c1f1557071a6cb4b33d4bd3e4d Bug 18789: (follow-up) Pass a Koha::Patron object from returns.pl - <td>[% name %]</td> + <td>[% INCLUDE 'patron-title.inc' patron=patron %]</td> We can simply remove the $name variable from the controller script. Test Plan: 1. Check out a book 2. Check in a book 3. Note what happens 4. Apply patch 5. Repeat step 1-2 6. Note that it doesn't break and everything still works JD Amended patch: fix commit message
I may be testing the wrong way, but I don't see the errors in any of the logs when checking out and checking in a book (using koha-testing-docker). Everything works as expected without or with the patch applied.
(In reply to David Nind from comment #4) > I may be testing the wrong way, but I don't see the errors in any of the > logs when checking out and checking in a book (using koha-testing-docker). > > Everything works as expected without or with the patch applied. Based on the code I think the warning only triggers when there is reserve attached to the item.
Created attachment 115319 [details] [review] Bug 27011: Remove unused $name variable The variable is no longer used in the template, it has been removed by: commit 76eb3a0549efe4c1f1557071a6cb4b33d4bd3e4d Bug 18789: (follow-up) Pass a Koha::Patron object from returns.pl - <td>[% name %]</td> + <td>[% INCLUDE 'patron-title.inc' patron=patron %]</td> We can simply remove the $name variable from the controller script. Test Plan: 1. Check out a book 2. Check in a book 3. Note what happens 4. Apply patch 5. Repeat step 1-2 6. Note that it doesn't break and everything still works JD Amended patch: fix commit message Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Slightly updated test plan: 1 - have a patron without a title 2 - place a hold on an item for that patron 3 - check item in 4 - see message in error log 5 - apply patch, restart services 6 - repeat 1-3, no error generated
(In reply to Joonas Kylmälä from comment #5) > Based on the code I think the warning only triggers when there is reserve > attached to the item. Thanks Joonas for clarifying!
Created attachment 115523 [details] [review] Bug 27011: Remove unused $name variable The variable is no longer used in the template, it has been removed by: commit 76eb3a0549efe4c1f1557071a6cb4b33d4bd3e4d Bug 18789: (follow-up) Pass a Koha::Patron object from returns.pl - <td>[% name %]</td> + <td>[% INCLUDE 'patron-title.inc' patron=patron %]</td> We can simply remove the $name variable from the controller script. Test Plan: 1. Check out a book 2. Check in a book 3. Note what happens 4. Apply patch 5. Repeat step 1-2 6. Note that it doesn't break and everything still works JD Amended patch: fix commit message Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Excellent work, Passing QA
Pushed to master for 21.05, thanks to everybody involved!
Pushed to 20.11.x for 20.11.03
Pushed to 20.05.x for 20.05.09
Backported: Pushed to 19.11.x branch for 19.11.15