Bug 34532 - Silence warns in Patroncard.pm when layout values are empty
Summary: Silence warns in Patroncard.pm when layout values are empty
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Label/patron card printing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Lucas Gass (lukeg)
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-08-14 21:10 UTC by Lucas Gass (lukeg)
Modified: 2024-07-04 20:40 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.04,22.11.11
Circulation function:


Attachments
Bug 34532: Silence warns in Patroncard.pm (5.38 KB, patch)
2023-08-14 21:13 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 34532: Silence warns in Patroncard.pm (4.46 KB, patch)
2023-08-14 21:15 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 34532: Silence warns in Patroncard.pm (4.55 KB, patch)
2023-08-21 14:34 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 34532: Silence warns in Patroncard.pm (9.17 KB, patch)
2023-09-01 13:37 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 34532: (follow-up) Fix typos llx => lly (1.38 KB, patch)
2023-09-14 18:54 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 34532: Silence warns in Patroncard.pm (9.27 KB, patch)
2023-09-15 06:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Gass (lukeg) 2023-08-14 21:10:23 UTC
To recreate: 

1. Go to patron card creator and make a patron card batch, card layout, and card template.
2. In the card layout leave some values blank for Lower left X coordinate and Lower left Y coordinate.
3. Now go create a card batch while tailing the plack-intranet-error log/
4. Notice in the logs:

[2023/08/14 20:50:54] [WARN] Argument "" isn't numeric in multiplication (*) at /kohadevbox/koha/C4/Patroncards/Patroncard.pm line 109.
[2023/08/14 20:50:54] [WARN] Argument "" isn't numeric in multiplication (*) at /kohadevbox/koha/C4/Patroncards/Patroncard.pm line 248.
[2023/08/14 20:50:54] [WARN] Argument "" isn't numeric in multiplication (*) at /kohadevbox/koha/C4/Patroncards/Patroncard.pm line 249.


The more patrons in a given batch the more of these errors you will see. 


If no llx or lly value is given we should assume it is 0.
Comment 1 Lucas Gass (lukeg) 2023-08-14 21:13:50 UTC
Created attachment 154415 [details] [review]
Bug 34532: Silence warns in Patroncard.pm

To test:

1. Go to patron card creator and make a patron card batch, card layout, and card template.
2. In the card layout leave some values blank for Lower left X coordinate and Lower left Y coordinate.
3. Now go create a card batch while tailing the plack-intranet-error log/
4. Notice in the logs:

[2023/08/14 20:50:54] [WARN] Argument "" isn't numeric in multiplication (*) at /kohadevbox/koha/C4/Patroncards/Patroncard.pm line 109.
[2023/08/14 20:50:54] [WARN] Argument "" isn't numeric in multiplication (*) at /kohadevbox/koha/C4/Patroncards/Patroncard.pm line 248.
[2023/08/14 20:50:54] [WARN] Argument "" isn't numeric in multiplication (*) at /kohadevbox/koha/C4/Patroncards/Patroncard.pm line 249.

5. Apply patch, restart_all
6. Try again and notice the WARNS should be gone from the logs now.
7. Make sure you can still create patron cards and the PDF's the generate look right.
Comment 2 Lucas Gass (lukeg) 2023-08-14 21:15:22 UTC
Created attachment 154416 [details] [review]
Bug 34532: Silence warns in Patroncard.pm

To test:

1. Go to patron card creator and make a patron card batch, card layout, and card template.
2. In the card layout leave some values blank for Lower left X coordinate and Lower left Y coordinate.
3. Now go create a card batch while tailing the plack-intranet-error log/
4. Notice in the logs:

[2023/08/14 20:50:54] [WARN] Argument "" isn't numeric in multiplication (*) at /kohadevbox/koha/C4/Patroncards/Patroncard.pm line 109.
[2023/08/14 20:50:54] [WARN] Argument "" isn't numeric in multiplication (*) at /kohadevbox/koha/C4/Patroncards/Patroncard.pm line 248.
[2023/08/14 20:50:54] [WARN] Argument "" isn't numeric in multiplication (*) at /kohadevbox/koha/C4/Patroncards/Patroncard.pm line 249.

5. Apply patch, restart_all
6. Try again and notice the WARNS should be gone from the logs now.
7. Make sure you can still create patron cards and the PDF's the generate look right.
Comment 3 Matt Blenkinsop 2023-08-21 14:34:25 UTC
Created attachment 154680 [details] [review]
Bug 34532: Silence warns in Patroncard.pm

To test:

1. Go to patron card creator and make a patron card batch, card layout, and card template.
2. In the card layout leave some values blank for Lower left X coordinate and Lower left Y coordinate.
3. Now go create a card batch while tailing the plack-intranet-error log/
4. Notice in the logs:

