[Bug 27071] New: Hold pickup library match not enforced correctly on intranet when using hold groups
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 Bug ID: 27071 Summary: Hold pickup library match not enforced correctly on intranet when using hold groups 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 Depends on: 22284 To recreate: - Create two local hold groups containing distinct lists of libraries. - In default rules for all libraries, set Hold Policy = "From local hold group" and Hold pickup library match to "Patron's hold group" - Make sure AllowHoldPolicyOverride is set to Don't Allow - Make sure you're logged in at a library in Group 1 - Find a bib with only 1 item from Group 1. Confirm you can place a hold on this title for a patron in Group 1 (correct), but not for a patron in Group 2 (correct). - Find a bib with only 1 item from Group 2. Confirm you cannot place a hold for a patron from Group 1 (correct), BUT you also cannot place a hold for a patron from Group 2 (incorrect) -- Koha gives the erroneous message "pickupNotInHoldGroup" - Change your library to a something in Group 2 - Find a bib with only 1 item from Group 1. Confirm you cannot place a hold on this title for a patron in Group 1 (incorrect), and not for a patron in Group 2 (correct). - Find a bib with only 1 item from Group 2. Confirm you cannot place a hold for a patron from Group 1 (correct), but you can place a hold for a patron from Group 2 (correct) Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22284 [Bug 22284] Add ability to define groups of locations for hold pickup -- 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=27071 --- Comment #1 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Confirmed this behavior in both master and 20.05. Holds from the OPAC can be placed correctly. -- 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=27071 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com, | |lisetteslatah@gmail.com, | |nick@bywatersolutions.com, | |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 113928 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113928&action=edit Bug 27071: Fix Hold pickup library when using hold groups -- 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=27071 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 113929 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113929&action=edit Bug 27071: Code simplification -- 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=27071 --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- In my understanding Koha::Library->validate_hold_sibling is plain wrong. It only tests for the first from @hold_libraries is not the one we are looking for, then it's considered an invalid one. However with this patch we have tests from t/db_dependent/Holds.t that are failing (maybe others as well). -- 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=27071 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Also, we should not see "pickupNotInHoldGroup" on the interface, the error code is not handled correctly to display a correct message to the user. -- 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=27071 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |agustinmoyano@theke.io |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=27071 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #3)
Created attachment 113929 [details] [review] Bug 27071: Code simplification
I've rather use List::MoreUtils::any, right? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Tomás Cohen Arazi from comment #6)
(In reply to Jonathan Druart from comment #3)
Created attachment 113929 [details] [review] [review] Bug 27071: Code simplification
I've rather use List::MoreUtils::any, right?
Well, if you want. But we don't need a module for a grep :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 113963 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=113963&action=edit Bug 27071: More tests for Koha::Library->validate_hold_sibling This patch adds tests for more than one non-same sibling libraries in the hold group. Tests pass which means it is working as expected. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Libraries.t => SUCCESS: Tests pass -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I wrote some tests trying to capture the failing situation. I didn't manage to make them fail, though. The fact that this works correctly in OPAC, makes me think of controller issues. I will try following Andrew's test plan now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- The problem is indeed in the controller: reserve/request.pl 554 my $can_item_be_reserved = CanItemBeReserved( $patron->borrowernumber, $itemnumber, $pickup )->{status}; $pickup is coming from: 70 my $pickup = $input->param('pickup') || C4::Context->userenv->{'branch'}; So we are testing if the hold can be picked up at the logged in library of the librarian. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|agustinmoyano@theke.io |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113928|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=27071 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113929|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=27071 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #113963|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=27071 --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 116599 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116599&action=edit Bug 27071: Fix Hold pickup library when using hold groups Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 116600 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116600&action=edit Bug 27071: Code simplification Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 116601 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116601&action=edit Bug 27071: More tests for Koha::Library->validate_hold_sibling This patch adds tests for more than one non-same sibling libraries in the hold group. Tests pass which means it is working as expected. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Libraries.t => SUCCESS: Tests pass Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 116602 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116602&action=edit Bug 27071: Use GetReservesControlBranch to pick the branch Pretty much like the opac-reserve.pl does, this patch makes the staff request.pl script get the branch from the specialized routine from C4::Reserves instead of falling back to userenv. To test: 1. Follow the original test plan: - Create two local hold groups containing distinct lists of libraries. - In default rules for all libraries, set Hold Policy = "From local hold group" and Hold pickup library match to "Patron's hold group" - Make sure AllowHoldPolicyOverride is set to Don't Allow - Make sure you're logged in at a library in Group 1 - Find a bib with only 1 item from Group 1. Confirm you can place a hold on this title for a patron in Group 1 (correct), but not for a patron in Group 2 (correct). - Find a bib with only 1 item from Group 2. Confirm you cannot place a hold for a patron from Group 1 (correct), BUT you also cannot place a hold for a patron from Group 2 (incorrect) -- Koha gives the erroneous message "pickupNotInHoldGroup" - Change your library to a something in Group 2 - Find a bib with only 1 item from Group 1. Confirm you cannot place a hold on this title for a patron in Group 1 (incorrect), and not for a patron in Group 2 (correct). - Find a bib with only 1 item from Group 2. Confirm you cannot place a hold for a patron from Group 1 (correct), but you can place a hold for a patron from Group 2 (correct) => FAIL: Things expected to fail on the plan, fail. 2. Apply this patches 3. Repeat 1 => SUCCESS: It now works! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 116603 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116603&action=edit Bug 27071: (follow-up) Provide a more meaningful error message Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=27664 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Status|Needs Signoff |Patch doesn't apply --- Comment #16 from David Nind <david@davidnind.com> --- Patch no longer applies 8-( Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 27071: Fix Hold pickup library when using hold groups Applying: Bug 27071: Code simplification error: sha1 information is lacking or useless (Koha/Library.pm). error: could not build fake ancestor Patch failed at 0001 Bug 27071: Code simplification -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 Tomás Cohen Arazi <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=27071 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116600|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=27071 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116601|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=27071 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116602|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=27071 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116603|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=27071 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116599|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=27071 --- Comment #17 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 117140 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117140&action=edit Bug 27071: Fix Hold pickup library when using hold groups Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 --- Comment #18 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 117141 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117141&action=edit Bug 27071: Code simplification Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 --- Comment #19 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 117142 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117142&action=edit Bug 27071: More tests for Koha::Library->validate_hold_sibling This patch adds tests for more than one non-same sibling libraries in the hold group. Tests pass which means it is working as expected. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Libraries.t => SUCCESS: Tests pass Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 --- Comment #20 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 117143 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117143&action=edit Bug 27071: Use GetReservesControlBranch to pick the branch Pretty much like the opac-reserve.pl does, this patch makes the staff request.pl script get the branch from the specialized routine from C4::Reserves instead of falling back to userenv. To test: 1. Follow the original test plan: - Create two local hold groups containing distinct lists of libraries. - In default rules for all libraries, set Hold Policy = "From local hold group" and Hold pickup library match to "Patron's hold group" - Make sure AllowHoldPolicyOverride is set to Don't Allow - Make sure you're logged in at a library in Group 1 - Find a bib with only 1 item from Group 1. Confirm you can place a hold on this title for a patron in Group 1 (correct), but not for a patron in Group 2 (correct). - Find a bib with only 1 item from Group 2. Confirm you cannot place a hold for a patron from Group 1 (correct), BUT you also cannot place a hold for a patron from Group 2 (incorrect) -- Koha gives the erroneous message "pickupNotInHoldGroup" - Change your library to a something in Group 2 - Find a bib with only 1 item from Group 1. Confirm you cannot place a hold on this title for a patron in Group 1 (incorrect), and not for a patron in Group 2 (correct). - Find a bib with only 1 item from Group 2. Confirm you cannot place a hold for a patron from Group 1 (correct), but you can place a hold for a patron from Group 2 (correct) => FAIL: Things expected to fail on the plan, fail. 2. Apply this patches 3. Repeat 1 => SUCCESS: It now works! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 --- Comment #21 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 117144 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117144&action=edit Bug 27071: (follow-up) Provide a more meaningful error message Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 --- Comment #22 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 117145 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117145&action=edit Bug 27071: (follow-up) Get rid of loop This patch gets rid of a loop on the librarary's groups siblings, and acknowledges ->get_hold_libraries returns a proper Koha::Libraries iterator that can be chained with a new query on the passed branchcode. This way we only fetch a count from the DB and no loop needs to take place on the code. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Libraries.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 --- Comment #23 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #7)
(In reply to Tomás Cohen Arazi from comment #6)
(In reply to Jonathan Druart from comment #3)
Created attachment 113929 [details] [review] [review] [review] Bug 27071: Code simplification
I've rather use List::MoreUtils::any, right?
Well, if you want. But we don't need a module for a grep :)
I wen't further and completely removed grep :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 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=27071 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117140|0 |1 is obsolete| | --- Comment #24 from David Nind <david@davidnind.com> --- Created attachment 117184 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117184&action=edit Bug 27071: Fix Hold pickup library when using hold groups Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=27071 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117141|0 |1 is obsolete| | --- Comment #25 from David Nind <david@davidnind.com> --- Created attachment 117185 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117185&action=edit Bug 27071: Code simplification Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=27071 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117142|0 |1 is obsolete| | --- Comment #26 from David Nind <david@davidnind.com> --- Created attachment 117186 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117186&action=edit Bug 27071: More tests for Koha::Library->validate_hold_sibling This patch adds tests for more than one non-same sibling libraries in the hold group. Tests pass which means it is working as expected. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Libraries.t => SUCCESS: Tests pass Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=27071 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117143|0 |1 is obsolete| | --- Comment #27 from David Nind <david@davidnind.com> --- Created attachment 117187 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117187&action=edit Bug 27071: Use GetReservesControlBranch to pick the branch Pretty much like the opac-reserve.pl does, this patch makes the staff request.pl script get the branch from the specialized routine from C4::Reserves instead of falling back to userenv. To test: 1. Follow the original test plan: - Create two local hold groups containing distinct lists of libraries. - In default rules for all libraries, set Hold Policy = "From local hold group" and Hold pickup library match to "Patron's hold group" - Make sure AllowHoldPolicyOverride is set to Don't Allow - Make sure you're logged in at a library in Group 1 - Find a bib with only 1 item from Group 1. Confirm you can place a hold on this title for a patron in Group 1 (correct), but not for a patron in Group 2 (correct). - Find a bib with only 1 item from Group 2. Confirm you cannot place a hold for a patron from Group 1 (correct), BUT you also cannot place a hold for a patron from Group 2 (incorrect) -- Koha gives the erroneous message "pickupNotInHoldGroup" - Change your library to a something in Group 2 - Find a bib with only 1 item from Group 1. Confirm you cannot place a hold on this title for a patron in Group 1 (incorrect), and not for a patron in Group 2 (correct). - Find a bib with only 1 item from Group 2. Confirm you cannot place a hold for a patron from Group 1 (correct), but you can place a hold for a patron from Group 2 (correct) => FAIL: Things expected to fail on the plan, fail. 2. Apply this patches 3. Repeat 1 => SUCCESS: It now works! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=27071 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117144|0 |1 is obsolete| | --- Comment #28 from David Nind <david@davidnind.com> --- Created attachment 117188 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117188&action=edit Bug 27071: (follow-up) Provide a more meaningful error message Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=27071 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117145|0 |1 is obsolete| | --- Comment #29 from David Nind <david@davidnind.com> --- Created attachment 117189 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117189&action=edit Bug 27071: (follow-up) Get rid of loop This patch gets rid of a loop on the librarary's groups siblings, and acknowledges ->get_hold_libraries returns a proper Koha::Libraries iterator that can be chained with a new query on the passed branchcode. This way we only fetch a count from the DB and no loop needs to take place on the code. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Libraries.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=27071 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com 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=27071 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117184|0 |1 is obsolete| | --- Comment #30 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117204 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117204&action=edit Bug 27071: Fix Hold pickup library when using hold groups Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=27071 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117185|0 |1 is obsolete| | --- Comment #31 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117205 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117205&action=edit Bug 27071: Code simplification Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=27071 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117186|0 |1 is obsolete| | --- Comment #32 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117206 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117206&action=edit Bug 27071: More tests for Koha::Library->validate_hold_sibling This patch adds tests for more than one non-same sibling libraries in the hold group. Tests pass which means it is working as expected. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Libraries.t => SUCCESS: Tests pass Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=27071 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117187|0 |1 is obsolete| | --- Comment #33 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117207 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117207&action=edit Bug 27071: Use GetReservesControlBranch to pick the branch Pretty much like the opac-reserve.pl does, this patch makes the staff request.pl script get the branch from the specialized routine from C4::Reserves instead of falling back to userenv. To test: 1. Follow the original test plan: - Create two local hold groups containing distinct lists of libraries. - In default rules for all libraries, set Hold Policy = "From local hold group" and Hold pickup library match to "Patron's hold group" - Make sure AllowHoldPolicyOverride is set to Don't Allow - Make sure you're logged in at a library in Group 1 - Find a bib with only 1 item from Group 1. Confirm you can place a hold on this title for a patron in Group 1 (correct), but not for a patron in Group 2 (correct). - Find a bib with only 1 item from Group 2. Confirm you cannot place a hold for a patron from Group 1 (correct), BUT you also cannot place a hold for a patron from Group 2 (incorrect) -- Koha gives the erroneous message "pickupNotInHoldGroup" - Change your library to a something in Group 2 - Find a bib with only 1 item from Group 1. Confirm you cannot place a hold on this title for a patron in Group 1 (incorrect), and not for a patron in Group 2 (correct). - Find a bib with only 1 item from Group 2. Confirm you cannot place a hold for a patron from Group 1 (correct), but you can place a hold for a patron from Group 2 (correct) => FAIL: Things expected to fail on the plan, fail. 2. Apply this patches 3. Repeat 1 => SUCCESS: It now works! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=27071 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117188|0 |1 is obsolete| | --- Comment #34 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117208 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117208&action=edit Bug 27071: (follow-up) Provide a more meaningful error message Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=27071 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117189|0 |1 is obsolete| | --- Comment #35 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 117209 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117209&action=edit Bug 27071: (follow-up) Get rid of loop This patch gets rid of a loop on the librarary's groups siblings, and acknowledges ->get_hold_libraries returns a proper Koha::Libraries iterator that can be chained with a new query on the passed branchcode. This way we only fetch a count from the DB and no loop needs to take place on the code. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Libraries.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=27071 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #36 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I like how this was broken down into nice manageable chunks :) Works as expected, passes QA scripts, includes tests and passes all tests.. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |When using library groups, release notes| |the rules for placing holds | |did not always work as | |expected. This fixes these | |rules so that when patrons | |are part of a library in a | |group, they can only place | |a hold for items held in | |that library group. It also | |improves the error | |messages. | | | |Example: | |- There | |are two library groups with | |distinct libraries in each | |(Group A and B). | |- Default | |rules for all libraries | |are: Hold Policy = "From | |local hold group" and Hold | |pickup library match to | |"Patron's hold group", | |AllowHoldPolicyOverride is | |Don't allow. | |- You can | |place a hold for a patron | |that belongs to one of the | |Group A libraries, for an | |item only held in a Group A | |library. | |- You can't place | |a hold for that item for a | |patron from a Group B | |library. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 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=27071 --- Comment #37 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=27071 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable Version(s)|21.05.00 |21.05.00,20.11.04 released in| | CC| |fridolin.somers@biblibre.co | |m --- Comment #38 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 20.11.x for 20.11.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|21.05.00,20.11.04 |21.05.00,20.11.04,20.05.10 released in| | Status|Pushed to stable |Pushed to oldstable --- Comment #39 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Pushed to 20.05.x for 20.05.10 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |RESOLVED Resolution|--- |FIXED CC| |victor@tuxayo.net --- Comment #40 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Missing dependencies for 19.11.x, not affected, no backport. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |28118 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28118 [Bug 28118] Fix missing "selected" attribute in "Pickup at" dropdown -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27071 Joonas Kylmälä <joonas.kylmala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |28541 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28541 [Bug 28541] Incorrect default pickup location passed to HTML, causing UI show items unreservable -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org