[Bug 24602] New: The fallback value for onshelfholds should be 0
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24602 Bug ID: 24602 Summary: The fallback value for onshelfholds should be 0 Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org The onshelfholds circulation rules values are more of an ENUM type (as opposed to other rules with integers like max*qty). In the quantities cases, it makes sense to have undef mean unlimited. In the enum type it is clearer to just set a default value (0) as the code already expects (Note: undef and 0 both eval to false so there's no behaviour problem, but warnings or we get forced to check for defined when the rule is used. It seems trivial to just make the rule fallback to a chosen default value) -- 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=24602 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 98532 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=98532&action=edit Bug 24602: Tests for get_onshlefholds_policy Ths patch introduces tests for get_onshelf_policy. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/CirculationRules.t => FAIL: Tests fail because the current code returns undef instead of 0 -- 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=24602 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 98533 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=98533&action=edit Bug 24602: Make get_onshelfholds_policy fallback to 0 The onshelfholds circulation rules values are more of an ENUM type (as opposed to other rules with integers like max*qty). In the quantities cases, it makes sense to have undef mean unlimited. In the enum type it is clearer to just set a default value (0) as the code already expects (Note: undef and 0 both eval to false so there's no behaviour problem, but warnings or we get forced to check for defined when the rule is used. It seems trivial to just make the rule fallback to a chosen default value). This patch makes it fallback to 0. To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/CirculationRules.t \ t/db_dependent/Reserves.t => FAIL: CirculationRules.t fails and Reserves.t prints a warning 2. Apply this patch 3. Repeat (1) => SUCCESS: Tests pass! No warning! 4. Sign off :-D -- 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=24602 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Status|NEW |Needs Signoff Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Depends on| |18936 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18936 [Bug 18936] Convert issuingrules fields to circulation_rules -- 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=24602 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #3 from David Nind <david@davidnind.com> --- Is this a warning? root@8881cb995d76:koha(bz24602)$ prove t/db_dependent/Reserves.t t/db_dependent/Reserves.t .. 8/62 Argument "book_reserved" isn't numeric in numeric eq (==) at /kohadevbox/koha/C4/Items.pm line 1776. t/db_dependent/Reserves.t .. ok All tests successful. Files=1, Tests=62, 8 wallclock secs ( 0.04 usr 0.00 sys + 6.64 cusr 0.63 csys = 7.31 CPU) Result: PASS -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24602 --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to David Nind from comment #3)
Is this a warning?
root@8881cb995d76:koha(bz24602)$ prove t/db_dependent/Reserves.t t/db_dependent/Reserves.t .. 8/62 Argument "book_reserved" isn't numeric in numeric eq (==) at /kohadevbox/koha/C4/Items.pm line 1776. t/db_dependent/Reserves.t .. ok All tests successful. Files=1, Tests=62, 8 wallclock secs ( 0.04 usr 0.00 sys + 6.64 cusr 0.63 csys = 7.31 CPU) Result: PASS
That's new, this is the warning I'm referring to: Use of uninitialized value $on_shelf_holds in numeric eq (==) at /kohadevbox/koha/C4/Reserves.pm line 1239. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24602 David Nind <david@davidnind.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=24602 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #98532|0 |1 is obsolete| | --- Comment #5 from David Nind <david@davidnind.com> --- Created attachment 98554 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=98554&action=edit Bug 24602: Tests for get_onshlefholds_policy Ths patch introduces tests for get_onshelf_policy. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/CirculationRules.t => FAIL: Tests fail because the current code returns undef instead of 0 Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24602 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #98533|0 |1 is obsolete| | --- Comment #6 from David Nind <david@davidnind.com> --- Created attachment 98555 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=98555&action=edit Bug 24602: Make get_onshelfholds_policy fallback to 0 The onshelfholds circulation rules values are more of an ENUM type (as opposed to other rules with integers like max*qty). In the quantities cases, it makes sense to have undef mean unlimited. In the enum type it is clearer to just set a default value (0) as the code already expects (Note: undef and 0 both eval to false so there's no behaviour problem, but warnings or we get forced to check for defined when the rule is used. It seems trivial to just make the rule fallback to a chosen default value). This patch makes it fallback to 0. To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/CirculationRules.t \ t/db_dependent/Reserves.t => FAIL: CirculationRules.t fails and Reserves.t prints a warning 2. Apply this patch 3. Repeat (1) => SUCCESS: Tests pass! No warning! 4. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24602 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24602 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #98554|0 |1 is obsolete| | Attachment #98555|0 |1 is obsolete| | --- Comment #7 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 98578 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=98578&action=edit Bug 24602: Tests for get_onshlefholds_policy Ths patch introduces tests for get_onshelf_policy. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/CirculationRules.t => FAIL: Tests fail because the current code returns undef instead of 0 Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24602 --- Comment #8 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 98579 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=98579&action=edit Bug 24602: Make get_onshelfholds_policy fallback to 0 The onshelfholds circulation rules values are more of an ENUM type (as opposed to other rules with integers like max*qty). In the quantities cases, it makes sense to have undef mean unlimited. In the enum type it is clearer to just set a default value (0) as the code already expects (Note: undef and 0 both eval to false so there's no behaviour problem, but warnings or we get forced to check for defined when the rule is used. It seems trivial to just make the rule fallback to a chosen default value). This patch makes it fallback to 0. To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/CirculationRules.t \ t/db_dependent/Reserves.t => FAIL: CirculationRules.t fails and Reserves.t prints a warning 2. Apply this patch 3. Repeat (1) => SUCCESS: Tests pass! No warning! 4. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24602 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |20.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24602 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24602 Bug 24602 depends on bug 18936, which changed state. Bug 18936 Summary: Convert issuingrules fields to circulation_rules https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18936 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24602 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED CC| |joy@bywatersolutions.com Status|Pushed to master |RESOLVED --- Comment #10 from Joy Nelson <joy@bywatersolutions.com> --- not pushed to 19.11.x branch due to dependencies -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org