[Bug 42700] New: In C4/SIP/ILS/Patron.pm, hold_items only counts waiting holds
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42700 Bug ID: 42700 Summary: In C4/SIP/ILS/Patron.pm, hold_items only counts waiting holds Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: SIP2 Assignee: koha-bugs@lists.koha-community.org Reporter: lucas@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org I can't find good documentation on this, but I think this is a bug: hold_items => $flags->{WAITING}->{itemlist}, holds_items is currently a count of waiting holds, when it should total holds. Meanwhile unavail_holds is a count of all holds except waiting holds. Some 3rd party SIP machines use this calculation to get a count of "holds to pick up": holds ready for pickup = hold_items - unavail_holds Currently this can produce a negative number. -- 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=42700 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com QA Contact|testopia@bugs.koha-communit |kyle@bywatersolutions.com |y.org | Depends on| |10817 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10817 [Bug 10817] SIPserver does not return holds info when requested -- 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=42700 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |3435 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=3435 [Bug 3435] Support 3M smart chute extensions -- 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=42700 --- Comment #1 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- (In reply to Lucas Gass (lukeg) from comment #0)
I can't find good documentation on this, but I think this is a bug:
hold_items => $flags->{WAITING}->{itemlist},
holds_items is currently a count of waiting holds, when it should total holds.
Meanwhile unavail_holds is a count of all holds except waiting holds.
Some 3rd party SIP machines use this calculation to get a count of "holds to pick up":
holds ready for pickup = hold_items - unavail_holds
Currently this can produce a negative number.
Bug 10817 seems to confirm this is a bug: Sip devices need this to show to the user what books they have on hold and what holds they have awaiting collection. The SIP2 protocol spec also seems to agree ( but not explicitly ). -- 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=42700 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | -- 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=42700 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|kyle@bywatersolutions.com |testopia@bugs.koha-communit | |y.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42700 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@openfifth.c | |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42700 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42700 --- Comment #2 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 199617 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199617&action=edit Bug 42700: Add unit tests Patch from commit 03c7ab1 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42700 --- Comment #3 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 199618 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199618&action=edit Bug 42700: hold_items in SIP patron info should contain all holds, not just waiting Some 3rd party SIP clients calculate the number of holds ready for pickup as: holds ready for pickup = hold_items - unavail_holds Right now this can produce a negative number since hold_items contains only waiting holds. This patch makes hold_items contain all of the patron's holds, so hold_items - unavail_holds correctly gives the count of holds ready to be picked up. Test Plan: 1) Apply the first patch only 2) prove t/db_dependent/SIP/Patron.t 3) Note the new "hold_items counts all holds" subtest fails 4) Apply this patch 5) prove t/db_dependent/SIP/Patron.t 6) Note the tests pass! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42700 --- Comment #4 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 199619 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199619&action=edit Bug 42700: Avoid duplicate holds query query when building SIP patron object This patch fetches the patron's holds once into a list of Koha::Hold objects and derives both hold_items and unavail_holds from that single fetch. Test Plan: 1) Apply this patch 2) prove t/db_dependent/SIP/ 3) Note all tests pass! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42700 --- Comment #5 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 199620 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199620&action=edit Bug 42700: Remove dead $flags->{WAITING} code from C4::Members::patronflags After the previous patch, $flags->{WAITING} from C4::Members::patronflags is no longer read anywhere in Koha. Test Plan: 1) Apply this patch 2) prove t/db_dependent/SIP/ t/db_dependent/Members.t 3) Note all tests pass! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42700 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=42700 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #199617|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=42700 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #199618|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=42700 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #199619|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=42700 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #199620|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=42700 --- Comment #6 from David Nind <david@davidnind.com> --- Created attachment 199636 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199636&action=edit Bug 42700: Add unit tests 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=42700 --- Comment #7 from David Nind <david@davidnind.com> --- Created attachment 199637 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199637&action=edit Bug 42700: hold_items in SIP patron info should contain all holds, not just waiting Some 3rd party SIP clients calculate the number of holds ready for pickup as: holds ready for pickup = hold_items - unavail_holds Right now this can produce a negative number since hold_items contains only waiting holds. This patch makes hold_items contain all of the patron's holds, so hold_items - unavail_holds correctly gives the count of holds ready to be picked up. Test Plan: 1) Apply the first patch only 2) prove t/db_dependent/SIP/Patron.t 3) Note the new "hold_items counts all holds" subtest fails 4) Apply this patch 5) prove t/db_dependent/SIP/Patron.t 6) Note the tests pass! 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=42700 --- Comment #8 from David Nind <david@davidnind.com> --- Created attachment 199638 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199638&action=edit Bug 42700: Avoid duplicate holds query query when building SIP patron object This patch fetches the patron's holds once into a list of Koha::Hold objects and derives both hold_items and unavail_holds from that single fetch. Test Plan: 1) Apply this patch 2) prove t/db_dependent/SIP/ 3) Note all tests pass! 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=42700 --- Comment #9 from David Nind <david@davidnind.com> --- Created attachment 199639 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199639&action=edit Bug 42700: Remove dead $flags->{WAITING} code from C4::Members::patronflags After the previous patch, $flags->{WAITING} from C4::Members::patronflags is no longer read anywhere in Koha. Test Plan: 1) Apply this patch 2) prove t/db_dependent/SIP/ t/db_dependent/Members.t 3) Note all tests pass! 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=42700 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |aleisha@catalyst.net.nz |y.org | 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=42700 --- Comment #10 from Aleisha Amohia <aleisha@catalyst.net.nz> --- QA tests fail, however this patchset isn't adding any new subroutines so I don't think it's within the scope of this bug to expect it. I've reported Bug 43324 to handle this separately. [FAIL] C4/SIP/ILS/Patron.pm FAIL pod_coverage sub AUTOLOAD has no pod coverage. sub _get_outstanding_holds has no pod coverage. sub _get_address has no pod coverage. sub DESTROY has no pod coverage. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42700 Aleisha Amohia <aleisha@catalyst.net.nz> 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=42700 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #199636|0 |1 is obsolete| | Attachment #199637|0 |1 is obsolete| | Attachment #199638|0 |1 is obsolete| | Attachment #199639|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=42700 --- Comment #11 from Aleisha Amohia <aleisha@catalyst.net.nz> --- Created attachment 203689 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203689&action=edit Bug 42700: Add unit tests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Aleisha Amohia <aleisha@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42700 --- Comment #12 from Aleisha Amohia <aleisha@catalyst.net.nz> --- Created attachment 203690 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203690&action=edit Bug 42700: hold_items in SIP patron info should contain all holds, not just waiting Some 3rd party SIP clients calculate the number of holds ready for pickup as: holds ready for pickup = hold_items - unavail_holds Right now this can produce a negative number since hold_items contains only waiting holds. This patch makes hold_items contain all of the patron's holds, so hold_items - unavail_holds correctly gives the count of holds ready to be picked up. Test Plan: 1) Apply the first patch only 2) prove t/db_dependent/SIP/Patron.t 3) Note the new "hold_items counts all holds" subtest fails 4) Apply this patch 5) prove t/db_dependent/SIP/Patron.t 6) Note the tests pass! Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Aleisha Amohia <aleisha@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42700 --- Comment #13 from Aleisha Amohia <aleisha@catalyst.net.nz> --- Created attachment 203691 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203691&action=edit Bug 42700: Avoid duplicate holds query query when building SIP patron object This patch fetches the patron's holds once into a list of Koha::Hold objects and derives both hold_items and unavail_holds from that single fetch. Test Plan: 1) Apply this patch 2) prove t/db_dependent/SIP/ 3) Note all tests pass! Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Aleisha Amohia <aleisha@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42700 --- Comment #14 from Aleisha Amohia <aleisha@catalyst.net.nz> --- Created attachment 203692 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203692&action=edit Bug 42700: Remove dead $flags->{WAITING} code from C4::Members::patronflags After the previous patch, $flags->{WAITING} from C4::Members::patronflags is no longer read anywhere in Koha. Test Plan: 1) Apply this patch 2) prove t/db_dependent/SIP/ t/db_dependent/Members.t 3) Note all tests pass! Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Aleisha Amohia <aleisha@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42700 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.11.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=42700 --- Comment #15 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.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42700 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_26_05_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42700 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable Version(s)|26.11.00 |26.11.00,26.05.05 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42700 --- Comment #16 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to 26.05.x for the upcoming 26.05.05 release. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42700 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_26_05_candidate | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org