From 6be52c790fec0ab9ecc0666a9e4c8d6103f4d14b Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 3 Nov 2016 13:20:17 +0000 Subject: [PATCH] [SIGNED-OFF] Bug 17560: Update current code This patch updates the current code to make it works with the new option's name of the syspref. It also refactor the tests to make them more reusable and robust. Sponsored-by: Cheshire Libraries Signed-off-by: Josef Moravec --- C4/Reserves.pm | 2 +- t/db_dependent/Reserves/GetReserveFee.t | 63 ++++++++++++++++++++------------- 2 files changed, 39 insertions(+), 26 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index ae1c7b7..9283d1a 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -730,7 +730,7 @@ SELECT COUNT(*) FROM reserves WHERE biblionumber=? AND borrowernumber<>? my $dbh = C4::Context->dbh; my ( $fee ) = $dbh->selectrow_array( $borquery, undef, ($borrowernumber) ); my $hold_fee_mode = C4::Context->preference('HoldFeeMode') || 'not_always'; - if( $fee and $fee > 0 and $hold_fee_mode ne 'always' ) { + if( $fee and $fee > 0 and $hold_fee_mode eq 'not_always' ) { # This is a reconstruction of the old code: # Compare number of items with items issued, and optionally check holds # If not all items are issued and there are no holds: charge no fee diff --git a/t/db_dependent/Reserves/GetReserveFee.t b/t/db_dependent/Reserves/GetReserveFee.t index 3b477ea..b86204f 100755 --- a/t/db_dependent/Reserves/GetReserveFee.t +++ b/t/db_dependent/Reserves/GetReserveFee.t @@ -21,7 +21,7 @@ use Modern::Perl; -use Test::More tests => 6; +use Test::More tests => 2; use Test::MockModule; use t::lib::TestBuilder; use t::lib::Mocks; @@ -93,30 +93,43 @@ my $acc1 = acctlines( $patron1->{borrowernumber} ); my $res1 = addreserve( $patron1->{borrowernumber} ); is( acctlines( $patron1->{borrowernumber} ), $acc1, 'No fee charged for patron 1' ); -# Issue item1 to patron1. Since there is still a reserve too, we should -# expect a charge for patron2. -C4::Circulation::AddIssue( $patron1, $item1->{barcode}, '2015-12-31', 0, undef, 0, {} ); # the date does not really matter -my $acc2 = acctlines( $patron2->{borrowernumber} ); -t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always'); -my $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->{biblionumber} ); -is( $fee > 0, 1, 'Patron 2 should be charged cf GetReserveFee' ); -C4::Reserves::ChargeReserveFee( $patron2->{borrowernumber}, $fee, $biblio->{title} ); -is( acctlines( $patron2->{borrowernumber} ), $acc2 + 1, 'Patron 2 has been charged by ChargeReserveFee' ); - -# If we delete the reserve, there should be no charge -$dbh->do( "DELETE FROM reserves WHERE reserve_id=?", undef, ( $res1 ) ); -$fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->{biblionumber} ); -is( $fee, 0, 'HoldFeeMode=not_always, Patron 2 should not be charged' ); - -t::lib::Mocks::mock_preference('HoldFeeMode', 'always'); -$fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->{biblionumber} ); -is( int($fee), 2, 'HoldFeeMode=always, Patron 2 should be charged' ); - -# If we delete the second item, there should be a charge -$dbh->do( "DELETE FROM items WHERE itemnumber=?", undef, ( $item2->{itemnumber} ) ); -$fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->{biblionumber} ); -is( int($fee), 2, 'Patron 2 should be charged again this time' ); -# End of tests +subtest 'GetReserveFee' => sub { + plan tests => 7; + + # Issue item1 to patron1. Since there is still a reserve too, we should + # expect a charge for patron2. + C4::Circulation::AddIssue( $patron1, $item1->{barcode}, '2015-12-31', 0, undef, 0, {} ); # the date does not really matter + my $acc2 = acctlines( $patron2->{borrowernumber} ); + + t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always'); + my $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->{biblionumber} ); + is( $fee > 0, 1, 'Patron 2 should be charged cf GetReserveFee' ); + C4::Reserves::ChargeReserveFee( $patron2->{borrowernumber}, $fee, $biblio->{title} ); + is( acctlines( $patron2->{borrowernumber} ), $acc2 + 1, 'Patron 2 has been charged by ChargeReserveFee' ); + + # If we delete the reserve, there should be no charge + $dbh->do( "DELETE FROM reserves WHERE reserve_id=?", undef, ( $res1 ) ); + $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->{biblionumber} ); + is( $fee, 0, 'HoldFeeMode=not_always, Patron 2 should not be charged' ); + + t::lib::Mocks::mock_preference('HoldFeeMode', 'any_time_is_placed'); + $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->{biblionumber} ); + is( int($fee), 2, 'HoldFeeMode=any_time_is_placed, Patron 2 should be charged' ); + + t::lib::Mocks::mock_preference('HoldFeeMode', 'any_time_is_collected'); + $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->{biblionumber} ); + is( int($fee), 2, 'HoldFeeMode=any_time_is_collected, Patron 2 should be charged' ); + + # If we delete the second item, there should be a charge + t::lib::Mocks::mock_preference('HoldFeeMode', 'any_time_is_placed'); + $dbh->do( "DELETE FROM items WHERE itemnumber=?", undef, ( $item2->{itemnumber} ) ); + $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->{biblionumber} ); + is( int($fee), 2, 'Patron 2 should be charged again this time' ); + + t::lib::Mocks::mock_preference('HoldFeeMode', 'any_time_is_collected'); + $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->{biblionumber} ); + is( int($fee), 2, 'HoldFeeMode=any_time_is_collected, Patron 2 should be charged' ); +}; sub acctlines { #calculate number of accountlines for a patron my @temp = $dbh->selectrow_array( "SELECT COUNT(*) FROM accountlines WHERE borrowernumber=?", undef, ( $_[0] ) ); -- 2.1.4