[Bug 20837] New: CanItemBeReserved should follow ReservesControlBranch and not CircControl
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 Bug ID: 20837 Summary: CanItemBeReserved should follow ReservesControlBranch and not CircControl 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 According to the pref descriptions CircControl shouldn't affect reserves, we should check the other pref. Also, the important note 'Default holds policy by item type' admin/smart-rules.pl is misleading: "Important: The policies are based on the patron's home library, not the library where the hold is being placed." -- 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=20837 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Small 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=20837 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 75684 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75684&action=edit Bug 20837: Unit tests To test: prove -v t/db_dependent/Holds.t -- 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=20837 --- Comment #2 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 75685 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75685&action=edit Bug 20837: Use ReservesControlBranch to determine holdability of items To test: 1 - Set a default holds plicy for an item type as 'from home library' 2 - Set CircControl to 'logged in library' 3 - Log in as staff from a library without the rule set 4 - Note you can set holds for patrons of any library on items of type above 5 - Toggle ReservesControlBranch and note that nothing changes 6 - Toggle CircControl and note some combinations don't allow holds 7 - Apply patch 8 - Toggle ReservesControlBranch and note that holdability is affected 9 - Toggle CircControl and note holdability does not change 10 - Read the note on admin/smartrules.pl and confirm it makes sense -- 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=20837 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | CC| |jonathan.druart@bugs.koha-c | |ommunity.org, | |jsasse@plumcreeklibrary.net | |, katrin.fischer@bsz-bw.de, | |kyle@bywatersolutions.com -- 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=20837 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- It makes sense to me yes. Note that there is another occurrence of C4::Circulation::_GetCircControlBranch in IsAvailableForItemLevelRequest We should also rename the variable name ($circ_control_branch) For discussion, from hea: ReservesControlBranch ItemHomeLibrary - 115 PatronLibrary - 944 (default) CircControl ItemHomeLibrary - 962 (default) PatronLibrary - 34 PickupLibrary - 65 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jzairo@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply CC| |wizzyrea@gmail.com --- Comment #4 from Liz Rea <wizzyrea@gmail.com> --- Hi Nick, Terribly sorry, the patches don't seem to apply: CONFLICT (content): Merge conflict in t/db_dependent/Holds.t error: Failed to merge in the changes. - This was a conflict on number of tests, easy to resolve so I did that, HOWEVER have also got... Applying: Bug 20837: Use ReservesControlBranch to determine holdability of items error: sha1 information is lacking or useless (C4/Reserves.pm). error: could not build fake ancestor A new patch set would be nice. :) Liz -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 Nick Clemens <nick@bywatersolutions.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=20837 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #75684|0 |1 is obsolete| | --- Comment #5 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 87021 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87021&action=edit Bug 20837: Unit tests To test: prove -v t/db_dependent/Holds.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #75685|0 |1 is obsolete| | --- Comment #6 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 87022 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87022&action=edit Bug 20837: Use ReservesControlBranch to determine holdability of items To test: 1 - Set a default holds plicy for an item type as 'from home library' 2 - Set CircControl to 'logged in library' 3 - Log in as staff from a library without the rule set 4 - Note you can set holds for patrons of any library on items of type above 5 - Toggle ReservesControlBranch and note that nothing changes 6 - Toggle CircControl and note some combinations don't allow holds 7 - Apply patch 8 - Toggle ReservesControlBranch and note that holdability is affected 9 - Toggle CircControl and note holdability does not change 10 - Read the note on admin/smartrules.pl and confirm it makes sense -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 --- Comment #7 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 87023 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87023&action=edit Bug 20837: Unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 --- Comment #8 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 87024 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87024&action=edit Bug 20837: Alter IsAvailableForItemForItemLevelRequest to use ReservesControlBranch Also use Koha objects for fetching itemtype info To test: Same test plan as above, ensure item availability is affected the the system preference prove t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 --- Comment #9 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 87025 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87025&action=edit Bug 20837: (follow-up) Rename variables -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #10 from Liz Rea <wizzyrea@gmail.com> --- Hi Nick, It looks like Holds.t isn't currently passing with all of the patches applied: not ok 52 - CanItemBeReserved should use item home library rule when RsservesControlBranch set to 'ItemsHomeLibrary' Arg. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87021|0 |1 is obsolete| | Attachment #87022|0 |1 is obsolete| | Attachment #87023|0 |1 is obsolete| | Attachment #87024|0 |1 is obsolete| | Attachment #87025|0 |1 is obsolete| | --- Comment #11 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 87409 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87409&action=edit Bug 20837: Unit tests To test: prove -v t/db_dependent/Holds.t prove -v t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 --- Comment #12 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 87410 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87410&action=edit Bug 20837: Use ReservesControlBranch to determine holdability of items To test: 1 - Set a default holds plicy for an item type as 'from home library' 2 - Set CircControl to 'logged in library' 3 - Log in as staff from a library without the rule set 4 - Note you can set holds for patrons of any library on items of type above 5 - Toggle ReservesControlBranch and note that nothing changes 6 - Toggle CircControl and note some combinations don't allow holds 7 - Apply patch 8 - Toggle ReservesControlBranch and note that holdability is affected 9 - Toggle CircControl and note holdability does not change 10 - Read the note on admin/smartrules.pl and confirm it makes sense -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 --- Comment #13 from Liz Rea <wizzyrea@gmail.com> --- I have questions about how this could affect current users and might cause some nasty surprises for existing users, but the behaviour is correct now based on the tooltips so new libraries might find this easier to understand so the change is probably in the better long term interest. Signing off. Liz -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 Liz Rea <wizzyrea@gmail.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=20837 Liz Rea <wizzyrea@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87409|0 |1 is obsolete| | Attachment #87410|0 |1 is obsolete| | --- Comment #14 from Liz Rea <wizzyrea@gmail.com> --- Created attachment 87673 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87673&action=edit Bug 20837: Unit tests To test: prove -v t/db_dependent/Holds.t prove -v t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 --- Comment #15 from Liz Rea <wizzyrea@gmail.com> --- Created attachment 87674 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87674&action=edit Bug 20837: Use ReservesControlBranch to determine holdability of items To test: 1 - Set a default holds plicy for an item type as 'from home library' 2 - Set CircControl to 'logged in library' 3 - Log in as staff from a library without the rule set 4 - Note you can set holds for patrons of any library on items of type above 5 - Toggle ReservesControlBranch and note that nothing changes 6 - Toggle CircControl and note some combinations don't allow holds 7 - Apply patch 8 - Toggle ReservesControlBranch and note that holdability is affected 9 - Toggle CircControl and note holdability does not change 10 - Read the note on admin/smartrules.pl and confirm it makes sense -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 --- Comment #16 from Liz Rea <wizzyrea@gmail.com> --- Created attachment 87675 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87675&action=edit Bug 20837: (QA Follow-up) adding missing filter to make QA tool happy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Keywords| |Manual, | |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Haha.. thanks for the follow-up Liz.. but you fell into the whole I nearly always also fall into 'follow-up' vs 'Follow-up'... QA script complained.. I've fixed it ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- ooh.. also you forgot your signoff lines.. must have been a late night for you aye... so glad it's not just me who does that -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87673|0 |1 is obsolete| | --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 88699 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=88699&action=edit Bug 20837: Unit tests To test: prove -v t/db_dependent/Holds.t prove -v t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t Signed-off-by: Liz Rea <wizzyrea@gmail.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=20837 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87674|0 |1 is obsolete| | --- Comment #20 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 88700 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=88700&action=edit Bug 20837: Use ReservesControlBranch to determine holdability of items To test: 1 - Set a default holds plicy for an item type as 'from home library' 2 - Set CircControl to 'logged in library' 3 - Log in as staff from a library without the rule set 4 - Note you can set holds for patrons of any library on items of type above 5 - Toggle ReservesControlBranch and note that nothing changes 6 - Toggle CircControl and note some combinations don't allow holds 7 - Apply patch 8 - Toggle ReservesControlBranch and note that holdability is affected 9 - Toggle CircControl and note holdability does not change 10 - Read the note on admin/smartrules.pl and confirm it makes sense Signed-off-by: Liz Rea <wizzyrea@gmail.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=20837 --- Comment #21 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 88701 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=88701&action=edit Bug 20837: (QA follow-up) adding missing filter to make QA tool happy Signed-off-by: Liz Rea <wizzyrea@gmail.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=20837 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |WARNING: This patch release notes| |corrects the behaviour of | |reserve rules such that | |they match the system | |preference descriptions. | |This may initial lead to | |confusion as prior to this | |the CircControl branches | |were used incorrectly. Keywords|Manual, | |release-notes-needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #87675|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=20837 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |Manual, | |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #22 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nick, could you check my release notes make sense? Otherwise, looks good and Passing QA. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|WARNING: This patch |WARNING: This patch release notes|corrects the behaviour of |corrects the behaviour of |reserve rules such that |reserve rules such that |they match the system |they match the system |preference descriptions. |preference descriptions. |This may initial lead to |This may initial lead to |confusion as prior to this |confusion as prior to this |the CircControl branches |the CircControl branches |were used incorrectly. |were used incorrectly. | |Settings for | |ReservesControlBranch and | |CircControl should be | |reviewed to ensure proper | |behaviour is enforced. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #23 from Nick Clemens <nick@bywatersolutions.com> --- Awesome work all! Pushed to master for 19.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |19.05.00 released in| | Status|Pushed to Master |RESOLVED Resolution|--- |FIXED --- Comment #24 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- This could be a rather confusing change for a stable branch.. opting not to backport to 18.11.x series. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20837 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|release-notes-needed | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org