From ed5f739cd6e4fb1ebf0c707856725df33ccc5320 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 3 Dec 2021 16:45:21 -0300 Subject: [PATCH] Bug 29349: Do not assume holding branch is a valid pickup location The original code for pickup locations when placing item-level holds picked the currently logged-in library. We made things more robust, as the logged-in library might not be a valid pickup location for the patron and item. But it was wrongly chosen to use the holding branch as the default. A more robust approach is needed, and this precedence is picked this time (it could be configuration-driven in the future): - Logged-in library - Empty To test: 1. Pick a biblio with various valid pickup locations, some not including the logged-in library. 2. Pick a patron for placing the hold => FAIL: Notice that (when valid pickup location) the holding branch is always chosen 3. Apply this patch 4. Repeat 2 => SUCCESS: If valid pickup location, the logged-in branch is picked as default for item-type level. When it is not, an empty dropdown is used as a fallback. 5. Sign off :-D Signed-off-by: Lucas Gass Signed-off-by: Martin Renvoize Signed-off-by: Fridolin Somers Signed-off-by: Kyle M Hall --- reserve/request.pl | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/reserve/request.pl b/reserve/request.pl index a635ab1348..1b6638307d 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -580,13 +580,20 @@ foreach my $biblionumber (@biblionumbers) { ) { # Send the pickup locations count to the UI, the pickup locations will be pulled using the API - my $pickup_locations = $item_object->pickup_locations({ patron => $patron }); - $item->{pickup_locations_count} = $pickup_locations->count; - if ( $item->{pickup_locations_count} > 0 ) { + my @pickup_locations = $item_object->pickup_locations({ patron => $patron })->as_list; + $item->{pickup_locations_count} = scalar @pickup_locations; + + if ( @pickup_locations ) { $num_available++; $item->{available} = 1; - # pass the holding branch for use as default - my $default_pickup_location = $pickup_locations->search({ branchcode => $item->{holdingbranch} })->next; + + my $default_pickup_location; + + # Default to logged-in, if valid + if ( C4::Context->userenv->{branch} ) { + ($default_pickup_location) = grep { $_->branchcode eq C4::Context->userenv->{branch} } @pickup_locations; + } + $item->{default_pickup_location} = $default_pickup_location; } else { -- 2.39.5