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 <lisetteslatah@gmail.com>

Signed-off-by: Jesse Maseto <jesse@bywatersolution.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index 9bac1e9..bc5963f 100644 (file)
--- 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 d87d8e5..ae14a8e 100755 (executable)
--- 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 e90dd87..5ddf97a 100644 (file)
--- 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' %]
 <title>Koha &rsaquo; Administration &rsaquo; Circulation and fine rules</title>
@@ -512,10 +513,12 @@
                                 [% branch_cat_rule_loo.maxonsiteissueqty | html %]
                             [% END %]
                         </td>
-                        <td>[% IF ( branch_cat_rule_loo.unlimited_max_holds ) %]
-                                Unlimited
+                        <td>
+                            [% 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 %]
                         </td>
 

diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t
index 4c1be77..25e12e0 100755 (executable)
--- 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' );