Bug 19059: Remove CancelReserve - move tests
authorJonathan Druart <jonathan.druart@bugs.koha-community.org>
Thu, 3 Aug 2017 18:12:25 +0000 (15:12 -0300)
committerJonathan Druart <jonathan.druart@bugs.koha-community.org>
Tue, 12 Sep 2017 15:42:58 +0000 (12:42 -0300)
Move the existing tests

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
t/db_dependent/Holds.t
t/db_dependent/Koha/Holds.t [new file with mode: 0644]
t/db_dependent/Reserves.t

index 241d024..cea4209 100755 (executable)
@@ -7,7 +7,7 @@ use t::lib::TestBuilder;
 
 use C4::Context;
 
-use Test::More tests => 55;
+use Test::More tests => 53;
 use MARC::Record;
 use C4::Biblio;
 use C4::Items;
@@ -127,11 +127,6 @@ $holds = $patron->holds;
 is( $holds->next->borrowernumber, $borrowernumbers[0], "Test Koha::Patron->holds");
 
 
-Koha::Holds->find( $reserve_id )->cancel;
-
-$holds = $biblio->holds;
-is( $holds->count, $borrowers_count - 1, "Koha::Hold->cancel" );
-
 $holds = $item->current_holds;
 $first_hold = $holds->next;
 $borrowernumber = $first_hold->borrowernumber;
@@ -219,7 +214,7 @@ is( $hold->priority, '1', "Test AlterPriority(), move up" );
 
 AlterPriority( 'bottom', $hold->reserve_id );
 $hold = Koha::Holds->find( $reserveid );
-is( $hold->priority, '5', "Test AlterPriority(), move to bottom" );
+is( $hold->priority, '6', "Test AlterPriority(), move to bottom" );
 
 # Regression test for bug 2394
 #
@@ -275,49 +270,22 @@ ok(
     '... unless canreservefromotherbranches is ON (bug 2394)'
 );
 
-# Regression test for bug 11336
-($bibnum, $title, $bibitemnum) = create_helper_biblio('DUMMY');
-($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $bibnum);
-AddReserve(
-    $branch_1,
-    $borrowernumbers[0],
-    $bibnum,
-    '',
-    1,
-);
-
-my $reserveid1 = Koha::Holds->search({ biblionumber => $bibnum, borrowernumber => $borrowernumbers[0] })->next->reserve_id;
-
-($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $bibnum);
-my $reserveid2 = AddReserve(
-    $branch_1,
-    $borrowernumbers[1],
-    $bibnum,
-    '',
-    2,
-);
-
-my $hold1 = Koha::Holds->find( $reserveid1 );
-$hold1->cancel;
-
-my $hold2 = Koha::Holds->find( $reserveid2 );
-is( $hold2->priority, 1, "After cancelreserve, the 2nd reserve becomes the first on the waiting list" );
-
-($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $bibnum);
-my $reserveid3 = AddReserve(
-    $branch_1,
-    $borrowernumbers[0],
-    $bibnum,
-    '',
-    2,
-);
-
-my $hold3 = Koha::Holds->find( $reserveid3 );
-is( $hold3->priority, 2, "New reserve for patron 0, the reserve has a priority = 2" );
-
-ModReserve({ reserve_id => $reserveid2, rank => 'del' });
-$hold3 = Koha::Holds->find( $reserveid3 );
-is( $hold3->priority, 1, "After ModReserve, the 3rd reserve becomes the first on the waiting list" );
+{
+    # Regression test for bug 11336 # Test if ModReserve correctly recalculate the priorities
+    ($bibnum, $title, $bibitemnum) = create_helper_biblio('DUMMY');
+    ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $bibnum);
+    my $reserveid1 = AddReserve($branch_1, $borrowernumbers[0], $bibnum, '', 1);
+    ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $bibnum);
+    my $reserveid2 = AddReserve($branch_1, $borrowernumbers[1], $bibnum, '', 2);
+    ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $bibnum);
+    my $reserveid3 = AddReserve($branch_1, $borrowernumbers[2], $bibnum, '', 3);
+    my $hhh = Koha::Holds->search({ biblionumber => $bibnum });
+    my $hold3 = Koha::Holds->find( $reserveid3 );
+    is( $hold3->priority, 3, "The 3rd hold should have a priority set to 3" );
+    ModReserve({ reserve_id => $reserveid1, rank => 'del' });
+    ModReserve({ reserve_id => $reserveid2, rank => 'del' });
+    is( $hold3->discard_changes->priority, 1, "After ModReserve, the 3rd reserve becomes the first on the waiting list" );
+}
 
 ModItem({ damaged => 1 }, $item_bibnum, $itemnumber);
 t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 1 );