[2023/08/14 20:50:54] [WARN] Argument "" isn't numeric in multiplication (*) at /kohadevbox/koha/C4/Patroncards/Patroncard.pm line 109.
[2023/08/14 20:50:54] [WARN] Argument "" isn't numeric in multiplication (*) at /kohadevbox/koha/C4/Patroncards/Patroncard.pm line 248.
[2023/08/14 20:50:54] [WARN] Argument "" isn't numeric in multiplication (*) at /kohadevbox/koha/C4/Patroncards/Patroncard.pm line 249.

5. Apply patch, restart_all
6. Try again and notice the WARNS should be gone from the logs now.
7. Make sure you can still create patron cards and the PDF's the generate look right.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 4 Marcel de Rooy 2023-09-01 08:29:34 UTC
Sorry (no tidy fan either):

   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 175, now: 180)
Comment 5 Lucas Gass (lukeg) 2023-09-01 13:37:20 UTC
Created attachment 155119 [details] [review]
Bug 34532: Silence warns in Patroncard.pm

To test:

1. Go to patron card creator and make a patron card batch, card layout, and card template.
2. In the card layout leave some values blank for Lower left X coordinate and Lower left Y coordinate.
3. Now go create a card batch while tailing the plack-intranet-error log/
4. Notice in the logs:

[2023/08/14 20:50:54] [WARN] Argument "" isn't numeric in multiplication (*) at /kohadevbox/koha/C4/Patroncards/Patroncard.pm line 109.
[2023/08/14 20:50:54] [WARN] Argument "" isn't numeric in multiplication (*) at /kohadevbox/koha/C4/Patroncards/Patroncard.pm line 248.
[2023/08/14 20:50:54] [WARN] Argument "" isn't numeric in multiplication (*) at /kohadevbox/koha/C4/Patroncards/Patroncard.pm line 249.

5. Apply patch, restart_all
6. Try again and notice the WARNS should be gone from the logs now.
7. Make sure you can still create patron cards and the PDF's the generate look right.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 6 Victor Grousset/tuxayo 2023-09-01 15:19:01 UTC
Thanks Lucas, more extract variable refactoring :)

Thanks Marcel to also look at this, I started to look at this yesterday and wondered if there was a reason to have the $llx & $lly default values set in the method instead of the constructor:
https://git.koha-community.org/Koha-community/Koha/src/commit/e26d7c783eb8c70d318e402e3a90930afcff44cd/C4/Patroncards/Patroncard.pm#L74
Currently there are likely the same warns in the other functions if llx & lly are undef.

And now I notice it's actually done twice, once in each method, so there is duplication.
And one occurrence is `my $lly           = $self->{'llx'} || 0;`
which very likely should be $self->{'lly'}
Comment 7 Marcel de Rooy 2023-09-04 09:33:09 UTC
 lly            => $llx + $llx_layout * $self->{'unitvalue'},

Typo
Comment 8 Nick Clemens (kidclamp) 2023-09-14 18:54:57 UTC
Created attachment 155630 [details] [review]
Bug 34532: (follow-up) Fix typos llx => lly
Comment 9 Marcel de Rooy 2023-09-15 06:51:54 UTC
Created attachment 155638 [details] [review]
Bug 34532: Silence warns in Patroncard.pm

To test:

1. Go to patron card creator and make a patron card batch, card layout, and card template.
2. In the card layout leave some values blank for Lower left X coordinate and Lower left Y coordinate.
3. Now go create a card batch while tailing the plack-intranet-error log/
4. Notice in the logs:

[2023/08/14 20:50:54] [WARN] Argument "" isn't numeric in multiplication (*) at /kohadevbox/koha/C4/Patroncards/Patroncard.pm line 109.
[2023/08/14 20:50:54] [WARN] Argument "" isn't numeric in multiplication (*) at /kohadevbox/koha/C4/Patroncards/Patroncard.pm line 248.
[2023/08/14 20:50:54] [WARN] Argument "" isn't numeric in multiplication (*) at /kohadevbox/koha/C4/Patroncards/Patroncard.pm line 249.

5. Apply patch, restart_all
6. Try again and notice the WARNS should be gone from the logs now.
7. Make sure you can still create patron cards and the PDF's the generate look right.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2023-09-15 06:52:13 UTC
Merged both patches
Comment 11 Tomás Cohen Arazi (tcohen) 2023-09-15 14:58:50 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 12 Fridolin Somers 2023-09-18 07:36:26 UTC
Pushed to 23.05.x for 23.05.04
Comment 13 Matt Blenkinsop 2023-10-12 15:32:16 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x