From 4acfbe283a4855a5dac246ad7d96548ed89bb12c Mon Sep 17 00:00:00 2001
From: Kyle M Hall <kyle@bywatersolutions.com>
Date: Wed, 28 Jun 2017 08:54:03 -0400
Subject: [PATCH] Bug 15524 - Set limit on maximum possible holds per patron by
 category
Content-Type: text/plain; charset=utf-8

It's possible to set a limit on the maximum number of holds for a particular branch/category/itemtype, but not on the total number of holds for a given patron (by branch/category).
This new rule works in conjunction with the existing branch/borrower/item rules in that Koha will use the lower of the two limits. This new rule counts all holds of all types, which prevents bib-level holds from not being counted for the purpose of these limits. This makes the most sense and was also requested by the sponsor.

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: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
---
 C4/Reserves.pm                                     | 23 ++++++++
 admin/smart-rules.pl                               | 35 ++++++-----
 installer/data/mysql/atomicupdate/bug_15524.perl   | 18 ++++++
 installer/data/mysql/kohastructure.sql             |  2 +
 .../prog/en/modules/admin/smart-rules.tt           |  8 +++
 t/db_dependent/Holds.t                             | 69 +++++++++++++++++++++-
 6 files changed, 140 insertions(+), 15 deletions(-)
 create mode 100644 installer/data/mysql/atomicupdate/bug_15524.perl

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index 73b86b6..bf8e499 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -398,6 +398,29 @@ sub CanItemBeReserved {
         return 'tooManyReserves';
     }
 
+    # Now we need to check hold limits by patron category
+    my $schema = Koha::Database->new()->schema();
+    my $rule = $schema->resultset('BranchBorrowerCircRule')->find(
+        {
+            branchcode   => $borrower->{branchcode},
+            categorycode => $borrower->{categorycode},
+        }
+    );
+    $rule ||= $schema->resultset('DefaultBorrowerCircRule')->find(
+        {
+            categorycode => $borrower->{categorycode}
+        }
+    );
+    if ( $rule && defined $rule->max_holds ) {
+        my $total_holds_count = Koha::Holds->search(
+            {
+                borrowernumber => $borrower->{borrowernumber}
+            }
+        )->count();
+
+        return 'tooManyReserves' if $total_holds_count >= $rule->max_holds;
+    }
+
     my $circ_control_branch =
       C4::Circulation::_GetCircControlBranch( $item->unblessed(), $borrower );
     my $branchitemrule =
diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl
index 718b682..3429778 100755
--- a/admin/smart-rules.pl
+++ b/admin/smart-rules.pl
@@ -254,10 +254,13 @@ elsif ($op eq "add-branch-cat") {
     my $categorycode  = $input->param('categorycode');
     my $maxissueqty   = $input->param('maxissueqty');
     my $maxonsiteissueqty = $input->param('maxonsiteissueqty');
+    my $max_holds = $input->param('max_holds');
     $maxissueqty =~ s/\s//g;
     $maxissueqty = undef if $maxissueqty !~ /^\d+/;
     $maxonsiteissueqty =~ s/\s//g;
     $maxonsiteissueqty = undef if $maxonsiteissueqty !~ /^\d+/;
+    $max_holds =~ s/\s//g;
+    $max_holds = undef if $max_holds !~ /^\d+/;
 
     if ($branch eq "*") {
         if ($categorycode eq "*") {
@@ -265,21 +268,22 @@ elsif ($op eq "add-branch-cat") {
                                             FROM default_circ_rules");
             my $sth_insert = $dbh->prepare(q|
                 INSERT INTO default_circ_rules
-                    (maxissueqty, maxonsiteissueqty)
-                    VALUES (?, ?)
+                    (maxissueqty, maxonsiteissueqty, max_holds)
+                    VALUES (?, ?, ?)
             |);
             my $sth_update = $dbh->prepare(q|
                 UPDATE default_circ_rules
                 SET maxissueqty = ?,
-                    maxonsiteissueqty = ?
+                    maxonsiteissueqty = ?,
+                    max_holds = ?
             |);
 
             $sth_search->execute();
             my $res = $sth_search->fetchrow_hashref();
             if ($res->{total}) {
-                $sth_update->execute($maxissueqty, $maxonsiteissueqty);
+                $sth_update->execute( $maxissueqty, $maxonsiteissueqty, $max_holds );
             } else {
-                $sth_insert->execute($maxissueqty, $maxonsiteissueqty);
+                $sth_insert->execute( $maxissueqty, $maxonsiteissueqty, $max_holds );
             }
         } else {
             my $sth_search = $dbh->prepare("SELECT count(*) AS total
@@ -287,21 +291,22 @@ elsif ($op eq "add-branch-cat") {
                                             WHERE categorycode = ?");
             my $sth_insert = $dbh->prepare(q|
                 INSERT INTO default_borrower_circ_rules
-                    (categorycode, maxissueqty, maxonsiteissueqty)
-                    VALUES (?, ?, ?)
+                    (categorycode, maxissueqty, maxonsiteissueqty, max_holds)
+                    VALUES (?, ?, ?, ?)
             |);
             my $sth_update = $dbh->prepare(q|
                 UPDATE default_borrower_circ_rules
                 SET maxissueqty = ?,
-                    maxonsiteissueqty = ?
+                    maxonsiteissueqty = ?,
+                    max_holds = ?
                 WHERE categorycode = ?
             |);
             $sth_search->execute($categorycode);
             my $res = $sth_search->fetchrow_hashref();
             if ($res->{total}) {
-                $sth_update->execute($maxissueqty, $maxonsiteissueqty, $categorycode);
+                $sth_update->execute( $maxissueqty, $maxonsiteissueqty, $categorycode, $max_holds );
             } else {
-                $sth_insert->execute($categorycode, $maxissueqty, $maxonsiteissueqty);
+                $sth_insert->execute( $categorycode, $maxissueqty, $maxonsiteissueqty, $max_holds );
             }
         }
     } elsif ($categorycode eq "*") {
@@ -333,13 +338,14 @@ elsif ($op eq "add-branch-cat") {
                                         AND   categorycode = ?");
         my $sth_insert = $dbh->prepare(q|
             INSERT INTO branch_borrower_circ_rules
-            (branchcode, categorycode, maxissueqty, maxonsiteissueqty)
-            VALUES (?, ?, ?, ?)
+            (branchcode, categorycode, maxissueqty, maxonsiteissueqty, max_holds)
+            VALUES (?, ?, ?, ?, ?)
         |);
         my $sth_update = $dbh->prepare(q|
             UPDATE branch_borrower_circ_rules
             SET maxissueqty = ?,
                 maxonsiteissueqty = ?
+                max_holds = ?
             WHERE branchcode = ?
             AND categorycode = ?
         |);
@@ -347,9 +353,9 @@ elsif ($op eq "add-branch-cat") {
         $sth_search->execute($branch, $categorycode);
         my $res = $sth_search->fetchrow_hashref();
         if ($res->{total}) {
-            $sth_update->execute($maxissueqty, $maxonsiteissueqty, $branch, $categorycode);
+            $sth_update->execute($maxissueqty, $maxonsiteissueqty, $max_holds, $branch, $categorycode);
         } else {
-            $sth_insert->execute($branch, $categorycode, $maxissueqty, $maxonsiteissueqty);
+            $sth_insert->execute($branch, $categorycode, $maxissueqty, $maxonsiteissueqty, $max_holds);
         }
     }
 }
@@ -545,6 +551,7 @@ my @sorted_branch_cat_rules = sort { $a->{'humancategorycode'} cmp $b->{'humanca
 foreach my $entry (@sorted_branch_cat_rules, @sorted_row_loop) {
     $entry->{unlimited_maxissueqty} = 1 unless defined($entry->{maxissueqty});
     $entry->{unlimited_maxonsiteissueqty} = 1 unless defined($entry->{maxonsiteissueqty});
+    $entry->{unlimited_max_holds} = 1 unless defined($entry->{max_holds});
 }
 
 @sorted_row_loop = sort by_category_and_itemtype @row_loop;
diff --git a/installer/data/mysql/atomicupdate/bug_15524.perl b/installer/data/mysql/atomicupdate/bug_15524.perl
new file mode 100644
index 0000000..4e23d91
--- /dev/null
+++ b/installer/data/mysql/atomicupdate/bug_15524.perl
@@ -0,0 +1,18 @@
+$DBversion = 'XXX';  # will be replaced by the RM
+if( CheckVersion( $DBversion ) ) {
+
+    if( !column_exists( 'branch_borrower_circ_rules', 'max_holds' ) ) {
+        $dbh->do(q{
+            ALTER TABLE branch_borrower_circ_rules ADD COLUMN max_holds INT(4) NULL DEFAULT NULL AFTER maxonsiteissueqty
+        });
+    }
+
+    if( !column_exists( 'default_borrower_circ_rules', 'max_holds' ) ) {
+        $dbh->do(q{
+            ALTER TABLE default_borrower_circ_rules ADD COLUMN max_holds INT(4) NULL DEFAULT NULL AFTER maxonsiteissueqty
+        });
+    }
+
+    SetVersion( $DBversion );
+    print "Upgrade to $DBversion done (Bug 15524 - Set limit on maximum possible holds per patron by category)\n";
+}
diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql
index 44901ab..3cddb8b 100644
--- a/installer/data/mysql/kohastructure.sql
+++ b/installer/data/mysql/kohastructure.sql
@@ -349,6 +349,7 @@ CREATE TABLE `branch_borrower_circ_rules` ( -- includes default circulation rule
   `categorycode` VARCHAR(10) NOT NULL, -- the patron category this rule applies to (categories.categorycode)
   `maxissueqty` int(4) default NULL, -- the maximum number of checkouts this patron category can have at this branch
   `maxonsiteissueqty` int(4) default NULL, -- the maximum number of on-site checkouts this patron category can have at this branch
+  max_holds INT(4) NULL DEFAULT NULL, -- the maximum number of holds a patron may have at a time
   PRIMARY KEY (`categorycode`, `branchcode`),
   CONSTRAINT `branch_borrower_circ_rules_ibfk_1` FOREIGN KEY (`categorycode`) REFERENCES `categories` (`categorycode`)
     ON DELETE CASCADE ON UPDATE CASCADE,
@@ -365,6 +366,7 @@ CREATE TABLE `default_borrower_circ_rules` ( -- default checkout rules found und
   `categorycode` VARCHAR(10) NOT NULL, -- patron category this rul
   `maxissueqty` int(4) default NULL,
   `maxonsiteissueqty` int(4) default NULL,
+  max_holds INT(4) NULL DEFAULT NULL, -- the maximum number of holds a patron may have at a time
   PRIMARY KEY (`categorycode`),
   CONSTRAINT `borrower_borrower_circ_rules_ibfk_1` FOREIGN KEY (`categorycode`) REFERENCES `categories` (`categorycode`)
     ON DELETE CASCADE ON UPDATE CASCADE
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 e1a5839..19b7b9a 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
@@ -464,6 +464,7 @@
                     <th>Patron category</th>
                     <th>Total current checkouts allowed</th>
                     <th>Total current on-site checkouts allowed</th>
+                    <th>Maximum total holds allowed (count)</th>
                     <th>&nbsp;</th>
                 </tr>
                 [% FOREACH branch_cat_rule_loo IN branch_cat_rule_loop %]
@@ -490,6 +491,12 @@
                                 [% branch_cat_rule_loo.maxonsiteissueqty %]
                             [% END %]
                         </td>
+                        <td>[% IF ( branch_cat_rule_loo.unlimited_max_holds ) %]
+                                Unlimited
+                            [% ELSE %]
+                                [% branch_cat_rule_loo.max_holds %]
+                            [% END %]
+                        </td>
 
                         <td class="actions">
                             <a class="btn btn-default btn-xs delete" href="/cgi-bin/koha/admin/smart-rules.pl?op=delete-branch-cat&amp;categorycode=[% branch_cat_rule_loo.categorycode %]&amp;branch=[% current_branch %]"><i class="fa fa-trash"></i> Delete</a>
@@ -506,6 +513,7 @@
                     </td>
                     <td><input name="maxissueqty" size="3" /></td>
                     <td><input name="maxonsiteissueqty" size="3" /></td>
+                    <td><input name="max_holds" size="3" /></td>
                     <td class="actions"><button type="submit" class="btn btn-default btn-xs"><i class="fa fa-plus"></i> Add</td>
                 </tr>
             </table>
diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t
index 1e6c234..29ce0c5 100755
--- a/t/db_dependent/Holds.t
+++ b/t/db_dependent/Holds.t
@@ -7,7 +7,7 @@ use t::lib::TestBuilder;
 
 use C4::Context;
 
-use Test::More tests => 55;
+use Test::More tests => 56;
 use MARC::Record;
 use C4::Biblio;
 use C4::Items;
@@ -411,6 +411,73 @@ my $res_id = AddReserve( $branch_1, $borrowernumbers[0], $bibnum, '', 1, );
 is( CanItemBeReserved( $borrowernumbers[0], $itemnumber ),
     'tooManyReserves', 'Patron cannot reserve item with hold limit of 1, 1 bib level hold placed' );
 
+subtest 'Test max_holds per library/patron category' => sub {
+    plan tests => 6;
+
+    $dbh->do('DELETE FROM reserves');
+    $dbh->do('DELETE FROM issuingrules');
+
+    ( $bibnum, $title, $bibitemnum ) = create_helper_biblio('TEST');
+    ( $item_bibnum, $item_bibitemnum, $itemnumber ) =
+      AddItem( { homebranch => $branch_1, holdingbranch => $branch_1 },
+        $bibnum );
+    $dbh->do(
+        q{
+            INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record)
+            VALUES (?, ?, ?, ?, ?)
+        },
+        {},
+        '*', '*', 'TEST', 99, 99
+    );
+    AddReserve( $branch_1, $borrowernumbers[0], $bibnum, '', 1, );
+    AddReserve( $branch_1, $borrowernumbers[0], $bibnum, '', 1, );
+    AddReserve( $branch_1, $borrowernumbers[0], $bibnum, '', 1, );
+
+    my $count =
+      Koha::Holds->search( { borrowernumber => $borrowernumbers[0] } )->count();
+    is( $count, 3, 'Patron now has 3 holds' );
+
+    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(
+        {
+            categorycode => $category->{categorycode},
+            max_holds    => 3,
+        }
+    )->insert();
+
+    my $rule_branch = $schema->resultset('BranchBorrowerCircRule')->new(
+        {
+            branchcode   => $branch_1,
+            categorycode => $category->{categorycode},
+            max_holds    => 5,
+        }
+    )->insert();
+
+    $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber );
+    is( $ret, 'OK', 'Patron can place hold with branch/category rule of 5, category rule of 3' );
+
+    $rule_branch->delete();
+
+    $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber );
+    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();
+
+    $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->update();
+    $rule_branch->max_holds(5);
+    $rule_branch->insert();
+
+    $ret = CanItemBeReserved( $borrowernumbers[0], $itemnumber );
+    is( $ret, 'OK', 'Patron can place hold with branch/category rule of 5, category rule of 5' );
+};
 
 # Helper method to set up a Biblio.
 sub create_helper_biblio {
-- 
2.1.4