@@ -, +, @@ charged --- C4/Reserves.pm | 5 ++++- .../bootstrap/en/modules/opac-reserve.tt | 21 ++++++++++--------- opac/opac-reserve.pl | 10 +++------ t/db_dependent/Reserves/GetReserveFee.t | 16 ++++++++++---- 4 files changed, 30 insertions(+), 22 deletions(-) --- a/C4/Reserves.pm +++ a/C4/Reserves.pm @@ -773,10 +773,13 @@ SELECT COUNT(*) FROM reserves WHERE biblionumber=? AND borrowernumber<>? my ( $notissued, $reserved ); ( $notissued ) = $dbh->selectrow_array( $issue_qry, undef, ( $biblionumber ) ); - if( $notissued ) { + if( $notissued == 0 ) { + # all items are issued ( $reserved ) = $dbh->selectrow_array( $holds_qry, undef, ( $biblionumber, $borrowernumber ) ); $fee = 0 if $reserved == 0; + } else { + $fee = 0; } } return $fee; --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt +++ a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt @@ -141,16 +141,6 @@

Confirm holds for:[% INCLUDE 'patron-title.inc' patron = logged_in_user %] ([% logged_in_user.cardnumber | html %])

[% END # / UNLESS none_available %] - [% IF (RESERVE_CHARGE) %] -
- [% IF Koha.Preference('HoldFeeMode') == 'any_time_is_collected' %] - You will be charged a hold fee of [% RESERVE_CHARGE | $Price %] when you collect this item - [% ELSE %] - You will be charged a hold fee of [% RESERVE_CHARGE | $Price %] for placing this hold - [% END %] -
- [% END %] - [% IF ( new_reserves_allowed ) %]
Sorry, you can only place [% new_reserves_allowed | html %] more holds. Please uncheck the checkboxes for the items you wish to not place holds on. @@ -178,6 +168,17 @@ [% END %]
[% END %] + + [% IF ( bibitemloo.reserve_charge ) %] +
+ [% IF Koha.Preference('HoldFeeMode') == 'any_time_is_collected' %] + You will be charged a hold fee of [% bibitemloo.reserve_charge | $Price %] when you collect this item + [% ELSE %] + You will be charged a hold fee of [% bibitemloo.reserve_charge | $Price %] for placing this hold + [% END %] +
+ [% END %] +

[% IF ( bibitemloo.holdable ) %] --- a/opac/opac-reserve.pl +++ a/opac/opac-reserve.pl @@ -25,7 +25,7 @@ use CGI qw ( -utf8 ); use C4::Auth qw( get_template_and_user ); use C4::Koha qw( getitemtypeimagelocation getitemtypeimagesrc ); use C4::Circulation qw( GetBranchItemRule ); -use C4::Reserves qw( CanItemBeReserved CanBookBeReserved AddReserve GetReservesControlBranch ItemsAnyAvailableAndNotRestricted IsAvailableForItemLevelRequest ); +use C4::Reserves qw( CanItemBeReserved CanBookBeReserved AddReserve GetReservesControlBranch ItemsAnyAvailableAndNotRestricted IsAvailableForItemLevelRequest GetReserveFee ); use C4::Biblio qw( GetBiblioData GetFrameworkCode ); use C4::Output qw( output_html_with_http_headers ); use C4::Context; @@ -72,12 +72,6 @@ unless ( $can_place_hold_if_available_at_pickup ) { } } -# Pass through any reserve charge -my $reservefee = $category->reservefee; -if ( $reservefee > 0){ - $template->param( RESERVE_CHARGE => $reservefee); -} - my $itemtypes = { map { $_->{itemtype} => $_ } @{ Koha::ItemTypes->search_with_localization->unblessed } }; # There are two ways of calling this script, with a single biblio num @@ -575,6 +569,8 @@ foreach my $biblioNum (@biblionumbers) { $biblioLoopIter{forced_hold_level} = $forced_hold_level; } + # Pass through any reserve charge + $biblioLoopIter{reserve_charge} = GetReserveFee( $patron->id, $biblioNum ); push @$biblioLoop, \%biblioLoopIter; --- a/t/db_dependent/Reserves/GetReserveFee.t +++ a/t/db_dependent/Reserves/GetReserveFee.t @@ -87,6 +87,7 @@ subtest 'GetReserveFee' => sub { plan tests => 5; C4::Circulation::AddIssue( $patron1, $item1->barcode, '2015-12-31', 0, undef, 0, {} ); # the date does not really matter + C4::Circulation::AddIssue( $patron3, $item2->barcode, '2015-12-31', 0, undef, 0, {} ); # the date does not really matter my $acc2 = acctlines( $patron2->{borrowernumber} ); my $res1 = addreserve( $patron1->{borrowernumber} ); @@ -138,8 +139,10 @@ subtest 'Integration with AddReserve' => sub { }; subtest 'Items are issued' => sub { - plan tests => 3; + plan tests => 4; + $dbh->do( "DELETE FROM issues WHERE itemnumber=?", undef, $item1->itemnumber); + $dbh->do( "DELETE FROM issues WHERE itemnumber=?", undef, $item2->itemnumber); C4::Circulation::AddIssue( $patron2, $item1->barcode, '2015-12-31', 0, undef, 0, {} ); t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always'); @@ -152,14 +155,19 @@ subtest 'Integration with AddReserve' => sub { $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} ); addreserve( $patron3->{borrowernumber} ); addreserve( $patron1->{borrowernumber} ); - # FIXME Are we sure it's the expected behavior? - is( acctlines( $patron1->{borrowernumber} ), 1, 'not_always - Patron should be charged if all the items are not checked out and at least 1 hold is already placed' ); + is( acctlines( $patron1->{borrowernumber} ), 0, 'not_always - Patron should not be charged if all the items are not checked out, even if 1 hold is already placed' ); C4::Circulation::AddIssue( $patron3, $item2->barcode, '2015-12-31', 0, undef, 0, {} ); $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber ); $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} ); addreserve( $patron1->{borrowernumber} ); - is( acctlines( $patron1->{borrowernumber} ), 1, 'not_always - Patron should be charged if all items are checked out' ); + is( acctlines( $patron1->{borrowernumber} ), 0, 'not_always - Patron should not be charged if all items are checked out but no holds are placed' ); + + $dbh->do( "DELETE FROM reserves WHERE biblionumber=?", undef, $biblio->biblionumber ); + $dbh->do( "DELETE FROM accountlines WHERE borrowernumber=?", undef, $patron1->{borrowernumber} ); + addreserve( $patron3->{borrowernumber} ); + addreserve( $patron1->{borrowernumber} ); + is( acctlines( $patron1->{borrowernumber} ), 1, 'not_always - Patron should only be charged if all items are checked out and at least 1 hold is already placed' ); }; }; --