https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35805 Bug ID: 35805 Summary: Two issues with HoldsQueue::MapItemsToHoldRequests Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: andreas.jonsson@kreablo.se QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com 1. This clause from the below if statement must be incorrect: ( !$request->{itemtype} # If hold itemtype is set, item's itemtype must match || ( $request->{itemnumber} && ( $items_by_itemnumber{ $request->{itemnumber} }->{itype} eq $request->{itemtype} ) ) ) Because $request->{itemnumber} is guaranteed to be undefined due to this clause: next if defined($request->{itemnumber}); # already handled these The clause was probably ment to be this: ( !$request->{itemtype} || ( $items_by_itemnumber{ $item->{itemnumber} }->{itype} eq $request->{itemtype} ) ) foreach my $request (@$hold_requests) { last if $num_items_remaining == 0; next if $request->{allocated}; next if defined($request->{itemnumber}); # already handled these # look for local match first my $pickup_branch = $request->{branchcode} || $request->{borrowerbranch}; my ($itemnumber, $holdingbranch); my $holding_branch_items = $items_by_branch{$pickup_branch}; my $priority_branch = C4::Context->preference('HoldsQueuePrioritizeBranch') // 'homebranch'; if ( $holding_branch_items ) { foreach my $item (@$holding_branch_items) { next unless $items_by_itemnumber{ $item->{itemnumber} }->{_object}->can_be_transferred( { to => $libraries->{ $request->{branchcode} } } ); if ( $request->{borrowerbranch} eq $item->{$priority_branch} && _checkHoldPolicy($item, $request) # Don't fill item level holds that contravene the hold pickup policy at this time && ( !$request->{itemtype} # If hold itemtype is set, item's itemtype must match || ( $request->{itemnumber} && ( $items_by_itemnumber{ $request->{itemnumber} }->{itype} eq $request->{itemtype} ) ) ) && ( !$request->{item_group_id} # If hold item_group is set, item's item_group must match || ( $item->{_object}->item_group && $item->{_object}->item_group->id eq $request->{item_group_id} ) ) ) { $itemnumber = $item->{itemnumber}; last; } } $holdingbranch = $pickup_branch; 2. Then the code continues: } elsif ($transport_cost_matrix) { But surely this should be: } if (!defined $itemnumber && defined $transport_cost_matrix) { Because the important condition here is that we failed to allocate from the local holding branch. Surely the transport_cost_matrix should still be used just even if we tried allocating locally first, but failed? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.