Bugzilla – Attachment 65608 Details for
Bug 19059
Move C4::Reserves::CancelReserve to the Koha::Hold->cancel
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 19059: Remove CancelReserve - move tests
Bug-19059-Remove-CancelReserve---move-tests.patch (text/plain), 14.67 KB, created by
Jonathan Druart
on 2017-08-07 21:11:46 UTC
(
hide
)
Description:
Bug 19059: Remove CancelReserve - move tests
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2017-08-07 21:11:46 UTC
Size:
14.67 KB
patch
obsolete
>From 8c3f8655c26c2da9bf8138862f81ab1c409908dc Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >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 <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; >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
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=65608">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 19059
:
65607
|
65608
|
65609
|
65610
|
65611
|
65630
|
65631
|
65632
|
65633
|
65634
|
65636
|
65637
|
65638
|
65639
|
65640
|
66783
|
66930
|
66975
|
66976
|
66977
|
66978
|
66979
|
66980
|
66981