From 25d163e7a99aa257f9484b6d81b0f4b4dd081781 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 3 Nov 2016 13:20:17 +0000 Subject: [PATCH] Bug 17560: Update current code Content-Type: text/plain; charset=utf-8 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 Signed-off-by: Marcel de Rooy --- 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 6a1756f..ce73140 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -731,7 +731,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