From 35265c0a9bbdb20f40fa49f592aa9a772d27f253 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 Signed-off-by: Lisette Scheer Signed-off-by: Jesse Maseto Signed-off-by: Josef Moravec Signed-off-by: Nick Clemens --- C4/Reserves.pm | 19 +++++--- admin/smart-rules.pl | 48 +++++++++++++++++-- .../prog/en/modules/admin/smart-rules.tt | 9 ++-- t/db_dependent/Holds.t | 30 +++++++----- 4 files changed, 81 insertions(+), 25 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 9bac1e9190..bc5963ff9c 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; @@ -412,26 +413,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 => $branchcode, categorycode => $borrower->{categorycode}, + branchcode => $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 { status => 'tooManyReserves', limit => $rule->max_holds } if $total_holds_count >= $rule->max_holds; + return { status => 'tooManyReserves', limit => $rule->rule_value} if $total_holds_count >= $rule->rule_value; } my $circ_control_branch = diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index d87d8e5451..ae14a8e8eb 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; use Koha::Caches; @@ -97,6 +98,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'); @@ -287,10 +297,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 @@ -310,10 +330,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 @@ -363,6 +393,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 e90dd8730a..5ddf97af3c 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,6 +1,7 @@ [% USE raw %] [% USE Asset %] [% USE Branches %] +[% USE CirculationRules %] [% SET footerjs = 1 %] [% INCLUDE 'doc-head-open.inc' %] Koha › Administration › Circulation and fine rules @@ -512,10 +513,12 @@ [% branch_cat_rule_loo.maxonsiteissueqty | html %] [% 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 | html %] + Unlimited [% END %] diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 4c1be773ec..25e12e0d83 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -21,6 +21,8 @@ use Koha::Biblios; use Koha::Holds; use Koha::Items; use Koha::Libraries; +use Koha::Patrons; +use Koha::CirculationRules; BEGIN { use FindBin; @@ -418,6 +420,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 ) = @@ -442,20 +445,25 @@ subtest 'Test max_holds per library/patron category' => sub { my $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber ); is( $ret->{status}, '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->{status}, 'OK', 'Patron can place hold with branch/category rule of 5, category rule of 3' ); @@ -466,16 +474,16 @@ subtest 'Test max_holds per library/patron category' => sub { is( $ret->{status}, '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->{status}, '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->{status}, 'OK', 'Patron can place hold with branch/category rule of 5, category rule of 5' ); -- 2.39.5