@@ -, +, @@ circulation rules 1) Go to /cgi-bin/koha/admin/preferences.pl and set 1) Go to /cgi-bin/koha/admin/smart-rules.pl circulation rules table 2) In that row add numerical values to type/patron categories 3) Prepare an item for our next step, pick a biblio that has item 3) Place a hold on that item and set the expiration date as the 4) Check in it after that. 5) Do SQL request to change expiration date manually to "yesterday": Or alternatively: 5) Wait for tomorrow, change server time or invent a time machine. 6) Run cancel_expired_holds.pl cron job. 7) Check that patron got charged in "accounting" left tab and 8) You can repeat this with other rules and different 9) If there's no matching smart rule or smart rule have an empty 10) But in case if matching rule has zero ("0") as a value - that --- Koha/CirculationRules.pm | 3 + Koha/Hold.pm | 19 +- admin/smart-rules.pl | 3 + .../prog/en/modules/admin/smart-rules.tt | 16 +- t/db_dependent/Holds/Holdfine.t | 258 ++++++++++++++++++ 5 files changed, 296 insertions(+), 3 deletions(-) create mode 100755 t/db_dependent/Holds/Holdfine.t --- a/Koha/CirculationRules.pm +++ a/Koha/CirculationRules.pm @@ -82,6 +82,9 @@ our $RULE_KINDS = { cap_fine_to_replacement_price => { scope => [ 'branchcode', 'categorycode', 'itemtype' ], }, + expire_reserves_charge => { + scope => [ 'branchcode', 'categorycode', 'itemtype' ], + }, chargeperiod => { scope => [ 'branchcode', 'categorycode', 'itemtype' ], }, --- a/Koha/Hold.pm +++ a/Koha/Hold.pm @@ -539,7 +539,24 @@ sub cancel { C4::Reserves::_FixPriority({ biblionumber => $self->biblionumber }); # and, if desired, charge a cancel fee - my $charge = C4::Context->preference("ExpireReservesMaxPickUpDelayCharge"); + my $charge; + if (my $item = $self->item) { + my $branchcode = C4::Reserves::GetReservesControlBranch($item->unblessed, $self->borrower->unblessed); + + my $rule = Koha::CirculationRules->get_effective_rule( + { + categorycode => $self->borrower->categorycode, + itemtype => $item->effective_itemtype, + branchcode => $branchcode, + rule_name => 'expire_reserves_charge', + } + ); + my $rule_value = $rule && $rule->rule_value // ''; + $charge = $rule_value ne '' ? $rule_value : C4::Context->preference("ExpireReservesMaxPickUpDelayCharge"); + } else { + $charge = C4::Context->preference("ExpireReservesMaxPickUpDelayCharge"); + } + if ( $charge && $params->{'charge_cancel_fee'} ) { my $account = Koha::Account->new( { patron_id => $self->borrowernumber } ); --- a/admin/smart-rules.pl +++ a/admin/smart-rules.pl @@ -110,6 +110,7 @@ if ($op eq 'delete') { opacitemholds => undef, overduefinescap => undef, cap_fine_to_replacement_price => undef, + expire_reserves_charge => undef, article_requests => undef, note => undef, } @@ -288,6 +289,7 @@ elsif ($op eq 'add') { my $article_requests = $input->param('article_requests') || 'no'; my $overduefinescap = $input->param('overduefinescap') || ''; my $cap_fine_to_replacement_price = ($input->param('cap_fine_to_replacement_price') || '') eq 'on'; + my $expire_reserves_charge = $input->param('expire_reserves_charge') // ''; my $note = $input->param('note'); my $decreaseloanholds = $input->param('decreaseloanholds') || undef; $debug and warn "Adding $br, $bor, $itemtype, $fine, $maxissueqty, $maxonsiteissueqty, $cap_fine_to_replacement_price"; @@ -322,6 +324,7 @@ elsif ($op eq 'add') { opacitemholds => $opacitemholds, overduefinescap => $overduefinescap, cap_fine_to_replacement_price => $cap_fine_to_replacement_price, + expire_reserves_charge => $expire_reserves_charge, article_requests => $article_requests, note => $note, decreaseloanholds => $decreaseloanholds, --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt @@ -123,6 +123,7 @@ Fine grace period Overdue fines cap (amount) Cap fine at replacement price + Expire reserve charge Suspension in days (day) Max. suspension duration (day) Suspension charging interval @@ -165,6 +166,7 @@ [% SET firstremind = all_rules.$c.$i.firstremind %] [% SET overduefinescap = all_rules.$c.$i.overduefinescap %] [% SET cap_fine_to_replacement_price = all_rules.$c.$i.cap_fine_to_replacement_price %] + [% SET expire_reserves_charge = all_rules.$c.$i.expire_reserves_charge %] [% SET finedays = all_rules.$c.$i.finedays %] [% SET maxsuspensiondays = all_rules.$c.$i.maxsuspensiondays %] [% SET suspension_chargeperiod = all_rules.$c.$i.suspension_chargeperiod %] @@ -184,7 +186,7 @@ [% SET rentaldiscount = all_rules.$c.$i.rentaldiscount %] [% SET decreaseloanholds = all_rules.$c.$i.decreaseloanholds %] - [% SET show_rule = note || maxissueqty || maxonsiteissueqty || issuelength || daysmode || lengthunit || hardduedate || hardduedatecompare || fine || chargeperiod || chargeperiod_charge_at || firstremind || overduefinescap || cap_fine_to_replacement_price || finedays || maxsuspensiondays || suspension_chargeperiod || renewalsallowed || unseenrenewalsallowed || renewalperiod || norenewalbefore || auto_renew || no_auto_renewal_after || no_auto_renewal_after_hard_limit || reservesallowed || holds_per_day || holds_per_record || onshelfholds || opacitemholds || article_requests || rentaldiscount || decreaseloanholds %] + [% SET show_rule = note || maxissueqty || maxonsiteissueqty || issuelength || daysmode || lengthunit || hardduedate || hardduedatecompare || fine || chargeperiod || chargeperiod_charge_at || firstremind || overduefinescap || cap_fine_to_replacement_price || expire_reserves_charge || finedays || maxsuspensiondays || suspension_chargeperiod || renewalsallowed || unseenrenewalsallowed || renewalperiod || norenewalbefore || auto_renew || no_auto_renewal_after || no_auto_renewal_after_hard_limit || reservesallowed || holds_per_day || holds_per_record || onshelfholds || opacitemholds || article_requests || rentaldiscount || decreaseloanholds %] [% IF show_rule %] [% SET row_count = row_count + 1 %] @@ -275,6 +277,13 @@ [% END %] + + [% IF expire_reserves_charge.defined && expire_reserves_charge != '' %] + [% expire_reserves_charge | html %] + [% ELSE %] + Use global + [% END %] + [% finedays | html %] [% maxsuspensiondays | html %] [% suspension_chargeperiod | html %] @@ -427,6 +436,7 @@ + @@ -500,6 +510,7 @@ Fine grace period Overdue fines cap (amount) Cap fine at replacement price + Expire reserve charge Suspension in days (day) Max. suspension duration (day) Suspension charging interval @@ -1138,10 +1149,11 @@ // Remove potential previous input added $(current_column).find("input").remove(); $(current_column).append(""); - } else if ( i == 5 || i == 6 || i == 26 || i == 27 || i == 28 ) { + } else if ( i == 5 || i == 6 || i == 17 || i == 27 || i == 28 || i == 29 ) { // If the value is not an integer for // - "Current checkouts allowed" // - "Current on-site checkouts allowed" + // - "Expire reserve charge" // - "Holds allowed (total)" // - "Holds allowed (daily)" // - "Holds per record (count)" --- a/t/db_dependent/Holds/Holdfine.t +++ a/t/db_dependent/Holds/Holdfine.t @@ -0,0 +1,258 @@ +#!/usr/bin/perl + +# Copyright The National Library of Finland, University of Helsinki 2020 +# Copyright Petro Vashchuk 2020 +# +# 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 . + +use Modern::Perl; + +use C4::Context; +use Koha::CirculationRules; + +use Test::More tests => 13; + +use t::lib::TestBuilder; +use t::lib::Mocks; +use Koha::Holds; + +use Koha::Account; +use Koha::Account::DebitTypes; + +BEGIN { + use_ok('C4::Reserves'); +} + +my $schema = Koha::Database->schema; +$schema->storage->txn_begin; +my $dbh = C4::Context->dbh; + +my $builder = t::lib::TestBuilder->new; + +my $library1 = $builder->build({ + source => 'Branch', +}); +my $library2 = $builder->build({ + source => 'Branch', +}); + +my $bib_title = "Test Title"; + +my $borrower = $builder->build({ + source => 'Borrower', + value => { + branchcode => $library1->{branchcode}, + } +}); + +my $itemtype1 = $builder->build({ + source => 'Itemtype', + value => {} +}); +my $itemtype2 = $builder->build({ + source => 'Itemtype', + value => {} +}); +my $itemtype3 = $builder->build({ + source => 'Itemtype', + value => {} +}); +my $itemtype4 = $builder->build({ + source => 'Itemtype', + value => {} +}); + +my $borrowernumber = $borrower->{borrowernumber}; + +my $library_A_code = $library1->{branchcode}; + +my $biblio = $builder->build_sample_biblio({itemtype => $itemtype1->{itemtype}}); +my $biblionumber = $biblio->biblionumber; +my $item1 = $builder->build_sample_item({ + biblionumber => $biblionumber, + itype => $itemtype1->{itemtype}, + homebranch => $library_A_code, + holdingbranch => $library_A_code +}); +my $item2 = $builder->build_sample_item({ + biblionumber => $biblionumber, + itype => $itemtype2->{itemtype}, + homebranch => $library_A_code, + holdingbranch => $library_A_code +}); +my $item3 = $builder->build_sample_item({ + biblionumber => $biblionumber, + itype => $itemtype3->{itemtype}, + homebranch => $library_A_code, + holdingbranch => $library_A_code +}); + +my $library_B_code = $library2->{branchcode}; + +my $biblio2 = $builder->build_sample_biblio({itemtype => $itemtype4->{itemtype}}); +my $biblionumber2 = $biblio2->biblionumber; +my $item4 = $builder->build_sample_item({ + biblionumber => $biblionumber2, + itype => $itemtype4->{itemtype}, + homebranch => $library_B_code, + holdingbranch => $library_B_code +}); + +$dbh->do("DELETE FROM circulation_rules"); +Koha::CirculationRules->set_rules( + { + itemtype => $itemtype1->{itemtype}, + categorycode => undef, + branchcode => undef, + rules => { + expire_reserves_charge => '111' + } + } +); +Koha::CirculationRules->set_rules( + { + itemtype => $itemtype2->{itemtype}, + categorycode => undef, + branchcode => undef, + rules => { + expire_reserves_charge => undef + } + } +); +Koha::CirculationRules->set_rules( + { + itemtype => undef, + categorycode => undef, + branchcode => $library_B_code, + rules => { + expire_reserves_charge => '444' + } + } +); + +t::lib::Mocks::mock_preference('ReservesControlBranch', 'ItemHomeLibrary'); + +my $reserve_id; +my $account; +my $status; +my $start_balance; + +# TEST: Hold itemtype1 item +$reserve_id = AddReserve( + { + branchcode => $library_A_code, + borrowernumber => $borrowernumber, + biblionumber => $biblionumber, + priority => 1, + itemnumber => $item1->itemnumber, + } +); + +$account = Koha::Account->new({ patron_id => $borrowernumber }); + +( $status ) = CheckReserves($item1->id); +is( $status, 'Reserved', "Hold for the itemtype1 created" ); + +$start_balance = $account->balance(); + +Koha::Holds->find( $reserve_id )->cancel({ charge_cancel_fee => 1 }); + +( $status ) = CheckReserves($item1->id); +is( $status, '', "Hold for the itemtype1 cancelled" ); + +is( $account->balance() - $start_balance, 111, "Used circulation rule for itemtype1" ); + +# TEST: circulation rule for itemtype2 has 'expire_reserves_charge' set undef, so it should use ExpireReservesMaxPickUpDelayCharge preference +t::lib::Mocks::mock_preference('ExpireReservesMaxPickUpDelayCharge', 222); + +$reserve_id = AddReserve( + { + branchcode => $library_A_code, + borrowernumber => $borrowernumber, + biblionumber => $biblionumber, + priority => 1, + itemnumber => $item2->itemnumber, + } +); + +$account = Koha::Account->new({ patron_id => $borrowernumber }); + +( $status ) = CheckReserves($item2->id); +is( $status, 'Reserved', "Hold for the itemtype2 created" ); + +$start_balance = $account->balance(); + +Koha::Holds->find( $reserve_id )->cancel({ charge_cancel_fee => 1 }); + +( $status ) = CheckReserves($item2->id); +is( $status, '', "Hold for the itemtype2 cancelled" ); + +is( $account->balance() - $start_balance, 222, "Used ExpireReservesMaxPickUpDelayCharge preference as expire_reserves_charge set to undef" ); + +# TEST: no circulation rules for itemtype3, it should use ExpireReservesMaxPickUpDelayCharge preference +t::lib::Mocks::mock_preference('ExpireReservesMaxPickUpDelayCharge', 333); + +$reserve_id = AddReserve( + { + branchcode => $library_A_code, + borrowernumber => $borrowernumber, + biblionumber => $biblionumber, + priority => 1, + itemnumber => $item3->itemnumber, + } +); + +$account = Koha::Account->new({ patron_id => $borrowernumber }); + +( $status ) = CheckReserves($item3->id); +is( $status, 'Reserved', "Hold for the itemtype3 created" ); + +$start_balance = $account->balance(); + +Koha::Holds->find( $reserve_id )->cancel({ charge_cancel_fee => 1 }); + +( $status ) = CheckReserves($item3->id); +is( $status, '', "Hold for the itemtype3 cancelled" ); + +is( $account->balance() - $start_balance, 333, "Used ExpireReservesMaxPickUpDelayCharge preference as there's no circulation rules for itemtype3" ); + +# TEST: circulation rule for itemtype4 with library_B_code +t::lib::Mocks::mock_preference('ExpireReservesMaxPickUpDelayCharge', 555); + +$reserve_id = AddReserve( + { + branchcode => $library_B_code, + borrowernumber => $borrowernumber, + biblionumber => $biblionumber2, + priority => 1, + itemnumber => $item4->itemnumber, + } +); + +$account = Koha::Account->new({ patron_id => $borrowernumber }); + +( $status ) = CheckReserves($item4->id); +is( $status, 'Reserved', "Hold for the itemtype4 created" ); + +$start_balance = $account->balance(); + +Koha::Holds->find( $reserve_id )->cancel({ charge_cancel_fee => 1 }); + +( $status ) = CheckReserves($item4->id); +is( $status, '', "Hold for the itemtype4 cancelled" ); + +is( $account->balance() - $start_balance, 444, "Used circulation rule for itemtype4 with library_B_code" ); + +$schema->storage->txn_rollback; --