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

(-)a/Koha/Biblio.pm (+32 lines)
Lines 252-257 sub can_be_transferred { Link Here
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
(-)a/Koha/Hold.pm (-1 / +2 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({ 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
(-)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 / +8 lines)
Lines 215-221 subtest 'is_serial() tests' => sub { Link Here
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') {
(-)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