Bug 21033 - Remove few warns in acqui/basket.pl
Summary: Remove few warns in acqui/basket.pl
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-07-03 13:51 UTC by Marcel de Rooy
Modified: 2019-10-14 19:58 UTC (History)
3 users (show)

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


Attachments
Bug 21033: Remove few warns in acqui/basket.pl (1.57 KB, patch)
2018-07-03 13:57 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 21033: Remove few warns in acqui/basket.pl (1.70 KB, patch)
2018-07-09 07:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 21033: Remove few warns in acqui/basket.pl (1.71 KB, patch)
2018-08-01 23:10 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 21033: Remove few warns in acqui/basket.pl (1.79 KB, patch)
2018-08-03 14:31 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2018-07-03 13:51:56 UTC

    
Comment 1 Marcel de Rooy 2018-07-03 13:57:55 UTC
Created attachment 76652 [details] [review]
Bug 21033: Remove few warns in acqui/basket.pl

Resolve (line numbers based on 16.11.x):
Use of uninitialized value in hash element at acqui/basket.pl line 337.
Use of uninitialized value in hash element at acqui/basket.pl line 338.
Use of uninitialized value in hash element at acqui/basket.pl line 340.
Use of uninitialized value in hash element at acqui/basket.pl line 342.
Use of uninitialized value in hash element at acqui/basket.pl line 344.

Argument "" isn't numeric in multiplication (*) at koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt line 486.
Argument "" isn't numeric in multiplication (*) at koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt line 591.

Test plan:
Open a basket that contains no prices.
Compare warnings before and after this patch.
Comment 2 Katrin Fischer 2018-07-08 14:27:34 UTC
Hi Marcel,

I've tried testing this, but can't reproduce the errors. What exactly do you mean by:

Open a basket that contains no prices.

?

I've tried closing and reopening a basket with one order line where I left all entries empty, but that didn't do it. Closing an empty basket is not possible, so I am a little lost here.
Comment 3 Marcel de Rooy 2018-07-09 07:21:21 UTC
(In reply to Katrin Fischer from comment #2)
> Hi Marcel,
> 
> I've tried testing this, but can't reproduce the errors. What exactly do you
> mean by:
> 
> Open a basket that contains no prices.
> 
> ?
> 
> I've tried closing and reopening a basket with one order line where I left
> all entries empty, but that didn't do it. Closing an empty basket is not
> possible, so I am a little lost here.

Thanks for testing! Hmm. I couldn't reproduce on master too. A closer look reveals that older production data has NULL values in fields like tax_rate_on_ordering whereas you cannot create these NULL values anymore on a new record.
I can imagine that other people might have some older acq data too and see the same warnings? In this case a dbrev replacing NULL by 0 would do the job too, but changing these two lines does not hurt?
Will adjust the commit message to reflect these findings.
Comment 4 Marcel de Rooy 2018-07-09 07:37:35 UTC
Created attachment 76778 [details] [review]
Bug 21033: Remove few warns in acqui/basket.pl

Resolve (line numbers based on 16.11.x):
Use of uninitialized value in hash element at acqui/basket.pl line 337.
Use of uninitialized value in hash element at acqui/basket.pl line 338.
Use of uninitialized value in hash element at acqui/basket.pl line 340.
Use of uninitialized value in hash element at acqui/basket.pl line 342.
Use of uninitialized value in hash element at acqui/basket.pl line 344.

Argument "" isn't numeric in multiplication (*) at koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt line 486.
Argument "" isn't numeric in multiplication (*) at koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt line 591.

Test plan:
If you have older acq data, you may have records in aqorders with field
tax_rate_on_ordering is NULL. These orders will trigger the above warns.
If you do not have, you could simulate by setting this field to NULL.
Comment 5 Katrin Fischer 2018-08-01 23:10:05 UTC
Created attachment 77418 [details] [review]
Bug 21033: Remove few warns in acqui/basket.pl

Resolve (line numbers based on 16.11.x):
Use of uninitialized value in hash element at acqui/basket.pl line 337.
Use of uninitialized value in hash element at acqui/basket.pl line 338.
Use of uninitialized value in hash element at acqui/basket.pl line 340.
Use of uninitialized value in hash element at acqui/basket.pl line 342.
Use of uninitialized value in hash element at acqui/basket.pl line 344.

Argument "" isn't numeric in multiplication (*) at koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt line 486.
Argument "" isn't numeric in multiplication (*) at koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt line 591.

Test plan:
If you have older acq data, you may have records in aqorders with field
tax_rate_on_ordering is NULL. These orders will trigger the above warns.
If you do not have, you could simulate by setting this field to NULL.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Marcel de Rooy 2018-08-02 06:26:48 UTC
(In reply to Katrin Fischer from comment #5)
> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Thanks Katrin.
Comment 7 Jonathan Druart 2018-08-03 14:31:02 UTC
Created attachment 77472 [details] [review]
Bug 21033: Remove few warns in acqui/basket.pl

Resolve (line numbers based on 16.11.x):
Use of uninitialized value in hash element at acqui/basket.pl line 337.
Use of uninitialized value in hash element at acqui/basket.pl line 338.
Use of uninitialized value in hash element at acqui/basket.pl line 340.
Use of uninitialized value in hash element at acqui/basket.pl line 342.
Use of uninitialized value in hash element at acqui/basket.pl line 344.

Argument "" isn't numeric in multiplication (*) at koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt line 486.
Argument "" isn't numeric in multiplication (*) at koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt line 591.

Test plan:
If you have older acq data, you may have records in aqorders with field
tax_rate_on_ordering is NULL. These orders will trigger the above warns.
If you do not have, you could simulate by setting this field to NULL.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Nick Clemens (kidclamp) 2018-08-09 12:25:32 UTC
Awesome work all!

Pushed to master for 18.11
Comment 9 Martin Renvoize (ashimema) 2018-08-10 08:11:36 UTC
Always good to clean up warnings.

Pushed to 18.05.x for 18.05.03
Comment 10 Fridolin Somers 2018-09-21 12:18:18 UTC
Pushed to 17.11.x for 17.11.10