Attachment #149411 for bug #33471

View | Details | Raw Unified | Return to bug 33471
Collapse All | Expand All




    return 0;
}

=head3 validate_pickup_location

    $biblio->validate_pickup_location( {patron => $patron, pickup_location_id => $branchcode } );

Returns 1 if the pickup location is valid, nothing if it is not. This function is similar to, but distinct
from pickup_locations in that it does not need to calculate all valid locations, but simply to check until
1 item is found that validates the passed location. Rather than confusing the logic in that sub, some code
is duplicated.

=cut

sub validate_pickup_location {
    my ( $self, $params ) = @_;

    my $patron = $params->{patron};
    my $branchcode = $params->{pickup_location_id};

    my %seen;
    foreach my $item ( $self->items->as_list ) {
        # The 5 variables below determine the valid locations for any item, no need to check twice
        my $cache_key = sprintf "Pickup_locations:%s:%s:%s:%s:%s",
           $item->itype,$item->homebranch,$item->holdingbranch,$item->ccode || "",$patron->branchcode||"" ;
        next if $seen{ $cache_key };

        my @item_pickup_locations = $item->pickup_locations( { patron => $patron } )->_resultset->get_column('branchcode')->all;
        $seen{ $cache_key} = 1;

        return 1 if grep { $branchcode eq $_ } @item_pickup_locations;
    }

    return;
}

=head3 pickup_locations






    if ( $self->itemnumber ) { # item-level
        $pickup_locations = $self->item->pickup_locations({ patron => $self->patron });
        return any { $_->branchcode eq $params->{library_id} } $pickup_locations->as_list;
    }
    else { # biblio-level
        $pickup_locations = $self->biblio->pickup_locations({ patron => $self->patron });
        return $self->biblio->validate_pickup_location({ patron => $self->patron, pickup_location_id => $params->{library_id} });
    }

    return any { $_->branchcode eq $params->{library_id} } $pickup_locations->as_list;
}

=head3 set_pickup_location

