[Bug 35432] New: Clarify and simplify the workings of MapItemsToHoldRequests
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35432 Bug ID: 35432 Summary: Clarify and simplify the workings of MapItemsToHoldRequests 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 This is a large and sparsely documented routine that could use some cleanup/explanation -- 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=35432 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch 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=35432 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 159351 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159351&action=edit Bug 35432: Add comments and POD -- 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=35432 --- Comment #2 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 159352 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159352&action=edit Bug 35432: Add _can_item_fill_request subroutine There are a series of repeated checks throughout MapItemsToHoldRequests. This patch simply consolidates them into a single routine that can be called. To test: 1 - prove t/db_dependent/HoldsQueue.t 2 - Confirm it passes before and after this patch is applied -- 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=35432 --- Comment #3 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 159353 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159353&action=edit Bug 35432: Simplay HoldsQueuePrioritize branch check The code here is going to check items held the the pickup location for a request or from the least cost branch matching the patron's home library against the HoldsQueuePrioritizeBranch setting The loop is the same in both case, so lets simplify this a bit to make the intent more clear To test: 1 - prove -v t/db_dependent/HoldsQueue.t 2 - It should pass before and after this patch -- 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=35432 --- Comment #4 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 159354 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159354&action=edit Bug 35432: Remove PULL_BRANCHES and use ifs over unless PULL_BRANCHES and PULL_BRANCHES2 felt a little arbitrary, moved to using a last in the outer loop. Swapped 'next unless' for an if to more directly show when we are acting -- 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=35432 --- Comment #5 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 159355 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159355&action=edit Bug 35432: Expand comments to explain the priority of filling requests -- 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=35432 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=33794 -- 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=35432 Phil Ringnalda <phil@chetcolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159351|0 |1 is obsolete| | --- Comment #6 from Phil Ringnalda <phil@chetcolibrary.org> --- Created attachment 159381 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159381&action=edit Bug 35432: Add comments and POD Signed-off-by: Phil Ringnalda <phil@chetcolibrary.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=35432 Phil Ringnalda <phil@chetcolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159352|0 |1 is obsolete| | --- Comment #7 from Phil Ringnalda <phil@chetcolibrary.org> --- Created attachment 159382 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159382&action=edit Bug 35432: Add _can_item_fill_request subroutine There are a series of repeated checks throughout MapItemsToHoldRequests. This patch simply consolidates them into a single routine that can be called. To test: 1 - prove t/db_dependent/HoldsQueue.t 2 - Confirm it passes before and after this patch is applied Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> -- 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=35432 Phil Ringnalda <phil@chetcolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159353|0 |1 is obsolete| | --- Comment #8 from Phil Ringnalda <phil@chetcolibrary.org> --- Created attachment 159383 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159383&action=edit Bug 35432: Simplay HoldsQueuePrioritize branch check The code here is going to check items held the the pickup location for a request or from the least cost branch matching the patron's home library against the HoldsQueuePrioritizeBranch setting The loop is the same in both case, so lets simplify this a bit to make the intent more clear To test: 1 - prove -v t/db_dependent/HoldsQueue.t 2 - It should pass before and after this patch Signed-off-by: Phil Ringnalda <phil@chetcolibrary.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=35432 Phil Ringnalda <phil@chetcolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159354|0 |1 is obsolete| | --- Comment #9 from Phil Ringnalda <phil@chetcolibrary.org> --- Created attachment 159384 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159384&action=edit Bug 35432: Remove PULL_BRANCHES and use ifs over unless PULL_BRANCHES and PULL_BRANCHES2 felt a little arbitrary, moved to using a last in the outer loop. Swapped 'next unless' for an if to more directly show when we are acting Signed-off-by: Phil Ringnalda <phil@chetcolibrary.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=35432 Phil Ringnalda <phil@chetcolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159355|0 |1 is obsolete| | --- Comment #10 from Phil Ringnalda <phil@chetcolibrary.org> --- Created attachment 159385 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159385&action=edit Bug 35432: Expand comments to explain the priority of filling requests Signed-off-by: Phil Ringnalda <phil@chetcolibrary.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=35432 Phil Ringnalda <phil@chetcolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |phil@chetcolibrary.org Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | Status|Needs Signoff |Signed Off -- 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=35432 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |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=35432 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |enhancement --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Moving to enh. Correct me if I am mistaken. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35432 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159381|0 |1 is obsolete| | --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 162194 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162194&action=edit Bug 35432: Add comments and POD Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> 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=35432 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159382|0 |1 is obsolete| | --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 162195 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162195&action=edit Bug 35432: Add _can_item_fill_request subroutine There are a series of repeated checks throughout MapItemsToHoldRequests. This patch simply consolidates them into a single routine that can be called. To test: 1 - prove t/db_dependent/HoldsQueue.t 2 - Confirm it passes before and after this patch is applied Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> 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=35432 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159383|0 |1 is obsolete| | --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 162196 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162196&action=edit Bug 35432: Simplay HoldsQueuePrioritize branch check The code here is going to check items held the the pickup location for a request or from the least cost branch matching the patron's home library against the HoldsQueuePrioritizeBranch setting The loop is the same in both case, so lets simplify this a bit to make the intent more clear To test: 1 - prove -v t/db_dependent/HoldsQueue.t 2 - It should pass before and after this patch Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> 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=35432 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159384|0 |1 is obsolete| | --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 162197 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162197&action=edit Bug 35432: Remove PULL_BRANCHES and use ifs over unless PULL_BRANCHES and PULL_BRANCHES2 felt a little arbitrary, moved to using a last in the outer loop. Swapped 'next unless' for an if to more directly show when we are acting Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> 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=35432 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159385|0 |1 is obsolete| | --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 162198 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162198&action=edit Bug 35432: Expand comments to explain the priority of filling requests Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> 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=35432 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Code is considerably clearer, unit tests all continue to pass 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=35432 --- Comment #18 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I wonder if we should not have tests for MapItemsToHoldRequests as well? But all holds related tests I could find still pass... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35432 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |24.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=35432 --- Comment #19 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35432 Andreas Jonsson <andreas.jonsson@kreablo.se> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andreas.jonsson@kreablo.se --- Comment #20 from Andreas Jonsson <andreas.jonsson@kreablo.se> --- *** Bug 35805 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35432 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Status|Pushed to main |Needs documenting --- Comment #21 from Fridolin Somers <fridolin.somers@biblibre.com> --- Not backported to 23.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35432 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|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=35432 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Needs documenting |RESOLVED CC| |caroline.cyr-la-rose@inlibr | |o.com --- Comment #22 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- Looks like backend modifications. Nothing to add/edit in the Koha manual. Please reopen with more details if that is not the case! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org