Bug 13918 [QA Followup] - Unit Tests

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

diff --git a/Koha/Hold.pm b/Koha/Hold.pm
index 107f361..518d0ae 100644 (file)
--- a/Koha/Hold.pm
+++ b/Koha/Hold.pm
@@ -109,18 +109,26 @@ sub is_in_transit {
 
 Returns true if hold is a cancelable hold
 
+Holds may be canceled if they not found, or
+are found and waiting. A hold found but in
+transit cannot be canceled.
+
 =cut
 
 sub is_cancelable {
     my ($self) = @_;
 
-    return ( $self->is_waiting() && !$self->is_found() )
-      || ( !$self->is_waiting() && !$self->is_in_transit() );
+    return 1 unless $self->is_found();
+    return 0 if $self->is_in_transit();
+    return 1 if $self->is_waiting();
+    return 0;
 }
 
 =head3 is_at_destination
 
-Returns true if hold is a in_transit hold
+Returns true if hold is waiting
+and the hold's pickup branch matches
+the hold item's holding branch
 
 =cut
 

diff --git a/t/db_dependent/Hold.t b/t/db_dependent/Hold.t
index 53ee364..ae8c19d 100755 (executable)
--- a/t/db_dependent/Hold.t
+++ b/t/db_dependent/Hold.t
@@ -18,10 +18,13 @@
 use Modern::Perl;
 
 use C4::Context;
+use C4::Biblio qw( AddBiblio );
 use Koha::Database;
 use Koha::Borrowers;
+use Koha::Branches;
+use Koha::Item;
 
-use Test::More tests => 16;
+use Test::More tests => 23;
 
 use_ok('Koha::Hold');
 
@@ -31,21 +34,48 @@ $schema->storage->txn_begin();
 my $dbh = C4::Context->dbh;
 $dbh->{RaiseError} = 1;
 
+my @branches = Koha::Branches->search();
 my $borrower = Koha::Borrowers->search()->next();
+
+my $biblio = MARC::Record->new();
+my $title  = 'Silence in the library';
+$biblio->append_fields(
+    MARC::Field->new( '100', ' ', ' ', a => 'Moffat, Steven' ),
+    MARC::Field->new( '245', ' ', ' ', a => $title ),
+);
+my ( $biblionumber, $biblioitemnumber ) = AddBiblio( $biblio, '' );
+
+my $item = Koha::Item->new(
+    {
+        biblionumber     => $biblionumber,
+        biblioitemnumber => $biblioitemnumber,
+        holdingbranch    => $branches[0]->branchcode(),
+        homebranch       => $branches[0]->branchcode(),
+    }
+);
+$item->store();
+
 my $hold = Koha::Hold->new(
     {
+        biblionumber     => $biblionumber,
+        itemnumber => $item->id(),
         found          => 'W',
         waitingdate    => '2000-01-01',
         borrowernumber => $borrower->borrowernumber(),
+        branchcode     => $branches[1]->branchcode(),
     }
 );
+$hold->store();
+
+$item = $hold->item();
+
 my $hold_borrower = $hold->borrower();
 ok( $hold_borrower, 'Got hold borrower' );
 is( $hold_borrower->borrowernumber(), $borrower->borrowernumber(), 'Hold borrower matches correct borrower' );
 
 C4::Context->set_preference( 'ReservesMaxPickUpDelay', '' );
 my $dt = $hold->waiting_expires_on();
-is( $dt, undef, "Koha::Hold->waiting_expires_on returns undef if ReservesMaxPickUpDelay is not set");
+is( $dt, undef, "Koha::Hold->waiting_expires_on returns undef if ReservesMaxPickUpDelay is not set" );
 
 is( $hold->is_waiting, 1, 'The hold is waiting' );
 is( $hold->is_found, 1, 'The hold is found');
@@ -53,22 +83,41 @@ ok( !$hold->is_in_transit, 'The hold is not in transit' );
 
 C4::Context->set_preference( 'ReservesMaxPickUpDelay', '5' );
 $dt = $hold->waiting_expires_on();
-is( $dt->ymd, "2000-01-06", "Koha::Hold->waiting_expires_on returns DateTime of waitingdate + ReservesMaxPickUpDelay if set");
+is( $dt->ymd, "2000-01-06",
+    "Koha::Hold->waiting_expires_on returns DateTime of waitingdate + ReservesMaxPickUpDelay if set" );
 
 $hold->found('T');
 $dt = $hold->waiting_expires_on();
-is( $dt, undef, "Koha::Hold->waiting_expires_on returns undef if found is not 'W' ( Set to 'T' )");
+is( $dt, undef, "Koha::Hold->waiting_expires_on returns undef if found is not 'W' ( Set to 'T' )" );
 isnt( $hold->is_waiting, 1, 'The hold is not waiting (T)' );
 is( $hold->is_found, 1, 'The hold is found');
 is( $hold->is_in_transit, 1, 'The hold is in transit' );
 
 $hold->found(q{});
 $dt = $hold->waiting_expires_on();
-is( $dt, undef, "Koha::Hold->waiting_expires_on returns undef if found is not 'W' ( Set to empty string )");
+is( $dt, undef, "Koha::Hold->waiting_expires_on returns undef if found is not 'W' ( Set to empty string )" );
 isnt( $hold->is_waiting, 1, 'The hold is not waiting (W)' );
 is( $hold->is_found, 0, 'The hold is not found' );
 ok( !$hold->is_in_transit, 'The hold is not in transit' );
 
+# Test method is_cancelable
+$hold->found(undef);
+ok( $hold->is_cancelable(), "Unfound hold is cancelable" );
+$hold->found('W');
+ok( $hold->is_cancelable, "Waiting hold is cancelable" );
+$hold->found('T');
+ok( !$hold->is_cancelable, "In transit hold is not cancelable" );
+
+# Test method is_at_destination
+$hold->found(undef);
+ok( !$hold->is_at_destination(), "Unfound hold cannot be at destination" );
+$hold->found('T');
+ok( !$hold->is_at_destination(), "In transit hold cannot be at destination" );
+$hold->found('W');
+ok( !$hold->is_at_destination(), "Waiting hold where hold branchcode is not the same as the item's holdingbranch is not at destination" );
+$item->holdingbranch( $branches[1]->branchcode() );
+ok( $hold->is_at_destination(), "Waiting hold where hold branchcode is the same as the item's holdingbranch is at destination" );
+
 $schema->storage->txn_rollback();
 
 1;