Lines 155-163 sub add { Link Here

(-)a/Koha/REST/V1/Holds.pm (-3 / +1 lines)
155	}	155	}
156	else {	156	else {
157	$valid_pickup_location =	157	$valid_pickup_location =
158	any { $_->branchcode eq $pickup_library_id }	158	$biblio->validate_pickup_location({ patron => $patron, pickup_location_id => $pickup_library_id });
159	$biblio->pickup_locations(
160	{ patron => $patron } )->as_list;
161	}	159	}
162		160
163	return $c->render(	161	return $c->render(

Lines 215-221 subtest 'is_serial() tests' => sub { Link Here

(-)a/t/db_dependent/Koha/Biblio.t (-1 / +8 lines)
215	};	215	};
216		216
217	subtest 'pickup_locations' => sub {	217	subtest 'pickup_locations' => sub {
218	plan tests => 9;	218	plan tests => 73;
219		219
220	$schema->storage->txn_begin;	220	$schema->storage->txn_begin;
221		221
Lines 433-438 subtest 'pickup_locations' => sub { Link Here
433	. ' but returns '	433	. ' but returns '
434	. scalar(@pl)	434	. scalar(@pl)
435	);	435	);
		436
		437	foreach my $branchcode (@branchcodes){
		438	my $valid;
		439	$valid = 1 if grep { $branchcode eq $_ } @pl;
		440	is( $valid, $biblio->validate_pickup_location({ patron => $patron, pickup_location_id => $branchcode }),
		441	"Validate pickup location returns the expected result");
		442	}
436	}	443	}
437		444
438	foreach my $cbranch ('ItemHomeLibrary','PatronLibrary') {	445	foreach my $cbranch ('ItemHomeLibrary','PatronLibrary') {




      ->status_is(201);

    # empty cases
    $mock_biblio->mock( 'validate_pickup_location', sub {
        return;
    });

    $t->post_ok( "//$userid:$password@/api/v1/holds" => json => $biblio_hold_data )

      ->json_is({ error => 'The supplied pickup location is not valid' });

    # empty cases
    $mock_item->mock( 'validate_pickup_location', sub {
        return;
    });

    $t->post_ok( "//$userid:$password@/api/v1/holds" => json => $item_hold_data )
- 

Return to bug 33471

Lines 252-257 sub can_be_transferred { Link Here

(-)a/Koha/Biblio.pm (+32 lines)
252	return 0;	252	return 0;
253	}	253	}
254		254
		255	=head3 validate_pickup_location
		256
		257	$biblio->validate_pickup_location( {patron => $patron, pickup_location_id => $branchcode } );
		258
		259	Returns 1 if the pickup location is valid, nothing if it is not. This function is similar to, but distinct
		260	from pickup_locations in that it does not need to calculate all valid locations, but simply to check until
		261	1 item is found that validates the passed location. Rather than confusing the logic in that sub, some code
		262	is duplicated.
		263
		264	=cut
		265
		266	sub validate_pickup_location {
		267	my ( $self, $params ) = @_;
		268
		269	my $patron = $params->{patron};
		270	my $branchcode = $params->{pickup_location_id};
		271
		272	my %seen;
		273	foreach my $item ( $self->items->as_list ) {
		274	# The 5 variables below determine the valid locations for any item, no need to check twice
		275	my $cache_key = sprintf "Pickup_locations:%s:%s:%s:%s:%s",
		276	$item->itype,$item->homebranch,$item->holdingbranch,$item->ccode \|\| "",$patron->branchcode\|\|"" ;
		277	next if $seen{ $cache_key };
		278
		279	my @item_pickup_locations = $item->pickup_locations( { patron => $patron } )->_resultset->get_column('branchcode')->all;
		280	$seen{ $cache_key} = 1;
		281
		282	return 1 if grep { $branchcode eq $_ } @item_pickup_locations;
		283	}
		284
		285	return;
		286	}
255		287
256	=head3 pickup_locations	288	=head3 pickup_locations
257		289

Lines 298-309 sub is_pickup_location_valid { Link Here

(-)a/Koha/Hold.pm (-1 / +2 lines)
298		298
299	if ( $self->itemnumber ) { # item-level	299	if ( $self->itemnumber ) { # item-level
300	$pickup_locations = $self->item->pickup_locations({ patron => $self->patron });	300	$pickup_locations = $self->item->pickup_locations({ patron => $self->patron });
		301	return any { $_->branchcode eq $params->{library_id} } $pickup_locations->as_list;
301	}	302	}
302	else { # biblio-level	303	else { # biblio-level
303	$pickup_locations = $self->biblio->pickup_locations({ patron => $self->patron });	304	$pickup_locations = $self->biblio->pickup_locations({ patron => $self->patron });
		305	return $self->biblio->validate_pickup_location({ patron => $self->patron, pickup_location_id => $params->{library_id} });
304	}	306	}
305		307
306	return any { $_->branchcode eq $params->{library_id} } $pickup_locations->as_list;
307	}	308	}
308		309
309	=head3 set_pickup_location	310	=head3 set_pickup_location

Lines 1248-1255 subtest 'add() tests' => sub { Link Here

(-)a/t/db_dependent/api/v1/holds.t (-5 / +4 lines)
1248	->status_is(201);	1248	->status_is(201);
1249		1249
1250	# empty cases	1250	# empty cases
1251	$mock_biblio->mock( 'pickup_locations', sub {	1251	$mock_biblio->mock( 'validate_pickup_location', sub {
1252	return Koha::Libraries->new->empty;	1252	return;
1253	});	1253	});
1254		1254
1255	$t->post_ok( "//$userid:$password@/api/v1/holds" => json => $biblio_hold_data )	1255	$t->post_ok( "//$userid:$password@/api/v1/holds" => json => $biblio_hold_data )
Lines 1257-1264 subtest 'add() tests' => sub { Link Here
1257	->json_is({ error => 'The supplied pickup location is not valid' });	1257	->json_is({ error => 'The supplied pickup location is not valid' });
1258		1258
1259	# empty cases	1259	# empty cases
1260	$mock_item->mock( 'pickup_locations', sub {	1260	$mock_item->mock( 'validate_pickup_location', sub {
1261	return Koha::Libraries->new->empty;	1261	return;
1262	});	1262	});
1263		1263
1264	$t->post_ok( "//$userid:$password@/api/v1/holds" => json => $item_hold_data )	1264	$t->post_ok( "//$userid:$password@/api/v1/holds" => json => $item_hold_data )
1265	-