From 8c3f8655c26c2da9bf8138862f81ab1c409908dc Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 3 Aug 2017 15:12:25 -0300 Subject: [PATCH] Bug 19059: Remove CancelReserve - move tests Move the existing tests --- t/db_dependent/Holds.t | 68 ++++++------------- t/db_dependent/Koha/Holds.t | 154 ++++++++++++++++++++++++++++++++++++++++++++ t/db_dependent/Reserves.t | 87 +------------------------ 3 files changed, 173 insertions(+), 136 deletions(-) create mode 100644 t/db_dependent/Koha/Holds.t diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 241d0247a4..cea420949e 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -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 index 0000000000..282159288b --- /dev/null +++ b/t/db_dependent/Koha/Holds.t @@ -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 . + +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; diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 5b6656d8df..d5330d2788 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 67; +use Test::More tests => 58; use Test::MockModule; use Test::Warn; @@ -321,11 +321,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 @@ -570,86 +565,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)); -- 2.11.0