[Bug 5801] New: C4::Circulation::_GetCircControlBranch doesn't work in opac-reserve.pl
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 Bug #: 5801 Summary: C4::Circulation::_GetCircControlBranch doesn't work in opac-reserve.pl Classification: Unclassified Change sponsored?: --- Product: Koha Version: rel_3_2 Platform: All OS/Version: All Status: NEW Severity: normal Priority: P5 Component: OPAC AssignedTo: oleonard@myacpl.org ReportedBy: fridolyn.somers@gmail.com QAContact: koha-bugs@lists.koha-community.org In opac-reserve.pl : my $branch = C4::Circulation::_GetCircControlBranch($itemLoopIter, $borr); This doesn't work because _GetCircControlBranch looks for homebranch nor holdingbranch. But $itemLoopIter doesn't contain those. I propose : my $branch = C4::Circulation::_GetCircControlBranch( $itemInfo , $borr); -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 --- Comment #1 from Owen Leonard <oleonard@myacpl.org> 2011-02-25 15:46:28 UTC --- Can you describe the problem this bug causes and how to test the proposed solution? -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 --- Comment #2 from Fridolyn SOMERS <fridolyn.somers@gmail.com> 2011-02-25 16:09:56 UTC --- (In reply to comment #1)
Can you describe the problem this bug causes and how to test the proposed solution?
You define a quota of holds in the issuing rules of your branch and borrower category. Despite this, you can't perform a hold. With the correction, the hold is possible. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 --- Comment #3 from Fridolyn SOMERS <fridolyn.somers@gmail.com> 2011-02-25 16:13:07 UTC --- (In reply to comment #2)
(In reply to comment #1)
Can you describe the problem this bug causes and how to test the proposed solution?
You define a quota of holds in the issuing rules of your branch and borrower category. Despite this, you can't perform a hold.
With the correction, the hold is possible.
System pref ReservesControlBranch = ItemHomeLibrary -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 --- Comment #4 from Owen Leonard <oleonard@myacpl.org> 2011-02-25 16:46:31 UTC ---
You define a quota of holds in the issuing rules of your branch and borrower category. Despite this, you can't perform a hold.
Can you be more specific about the conditions under which one cannot place a hold? I don't have any problems placing a hold with or without this change. In fact, I would expect the rule limiting [Library 1] Staff patrons to 2 holds would prevent me from placing more than 2 holds on items from [Library 1] but I'm not being prevented from doing so. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 Ian Walls <ian.walls@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |ian.walls@bywatersolutions. | |com --- Comment #5 from Ian Walls <ian.walls@bywatersolutions.com> 2011-12-05 21:23:49 UTC --- Just looking at the code, it's pretty clear there are some problems; _GetCircControlBranch is an internal function (hence the _ at the beginning), so it shouldn't be called in this script. Also, as Fridoyln points out, the hashref passed doesn't contain the necessary information, so results are inaccurate. And shouldn't we be using ReservesControlBranch, not CircControl and HomeOrHoldingBranch, to get our rules for a hold? -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |gmcharlt@gmail.com Component|OPAC |Hold requests Version|rel_3_2 |master Assignee|oleonard@myacpl.org |koha.sekjal@gmail.com QA Contact|koha-bugs@lists.koha-commun |koha.sekjal@gmail.com |ity.org | -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Assignee|koha.sekjal@gmail.com |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 12625 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=12625&action=edit Bug 5801 - C4::Circulation::_GetCircControlBranch doesn't work in opac-reserve.pl -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 --- Comment #7 from Kyle M Hall <kyle@bywatersolutions.com> --- Note, I think there is another improper use of C4::Circulation::_GetCircControlBranch in C4::Reserves::CheckReserves -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 --- Comment #8 from Owen Leonard <oleonard@myacpl.org> --- (In reply to comment #2)
(In reply to comment #1)
Can you describe the problem this bug causes and how to test the proposed solution?
You define a quota of holds in the issuing rules of your branch and borrower category. Despite this, you can't perform a hold.
I still don't understand how to reproduce this bug. I have ReservesControlBranch set to 'ItemHomeLibrary' and I don't have any problems placing a hold without this patch. Kyle, were you able to reproduce the problem? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 --- Comment #9 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to comment #8)
Kyle, were you able to reproduce the problem?
Yes, I was able to reproduce the problem, but that was a while back. I think this patch is still valid because we shouldn't be using _GetCircControlBranch here anyway, as this is reserves related. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 --- Comment #10 from Owen Leonard <oleonard@myacpl.org> --- (In reply to comment #9)
I think this patch is still valid because we shouldn't be using _GetCircControlBranch here anyway, as this is reserves related.
Does the patch solve a problem with the way holds work? If so, how does one reproduce the problem and test the patch? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to comment #10)
(In reply to comment #9)
I think this patch is still valid because we shouldn't be using _GetCircControlBranch here anyway, as this is reserves related.
Does the patch solve a problem with the way holds work? If so, how does one reproduce the problem and test the patch?
The problem is that CircControl is meant to return the library code to be used to determine which circulation policy applies to a transaction, *not* reserves policy. So, a library has set CircControl to use the checkout and fines rules of the library the item is from. The library has also set ReservesControlBranch to check the patron's home library to see if the patron can place a hold on the item. In this case, the reserves rules used will be those of the item's home library, not those of the patron's home library. This seems like a fairly serious bug. Koha is not doing what it says it will do. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@bigballofwax.co.nz Patch complexity|--- |Trivial patch Summary|C4::Circulation::_GetCircCo |C4::Circulation::_GetCircCo |ntrolBranch doesn't work in |ntrolBranch should not be |opac-reserve.pl |used in opac-reserve.pl Severity|normal |major --- Comment #12 from Chris Cormack <chris@bigballofwax.co.nz> --- Updating the bug title to more accurately reflect the problem. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #12625|0 |1 is obsolete| | --- Comment #13 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 13718 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=13718&action=edit Bug 5801 - C4::Circulation::_GetCircControlBranch wrongly used in opac-reserve.pl Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 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=5801 --- Comment #14 from Fridolyn SOMERS <fridolyn.somers@biblibre.com> --- Should _GetCircControlBranch also be removed from C4::Reserves::CheckReserves ? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de QA Contact|koha.sekjal@gmail.com |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 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=5801 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #13718|0 |1 is obsolete| | --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 13755 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=13755&action=edit [SIGNED-OFF] Bug 5801 - C4::Circulation::_GetCircControlBranch wrongly used in opac-reserve.pl Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> All tests and perlcritic pass. Tests done: holds policy vs. circulation rules seemed to make the difference for me, I am not totally sure why that is, but see no reason to fail this patch as results are ok. System preferences - CircControlBranch = the library the item is from - ReservesControlBranch = patron's home library Circulation rules - Library A - Holds policy: Books can not be put on hold. - Library B - Holds policy: Books can be put on hold. (no special setting needed) Catalog data - A record with an item for library B, itemtype Books. Test without patch: 1) Patron A (home library A) places a hold in OPAC on an item from library B. Should be: hold is NOT allowed. Before patch: hold IS allowed. After patch: hold is NOT allowed. OK 2) ReserveControlbranch = item's home library Patron A (home library A) places a hold in OPAC on an item from library B. Should be: hold is allowed. Before patch: hold is allowed. After patch: hold is allowed. OK 3) ReservesControlBranch = patron's home library Circulation rules: Remove hold policy for library B. Library A: no holds allowed for books using 'Holds allowed (count)' = 0 Library B: 5 holds allowed for books using 'Holds allowed (count)' = 5 Patron A (home library A) places a hold in OPAC on an item from library B. Should be: hold is NOT allowed Before patch: hold is NOT allowed After patch: hold is NOT allowed OK 4) ReserveControlbranch = item's home library Should be: hold is allowed Before patch: hold is allowed After patch: hold is allowed OK -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |jcamins@cpbibliography.com --- Comment #16 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- This patch has been pushed to master. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5801 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable --- Comment #17 from Chris Cormack <chris@bigballofwax.co.nz> --- Pushed to 3.10.x and 3.8.x, will be in 3.10.1 and 3.8.8 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org