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

(-)a/Koha/Biblio.pm (+35 lines)
Lines 285-290 sub can_be_transferred { Link Here
285
}
285
}
286
286
287
287
288
=head3 validate_pickup_location
289
290
    $biblio->validate_pickup_location( {patron => $patron, pickup_location_id => $branchcode } );
291
292
Returns 1 if the pickup location is valid, nothing if it is not. This function is similar to, but distinct
293
from pickup_locations in that it does not need to calculate all valid locations, but simply to check until
294
1 item is found that validates the passed location. Rather than confusing the logic in that sub, some code
295
is duplicated.
296
297
=cut
298
299
sub validate_pickup_location {
300
    my ( $self, $params ) = @_;
301
302
    my $patron     = $params->{patron};
303
    my $branchcode = $params->{pickup_location_id};
304
305
    my %seen;
306
    foreach my $item ( $self->items->as_list ) {
307
308
        # The 5 variables below determine the valid locations for any item, no need to check twice
309
        my $cache_key = sprintf "Pickup_locations:%s:%s:%s:%s:%s",
310
            $item->itype, $item->homebranch, $item->holdingbranch, $item->ccode || "", $patron->branchcode || "";
311
        next if $seen{$cache_key};
312
313
        my @item_pickup_locations =
314
            $item->pickup_locations( { patron => $patron } )->_resultset->get_column('branchcode')->all;
315
        $seen{$cache_key} = 1;
316
317
        return 1 if grep { $branchcode eq $_ } @item_pickup_locations;
318
    }
319
320
    return;
321
}
322
288
=head3 pickup_locations
323
=head3 pickup_locations
289
324
290
    my $pickup_locations = $biblio->pickup_locations({ patron => $patron });
325
    my $pickup_locations = $biblio->pickup_locations({ patron => $patron });
(-)a/Koha/Hold.pm (-1 / +3 lines)
Lines 298-309 sub is_pickup_location_valid { Link Here
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(
306
            { patron => $self->patron, pickup_location_id => $params->{library_id} } );
304
    }
307
    }
305
308
306
    return any { $_->branchcode eq $params->{library_id} } $pickup_locations->as_list;
307
}
309
}
308
310
309
=head3 set_pickup_location
311
=head3 set_pickup_location
(-)a/Koha/REST/V1/Holds.pm (-3 / +1 lines)
Lines 155-163 sub add { Link Here
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(
(-)a/t/db_dependent/Koha/Biblio.t (-1 / +10 lines)
Lines 217-223 subtest 'is_serial() tests' => sub { Link Here
217
217
218
subtest 'pickup_locations() tests' => sub {
218
subtest 'pickup_locations() tests' => sub {
219
219
220
    plan tests => 11;
220
    plan tests => 75;
221
221
222
    $schema->storage->txn_begin;
222
    $schema->storage->txn_begin;
223
223
Lines 442-447 subtest 'pickup_locations() tests' => sub { Link Here
442
              . ' but returns '
442
              . ' but returns '
443
              . scalar(@pl)
443
              . scalar(@pl)
444
        );
444
        );
445
446
        foreach my $branchcode (@branchcodes) {
447
            my $valid;
448
            $valid = 1 if grep { $branchcode eq $_ } @pl;
449
            is(
450
                $valid, $biblio->validate_pickup_location( { patron => $patron, pickup_location_id => $branchcode } ),
451
                "Validate pickup location returns the expected result"
452
            );
453
        }
445
    }
454
    }
446
455
447
    foreach my $cbranch ('ItemHomeLibrary','PatronLibrary') {
456
    foreach my $cbranch ('ItemHomeLibrary','PatronLibrary') {
(-)a/t/db_dependent/api/v1/holds.t (-5 / +4 lines)
Lines 1248-1255 subtest 'add() tests' => sub { Link Here
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
- 

Return to bug 33471