[Bug 41404] New: No need to check related guarantor/guarantee charges when the limits are not set
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41404 Bug ID: 41404 Summary: No need to check related guarantor/guarantee charges when the limits are not set Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org In the code: # Check the debt of this patrons guarantees if ( defined $no_issues_charge_guarantees ) { # Check the debt of this patrons guarantors *and* the guarantees of those guarantors if ( defined $no_issues_charge_guarantors_with_guarantees ) { It is not possible to undef the system preferences, they are set as blank for no value - we need to check if they are numbers -- 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=41404 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=41404 --- Comment #1 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 190307 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190307&action=edit Bug 41404: Use looks_like_a_number to verify if prefs are set This check adds a check to ensure we have number, not just defined To test: 1 - Apply only unit tests patch 2 - Run the tests prove -v t/db_dependent/Koha/Patron.t 3 - Apply this patch 4 - Run the tests 5 - They pass! -- 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=41404 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=41405 -- 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=41404 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- 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=41404 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190307|0 |1 is obsolete| | --- Comment #2 from David Nind <david@davidnind.com> --- Created attachment 190313 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190313&action=edit Bug 41404: Use looks_like_a_number to verify if prefs are set This check adds a check to ensure we have number, not just defined To test: 1 - Apply only unit tests patch 2 - Run the tests prove -v t/db_dependent/Koha/Patron.t 3 - Apply this patch 4 - Run the tests 5 - They pass! Signed-off-by: David Nind <you@example.com> -- 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=41404 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #3 from David Nind <david@davidnind.com> --- There was no unit tests patch (maybe it is missing), but the tests passed after applying the patch. -- 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=41404 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190313|0 |1 is obsolete| | --- Comment #4 from David Nind <david@davidnind.com> --- Created attachment 190322 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190322&action=edit Bug 41404: Use looks_like_a_number to verify if prefs are set This check adds a check to ensure we have number, not just defined To test: 1 - Apply only unit tests patch 2 - Run the tests prove -v t/db_dependent/Koha/Patron.t 3 - Apply this patch 4 - Run the tests 5 - They pass! Signed-off-by: David Nind <david@davidnind.com> -- 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=41404 --- Comment #5 from David Nind <david@davidnind.com> --- My sign-off line was incorrect, so have updated. -- 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=41404 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | -- 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=41404 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@openfifth.c | |o.uk --- Comment #6 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- 1) Should we also make the same change for noissuescharge? 2) Remind me.. is '0' disabled or 'block all' in this case? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41404 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Trivial patch QA Contact|testopia@bugs.koha-communit |martin.renvoize@openfifth.c |y.org |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41404 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190322|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=41404 --- Comment #7 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 190699 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190699&action=edit Bug 41404: Use looks_like_a_number to verify if prefs are set This check adds a check to ensure we have number, not just defined To test: 1 - Apply only unit tests patch 2 - Run the tests prove -v t/db_dependent/Koha/Patron.t 3 - Apply this patch 4 - Run the tests 5 - They pass! Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41404 --- Comment #8 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 190700 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190700&action=edit Bug 41404: (QA follow-up) Add tests for empty string and zero edge cases This QA follow-up adds comprehensive test coverage for edge cases that the original patch handles: 1. Empty string values ('') - the default value in sysprefs - Verifies that empty strings are treated as "not set" - Confirms charge calculations are skipped (performance optimization) - Ensures patrons are not blocked 2. Zero values (0) - valid numeric limit - Verifies that zero triggers charge calculation (valid number) - Confirms patrons are not blocked (0 means "no limit") - Documents intended behavior that 0 disables blocking 3. Positive numeric values - normal case - Confirms charges are calculated and blocking works correctly These tests validate that looks_like_number() is the correct approach rather than simple truthiness checking, as it properly distinguishes between: - Empty string '' (not a number, skip calculation) - Zero 0 (is a number, calculate but don't block) Test plan: 1. Apply this patch 2. Run the tests: prove t/db_dependent/Koha/Patron.t 3. Verify all tests pass Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41404 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.05.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41404 --- Comment #9 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 26.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41404 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41404 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable Version(s)|26.05.00 |26.05.00,25.11.01 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41404 --- Comment #10 from Jacob O'Mara <jacob.omara@openfifth.co.uk> --- Thanks all, pushed to 25.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41404 Laura Escamilla <Laura.escamilla@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |Laura.escamilla@bywatersolu | |tions.com Status|Pushed to stable |Needs documenting --- Comment #11 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- This patch doesn’t apply cleanly to 25.05, so I’m not planning to backport it at this time. If you’re able to update it for 25.05 compatibility, I’d be happy to take another look. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41404 --- Comment #12 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 193157 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193157&action=edit Bug 41404: [25.05.x] Use looks_like_a_number to verify if prefs are set Bug 41404: Use looks_like_a_number to verify if prefs are set This check adds a check to ensure we have number, not just defined To test: 1 - Apply only unit tests patch 2 - Run the tests prove -v t/db_dependent/Koha/Patron.t 3 - Apply this patch 4 - Run the tests 5 - They pass! Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Bug 41404: (QA follow-up) Add tests for empty string and zero edge cases This QA follow-up adds comprehensive test coverage for edge cases that the original patch handles: 1. Empty string values ('') - the default value in sysprefs - Verifies that empty strings are treated as "not set" - Confirms charge calculations are skipped (performance optimization) - Ensures patrons are not blocked 2. Zero values (0) - valid numeric limit - Verifies that zero triggers charge calculation (valid number) - Confirms patrons are not blocked (0 means "no limit") - Documents intended behavior that 0 disables blocking 3. Positive numeric values - normal case - Confirms charges are calculated and blocking works correctly These tests validate that looks_like_number() is the correct approach rather than simple truthiness checking, as it properly distinguishes between: - Empty string '' (not a number, skip calculation) - Zero 0 (is a number, calculate but don't block) Test plan: 1. Apply this patch 2. Run the tests: prove t/db_dependent/Koha/Patron.t 3. Verify all tests pass Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41404 --- Comment #13 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- (In reply to Laura Escamilla from comment #11)
This patch doesn’t apply cleanly to 25.05, so I’m not planning to backport it at this time. If you’re able to update it for 25.05 compatibility, I’d be happy to take another look.
Rebased for 25.05.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org