Bug 4045 - No check for maximum number of allowed holds.

Re-add the system preference maxreserves.

All the code using maxreserves is still in place. Though it
is not used in the Reserves module, it is used in all the
scripts where holds are placed.

Also adds a check so that a borrower cannot exceed the maximum
number of allowed holds by using the multi-hold feature via
the opac.

Test Plan:
1) Apply this patch
2) Run updatedatabase
3) Set maxreserves to 3, set opactheme to bootstrap
4) Log into the opac as a patron
5) Place 3 holds
6) Attempt to place a 4th hold
7) Note you get an error message and cannot place a forth hold
8) Delete two of those holds
9) Attempt to place 3 or more holds as a multi-hold
10) You should see a warning that you cannot place this many holds
11) Try to anyway
12) You should see an alert to tell you to reduce the number of holds
    you are placing.
13) Reduce the number for holds you are placing to 2
14) Your holds should now be placed

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl
index ad32a87..1d4e96d 100755 (executable)
--- a/installer/data/mysql/updatedatabase.pl
+++ b/installer/data/mysql/updatedatabase.pl
@@ -8596,6 +8596,13 @@ if ( CheckVersion($DBversion) ) {
     SetVersion ($DBversion);
 }
 
+$DBversion = "3.17.00.XXX";
+if ( CheckVersion($DBversion) ) {
+    $dbh->do("INSERT IGNORE INTO systempreferences (variable,value,explanation,options,type) VALUES ('maxreserves',50,'System-wide maximum number of holds a patron can place','','Integer')");
+    print "Upgrade to $DBversion done (Re-add system preference maxreserves)\n";
+    SetVersion($DBversion);
+}
+
 =head1 FUNCTIONS
 
 =head2 TableExists($table)

diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
index 0ed677b..fadddcc 100644 (file)
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
@@ -105,6 +105,12 @@
                             </div>
                         [% END %]
 
+                        [% IF ( new_reserves_allowed ) %]
+                            <div id="new_reserves_allowed" class="alert">
+                                <strong>Sorry,</strong> you can only place [% new_reserves_allowed %] more holds. Please uncheck the checkboxes for the items you wish to not place holds on.
+                            </div>
+                        [% END %]
+
                         <form action="/cgi-bin/koha/opac-reserve.pl" method="post" id="hold-request-form">
                             <input type="hidden" name="place_reserve" value="1"/>
                             <!-- These values are set dynamically by js -->
@@ -563,6 +569,13 @@
             var biblionumbers = "";
             var selections = "";
 
