[Bug 29196] New: Follow-up to Bug 27068 - fix remaining ocurrence of hold policy check
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29196 Bug ID: 29196 Summary: Follow-up to Bug 27068 - fix remaining ocurrence of hold policy check 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 726 # Don't fill item level holds that contravene the hold pickup policy at this time 727 next unless $item->{hold_fulfillment_policy} eq 'any' 728 || $request->{branchcode} eq $item->{ $item->{hold_fulfillment_policy} }; Should be: # Don't fill item level holds that contravene the hold pickup policy at this time next unless _checkHoldPolicy($item, $request); -- 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=29196 Nick Clemens <nick@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.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29196 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 125963 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125963&action=edit Bug 29196: Bug 27068 follow-up Test plan stolen/updated from 27068 To test: 1) In library groups add a root group and check it as hold group. 2) Add two libraries to the group 3) In circulation and fines rules, in 'Default checkout, hold and return policy', in Hold pickup library match change the value to 'From patron's hold group' 4) Place a hold from a patron whose home library is from the group 4.5) perl misc/cronjobs/holds/build_holdsqueue.pl 5) Go to /cgi-bin/koha/circ/view_holdsqueue.pl 6) Select the holding branch of the item with a hold 7) observe no results 8) Apply Patch 9) Repeat 5-6 10) The item should come up on the holds queue results 11) Place a hold on an item where 1 record has 2 copies, 1 in the hold group, 1 not. 12) Run the HoldQueue for the library not in the group and make sure the hold isn't showing. 13) Turn on transportation cost matrix and set costs for the libraries within the group. 14) Place a hold for a patron where multiple copies are on the bib. 15) Check both branch's hold queue for the item, it should only show on the lower cost branch's list if both copies are available. -- 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=29196 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |major -- 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=29196 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |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=29196 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Nick and I talked about this. We are quite sure that line of code is superfluous and can be safely removed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29196 --- Comment #3 from Kyle M Hall <kyle@bywatersolutions.com> --- *** Bug 29219 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29196 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Follow-up to Bug 27068 - |Follow-up to Bug 27068 - |fix remaining ocurrence of |Remove unnecessary check |hold policy check | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29196 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125963|0 |1 is obsolete| | --- Comment #4 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 126226 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126226&action=edit Bug 29196: Follow-up to Bug 27068 - Remove unnecessary check The introduction of _checkHoldPolicy has made this check superfluous. Test Plan: 1) Apply this patch 2) prove t/db_dependent/HoldsQueue.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29196 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=29196 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126226|0 |1 is obsolete| | --- Comment #5 from David Nind <david@davidnind.com> --- Created attachment 126788 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126788&action=edit Bug 29196: Follow-up to Bug 27068 - Remove unnecessary check The introduction of _checkHoldPolicy has made this check superfluous. Test Plan: 1) Apply this patch 2) prove t/db_dependent/HoldsQueue.t 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=29196 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |joonas.kylmala@iki.fi --- Comment #6 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- Kyle, please provide unit tests for the 'holdgroup' and 'patrongroup' cases to highlight this bug and fix the commit message to describe the bug (so basically copy&paste the commit message from Nick's patch) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29196 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29196 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126788|0 |1 is obsolete| | --- Comment #7 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 140832 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=140832&action=edit Bug 29196: Follow-up to Bug 27068 - Remove unnecessary check The introduction of _checkHoldPolicy has made this check superfluous. Test plan stolen/updated from 27068 To test: 1) In library groups add a root group and check it as hold group. 2) Add two libraries to the group 3) In circulation and fines rules, in 'Default checkout, hold and return policy', in Hold pickup library match change the value to 'From patron's hold group' 4) Place a hold from a patron whose home library is from the group 4.5) perl misc/cronjobs/holds/build_holdsqueue.pl 5) Go to /cgi-bin/koha/circ/view_holdsqueue.pl 6) Select the holding branch of the item with a hold 7) observe no results 8) Apply Patch 9) Repeat 5-6 10) The item should come up on the holds queue results 11) Place a hold on an item where 1 record has 2 copies, 1 in the hold group, 1 not. 12) Run the HoldQueue for the library not in the group and make sure the hold isn't showing. 13) Turn on transportation cost matrix and set costs for the libraries within the group. 14) Place a hold for a patron where multiple copies are on the bib. 15) Check both branch's hold queue for the item, it should only show on the lower cost branch's list if both copies are available. 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=29196 --- Comment #8 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Joonas Kylmälä from comment #6)
Kyle, please provide unit tests for the 'holdgroup' and 'patrongroup' cases to highlight this bug and fix the commit message to describe the bug (so basically copy&paste the commit message from Nick's patch)
Are tests from 27068 not sufficient? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29196 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Status|Signed Off |Failed QA --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- * Commit title does not contain 'follow-up' correctly spelt - c61176ac2a -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29196 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29196 --- Comment #10 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 141131 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141131&action=edit Bug 29196: (Bug 27068 follow-up) - Remove unnecessary check The introduction of _checkHoldPolicy has made this check superfluous. Test plan stolen/updated from 27068 To test: 1) In library groups add a root group and check it as hold group. 2) Add two libraries to the group 3) In circulation and fines rules, in 'Default checkout, hold and return policy', in Hold pickup library match change the value to 'From patron's hold group' 4) Place a hold from a patron whose home library is from the group 4.5) perl misc/cronjobs/holds/build_holdsqueue.pl 5) Go to /cgi-bin/koha/circ/view_holdsqueue.pl 6) Select the holding branch of the item with a hold 7) observe no results 8) Apply Patch 9) Repeat 5-6 10) The item should come up on the holds queue results 11) Place a hold on an item where 1 record has 2 copies, 1 in the hold group, 1 not. 12) Run the HoldQueue for the library not in the group and make sure the hold isn't showing. 13) Turn on transportation cost matrix and set costs for the libraries within the group. 14) Place a hold for a patron where multiple copies are on the bib. 15) Check both branch's hold queue for the item, it should only show on the lower cost branch's list if both copies are available. 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=29196 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #140832|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=29196 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29196 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #141131|0 |1 is obsolete| | --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 141155 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141155&action=edit Bug 29196: (Bug 27068 follow-up) - Remove unnecessary check The introduction of _checkHoldPolicy has made this check superfluous. Test plan stolen/updated from 27068 To test: 1) In library groups add a root group and check it as hold group. 2) Add two libraries to the group 3) In circulation and fines rules, in 'Default checkout, hold and return policy', in Hold pickup library match change the value to 'From patron's hold group' 4) Place a hold from a patron whose home library is from the group 4.5) perl misc/cronjobs/holds/build_holdsqueue.pl 5) Go to /cgi-bin/koha/circ/view_holdsqueue.pl 6) Select the holding branch of the item with a hold 7) observe no results 8) Apply Patch 9) Repeat 5-6 10) The item should come up on the holds queue results 11) Place a hold on an item where 1 record has 2 copies, 1 in the hold group, 1 not. 12) Run the HoldQueue for the library not in the group and make sure the hold isn't showing. 13) Turn on transportation cost matrix and set costs for the libraries within the group. 14) Place a hold for a patron where multiple copies are on the bib. 15) Check both branch's hold queue for the item, it should only show on the lower cost branch's list if both copies are available. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29196 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |kyle@bywatersolutions.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29196 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |22.11.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=29196 --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 22.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org