@@ -, +, @@ --- C4/Reserves.pm | 2 +- t/db_dependent/Reserves/GetReserveFee.t | 63 ++++++++++++++++++++------------- 2 files changed, 39 insertions(+), 26 deletions(-) --- a/C4/Reserves.pm +++ a/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 --- a/t/db_dependent/Reserves/GetReserveFee.t +++ a/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] ) ); --