Bug 27064: Correct transferbook.t tests to reflect new behaviour of transferbook()
authorJoonas Kylmälä <joonas.kylmala@helsinki.fi>
Tue, 15 Jun 2021 08:56:43 +0000 (11:56 +0300)
committerJonathan Druart <jonathan.druart@bugs.koha-community.org>
Tue, 15 Jun 2021 09:42:35 +0000 (11:42 +0200)
The transferbook() behaviour was changed so that it only allows
transferring the item with a reserve if ignore_reserves=1 is passed to
it. The tests are changed here to reflect that. Note that however the
tests were buggy already before this change because the transfer's
"from" and "to" branches were the same and so the transfer should have
failed due to the error DestinationEqualsHolding, but futher though
the transferbook() code was buggy and it override the
DestinationEqualsHolding checking totally if there was a transfer! So
the tests were earlier working due to a bug in transferbook().

To test):
   1) Make sure the new test scenarios make sense
   2) prove t/db_dependent/Circulation/transferbook.t

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
t/db_dependent/Circulation/transferbook.t

index 03c735a..2e2ac0f 100755 (executable)
@@ -148,7 +148,7 @@ subtest 'transfer already at destination' => sub {
         barcode => $item->barcode,
         trigger => "Manual"
     });
-    is( $dotransfer, 1, 'Transfer of reserved item succeeded without ignore reserves' );
+    is( $dotransfer, 0, 'Transfer of reserved item doesn\'t succeed without ignore_reserves' );
     is( $messages->{ResFound}->{ResFound}, 'Reserved', "We found the reserve");
     is( $messages->{ResFound}->{itemnumber}, $item->itemnumber, "We got the reserve info");
 };
@@ -205,15 +205,16 @@ subtest 'transfer an issued item' => sub {
         barcode => $item->barcode,
         trigger => "Manual"
     });
-    is( $dotransfer, 1, 'Transfer of reserved item succeeded without ignore reserves' );
+    is( $dotransfer, 0, 'Transfer of reserved item doesn\'t succeed without ignore_reserves' );
     is( $messages->{ResFound}->{ResFound}, 'Reserved', "We found the reserve");
     is( $messages->{ResFound}->{itemnumber}, $item->itemnumber, "We got the reserve info");
     is( $messages->{WasReturned}, $patron->borrowernumber, "We got the return info");
 };
 
 subtest 'ignore_reserves flag' => sub {
-    plan tests => 10;
+    plan tests => 9;
     my $library = $builder->build_object( { class => 'Koha::Libraries' } )->store;
+    my $library2 = $builder->build_object( { class => 'Koha::Libraries' } )->store;
     t::lib::Mocks::mock_userenv( { branchcode => $library->branchcode } );
 
     my $patron = $builder->build_object(
@@ -241,42 +242,37 @@ subtest 'ignore_reserves flag' => sub {
     #   found will override all other measures that may prevent transfer and force a transfer.
     my ($dotransfer, $messages ) = transferbook({
         from_branch => $item->homebranch,
-        to_branch => $library->branchcode,
+        to_branch => $library2->branchcode,
         barcode => $item->barcode,
         trigger => "Manual"
     });
-    is( $dotransfer, 1, 'Transfer of reserved item succeeded without ignore reserves' );
+    is( $dotransfer, 0, 'Transfer of reserved item doesn\'t succeed without ignore_reserves' );
     is( $messages->{ResFound}->{ResFound}, 'Reserved', "We found the reserve");
     is( $messages->{ResFound}->{itemnumber}, $item->itemnumber, "We got the reserve info");
 
     my $ignore_reserves = 0;
     ($dotransfer, $messages ) = transferbook({
         from_branch => $item->homebranch,
-        to_branch => $library->branchcode,
+        to_branch => $library2->branchcode,
         barcode => $item->barcode,
         ignore_reserves => $ignore_reserves,
         trigger => "Manual"
     });
-    is( $dotransfer, 1, 'Transfer of reserved item succeeded with ignore reserves: false' );
+    is( $dotransfer, 0, 'Transfer of reserved item doesn\'t succeed without ignore_reserves' );
     is( $messages->{ResFound}->{ResFound}, 'Reserved', "We found the reserve");
     is( $messages->{ResFound}->{itemnumber}, $item->itemnumber, "We got the reserve info");
 
     $ignore_reserves = 1;
     ($dotransfer, $messages ) = transferbook({
         from_branch => $item->homebranch,
-        to_branch => $library->branchcode,
+        to_branch => $library2->branchcode,
         barcode => $item->barcode,
         ignore_reserves => $ignore_reserves,
         trigger => "Manual"
     });
-    is( $dotransfer, 0, 'Transfer of reserved item failed with ignore reserves: true' );
-    is_deeply(
-        $messages,
-        { 'DestinationEqualsHolding' => 1 },
-        "We got the expected failure message: DestinationEqualsHolding"
-    );
-    isnt( $messages->{ResFound}->{ResFound}, 'Reserved', "We did not return that we found a reserve");
-    isnt( $messages->{ResFound}->{itemnumber}, $item->itemnumber, "We did not return the reserve info");
+    is( $dotransfer, 1, 'Transfer of reserved item succeed with ignore reserves: true' );
+    is( $messages->{ResFound}->{ResFound}, 'Reserved', "We found the reserve");
+    is( $messages->{ResFound}->{itemnumber}, $item->itemnumber, "We got the reserve info");
 };
 
 subtest 'transferbook test from branch' => sub {