diff --git a/t/db_dependent/Koha/Holds.t b/t/db_dependent/Koha/Holds.t
new file mode 100644 (file)
index 0000000..2821592
--- /dev/null
@@ -0,0 +1,154 @@
+#!/usr/bin/perl
+
+# Copyright 2017 Koha Development team
+#
+# This file is part of Koha
+#
+# Koha is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# Koha is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Koha; if not, see <http://www.gnu.org/licenses>.
+
+use Modern::Perl;
+
+use Test::More tests => 1;
+
+use C4::Reserves;
+use Koha::Holds;
+use Koha::Database;
+
+use t::lib::Mocks;
+use t::lib::TestBuilder;
+
+my $schema = Koha::Database->new->schema;
+$schema->storage->txn_begin;
+
+my $builder = t::lib::TestBuilder->new;
+
+subtest 'cancel' => sub {
+    plan tests => 10;
+    my $biblioitem = $builder->build_object( { class => 'Koha::Biblioitems' } );
+    my $library    = $builder->build_object( { class => 'Koha::Libraries' } );
+    my $itemtype   = $builder->build_object( { class => 'Koha::ItemTypes', value => { rentalcharge => 0 } } );
+    my $item_info  = {
+        biblionumber     => $biblioitem->biblionumber,
+        biblioitemnumber => $biblioitem->biblioitemnumber,
+        homebranch       => $library->branchcode,
+        holdingbranch    => $library->branchcode,
+        itype            => $itemtype->itemtype,
+    };
+    my $item = $builder->build_object( { class => 'Koha::Items', value => $item_info } );
+
+    my ( @patrons, @holds );
+    for my $i ( 0 .. 2 ) {
+        my $priority = $i + 1;
+        my $patron   = $builder->build_object(
+            {
+                class => 'Koha::Patrons',
+                value => { branchcode => $library->branchcode, }
+            }
+        );
+        my $reserve_id = C4::Reserves::AddReserve(
+            $library->branchcode, $patron->borrowernumber,
+            $item->biblionumber,  '',
+            $priority,            undef,
+            undef,                '',
+            "title for fee",      $item->itemnumber,
+        );
+        my $hold = Koha::Holds->find($reserve_id);
+        push @patrons, $patron;
+        push @holds,   $hold;
+    }
+
+    # There are 3 holds on this records
+    my $nb_of_holds =
+      Koha::Holds->search( { biblionumber => $item->biblionumber } )->count;
+    is( $nb_of_holds, 3,
+        'There should have 3 holds placed on this biblio record' );
+    my $first_hold  = $holds[0];
+    my $second_hold = $holds[1];
+    my $third_hold  = $holds[2];
+    is( ref($second_hold), 'Koha::Hold',
+        'We should play with Koha::Hold objects' );
+    is( $second_hold->priority, 2,
+        'Second hold should have a priority set to 3' );
+
+    # Remove the second hold, only 2 should still exist in DB and priorities must have been updated
+    my $is_cancelled = $second_hold->cancel;
+    is( ref($is_cancelled), 'Koha::Hold',
+        'Koha::Hold->cancel should return the Koha::Hold (?)' )
+      ;    # This is can reconsidered
+    is( $second_hold->in_storage, 0,
+        'The hold has been cancelled and does not longer exist in DB' );
+    $nb_of_holds =
+      Koha::Holds->search( { biblionumber => $item->biblionumber } )->count;
+    is( $nb_of_holds, 2,
+        'a hold has been cancelled, there should have only 2 holds placed on this biblio record'
+    );
+
+    # discard_changes to refetch
+    is( $first_hold->discard_changes->priority, 1, 'First hold should still be first' );
+    is( $third_hold->discard_changes->priority, 2, 'Third hold should now be second' );
+
+    subtest 'charge_cancel_fee parameter' => sub {
+        plan tests => 4;
+        my $patron_category = $builder->build_object({ class => 'Koha::Patron::Categories', value => { reservefee => 0 } } );
+        my $patron = $builder->build_object({ class => 'Koha::Patrons', value => { categorycode => $patron_category->categorycode } });
+        is( $patron->account->balance, 0, 'A new patron does not have any charges' );
+
+        my @hold_info = (
+            $library->branchcode, $patron->borrowernumber,
+            $item->biblionumber,  '',
+            1,                    undef,
+            undef,                '',
+            "title for fee",      $item->itemnumber,
+        );
+
+        # First, test cancelling a reserve when there's no charge configured.
+        t::lib::Mocks::mock_preference('ExpireReservesMaxPickUpDelayCharge', 0);
+        my $reserve_id = C4::Reserves::AddReserve( @hold_info );
+        Koha::Holds->find( $reserve_id )->cancel( { charge_cancel_fee => 1 } );
+        is( $patron->account->balance, 0, 'ExpireReservesMaxPickUpDelayCharge=0 - The patron should not have been charged' );
+
+        # Then, test cancelling a reserve when there's no charge desired.
+        t::lib::Mocks::mock_preference('ExpireReservesMaxPickUpDelayCharge', 42);
+        $reserve_id = C4::Reserves::AddReserve( @hold_info );
+        Koha::Holds->find( $reserve_id )->cancel(); # charge_cancel_fee => 0
+        is( $patron->account->balance, 0, 'ExpireReservesMaxPickUpDelayCharge=42, but charge_cancel_fee => 0, The patron should not have been charged' );
+
+
+        # Finally, test cancelling a reserve when there's a charge desired and configured.
+        t::lib::Mocks::mock_preference('ExpireReservesMaxPickUpDelayCharge', 42);
+        $reserve_id = C4::Reserves::AddReserve( @hold_info );
+        Koha::Holds->find( $reserve_id )->cancel( { charge_cancel_fee => 1 } );
+        is( int($patron->account->balance), 42, 'ExpireReservesMaxPickUpDelayCharge=42 and charge_cancel_fee => 1, The patron should have been charged!' );
+    };
+
+    subtest 'waiting hold' => sub {
+        plan tests => 1;
+        my $patron = $builder->build_object({ class => 'Koha::Patrons' });
+        my $reserve_id = C4::Reserves::AddReserve(
+            $library->branchcode, $patron->borrowernumber,
+            $item->biblionumber,  '',
+            1,                    undef,
+            undef,                '',
+            "title for fee",      $item->itemnumber,
+            'W',
+        );
+        Koha::Holds->find( $reserve_id )->cancel;
+        my $hold_old = Koha::Old::Holds->find( $reserve_id );
+        is( $hold_old->found, 'W', 'The found column should have been kept and a hold is cancelled' );
+    };
+};
+
+$schema->storage->txn_rollback;
+
+1;
index 2a927b3..fd79427 100755 (executable)
@@ -17,7 +17,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 67;
+use Test::More tests => 58;
 use Test::MockModule;
 use Test::Warn;
 
