From f3aa9c219dc84a8bd8b2de100bff03d0d46f939e Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Fri, 30 Jun 2017 14:23:55 -0400 Subject: [PATCH] Bug 18887 - Port max_holds rules to new CirculationRules system This is the first step in the circulation rules revamp as detailed in the RFF https://wiki.koha-community.org/wiki/Circulation_Rules_Interface_and_Backend_Revamp_RFC This patch moves the recent max_holds rule to the new circulation_rules table. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 3) Go to the circ rules editor, note the new max holds rules by patron category in the "Checkout limit by patron category". ( Should we rename this section? ) 4) Create find a patron that is allowed to place a hold, count the number of holds that patron has. Lets make that number 'X'. 5) Set the new max holds rule to X for "All libraries" 6) Note the patron can no longer place another hold 7) Set the new max holds rule to X + 1 for the patron's home library 8) Note the patron can again place another hold 9) Set the new max holds rule to X for the patron's home library 10) Note the patron can no longer place another hold --- C4/Reserves.pm | 19 +++++---- admin/smart-rules.pl | 48 ++++++++++++++++++++-- .../prog/en/modules/admin/smart-rules.tt | 9 ++-- t/db_dependent/Holds.t | 29 ++++++++----- 4 files changed, 80 insertions(+), 25 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index a149040791..1796420f32 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -48,6 +48,7 @@ use Koha::IssuingRules; use Koha::Items; use Koha::ItemTypes; use Koha::Patrons; +use Koha::CirculationRules; use List::MoreUtils qw( firstidx any ); use Carp; @@ -410,26 +411,30 @@ sub CanItemBeReserved { } # Now we need to check hold limits by patron category - my $schema = Koha::Database->new()->schema(); - my $rule = $schema->resultset('BranchBorrowerCircRule')->find( + my $rule = Koha::CirculationRules->find( { - branchcode => $borrower->{branchcode}, categorycode => $borrower->{categorycode}, + branchcode => $borrower->{branchcode}, + itemtype => undef, + rule_name => 'max_holds', } ); - $rule ||= $schema->resultset('DefaultBorrowerCircRule')->find( + $rule ||= Koha::CirculationRules->find( { - categorycode => $borrower->{categorycode} + categorycode => $borrower->{categorycode}, + branchcode => undef,, + itemtype => undef, + rule_name => 'max_holds', } ); - if ( $rule && defined $rule->max_holds ) { + if ( $rule ) { my $total_holds_count = Koha::Holds->search( { borrowernumber => $borrower->{borrowernumber} } )->count(); - return 'tooManyReserves' if $total_holds_count >= $rule->max_holds; + return 'tooManyReserves' if $total_holds_count >= $rule->rule_value; } my $circ_control_branch = diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index eb4ef7b54f..acf71c8562 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -32,6 +32,7 @@ use Koha::Logger; use Koha::RefundLostItemFeeRule; use Koha::RefundLostItemFeeRules; use Koha::Libraries; +use Koha::CirculationRules; use Koha::Patron::Categories; my $input = CGI->new; @@ -93,6 +94,15 @@ elsif ($op eq 'delete-branch-cat') { AND categorycode = ?"); $sth_delete->execute($branch, $categorycode); } + Koha::CirculationRules->set_rule( + { + branchcode => $branch, + categorycode => $categorycode, + itemtype => undef, + rule_name => 'max_holds', + rule_value => undef, + } + ); } elsif ($op eq 'delete-branch-item') { my $itemtype = $input->param('itemtype'); @@ -283,10 +293,20 @@ elsif ($op eq "add-branch-cat") { $sth_search->execute(); my $res = $sth_search->fetchrow_hashref(); if ($res->{total}) { - $sth_update->execute( $maxissueqty, $maxonsiteissueqty, $max_holds ); + $sth_update->execute( $maxissueqty, $maxonsiteissueqty ); } else { - $sth_insert->execute( $maxissueqty, $maxonsiteissueqty, $max_holds ); + $sth_insert->execute( $maxissueqty, $maxonsiteissueqty ); } + + Koha::CirculationRules->set_rule( + { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rule_name => 'max_holds', + rule_value => $max_holds, + } + ); } else { my $sth_search = $dbh->prepare("SELECT count(*) AS total FROM default_borrower_circ_rules @@ -306,10 +326,20 @@ elsif ($op eq "add-branch-cat") { $sth_search->execute($categorycode); my $res = $sth_search->fetchrow_hashref(); if ($res->{total}) { - $sth_update->execute( $maxissueqty, $maxonsiteissueqty, $max_holds, $categorycode ); + $sth_update->execute( $maxissueqty, $maxonsiteissueqty, $categorycode ); } else { - $sth_insert->execute( $categorycode, $maxissueqty, $maxonsiteissueqty, $max_holds ); + $sth_insert->execute( $categorycode, $maxissueqty, $maxonsiteissueqty ); } + + Koha::CirculationRules->set_rule( + { + branchcode => undef, + categorycode => $categorycode, + itemtype => undef, + rule_name => 'max_holds', + rule_value => $max_holds, + } + ); } } elsif ($categorycode eq "*") { my $sth_search = $dbh->prepare("SELECT count(*) AS total @@ -359,6 +389,16 @@ elsif ($op eq "add-branch-cat") { } else { $sth_insert->execute($branch, $categorycode, $maxissueqty, $maxonsiteissueqty, $max_holds); } + + Koha::CirculationRules->set_rule( + { + branchcode => $branch, + categorycode => $categorycode, + itemtype => undef, + rule_name => 'max_holds', + rule_value => $max_holds, + } + ); } } elsif ($op eq "add-branch-item") { diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt index 9d21f0661b..fe23c9bf69 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt @@ -1,5 +1,6 @@ [% USE Asset %] [% USE Branches %] +[% USE CirculationRules %] [% SET footerjs = 1 %] [% INCLUDE 'doc-head-open.inc' %] Koha › Administration › Circulation and fine rules @@ -511,10 +512,12 @@ [% branch_cat_rule_loo.maxonsiteissueqty %] [% END %] - [% IF ( branch_cat_rule_loo.unlimited_max_holds ) %] - Unlimited + + [% SET rule_value = CirculationRules.Get( branch_cat_rule_loo.branchcode, branch_cat_rule_loo.categorycode, branch_cat_rule_loo.itemtype, 'max_holds' ) %] + [% IF rule_value %] + [% rule_value %] [% ELSE %] - [% branch_cat_rule_loo.max_holds %] + Unlimited [% END %] diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 69eff35be2..c7767330db 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -18,6 +18,7 @@ use Koha::DateUtils qw( dt_from_string output_pref ); use Koha::Biblios; use Koha::Holds; use Koha::Patrons; +use Koha::CirculationRules; BEGIN { use FindBin; @@ -416,6 +417,7 @@ subtest 'Test max_holds per library/patron category' => sub { $dbh->do('DELETE FROM reserves'); $dbh->do('DELETE FROM issuingrules'); + $dbh->do('DELETE FROM circulation_rules'); ( $bibnum, $title, $bibitemnum ) = create_helper_biblio('TEST'); ( $item_bibnum, $item_bibitemnum, $itemnumber ) = @@ -440,20 +442,25 @@ subtest 'Test max_holds per library/patron category' => sub { my $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber ); is( $ret, 'OK', 'Patron can place hold with no borrower circ rules' ); - my $rule_all = $schema->resultset('DefaultBorrowerCircRule')->new( + my $rule_all = Koha::CirculationRules->set_rule( { categorycode => $category->{categorycode}, - max_holds => 3, + branchcode => undef, + itemtype => undef, + rule_name => 'max_holds', + rule_value => 3, } - )->insert(); + ); - my $rule_branch = $schema->resultset('BranchBorrowerCircRule')->new( + my $rule_branch = Koha::CirculationRules->set_rule( { branchcode => $branch_1, categorycode => $category->{categorycode}, - max_holds => 5, + itemtype => undef, + rule_name => 'max_holds', + rule_value => 5, } - )->insert(); + ); $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber ); is( $ret, 'OK', 'Patron can place hold with branch/category rule of 5, category rule of 3' ); @@ -464,16 +471,16 @@ subtest 'Test max_holds per library/patron category' => sub { is( $ret, 'tooManyReserves', 'Patron cannot place hold with only a category rule of 3' ); $rule_all->delete(); - $rule_branch->max_holds(3); - $rule_branch->insert(); + $rule_branch->rule_value(3); + $rule_branch->store(); $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber ); is( $ret, 'tooManyReserves', 'Patron cannot place hold with only a branch/category rule of 3' ); - $rule_branch->max_holds(5); + $rule_branch->rule_value(5); $rule_branch->update(); - $rule_all->max_holds(5); - $rule_all->insert(); + $rule_branch->rule_value(5); + $rule_branch->store(); $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber ); is( $ret, 'OK', 'Patron can place hold with branch/category rule of 5, category rule of 5' ); -- 2.11.0