In opac/opac-reserve.pl: 243 # When choosing a specific item, the default pickup library should be dictated by the default hold policy 244 if ( ! C4::Context->preference("OPACAllowUserToChooseBranch") && $itemNum ) { 245 my $item = Koha::Items->find( $itemNum ); 246 my $type = $item->effective_itemtype; 247 my $rule = GetBranchItemRule( $patron->branchcode, $type ); 248 if ( $rule->{hold_fulfillment_policy} eq 'any' ) { 249 $branch = $patron->branchcode; 250 } else { 251 my $policy = $rule->{hold_fulfillment_policy}; 252 $branch = $item->$policy; 253 } 254 } 255 $branch = $item->$policy; is problematic for group values To test: 1 - Set 'Default checkout, hold and return policy' -> 'Hold pickup library match' to item's hold group or patron's hold group 2 - Set OPACAllowUserToChooseBranch to 'Don't allow' 3 - Try to place an item level hold on the opac 4 - Ka-boom The method Koha::Item->patrongroup is not covered by tests! The method Koha::Item->itemgroup is not covered by tests!
The behavior with hold groups should mirror the behavior without them. If we set hold_fulfillment_policy to Any Library and OPACAllowUserToChooseBranch to not allowed, Koha defaults OPAC holds to the patron's home library for the pickup location. So when hold_fulfillment_policy is set to patron's hold group, we should do the same and set to the pickup location to the patron's home library, which will by definition be in the patron's hold group and therefore a valid pickup location.
(In reply to Andrew Fuerste-Henry from comment #1) > The behavior with hold groups should mirror the behavior without them. If we > set hold_fulfillment_policy to Any Library and OPACAllowUserToChooseBranch > to not allowed, Koha defaults OPAC holds to the patron's home library for > the pickup location. So when hold_fulfillment_policy is set to patron's hold > group, we should do the same and set to the pickup location to the patron's > home library, which will by definition be in the patron's hold group and > therefore a valid pickup location. Agreed, this is what we expected when we set up hold groups.
Created attachment 115782 [details] [review] Bug 27529: Choose patron's branch or item's homebranch if following group rules and patron cannot choose branch This patch defaults the holds pickup location to the items homebranch or the patron's branch when a group option is selected for the hold fulfillment policy and the patron is not allowed to choose the branch on the OPAC To test: 1 - Set 'Default checkout, hold and return policy' -> 'Hold pickup library match' to item's hold group or patron's hold group 2 - Set OPACAllowUserToChooseBranch to 'Don't allow' 3 - Try to place an item level hold on the opac 4 - Ka-boom, etiher: The method Koha::Item->patrongroup is not covered by tests! The method Koha::Item->itemgroup is not covered by tests! 5 - Apply patch 6 - Repeat 7 - Note the pickup location is set to either the patron's branch or the items homebranch 8 - Repeat plan with the other group setting
I'm still getting an error after applying the patch: The method Koha::Item->holdgroup is not covered by tests! Trace begun at /kohadevbox/koha/Koha/Object.pm line 827 Koha::Object::AUTOLOAD('Koha::Item=HASH(0x55b488f14d30)') called at /kohadevbox/koha/opac/opac-reserve.pl line 255 eval {...} at /kohadevbox/koha/opac/opac-reserve.pl line 2 That's with Hold Pickup Library Match set to item's hold group, reservescontrol set to patron's library.
Created attachment 118975 [details] [review] Bug 27529: Choose patron's branch or item's homebranch if following group rules and patron cannot choose branch This patch defaults the holds pickup location to the items homebranch or the patron's branch when a group option is selected for the hold fulfillment policy and the patron is not allowed to choose the branch on the OPAC To test: 1 - Set 'Default checkout, hold and return policy' -> 'Hold pickup library match' to item's hold group or patron's hold group 2 - Set OPACAllowUserToChooseBranch to 'Don't allow' 3 - Try to place an item level hold on the opac 4 - Ka-boom, etiher: The method Koha::Item->patrongroup is not covered by tests! The method Koha::Item->itemgroup is not covered by tests! 5 - Apply patch 6 - Repeat 7 - Note the pickup location is set to either the patron's branch or the items homebranch 8 - Repeat plan with the other group setting
Created attachment 119162 [details] [review] Bug 27529: Choose patron's branch or item's homebranch if following group rules and patron cannot choose branch This patch defaults the holds pickup location to the items homebranch or the patron's branch when a group option is selected for the hold fulfillment policy and the patron is not allowed to choose the branch on the OPAC To test: 1 - Set 'Default checkout, hold and return policy' -> 'Hold pickup library match' to item's hold group or patron's hold group 2 - Set OPACAllowUserToChooseBranch to 'Don't allow' 3 - Try to place an item level hold on the opac 4 - Ka-boom, etiher: The method Koha::Item->patrongroup is not covered by tests! The method Koha::Item->itemgroup is not covered by tests! 5 - Apply patch 6 - Repeat 7 - Note the pickup location is set to either the patron's branch or the items homebranch 8 - Repeat plan with the other group setting Signed-off-by: David Nind <david@davidnind.com>
Created attachment 119708 [details] [review] Bug 27529: Choose patron's branch or item's homebranch if following group rules and patron cannot choose branch This patch defaults the holds pickup location to the items homebranch or the patron's branch when a group option is selected for the hold fulfillment policy and the patron is not allowed to choose the branch on the OPAC To test: 1 - Set 'Default checkout, hold and return policy' -> 'Hold pickup library match' to item's hold group or patron's hold group 2 - Set OPACAllowUserToChooseBranch to 'Don't allow' 3 - Try to place an item level hold on the opac 4 - Ka-boom, etiher: The method Koha::Item->patrongroup is not covered by tests! The method Koha::Item->itemgroup is not covered by tests! 5 - Apply patch 6 - Repeat 7 - Note the pickup location is set to either the patron's branch or the items homebranch 8 - Repeat plan with the other group setting Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Trivial patch works as expected, no QA script failures. Passing QA
Pushed to master for 21.05, thanks to everybody involved!
Pushed to 20.11.x for 20.11.05
Pushed to 20.05.x for 20.05.11
Missing dependencies for 19.11.x, it shouldn't be affected, no backport.