[Bug 26634] New: Hold rules applied incorrectly when All Libraries rules are more specific than branch rules
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 Bug ID: 26634 Summary: Hold rules applied incorrectly when All Libraries rules are more specific than branch rules Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: critical Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: andrew@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com To recreate: - At the All Libraries level, create a circ rule for a specific patron category and a specific item type that only allows 1 hold - At the branch-specific level for Branch A, create an All/All rule that allows 2 holds - confirm ReservesControll is set to patron's library - find a patron from Branch A of the category for which you made your rule - find two bibs with items of the itype got which you made your rule - place a hold on one bib. success! - try to place a hold on the second bib. you're told you cannot because the patron is only allowed 1 hold We should be enforcing the All/All rule for Branch A. If we put in a rule for the category and itype for Branch A, that rule will be enforced and we'll be allowed a second hold. In GetHoldRule, we tell get_effective_rules to return those rules ordered by 'categorycode', 'itemtype', 'branchcode.' We should just let get_effective_rule handle that ordering on its own. -- 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=26634 Andrew Fuerste-Henry <andrew@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=26634 --- Comment #1 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 111379 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111379&action=edit Bug 26634: Do not reorder effective rules To test: 1 - At the All Libraries level, create a circ rule for a specific patron category and a specific item type that only allows 1 hold 2 - At the branch-specific level for Branch A, create an All/All rule that allows 2 holds 3 - confirm ReservesControll is set to patron's library 4 - find a patron from Branch A of the category for which you made your rule 5 - find two bibs with items of the itype got which you made your rule 6 - place a hold on one bib. success! 7 - try to place a hold on the second bib. you're told you cannot because the patron is only allowed 1 hold 8 - apply patch, restart services 9 - try to place your second hold again, success! -- 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=26634 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bwsdonna@gmail.com, | |kelly@bywatersolutions.com, | |kyle@bywatersolutions.com, | |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org, | |julian.maurice@biblibre.com | |, katrin.fischer@bsz-bw.de, | |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=26634 ByWater Sandboxes <bws.sandboxes@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #111379|0 |1 is obsolete| | --- Comment #2 from ByWater Sandboxes <bws.sandboxes@gmail.com> --- Created attachment 111383 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111383&action=edit Bug 26634: Do not reorder effective rules To test: 1 - At the All Libraries level, create a circ rule for a specific patron category and a specific item type that only allows 1 hold 2 - At the branch-specific level for Branch A, create an All/All rule that allows 2 holds 3 - confirm ReservesControll is set to patron's library 4 - find a patron from Branch A of the category for which you made your rule 5 - find two bibs with items of the itype got which you made your rule 6 - place a hold on one bib. success! 7 - try to place a hold on the second bib. you're told you cannot because the patron is only allowed 1 hold 8 - apply patch, restart services 9 - try to place your second hold again, success! Signed-off-by: Lisette Scheer <lisetteslatah@gmail.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=26634 Lisette Scheer <lisetteslatah@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |lisetteslatah@gmail.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=26634 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |andrew@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=26634 --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Hmmm, so the ordering came from bug 14695 in the first place.. I'm wondering if it was a deliberate diversion from the norm or a mistake in the first place. It does make sense to make it consistent with the rest of the system however. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #111383|0 |1 is obsolete| | --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 111398 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111398&action=edit Bug 26634: Do not reorder effective rules To test: 1 - At the All Libraries level, create a circ rule for a specific patron category and a specific item type that only allows 1 hold 2 - At the branch-specific level for Branch A, create an All/All rule that allows 2 holds 3 - confirm ReservesControll is set to patron's library 4 - find a patron from Branch A of the category for which you made your rule 5 - find two bibs with items of the itype got which you made your rule 6 - place a hold on one bib. success! 7 - try to place a hold on the second bib. you're told you cannot because the patron is only allowed 1 hold 8 - apply patch, restart services 9 - try to place your second hold again, success! Signed-off-by: Lisette Scheer <lisetteslatah@gmail.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=26634 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- So.. it appears the original unit tests don't test for this scenario.. any chance of adding such a test to db_dependent/Reserves/MultiplePerRecord.t Andrew? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 --- Comment #6 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Kyle, Nick, and I couldn't find or remember any reason for this grouping from that original bug. If anyone can identify a reason for it, I'm open to keeping it, but then we need to change a bunch of documentation to reflect it. I very much don't know how to write new tests, but I'll bug Kyle about it :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 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=26634 --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Martin Renvoize from comment #3)
Hmmm, so the ordering came from bug 14695 in the first place.. I'm wondering if it was a deliberate diversion from the norm or a mistake in the first place.
Nope, it existed prior to 14695: commit 3c741d2376e939dea0554a05eddd4f9e9b2d9449 Date: Mon Aug 24 22:10:21 2009 +0200 Smart Rules enhancements + $rightsquery .= " ORDER BY categorycode DESC, itemtype DESC, branchcode DESC"; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #8 from David Cook <dcook@prosentient.com.au> --- I think I had sometimes similar happen recently with loan period... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #111398|0 |1 is obsolete| | --- Comment #9 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 113966 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113966&action=edit Bug 26634: Remove GetHoldRule subroutine in C4::Reserves This routine is only used internally and incorrectly overrides the precedence of holds rules - it should be removed This patch removes the routine, adjusts tests, and adds test to confirm correct precedence is followed To test: 1 - At the All Libraries level, create a circ rule for a specific patron category and a specific item type that only allows 1 hold 2 - At the branch-specific level for Branch A, create an All/All rule that allows 2 holds 3 - confirm ReservesControll is set to patron's library 4 - find a patron from Branch A of the category for which you made your rule 5 - find two bibs with items of the itype got which you made your rule 6 - place a hold on one bib. success! 7 - try to place a hold on the second bib. you're told you cannot because the patron is only allowed 1 hold 8 - apply patch, restart services 9 - try to place your second hold again, success! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|andrew@bywatersolutions.com |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Working through testing this one now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113966|0 |1 is obsolete| | --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 114017 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114017&action=edit Bug 26634: Remove GetHoldRule subroutine in C4::Reserves This routine is only used internally and incorrectly overrides the precedence of holds rules - it should be removed This patch removes the routine, adjusts tests, and adds test to confirm correct precedence is followed To test: 1 - At the All Libraries level, create a circ rule for a specific patron category and a specific item type that only allows 1 hold 2 - At the branch-specific level for Branch A, create an All/All rule that allows 2 holds 3 - confirm ReservesControll is set to patron's library 4 - find a patron from Branch A of the category for which you made your rule 5 - find two bibs with items of the itype got which you made your rule 6 - place a hold on one bib. success! 7 - try to place a hold on the second bib. you're told you cannot because the patron is only allowed 1 hold 8 - apply patch, restart services 9 - try to place your second hold again, success! 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=26634 Martin Renvoize <martin.renvoize@ptfs-europe.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=26634 --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Re-tested and it's working well and provides a more comprehensive cleanup. Signing off. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Re-tested and it's working well and provides a more comprehensive cleanup. Signing off. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|martin.renvoize@ptfs-europe |testopia@bugs.koha-communit |.com |y.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114017|0 |1 is obsolete| | --- Comment #14 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 114260 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114260&action=edit Bug 26634: Remove GetHoldRule subroutine in C4::Reserves This routine is only used internally and incorrectly overrides the precedence of holds rules - it should be removed This patch removes the routine, adjusts tests, and adds test to confirm correct precedence is followed To test: 1 - At the All Libraries level, create a circ rule for a specific patron category and a specific item type that only allows 1 hold 2 - At the branch-specific level for Branch A, create an All/All rule that allows 2 holds 3 - confirm ReservesControll is set to patron's library 4 - find a patron from Branch A of the category for which you made your rule 5 - find two bibs with items of the itype got which you made your rule 6 - place a hold on one bib. success! 7 - try to place a hold on the second bib. you're told you cannot because the patron is only allowed 1 hold 8 - apply patch, restart services 9 - try to place your second hold again, success! Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #15 from Victor Grousset/tuxayo <victor@tuxayo.net> --- As a step towards QA, here is another signoff. And QA script ok -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114260|0 |1 is obsolete| | --- Comment #16 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 114439 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114439&action=edit Bug 26634: Remove GetHoldRule subroutine in C4::Reserves This routine is only used internally and incorrectly overrides the precedence of holds rules - it should be removed This patch removes the routine, adjusts tests, and adds test to confirm correct precedence is followed To test: 1 - At the All Libraries level, create a circ rule for a specific patron category and a specific item type that only allows 1 hold 2 - At the branch-specific level for Branch A, create an All/All rule that allows 2 holds 3 - confirm ReservesControll is set to patron's library 4 - find a patron from Branch A of the category for which you made your rule 5 - find two bibs with items of the itype got which you made your rule 6 - place a hold on one bib. success! 7 - try to place a hold on the second bib. you're told you cannot because the patron is only allowed 1 hold 8 - apply patch, restart services 9 - try to place your second hold again, success! Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA QA Contact|testopia@bugs.koha-communit |josef.moravec@gmail.com |y.org | CC| |josef.moravec@gmail.com --- Comment #17 from Josef Moravec <josef.moravec@gmail.com> --- QA script passed tests passed code is readable and does have test Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |21.05.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=26634 --- Comment #18 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 21.05, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable CC| |fridolin.somers@biblibre.co | |m Version(s)|21.05.00 |21.05.00,20.11.01 released in| | --- Comment #19 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 20.11.x for 20.11.01 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 --- Comment #20 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Doesn't apply cleanly to 20.05.x, please rebase for backport -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 Niamh Walker-Headon <Niamh.WalkerHeadon@hse.ie> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |Niamh.WalkerHeadon@hse.ie -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 --- Comment #21 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 116519 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116519&action=edit Bug 26634: [20.05.x] Remove GetHoldRule subroutine in C4::Reserves This routine is only used internally and incorrectly overrides the precedence of holds rules - it should be removed This patch removes the routine, adjusts tests, and adds test to confirm correct precedence is followed To test: 1 - At the All Libraries level, create a circ rule for a specific patron category and a specific item type that only allows 1 hold 2 - At the branch-specific level for Branch A, create an All/All rule that allows 2 holds 3 - confirm ReservesControll is set to patron's library 4 - find a patron from Branch A of the category for which you made your rule 5 - find two bibs with items of the itype got which you made your rule 6 - place a hold on one bib. success! 7 - try to place a hold on the second bib. you're told you cannot because the patron is only allowed 1 hold 8 - apply patch, restart services 9 - try to place your second hold again, success! Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|21.05.00,20.11.01 |21.05.00,20.11.01,20.05.09 released in| | Status|Pushed to stable |Pushed to oldstable --- Comment #22 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Pushed to 20.05.x for 20.05.09 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 --- Comment #23 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Can't backport to 19.11.x: can't solve the many conflicts. 💥💥💥 If there is an interest in having this backported, please submit a patch for 19.11. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26634 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |caroline.cyr-la-rose@inlibr | |o.com -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org