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 <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit ed5f739cd6)

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
This commit is contained in:
Tomas Cohen Arazi 2021-12-03 16:45:21 -03:00 committed by Andrew Fuerste-Henry
parent 7a335a2e3c
commit 4ac0d32472
1 changed files with 12 additions and 5 deletions

View File

@ -569,13 +569,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 {