@@ -320,11 +320,6 @@ $holds = $biblio->holds;
 is($holds->count, 1, "Only one reserves for this biblio");
 my $reserve_id = $holds->next->reserve_id;
 
-Koha::Holds->find( $reserve_id )->cancel;
-
-my $hold = Koha::Holds->find( $reserve_id );
-is($hold, undef, "Koha::Holds->cancel should have cancel the reserve");
-
 # Tests for bug 9761 (ConfirmFutureHolds): new CheckReserves lookahead parameter, and corresponding change in AddReturn
 # Note that CheckReserve uses its lookahead parameter and does not check ConfirmFutureHolds pref (it should be passed if needed like AddReturn does)
 # Test 9761a: Add a reserve without date, CheckReserve should return it
@@ -569,86 +564,6 @@ $dbh->do(
 );
 ok( !C4::Reserves::OnShelfHoldsAllowed($item, $borrower), "OnShelfHoldsAllowed() disallowed" );
 
-# Tests for bug 14464
-
-$dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum));
-my $patron = Koha::Patrons->find( $borrowernumber );
-my $bz14464_fines = $patron->account->balance;
-is( !$bz14464_fines || $bz14464_fines==0, 1, 'Bug 14464 - No fines at beginning' );
-
-# First, test cancelling a reserve when there's no charge configured.
-t::lib::Mocks::mock_preference('ExpireReservesMaxPickUpDelayCharge', 0);
-
-my $bz14464_reserve = AddReserve(
-    $branch_1,
-    $borrowernumber,
-    $bibnum,
-    undef,
-    '1',
-    undef,
-    undef,
-    '',
-    $title,
-    $itemnumber,
-    'W'
-);
-
-ok( $bz14464_reserve, 'Bug 14464 - 1st reserve correctly created' );
-
-Koha::Holds->find( $bz14464_reserve )->cancel( { charge_cancel_fee => 1 } );
-
-my $old_reserve = Koha::Database->new()->schema()->resultset('OldReserve')->find( $bz14464_reserve );
-is($old_reserve->get_column('found'), 'W', 'Bug 14968 - Keep found column from reserve');
-
-$bz14464_fines = $patron->account->balance;
-is( !$bz14464_fines || $bz14464_fines==0, 1, 'Bug 14464 - No fines after cancelling reserve with no charge configured' );
-
-# Then, test cancelling a reserve when there's no charge desired.
-t::lib::Mocks::mock_preference('ExpireReservesMaxPickUpDelayCharge', 42);
-
-$bz14464_reserve = AddReserve(
-    $branch_1,
-    $borrowernumber,
-    $bibnum,
-    undef,
-    '1',
-    undef,
-    undef,
-    '',
-    $title,
-    $itemnumber,
-    'W'
-);
-
-ok( $bz14464_reserve, 'Bug 14464 - 2nd reserve correctly created' );
-
-Koha::Holds->find( $bz14464_reserve )->cancel();
-
-$bz14464_fines = $patron->account->balance;
-is( !$bz14464_fines || $bz14464_fines==0, 1, 'Bug 14464 - No fines after cancelling reserve with no charge desired' );
-
-# Finally, test cancelling a reserve when there's a charge desired and configured.
-$bz14464_reserve = AddReserve(
-    $branch_1,
-    $borrowernumber,
-    $bibnum,
-    undef,
-    '1',
-    undef,
-    undef,
-    '',
-    $title,
-    $itemnumber,
-    'W'
-);
-
-ok( $bz14464_reserve, 'Bug 14464 - 1st reserve correctly created' );
-
-Koha::Holds->find( $bz14464_reserve )->cancel( { charge_cancel_fee => 1 } );
-
-$bz14464_fines = $patron->account->balance;
-is( int( $bz14464_fines ), 42, 'Bug 14464 - Fine applied after cancelling reserve with charge desired and configured' );
-
 # tests for MoveReserve in relation to ConfirmFutureHolds (BZ 14526)
 #   hold from A pos 1, today, no fut holds: MoveReserve should fill it
 $dbh->do('DELETE FROM reserves', undef, ($bibnum));