gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 27209: (follow-up) Add ->is_pickup_location_valid 

This patch simply refactors the pickup location check into a method that
can be called on its own. Tests are added, and the tests for
->set_pickup_location should pass unmodified.

To test:
1. Apply the first two patches
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Hold.t
=> SUCCESS: tests pass
3. Apply this refactoring patch
4. Notice the tests are similar, but check for boolean output
5. Repeat 2
=> SUCCESS: New tests pass, set_pickup_location() behavior unchanged.
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 865a1dab5c)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This commit is contained in:
Tomas Cohen Arazi 2020-12-14 07:37:53 -03:00 committed by Fridolin Somers
parent 18578794d9
commit 02df63c430
2 changed files with 90 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
Koha/Hold.pm
View File

@ -222,6 +222,36 @@ sub set_waiting {
return $self;
}
=head3 is_pickup_location_valid
if ($hold->is_pickup_location_valid({ library_id => $library->id }) ) {
...
}
Returns a I<boolean> representing if the passed pickup location is valid for the hold.
It throws a I<Koha::Exceptions::_MissingParameter> if the library_id parameter is not
passed.
=cut
sub is_pickup_location_valid {
my ( $self, $params ) = @_;
Koha::Exceptions::MissingParameter->throw('The library_id parameter is mandatory')
unless $params->{library_id};
my @pickup_locations;
if ( $self->itemnumber ) { # item-level
@pickup_locations = $self->item->pickup_locations({ patron => $self->patron });
}
else { # biblio-level
@pickup_locations = $self->biblio->pickup_locations({ patron => $self->patron });
}
return any { $_->branchcode eq $params->{library_id} } @pickup_locations;
}
=head3 set_pickup_location
$hold->set_pickup_location({ library_id => $library->id });
@ -237,16 +267,7 @@ sub set_pickup_location {
Koha::Exceptions::MissingParameter->throw('The library_id parameter is mandatory')
unless $params->{library_id};
my @pickup_locations;
if ( $self->itemnumber ) { # item-level
@pickup_locations = $self->item->pickup_locations({ patron => $self->patron });
}
else { # biblio-level
@pickup_locations = $self->biblio->pickup_locations({ patron => $self->patron });
}
if ( any { $_->branchcode eq $params->{library_id} } @pickup_locations ) {
if ( $self->is_pickup_location_valid({ library_id => $params->{library_id} }) ) {
# all good, set the new pickup location
$self->branchcode( $params->{library_id} )->store;
}

60
t/db_dependent/Koha/Hold.t
View File

@ -19,7 +19,7 @@
use Modern::Perl;
use Test::More tests => 2;
use Test::More tests => 3;
use Test::Exception;
use Test::MockModule;
@ -140,3 +140,61 @@ subtest 'set_pickup_location() tests' => sub {
$schema->storage->txn_rollback;
};
subtest 'is_pickup_location_valid() tests' => sub {
plan tests => 4;
$schema->storage->txn_begin;
my $mock_biblio = Test::MockModule->new('Koha::Biblio');
my $mock_item = Test::MockModule->new('Koha::Item');
my $library_1 = $builder->build_object({ class => 'Koha::Libraries' });
my $library_2 = $builder->build_object({ class => 'Koha::Libraries' });
my $library_3 = $builder->build_object({ class => 'Koha::Libraries' });
# let's control what Koha::Biblio->pickup_locations returns, for testing
$mock_biblio->mock( 'pickup_locations', sub {
return Koha::Libraries->search( { branchcode => [ $library_2->branchcode, $library_3->branchcode ] } );
});
# let's mock what Koha::Item->pickup_locations returns, for testing
$mock_item->mock( 'pickup_locations', sub {
return Koha::Libraries->search( { branchcode => [ $library_2->branchcode, $library_3->branchcode ] } );
});
my $biblio = $builder->build_sample_biblio;
my $item = $builder->build_sample_item({ biblionumber => $biblio->biblionumber });
# Test biblio-level holds
my $biblio_hold = $builder->build_object(
{
class => "Koha::Holds",
value => {
biblionumber => $biblio->biblionumber,
branchcode => $library_3->branchcode,
itemnumber => undef,
}
}
);
ok( !$biblio_hold->is_pickup_location_valid({ library_id => $library_1->branchcode }), 'Pickup location invalid');
ok( $biblio_hold->is_pickup_location_valid({ library_id => $library_2->id }), 'Pickup location valid');
# Test item-level holds
my $item_hold = $builder->build_object(
{
class => "Koha::Holds",
value => {
biblionumber => $biblio->biblionumber,
branchcode => $library_3->branchcode,
itemnumber => $item->itemnumber,
}
}
);
ok( !$item_hold->is_pickup_location_valid({ library_id => $library_1->branchcode }), 'Pickup location invalid');
ok( $item_hold->is_pickup_location_valid({ library_id => $library_2->id }), 'Pickup location valid' );
$schema->storage->txn_rollback;
};