Bug 21033

Summary: Remove few warns in acqui/basket.pl
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: AcquisitionsAssignee: Marcel de Rooy <m.de.rooy>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: trivial    
Priority: P5 - low CC: fridolin.somers, katrin.fischer, martin.renvoize, nick
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 21033: Remove few warns in acqui/basket.pl
Bug 21033: Remove few warns in acqui/basket.pl
Bug 21033: Remove few warns in acqui/basket.pl
Bug 21033: Remove few warns in acqui/basket.pl

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 2018-08-09 12:25:32 UTC
Awesome work all!

Pushed to master for 18.11
Comment 9 Martin Renvoize 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