+            [% IF new_reserves_allowed %]
+                if ($(".confirmjs:checked").size() > [% new_reserves_allowed %] ) {
+                    alert(MSG_MAX_HOLDS_EXCEEDED);
+                    return false;
+                }
+            [% END %]
+
             if ($(".confirmjs:checked").size() == 0) {
                 alert(MSG_NO_RECORD_SELECTED);
                 return false;

diff --git a/koha-tmpl/opac-tmpl/bootstrap/js/global.js b/koha-tmpl/opac-tmpl/bootstrap/js/global.js
index 5342113..402026a 100644 (file)
--- a/koha-tmpl/opac-tmpl/bootstrap/js/global.js
+++ b/koha-tmpl/opac-tmpl/bootstrap/js/global.js
@@ -1,3 +1,15 @@
+// http://stackoverflow.com/questions/1038746/equivalent-of-string-format-in-jquery/5341855#5341855
+String.prototype.format = function() { return formatstr(this, arguments) }
+function formatstr(str, col) {
+    col = typeof col === 'object' ? col : Array.prototype.slice.call(arguments, 1);
+    var idx = 0;
+    return str.replace(/%%|%s|%(\d+)\$s/g, function (m, n) {
+        if (m == "%%") { return "%"; }
+        if (m == "%s") { return col[idx++]; }
+        return col[n];
+    });
+};
+
 function confirmDelete(message) {
     return (confirm(message) ? true : false);
 }
@@ -22,4 +34,4 @@ function prefixOf (s, tok) {
 function suffixOf (s, tok) {
     var index = s.indexOf(tok);
     return s.substring(index + 1);
-}
\ No newline at end of file
+}

diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
index 2f3c652..45d03bd 100755 (executable)
--- a/opac/opac-reserve.pl
+++ b/opac/opac-reserve.pl
@@ -38,7 +38,7 @@ use Koha::DateUtils;
 use Date::Calc qw/Today Date_to_Days/;
 # use Data::Dumper;
 
-my $MAXIMUM_NUMBER_OF_RESERVES = C4::Context->preference("maxreserves");
+my $maxreserves = C4::Context->preference("maxreserves");
 
 my $query = new CGI;
 my ( $template, $borrowernumber, $cookie ) = get_template_and_user(
@@ -114,6 +114,7 @@ if (($#biblionumbers < 0) && (! $query->param('place_reserve'))) {
     &get_out($query, $cookie, $template->output);
 }
 
+
 # pass the pickup branch along....
 my $branch = $query->param('branch') || $borr->{'branchcode'} || C4::Context->userenv->{branch} || '' ;
 ($branches->{$branch}) or $branch = "";     # Confirm branch is real
@@ -183,7 +184,7 @@ foreach my $biblioNumber (@biblionumbers) {
 #
 if ( $query->param('place_reserve') ) {
     my $reserve_cnt = 0;
-    if ($MAXIMUM_NUMBER_OF_RESERVES) {
+    if ($maxreserves) {
         $reserve_cnt = GetReservesFromBorrowernumber( $borrowernumber );
     }
 
@@ -267,8 +268,8 @@ if ( $query->param('place_reserve') ) {
         }
         my $notes = $query->param('notes_'.$biblioNum)||'';
 
-        if (   $MAXIMUM_NUMBER_OF_RESERVES
-            && $reserve_cnt >= $MAXIMUM_NUMBER_OF_RESERVES )
+        if (   $maxreserves
+            && $reserve_cnt >= $maxreserves )
         {
             $canreserve = 0;
         }
@@ -308,32 +309,41 @@ if ( $borr->{'amountoutstanding'} && ($borr->{'amountoutstanding'} > $maxoutstan
 if ( $borr->{gonenoaddress} && ($borr->{gonenoaddress} == 1) ) {
     $noreserves = 1;
     $template->param(
-                     message => 1,
-                     GNA     => 1
-                    );
+        message => 1,
+        GNA     => 1
+    );
 }
 if ( $borr->{lost} && ($borr->{lost} == 1) ) {
     $noreserves = 1;
     $template->param(
-                     message => 1,
-                     lost    => 1
-                    );
+        message => 1,
+        lost    => 1
+    );
 }
 if ( $borr->{'debarred'} ) {
     $noreserves = 1;
     $template->param(
-                     message  => 1,
-                     debarred => 1
-                    );
+        message  => 1,
+        debarred => 1
+    );
 }
 
 my @reserves = GetReservesFromBorrowernumber( $borrowernumber );
+my $reserves_count = scalar(@reserves);
 $template->param( RESERVES => \@reserves );
-if ( $MAXIMUM_NUMBER_OF_RESERVES && (scalar(@reserves) >= $MAXIMUM_NUMBER_OF_RESERVES) ) {
+if ( $maxreserves && ( $reserves_count >= $maxreserves ) ) {
     $template->param( message => 1 );
     $noreserves = 1;
     $template->param( too_many_reserves => scalar(@reserves));
 }
+
+unless ( $noreserves ) {
+    my $requested_reserves_count = scalar( @biblionumbers );
+    if ( $maxreserves && ( $reserves_count + $requested_reserves_count > $maxreserves ) ) {
+        $template->param( new_reserves_allowed => $maxreserves - $reserves_count );
+    }
+}
+
 foreach my $res (@reserves) {
     foreach my $biblionumber (@biblionumbers) {
         if ( $res->{'biblionumber'} == $biblionumber && $res->{'borrowernumber'} == $borrowernumber) {

diff --git a/reserve/request.pl b/reserve/request.pl
index 5bd9128..635c773 100755 (executable)
--- a/reserve/request.pl
+++ b/reserve/request.pl
@@ -78,6 +78,8 @@ $findborrower =~ s|,| |g;
 my $borrowernumber_hold = $input->param('borrowernumber') || '';
 my $messageborrower;
 my $maxreserves;
+my $warnings;
+my $messages;
 
 my $date = C4::Dates->today('iso');
 my $action = $input->param('action');
@@ -122,13 +124,14 @@ if ($borrowernumber_hold && !$action) {
     my @getreservloop;
     my $count_reserv = 0;
 
-#   we check the reserves of the borrower, and if he can reserv a document
-# FIXME At this time we have a simple count of reservs, but, later, we could improve the infos "title" ...
+    # we check the reserves of the borrower, and if he can reserv a document
+    # FIXME At this time we have a simple count of reservs, but, later, we could improve the infos "title" ...
 
     my $number_reserves =
       GetReserveCount( $borrowerinfo->{'borrowernumber'} );
 
     if ( C4::Context->preference('maxreserves') && ($number_reserves >= C4::Context->preference('maxreserves')) ) {
+        $warnings = 1;
         $maxreserves = 1;
     }
 
@@ -136,7 +139,7 @@ if ($borrowernumber_hold && !$action) {
     my $expiry_date = $borrowerinfo->{dateexpiry};
     my $expiry = 0; # flag set if patron account has expired
     if ($expiry_date and $expiry_date ne '0000-00-00' and
-            Date_to_Days(split /-/,$date) > Date_to_Days(split /-/,$expiry_date)) {
+        Date_to_Days(split /-/,$date) > Date_to_Days(split /-/,$expiry_date)) {
         $expiry = 1;
     }
 
@@ -162,6 +165,8 @@ if ($borrowernumber_hold && !$action) {
                 cardnumber          => $borrowerinfo->{'cardnumber'},
                 expiry              => $expiry,
                 diffbranch          => $diffbranch,
+                messages            => $messages,
+                warnings            => $warnings
     );
 }
 
@@ -417,8 +422,7 @@ foreach my $biblionumber (@biblionumbers) {
                 $num_available++;
             }
             elsif ( C4::Context->preference('AllowHoldPolicyOverride') ) {
-
-# If AllowHoldPolicyOverride is set, it should override EVERY restriction, not just branch item rules
+                # If AllowHoldPolicyOverride is set, it should override EVERY restriction, not just branch item rules
                 $item->{override} = 1;
                 $num_override++;
             }