[Bug 42565] New: GetIssuingCharges returns undef when rentalcharge is NULL
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42565 Bug ID: 42565 Summary: GetIssuingCharges returns undef when rentalcharge is NULL Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle@bywatersolutions.com GetIssuingCharges initializes $charge = 0 but then overwrites it with the raw value from itemtypes.rentalcharge. When no rental charge is configured (NULL in the database, which is the default), the function returns undef instead of 0. This causes 'Use of uninitialized value' warnings in every caller that does $charge > 0, notably during automatic renewals (cron), SIP checkout/renew, and the OPAC user page. The fix is trivial: normalize NULL to 0 at the assignment point in GetIssuingCharges. All callers already treat the return as numeric and only act on positive values, so this is safe. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42565 --- Comment #1 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198858 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198858&action=edit Bug 42565: Fix uninitialized value warning in GetIssuingCharges GetIssuingCharges overwrites its initial $charge = 0 with the value of itemtypes.rentalcharge, which is NULL when no rental charge is configured. This causes 'uninitialized value' warnings in every caller that compares $charge > 0 (automatic renewals, SIP checkout/renew, OPAC). The fix normalizes NULL to 0 at the source, which is semantically correct: NULL rentalcharge means 'no charge', same as 0. All callers already treat the return value as numeric, so this is safe. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Circulation.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42565 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|minor |trivial CC| |tomascohen@gmail.com Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42565 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198858|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42565 --- Comment #2 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198859 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198859&action=edit Bug 42565: Fix uninitialized value warning in GetIssuingCharges GetIssuingCharges overwrites its initial $charge = 0 with the value of itemtypes.rentalcharge, which is NULL when no rental charge is configured. This causes 'uninitialized value' warnings in every caller that compares $charge > 0 (automatic renewals, SIP checkout/renew, OPAC). The fix normalizes NULL to 0 at the source. All callers already treat the return value as numeric, so this is safe. Also removes a now-redundant $charge && guard in AddIssue that was working around this. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Circulation.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42565 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42565 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198859|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42565 --- Comment #3 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198860 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198860&action=edit Bug 42565: Fix uninitialized value warning in GetIssuingCharges GetIssuingCharges overwrites its initial $charge = 0 with the value of itemtypes.rentalcharge, which is NULL when no rental charge is configured. This causes 'uninitialized value' warnings in every caller that compares $charge > 0 (automatic renewals, SIP checkout/renew, OPAC). The fix normalizes NULL to 0 at the source. All callers already treat the return value as numeric, so this is safe. Also removes a now-redundant $charge && guard in AddIssue that was working around this. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Circulation.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42565 --- Comment #4 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Added a test for this, fixed some QA script errors (missed it sorry) and also removed defensive code related to GetIssuingCharges usage now that we solved it at the source. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42565 Chris Mathevet <chris.mathevet@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42565 Chris Mathevet <chris.mathevet@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198860|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42565 --- Comment #5 from Chris Mathevet <chris.mathevet@inLibro.com> --- Created attachment 198876 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198876&action=edit Bug 42565: Fix uninitialized value warning in GetIssuingCharges GetIssuingCharges overwrites its initial $charge = 0 with the value of itemtypes.rentalcharge, which is NULL when no rental charge is configured. This causes 'uninitialized value' warnings in every caller that compares $charge > 0 (automatic renewals, SIP checkout/renew, OPAC). The fix normalizes NULL to 0 at the source. All callers already treat the return value as numeric, so this is safe. Also removes a now-redundant $charge && guard in AddIssue that was working around this. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Circulation.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Chris Mathevet <chris.mathevet@inLibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42565 Lari Taskula <lari.taskula@hypernova.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #6 from Lari Taskula <lari.taskula@hypernova.fi> --- *** Bug 30815 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42565 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |baptiste.wojtkowski@biblibr |y.org |e.com CC| |lisette@bywatersolutions.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42565 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA QA Contact|baptiste.wojtkowski@biblibr |andrew@bywatersolutions.com |e.com | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42565 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198876|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42565 --- Comment #7 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 204507 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204507&action=edit Bug 42565: Fix uninitialized value warning in GetIssuingCharges GetIssuingCharges overwrites its initial $charge = 0 with the value of itemtypes.rentalcharge, which is NULL when no rental charge is configured. This causes 'uninitialized value' warnings in every caller that compares $charge > 0 (automatic renewals, SIP checkout/renew, OPAC). The fix normalizes NULL to 0 at the source. All callers already treat the return value as numeric, so this is safe. Also removes a now-redundant $charge && guard in AddIssue that was working around this. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Circulation.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Chris Mathevet <chris.mathevet@inLibro.com> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42565 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |26.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42565 --- Comment #8 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Thanks everyone! Pushed to main for 26.11! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org