[Bug 29102] New: DecreaseLoanHighHolds will decrease lona period if borrower has an 'unfound' hold
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29102 Bug ID: 29102 Summary: DecreaseLoanHighHolds will decrease lona period if borrower has an 'unfound' hold Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com Similar to bug 25566 If a patron has a hold on a record and there are other holds -the hold is not found -the rules limit to 1 hold per record -no items will be holdable (already have the hone hold) -Loan will be decreased if using decreaseLoanHighHoldsControl - 'over the number of holdable items'/dynamic -- 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=29102 Nick Clemens <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=29102 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 128901 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128901&action=edit Bug 29102: Unit tests -- 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=29102 --- Comment #2 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 128902 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128902&action=edit Bug 29102: Do not count patron's own hold against limits This patch makes two changes: 1 - The borrower's own holds are not counted towards HighHolds limit 2 - We exclude all hold counts from CanItemBeReserved Previously a patron's hold could put the count over the threshhold, and if the patron is only allowed 1 hold per record, and the hold wasn't found before the checkout, it would make all items unholdable, thus lowering the theshhold for dynamic HighHolds To test: 1 - Set sysaprefs: decreaseLoanHighHolds - enable decreaseLoanHighHoldsDuration - 1 decreaseLoanHighHoldsValue - 1 decreaseLoanHighHoldsControl - dynamic decreaseLoanHighHoldsIgnoreStatuses - blank 2 - Set circ rules to allow 1 hold per record and loan period of 5 3 - Find/create a record with 3 items 4 - Place a title level hold for a patron 5 - Attempt to checkout item - note warning about high holds 6 - Cancel checkout 7 - Set decreaseLoanHighHoldsControl - static 8 - Attempt checkout - note warning about high holds 9 - Apply patrch 10 - Checkout item - no warning 11 - checkin item, replace hold 12 - Set decreaseLoanHighHoldsControl - dynamic 13 - Checkout item - no warning -- 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=29102 --- Comment #3 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 128903 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128903&action=edit Bug 29102: Remove ignore_found_holds If not counting patrons holds, found or unfound, we no longer need this option introduced by bug 28078 -- 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=29102 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|DecreaseLoanHighHolds will |DecreaseLoanHighHolds will |decrease lona period if |decrease loan period if |borrower has an 'unfound' |borrower has an 'unfound' |hold |hold CC| |cbrannon@cdalibrary.org Assignee|koha-bugs@lists.koha-commun |nick@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=29102 Donna <bwsdonna@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bwsdonna@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29102 --- Comment #4 from Christopher Brannon <cbrannon@cdalibrary.org> --- I'm using a sandbox, and I haven't applied the patch yet and I am not running into the problem in step 5. It just checks out the item and removes the hold. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29102 --- Comment #5 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Christopher Brannon from comment #4)
I'm using a sandbox, and I haven't applied the patch yet and I am not running into the problem in step 5. It just checks out the item and removes the hold.
Ah, I described it in initial comment "and there are other holds" When set to dynamic we check: $threshold = $items_count + $decreaseLoanHighHoldsValue; $holds->count() <= $threshold items_count will be 0 (none holdable) but holds count is still 1, so not over the threshold Amended steps below: step 4 - place title level holds for 2 patrons step 5 - attempt to checkout to one of the patrons who have a hold -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29102 e.betemps <e.betemps@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |e.betemps@gmail.com --- Comment #6 from e.betemps <e.betemps@gmail.com> --- For the syspref 'decreaseLoanHighHoldsControl', I do not see nor 'static' nor 'dynamic'. I can choose between 'on the record' and 'over the number of holdable items on the record'. -> Which of these two is dynamic ? I assumed it was 'over the number...' but I'd like to be sure. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29102 --- Comment #7 from Christopher Brannon <cbrannon@cdalibrary.org> --- (In reply to Emmanuel Bétemps from comment #6)
For the syspref 'decreaseLoanHighHoldsControl', I do not see nor 'static' nor 'dynamic'.
I can choose between 'on the record' and 'over the number of holdable items on the record'.
-> Which of these two is dynamic ? I assumed it was 'over the number...' but I'd like to be sure.
I believe 'on the record' is static, and 'over the number of holdable items on the record' is dynamic. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29102 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #8 from David Nind <david@davidnind.com> --- Patch no longer applies 8-(.. Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 29102: Unit tests Using index info to reconstruct a base tree... M t/db_dependent/DecreaseLoanHighHolds.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/DecreaseLoanHighHolds.t Applying: Bug 29102: Do not count patron's own hold against limits Using index info to reconstruct a base tree... M C4/Circulation.pm M t/db_dependent/DecreaseLoanHighHolds.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/DecreaseLoanHighHolds.t CONFLICT (content): Merge conflict in t/db_dependent/DecreaseLoanHighHolds.t Auto-merging C4/Circulation.pm CONFLICT (content): Merge conflict in C4/Circulation.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 29102: Do not count patron's own hold against limits -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29102 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29102 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29102 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #128901|0 |1 is obsolete| | Attachment #128902|0 |1 is obsolete| | Attachment #128903|0 |1 is obsolete| | --- Comment #9 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 137722 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=137722&action=edit Bug 29102: Do not count patron's own hold against limits This patch makes two changes: 1 - The borrower's own holds are not counted towards HighHolds limit 2 - We exclude all hold counts from CanItemBeReserved Previously a patron's hold could put the count over the threshhold, and if the patron is only allowed 1 hold per record, and the hold wasn't found before the checkout, it would make all items unholdable, thus lowering the theshhold for dynamic HighHolds To test: 1 - Set sysaprefs: decreaseLoanHighHolds - enable decreaseLoanHighHoldsDuration - 1 decreaseLoanHighHoldsValue - 1 decreaseLoanHighHoldsControl - dynamic decreaseLoanHighHoldsIgnoreStatuses - blank 2 - Set circ rules to allow 1 hold per record and loan period of 5 3 - Find/create a record with 3 items 4 - Place a title level hold for a patron 5 - Attempt to checkout item - note warning about high holds 6 - Cancel checkout 7 - Set decreaseLoanHighHoldsControl - static 8 - Attempt checkout - note warning about high holds 9 - Apply patrch 10 - Checkout item - no warning 11 - checkin item, replace hold 12 - Set decreaseLoanHighHoldsControl - dynamic 13 - Checkout item - no warning 14 - prove t/db_dependent/DecreaseLoanHighHolds.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29102 --- Comment #10 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 137723 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=137723&action=edit Bug 29102: Remove ignore_found_holds If not counting patrons holds, found or unfound, we no longer need this option introduced by bug 28078 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29102 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #137722|0 |1 is obsolete| | Attachment #137723|0 |1 is obsolete| | --- Comment #11 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 137745 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=137745&action=edit Bug 29102: Do not count patron's own hold against limits This patch makes three changes: 1 - The borrower's own holds are not counted towards HighHolds limit 2 - We exclude all hold counts from CanItemBeReserved 3 - Static mode should only decrease hold when over the decreaseLoanHighHoldsValue, not when equal Previously a patron's hold could put the count over the threshhold, and if the patron is only allowed 1 hold per record, and the hold wasn't found before the checkout, it would make all items unholdable, thus lowering the theshhold for dynamic HighHolds To test: 1 - Set sysaprefs: decreaseLoanHighHolds - enable decreaseLoanHighHoldsDuration - 1 decreaseLoanHighHoldsValue - 1 decreaseLoanHighHoldsControl - "over the number of holdable items on the record" / dynamic decreaseLoanHighHoldsIgnoreStatuses - blank 2 - Set circ rules to allow 1 hold per record and loan period of 5 3 - Find/create a record with 3 items 4 - Place a title level hold for two different patrons 5 - Attempt to checkout item - note warning about high holds 6 - Cancel checkout 7 - Set decreaseLoanHighHoldsControl - "on the record" / static 8 - Attempt checkout - note warning about high holds 9 - Apply patch 10 - Checkout item - no warning 11 - checkin item, replace hold 12 - Set decreaseLoanHighHoldsControl - "over the number of holdable items on the record" / dynamic 13 - Checkout item - no warning 14 - prove t/db_dependent/DecreaseLoanHighHolds.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29102 --- Comment #12 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 137746 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=137746&action=edit Bug 29102: Remove ignore_found_holds If not counting patrons holds, found or unfound, we no longer need this option introduced by bug 28078 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29102 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=29102 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #137745|0 |1 is obsolete| | --- Comment #13 from David Nind <david@davidnind.com> --- Created attachment 140970 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=140970&action=edit Bug 29102: Do not count patron's own hold against limits This patch makes three changes: 1 - The borrower's own holds are not counted towards HighHolds limit 2 - We exclude all hold counts from CanItemBeReserved 3 - Static mode should only decrease hold when over the decreaseLoanHighHoldsValue, not when equal Previously a patron's hold could put the count over the threshhold, and if the patron is only allowed 1 hold per record, and the hold wasn't found before the checkout, it would make all items unholdable, thus lowering the theshhold for dynamic HighHolds To test: 1 - Set sysaprefs: decreaseLoanHighHolds - enable decreaseLoanHighHoldsDuration - 1 decreaseLoanHighHoldsValue - 1 decreaseLoanHighHoldsControl - "over the number of holdable items on the record" / dynamic decreaseLoanHighHoldsIgnoreStatuses - blank 2 - Set circ rules to allow 1 hold per record and loan period of 5 3 - Find/create a record with 3 items 4 - Place a title level hold for two different patrons 5 - Attempt to checkout item - note warning about high holds 6 - Cancel checkout 7 - Set decreaseLoanHighHoldsControl - "on the record" / static 8 - Attempt checkout - note warning about high holds 9 - Apply patch 10 - Checkout item - no warning 11 - checkin item, replace hold 12 - Set decreaseLoanHighHoldsControl - "over the number of holdable items on the record" / dynamic 13 - Checkout item - no warning 14 - prove t/db_dependent/DecreaseLoanHighHolds.t 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=29102 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #137746|0 |1 is obsolete| | --- Comment #14 from David Nind <david@davidnind.com> --- Created attachment 140971 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=140971&action=edit Bug 29102: Remove ignore_found_holds If not counting patrons holds, found or unfound, we no longer need this option introduced by bug 28078 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=29102 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |katrin.fischer@bsz-bw.de |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29102 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|katrin.fischer@bsz-bw.de |testopia@bugs.koha-communit | |y.org Summary|DecreaseLoanHighHolds will |DecreaseLoanHighHolds will |decrease loan period if |decrease loan period if |borrower has an 'unfound' |patron has an 'unfound' |hold |hold -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29102 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #140970|0 |1 is obsolete| | --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 142990 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142990&action=edit Bug 29102: Do not count patron's own hold against limits This patch makes three changes: 1 - The borrower's own holds are not counted towards HighHolds limit 2 - We exclude all hold counts from CanItemBeReserved 3 - Static mode should only decrease hold when over the decreaseLoanHighHoldsValue, not when equal Previously a patron's hold could put the count over the threshhold, and if the patron is only allowed 1 hold per record, and the hold wasn't found before the checkout, it would make all items unholdable, thus lowering the theshhold for dynamic HighHolds To test: 1 - Set sysaprefs: decreaseLoanHighHolds - enable decreaseLoanHighHoldsDuration - 1 decreaseLoanHighHoldsValue - 1 decreaseLoanHighHoldsControl - "over the number of holdable items on the record" / dynamic decreaseLoanHighHoldsIgnoreStatuses - blank 2 - Set circ rules to allow 1 hold per record and loan period of 5 3 - Find/create a record with 3 items 4 - Place a title level hold for two different patrons 5 - Attempt to checkout item - note warning about high holds 6 - Cancel checkout 7 - Set decreaseLoanHighHoldsControl - "on the record" / static 8 - Attempt checkout - note warning about high holds 9 - Apply patch 10 - Checkout item - no warning 11 - checkin item, replace hold 12 - Set decreaseLoanHighHoldsControl - "over the number of holdable items on the record" / dynamic 13 - Checkout item - no warning 14 - prove t/db_dependent/DecreaseLoanHighHolds.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29102 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #140971|0 |1 is obsolete| | --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 142991 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142991&action=edit Bug 29102: Remove ignore_found_holds If not counting patrons holds, found or unfound, we no longer need this option introduced by bug 28078 Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29102 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA CC| |martin.renvoize@ptfs-europe | |.com --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Works as described, nice clean code., unit tests and QA script is happy. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29102 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29102 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |22.11.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=29102 --- Comment #18 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 22.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org