[Bug 42385] New: Extract _GetCircControlBranch into Koha::Policy::Circulation
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42385 Bug ID: 42385 Summary: Extract _GetCircControlBranch into Koha::Policy::Circulation Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement 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 Depends on: 41728 _GetCircControlBranch is a private function in C4::Circulation that is exported (the code itself comments "This is wrong!") and used from Koha::Item, Koha::Recall, and multiple places in C4::Circulation. It resolves which library controls circulation rules given the CircControl and HomeOrHoldingBranch system preferences. This is the exact same pattern as Koha::Policy::Holds->holds_control_library, which resolves the controlling library for hold rules based on ReservesControlBranch. This bug proposes: - Add Koha::Policy::Circulation with a circ_control_library($item, $patron) class method - Replace all _GetCircControlBranch calls with the new method - Remove the exported private function from C4::Circulation - Add unit tests This is a prerequisite for the checkout availability work, as the future Koha::Item::Checkout::Availability class would need to resolve the controlling library. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41728 [Bug 41728] Add `Koha::Item::Checkin::Availability` to centralize logic -- 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=42385 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |42386 Blocks| |42387 Patch complexity|--- |Small patch Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Status|NEW |Needs Signoff Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42386 [Bug 42386] Unify hold availability checks into Koha::Availability pattern https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42387 [Bug 42387] [UMBRELLA] Extend Koha::Availability pattern to checkout and hold operations -- 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=42385 --- Comment #1 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198614 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198614&action=edit Bug 42385: Add Koha::Policy::Circulation Adds Koha::Policy::Circulation->circ_control_library($item, $patron) class method that resolves which library controls circulation rules, based on the CircControl and HomeOrHoldingBranch system preferences. This follows the same pattern as Koha::Policy::Holds->holds_control_library. An optional pickup_library_id parameter allows callers in API/daemon contexts to pass the library explicitly instead of relying on userenv. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Policy/Circulation.t => SUCCESS: Tests pass! 3. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42385 --- Comment #2 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198615 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198615&action=edit Bug 42385: Use Koha::Policy::Circulation everywhere Replaces all calls to _GetCircControlBranch with Koha::Policy::Circulation->circ_control_library in: - C4::Circulation (10 call sites) - Koha::Item (1 call site) - Koha::Recall (1 call site) - reserve/request.pl (1 call site) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Policy/Circulation.t \ t/db_dependent/Circulation.t \ t/db_dependent/Koha/Items.t \ t/db_dependent/Koha/Recall.t => SUCCESS: Tests pass! 3. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42385 --- Comment #3 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198616 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198616&action=edit Bug 42385: Remove _GetCircControlBranch Removes the deprecated _GetCircControlBranch function and its export from C4::Circulation. Removes the corresponding tests from Circulation.t (now covered by t/db_dependent/Koha/Policy/Circulation.t). Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Circulation.t \ t/db_dependent/Koha/Policy/Circulation.t => SUCCESS: Tests pass! 3. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42385 Alex Carver [Acerock7] <alex@rcls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |alex@rcls.org Status|Needs Signoff |Patch doesn't apply --- Comment #4 from Alex Carver [Acerock7] <alex@rcls.org> --- Patch application failed for attachment 198616 - Bug 42385: Remove _GetCircControlBranch Error: Apply failed: Git command (git am -3 /tmp/ycBzVQ_95h/0003-198616.patch) failed: error: sha1 information is lacking or useless (C4/Circulation.pm). error: could not build fake ancestor -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42385 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|42387 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42387 [Bug 42387] [UMBRELLA] Extend Koha::Availability pattern to checkout and hold operations -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42385 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.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=42385 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198614|0 |1 is obsolete| | Attachment #198615|0 |1 is obsolete| | Attachment #198616|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=42385 --- Comment #5 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 200348 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200348&action=edit Bug 42385: Add Koha::Policy::Circulation Adds Koha::Policy::Circulation->circ_control_library($item, $patron) class method that resolves which library controls circulation rules, based on the CircControl and HomeOrHoldingBranch system preferences. This follows the same pattern as Koha::Policy::Holds->holds_control_library. An optional pickup_library_id parameter allows callers in API/daemon contexts to pass the library explicitly instead of relying on userenv. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Policy/Circulation.t => SUCCESS: Tests pass! 3. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42385 --- Comment #6 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 200350 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200350&action=edit Bug 42385: Use Koha::Policy::Circulation everywhere Replaces all calls to _GetCircControlBranch with Koha::Policy::Circulation->circ_control_library in: - C4::Circulation (10 call sites) - Koha::Item (1 call site) - Koha::Recall (1 call site) - reserve/request.pl (1 call site) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Policy/Circulation.t \ t/db_dependent/Circulation.t \ t/db_dependent/Koha/Items.t \ t/db_dependent/Koha/Recall.t => SUCCESS: Tests pass! 3. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42385 --- Comment #7 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 200352 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200352&action=edit Bug 42385: Remove _GetCircControlBranch Removes the deprecated _GetCircControlBranch function and its export from C4::Circulation. Removes the corresponding tests from Circulation.t (now covered by t/db_dependent/Koha/Policy/Circulation.t). Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Circulation.t \ t/db_dependent/Koha/Policy/Circulation.t => SUCCESS: Tests pass! 3. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42385 Alex Carver [Acerock7] <alex@rcls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #8 from Alex Carver [Acerock7] <alex@rcls.org> --- 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=42385 Alex Carver [Acerock7] <alex@rcls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #200348|0 |1 is obsolete| | Attachment #200350|0 |1 is obsolete| | Attachment #200352|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=42385 --- Comment #9 from Alex Carver [Acerock7] <alex@rcls.org> --- Created attachment 200357 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200357&action=edit Bug 42385: Add Koha::Policy::Circulation Adds Koha::Policy::Circulation->circ_control_library($item, $patron) class method that resolves which library controls circulation rules, based on the CircControl and HomeOrHoldingBranch system preferences. This follows the same pattern as Koha::Policy::Holds->holds_control_library. An optional pickup_library_id parameter allows callers in API/daemon contexts to pass the library explicitly instead of relying on userenv. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Policy/Circulation.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Alex Carver [Acerock7] <alex@rcls.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42385 --- Comment #10 from Alex Carver [Acerock7] <alex@rcls.org> --- Created attachment 200358 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200358&action=edit Bug 42385: Use Koha::Policy::Circulation everywhere Replaces all calls to _GetCircControlBranch with Koha::Policy::Circulation->circ_control_library in: - C4::Circulation (10 call sites) - Koha::Item (1 call site) - Koha::Recall (1 call site) - reserve/request.pl (1 call site) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Policy/Circulation.t \ t/db_dependent/Circulation.t \ t/db_dependent/Koha/Items.t \ t/db_dependent/Koha/Recall.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Alex Carver [Acerock7] <alex@rcls.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42385 --- Comment #11 from Alex Carver [Acerock7] <alex@rcls.org> --- Created attachment 200359 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200359&action=edit Bug 42385: Remove _GetCircControlBranch Removes the deprecated _GetCircControlBranch function and its export from C4::Circulation. Removes the corresponding tests from Circulation.t (now covered by t/db_dependent/Koha/Policy/Circulation.t). Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Circulation.t \ t/db_dependent/Koha/Policy/Circulation.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Alex Carver [Acerock7] <alex@rcls.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42385 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|Small patch |Medium patch QA Contact|testopia@bugs.koha-communit |martin.renvoize@openfifth.c |y.org |o.uk 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=42385 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #200357|0 |1 is obsolete| | Attachment #200358|0 |1 is obsolete| | Attachment #200359|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=42385 --- Comment #12 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 200677 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200677&action=edit Bug 42385: Add Koha::Policy::Circulation Adds Koha::Policy::Circulation->circ_control_library($item, $patron) class method that resolves which library controls circulation rules, based on the CircControl and HomeOrHoldingBranch system preferences. This follows the same pattern as Koha::Policy::Holds->holds_control_library. An optional pickup_library_id parameter allows callers in API/daemon contexts to pass the library explicitly instead of relying on userenv. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Policy/Circulation.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Alex Carver [Acerock7] <alex@rcls.org> 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=42385 --- Comment #13 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 200678 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200678&action=edit Bug 42385: Use Koha::Policy::Circulation everywhere Replaces all calls to _GetCircControlBranch with Koha::Policy::Circulation->circ_control_library in: - C4::Circulation (10 call sites) - Koha::Item (1 call site) - Koha::Recall (1 call site) - reserve/request.pl (1 call site) - svc/letters/preview (2 call sites) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Policy/Circulation.t \ t/db_dependent/Circulation.t \ t/db_dependent/Koha/Items.t \ t/db_dependent/Koha/Recall.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Alex Carver [Acerock7] <alex@rcls.org> 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=42385 --- Comment #14 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 200679 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200679&action=edit Bug 42385: Remove _GetCircControlBranch Removes the deprecated _GetCircControlBranch function and its export from C4::Circulation. Removes the corresponding tests from Circulation.t (now covered by t/db_dependent/Koha/Policy/Circulation.t). Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Circulation.t \ t/db_dependent/Koha/Policy/Circulation.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Alex Carver [Acerock7] <alex@rcls.org> 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=42385 --- Comment #15 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 200680 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200680&action=edit Bug 42385: (QA follow-up) Test PickupLibrary fall-through to ItemHomeLibrary The original _GetCircControlBranch tests covered the case where CircControl is PickupLibrary but no userenv branch is available, in which case the resolution falls through to the ItemHomeLibrary behaviour. That coverage was lost when the tests moved to t/db_dependent/Koha/Policy/Circulation.t, since the PickupLibrary subtest always mocks a userenv. This restores coverage for the fall-through path: with CircControl set to PickupLibrary, no userenv, and no pickup_library_id, the controlling library is resolved from the item's home library. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Policy/Circulation.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org