[Bug 10272] New: CheckReserves returns not respecting ReservesControlBranch
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Bug ID: 10272 Summary: CheckReserves returns not respecting ReservesControlBranch Classification: Unclassified 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: kyle.m.hall@gmail.com CC: gmcharlt@gmail.com CheckReserves is using the CircControl system preference to determine what patrons an item can fill a hold for. It should be using ReservesControlBranch instead. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 18189 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=18189&action=edit Bug 10272 - CheckReserves returns not respecting ReservesControlBranch CheckReserves is using the CircControl system preference to determine what patrons an item can fill a hold for. It should be using ReservesControlBranch instead. Test Plan: 1) Create an item at Library A, place holds for it for patrons at Library B, Library C, and Library A in that order, for pickup at the patrons home library. 2) Make sure the holds policy for Library A is set to Hold Policy = "From home library" and Return Policy = "Item returns home". Make sure the holds policies for the other libraries are set to Hold Policy = "From any library". 3) Check the item in at Library C, the hold for the patron at Library B should pop up, even though it's in violation of the circulation rules. Don't click the confirm button! 4) Apply this patch, and reload the page, now the hold listed should be for the last hold, the hold for the patron at Library A, which is correct. This patch adds the subroutine C4::Reserves::GetReservesControlBranch as an equivilent to C4::Circulation::_GetCircControlBranch. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle.m.hall@gmail.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #18189|0 |1 is obsolete| | --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 18190 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=18190&action=edit Bug 10272 - CheckReserves returns not respecting ReservesControlBranch Perltidy'ed the new sub. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #18190|0 |1 is obsolete| | --- Comment #3 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 18208 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=18208&action=edit Bug 10272 - CheckReserves returns not respecting ReservesControlBranch CheckReserves is using the CircControl system preference to determine what patrons an item can fill a hold for. It should be using ReservesControlBranch instead. Test Plan: 1) Create an item at Library A, place holds for it for patrons at Library B, Library C, and Library A in that order, for pickup at the patrons home library. 2) Make sure the holds policy for Library A is set to Hold Policy = "From home library" and Return Policy = "Item returns home". Make sure the holds policies for the other libraries are set to Hold Policy = "From any library". 3) Check the item in at Library C, the hold for the patron at Library B should pop up, even though it's in violation of the circulation rules. Don't click the confirm button! 4) Apply this patch, and reload the page, now the hold listed should be for the last hold, the hold for the patron at Library A, which is correct. This patch adds the subroutine C4::Reserves::GetReservesControlBranch as an equivilent to C4::Circulation::_GetCircControlBranch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |jonathan.druart@biblibre.co | |m --- Comment #4 from Jonathan Druart <jonathan.druart@biblibre.com> --- Kyle, The new routine GetReservesControlBranch needs a unit test. Marked as Failed QA. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #18208|0 |1 is obsolete| | --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 18907 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=18907&action=edit Bug 10272 - CheckReserves returns not respecting ReservesControlBranch CheckReserves is using the CircControl system preference to determine what patrons an item can fill a hold for. It should be using ReservesControlBranch instead. Test Plan: 1) Create an item at Library A, place holds for it for patrons at Library B, Library C, and Library A in that order, for pickup at the patrons home library. 2) Make sure the holds policy for Library A is set to Hold Policy = "From home library" and Return Policy = "Item returns home". Make sure the holds policies for the other libraries are set to Hold Policy = "From any library". 3) Check the item in at Library C, the hold for the patron at Library B should pop up, even though it's in violation of the circulation rules. Don't click the confirm button! 4) Apply this patch, and reload the page, now the hold listed should be for the last hold, the hold for the patron at Library A, which is correct. This patch adds the subroutine C4::Reserves::GetReservesControlBranch as an equivilent to C4::Circulation::_GetCircControlBranch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- Good catch! Unit test added! (In reply to comment #4)
Kyle,
The new routine GetReservesControlBranch needs a unit test.
Marked as Failed QA.
-- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #18907|0 |1 is obsolete| | --- Comment #7 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 18922 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=18922&action=edit Bug 10272 - CheckReserves returns not respecting ReservesControlBranch CheckReserves is using the CircControl system preference to determine what patrons an item can fill a hold for. It should be using ReservesControlBranch instead. Test Plan: 1) Create an item at Library A, place holds for it for patrons at Library B, Library C, and Library A in that order, for pickup at the patrons home library. 2) Make sure the holds policy for Library A is set to Hold Policy = "From home library" and Return Policy = "Item returns home". Make sure the holds policies for the other libraries are set to Hold Policy = "From any library". 3) Check the item in at Library C, the hold for the patron at Library B should pop up, even though it's in violation of the circulation rules. Don't click the confirm button! 4) Apply this patch, and reload the page, now the hold listed should be for the last hold, the hold for the patron at Library A, which is correct. This patch adds the subroutine C4::Reserves::GetReservesControlBranch as an equivilent to C4::Circulation::_GetCircControlBranch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- Last patch changes the expected number of tests (4 to 6). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 --- Comment #9 from Kyle M Hall <kyle@bywatersolutions.com> --- Jonathan, did you mean to sign-off on this patch, or just revise the number of tests? Either way, thanks for fixing that! Kyle (In reply to comment #8)
Last patch changes the expected number of tests (4 to 6).
-- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 --- Comment #10 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to comment #9)
Jonathan, did you mean to sign-off on this patch, or just revise the number of tests? Either way, thanks for fixing that!
No I didn't test the patch, just fix the unit tests. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #18922|0 |1 is obsolete| | --- Comment #11 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 19012 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19012&action=edit [SIGNED-OFF] Bug 10272 - CheckReserves returns not respecting ReservesControlBranch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |julian.maurice@biblibre.com --- Comment #12 from Julian Maurice <julian.maurice@biblibre.com> --- Thanks for the test plan, Kyle :) Tested OK. Unit tests pass. Signed off. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff --- Comment #13 from Julian Maurice <julian.maurice@biblibre.com> --- In fact it seems that it don't work. I retested it with Jonathan and at step 4, the hold returned is still the same (the one for patron at library B) But why should the hold for library A's patron be returned whereas it has the lowest priority ? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #18922|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19012|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #18922|0 |1 is obsolete| | --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 19081 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19081&action=edit Bug 10272 - CheckReserves returns not respecting ReservesControlBranch CheckReserves is using the CircControl system preference to determine what patrons an item can fill a hold for. It should be using ReservesControlBranch instead. Test Plan: 1) Set ReservesControlBranch to "item's home library". 2) Create an item at Library A, place holds for it for patrons at Library B, Library C, and Library A in that order, for pickup at the patrons home library. 3) Make sure the holds policy for Library A is set to Hold Policy = "From home library" and Return Policy = "Item returns home". Make sure the holds policies for the other libraries are set to Hold Policy = "From any library". 4) Check the item in at Library C, the hold for the patron at Library B should pop up, even though it's in violation of the circulation rules. Don't click the confirm button! 5) Apply this patch, and reload the page, now the hold listed should be for the last hold, the hold for the patron at Library A, which is correct. This patch adds the subroutine C4::Reserves::GetReservesControlBranch as an equivilent to C4::Circulation::_GetCircControlBranch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- I forgot a step in the test plan! ReservesControlBranch needs to be set to "item's home library". (In reply to comment #13)
In fact it seems that it don't work. I retested it with Jonathan and at step 4, the hold returned is still the same (the one for patron at library B) But why should the hold for library A's patron be returned whereas it has the lowest priority ?
-- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19081|0 |1 is obsolete| | --- Comment #16 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 19435 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19435&action=edit Bug 10272 - CheckReserves returns not respecting ReservesControlBranch CheckReserves is using the CircControl system preference to determine what patrons an item can fill a hold for. It should be using ReservesControlBranch instead. Test Plan: 1) Set ReservesControlBranch to "item's home library". 2) Create an item at Library A, place holds for it for patrons at Library B, Library C, and Library A in that order, for pickup at the patrons home library. 3) Make sure the holds policy for Library A is set to Hold Policy = "From home library" and Return Policy = "Item returns home". Make sure the holds policies for the other libraries are set to Hold Policy = "From any library". 4) Check the item in at Library C, the hold for the patron at Library B should pop up, even though it's in violation of the circulation rules. Don't click the confirm button! 5) Apply this patch, and reload the page, now the hold listed should be for the last hold, the hold for the patron at Library A, which is correct. This patch adds the subroutine C4::Reserves::GetReservesControlBranch as an equivilent to C4::Circulation::_GetCircControlBranch. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply CC| |katrin.fischer@bsz-bw.de --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Kyle, can you please rebase and switch back to signed off? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19435|0 |1 is obsolete| | --- Comment #18 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 20778 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20778&action=edit Bug 10272 - CheckReserves returns not respecting ReservesControlBranch CheckReserves is using the CircControl system preference to determine what patrons an item can fill a hold for. It should be using ReservesControlBranch instead. Test Plan: 1) Set ReservesControlBranch to "item's home library". 2) Create an item at Library A, place holds for it for patrons at Library B, Library C, and Library A in that order, for pickup at the patrons home library. 3) Make sure the holds policy for Library A is set to Hold Policy = "From home library" and Return Policy = "Item returns home". Make sure the holds policies for the other libraries are set to Hold Policy = "From any library". 4) Check the item in at Library C, the hold for the patron at Library B should pop up, even though it's in violation of the circulation rules. Don't click the confirm button! 5) Apply this patch, and reload the page, now the hold listed should be for the last hold, the hold for the patron at Library A, which is correct. This patch adds the subroutine C4::Reserves::GetReservesControlBranch as an equivilent to C4::Circulation::_GetCircControlBranch. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20778|0 |1 is obsolete| | --- Comment #19 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 20864 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20864&action=edit [PASSED QA] Bug 10272 - CheckReserves returns not respecting ReservesControlBranch CheckReserves is using the CircControl system preference to determine what patrons an item can fill a hold for. It should be using ReservesControlBranch instead. Test Plan: 1) Set ReservesControlBranch to "item's home library". 2) Create an item at Library A, place holds for it for patrons at Library B, Library C, and Library A in that order, for pickup at the patrons home library. 3) Make sure the holds policy for Library A is set to Hold Policy = "From home library" and Return Policy = "Item returns home". Make sure the holds policies for the other libraries are set to Hold Policy = "From any library". 4) Check the item in at Library C, the hold for the patron at Library B should pop up, even though it's in violation of the circulation rules. Don't click the confirm button! 5) Apply this patch, and reload the page, now the hold listed should be for the last hold, the hold for the patron at Library A, which is correct. This patch adds the subroutine C4::Reserves::GetReservesControlBranch as an equivilent to C4::Circulation::_GetCircControlBranch. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Fixed POD so that arguments and explanation match (C<$item>). Also tested opac-reserves.pl for regressions. Passes all tests, QA script, and Reserves.t. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #20 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to master, along with a follow-up that adds a regression test. Thanks, Kyle! As a side-note, I care a /lot/ about there being regression tests, especially for changes that touch circulation and/or hold requests. While the unit test for the new routine was necessary, it wasn't sufficient. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |tomascohen@gmail.com --- Comment #21 from Tomás Cohen Arazi <tomascohen@gmail.com> --- This patch has been pushed to 3.12.x, will be in 3.12.6. Thanks Kyle! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10272 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bgkriegel@gmail.com --- Comment #22 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Pushed to 3.10.x, will be in 3.10.12 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org