From 540b4284338046980f6c13fc437053b95a0318dc Mon Sep 17 00:00:00 2001
From: Lari Taskula <lari.taskula@hypernova.fi>
Date: Tue, 31 Mar 2020 16:25:59 +0000
Subject: [PATCH] Bug 25089: Remove on-site specific circulation rules

To apply:
1. perl installer/data/mysql/updatedatabase.pl

To test:
1. Search for patron_maxonsiteissueqty rule

   $ grep -rn 'patron_maxonsiteissueqty'

2. Observe no results in Koha source files
3. Search for maxonsiteissueqty rule

   $ grep -rn 'maxonsiteissueqty'

4. Observe no results (apart from .git)
5. Check modification made to C4::Circulation::GetBranchBorrowerCircRule().
   Make sure the new parameter is being used everywhere. You can search
   the usage of this subroutine with similar grep commands as above.

6. Run the following tests:

prove t/db_dependent/Circulation/Branch.t
prove t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
prove t/db_dependent/Circulation/TooMany.t

Sponsored-by: The National Library of Finland
---
 C4/Circulation.pm                             | 49 +++++++------------
 Koha/CirculationRules.pm                      |  6 ---
 admin/smart-rules.pl                          | 19 +------
 ...bug_24101_2_convert_circulation_rules.perl | 33 +++++++++++++
 installer/onboarding.pl                       |  2 -
 .../prog/en/modules/admin/smart-rules.tt      | 38 ++------------
 t/db_dependent/Circulation/Branch.t           | 47 ++++++++++++------
 .../Circulation/SwitchOnSiteCheckouts.t       | 26 +++++++++-
 t/db_dependent/Circulation/TooMany.t          | 36 +++++++++-----
 9 files changed, 138 insertions(+), 118 deletions(-)
 create mode 100644 installer/data/mysql/atomicupdate/bug_24101_2_convert_circulation_rules.perl

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index efdc2b15a0..acc1f14786 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -385,6 +385,11 @@ sub TooMany {
 	# Get which branchcode we need
     $branch = _GetCircControlBranch($item_object->unblessed,$borrower);
     my $type = $item_object->effective_itemtype;
+    my $checkout_type = $onsite_checkout
+        ? $switch_onsite_checkout
+            ? $Koha::Checkouts::type->{checkout}
+            : $Koha::Checkouts::type->{onsite_checkout}
+        : $Koha::Checkouts::type->{checkout};
 
     # given branch, patron category, and item type, determine
     # applicable issuing rule
@@ -396,14 +401,6 @@ sub TooMany {
             rule_name    => 'maxissueqty',
         }
     );
-    my $maxonsiteissueqty_rule = Koha::CirculationRules->get_effective_rule(
-        {
-            categorycode => $cat_borrower,
-            itemtype     => $type,
-            branchcode   => $branch,
-            rule_name    => 'maxonsiteissueqty',
-        }
-    );
 
 
     # if a rule is found and has a loan limit set, count
@@ -473,14 +470,13 @@ sub TooMany {
         my ( $checkout_count, $onsite_checkout_count ) = $dbh->selectrow_array( $count_query, {}, @bind_params );
 
         my $max_checkouts_allowed = $maxissueqty_rule ? $maxissueqty_rule->rule_value : undef;
-        my $max_onsite_checkouts_allowed = $maxonsiteissueqty_rule ? $maxonsiteissueqty_rule->rule_value : undef;
 
-        if ( $onsite_checkout and $max_onsite_checkouts_allowed ne '' ) {
-            if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed )  {
+        if ( $onsite_checkout and $max_checkouts_allowed ne '' ) {
+            if ( $onsite_checkout_count >= $max_checkouts_allowed )  {
                 return {
                     reason => 'TOO_MANY_ONSITE_CHECKOUTS',
                     count => $onsite_checkout_count,
-                    max_allowed => $max_onsite_checkouts_allowed,
+                    max_allowed => $max_checkouts_allowed,
                 }
             }
         }
@@ -505,7 +501,7 @@ sub TooMany {
     }
 
     # Now count total loans against the limit for the branch
-    my $branch_borrower_circ_rule = GetBranchBorrowerCircRule($branch, $cat_borrower);
+    my $branch_borrower_circ_rule = GetBranchBorrowerCircRule($branch, $cat_borrower, $checkout_type );
     if (defined($branch_borrower_circ_rule->{patron_maxissueqty}) and $branch_borrower_circ_rule->{patron_maxissueqty} ne '') {
         my @bind_params = ();
         my $branch_count_query = qq|
@@ -527,14 +523,13 @@ sub TooMany {
         }
         my ( $checkout_count, $onsite_checkout_count ) = $dbh->selectrow_array( $branch_count_query, {}, @bind_params );
         my $max_checkouts_allowed = $branch_borrower_circ_rule->{patron_maxissueqty};
-        my $max_onsite_checkouts_allowed = $branch_borrower_circ_rule->{patron_maxonsiteissueqty};
 
-        if ( $onsite_checkout and $max_onsite_checkouts_allowed ne '' ) {
-            if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed )  {
+        if ( $onsite_checkout and $max_checkouts_allowed ne '' ) {
+            if ( $onsite_checkout_count >= $max_checkouts_allowed )  {
                 return {
                     reason => 'TOO_MANY_ONSITE_CHECKOUTS',
                     count => $onsite_checkout_count,
-                    max_allowed => $max_onsite_checkouts_allowed,
+                    max_allowed => $max_checkouts_allowed,
                 }
             }
         }
@@ -1601,7 +1596,7 @@ sub GetHardDueDate {
 
 =head2 GetBranchBorrowerCircRule
 
-  my $branch_cat_rule = GetBranchBorrowerCircRule($branchcode, $categorycode);
+  my $branch_cat_rule = GetBranchBorrowerCircRule($branchcode, $categorycode, $checkout_type);
 
 Retrieves circulation rule attributes that apply to the given
 branch and patron category, regardless of item type.  
@@ -1611,21 +1606,13 @@ patron_maxissueqty - maximum number of loans that a
 patron of the given category can have at the given
 branch.  If the value is undef, no limit.
 
-patron_maxonsiteissueqty - maximum of on-site checkouts that a
-patron of the given category can have at the given
-branch.  If the value is undef, no limit.
-
-This will check for different branch/category combinations in the following order:
-branch and category
-branch only
-category only
-default branch and category
+The order in which rules are searched is defined in circulation
+rules user interface.
 
 If no rule has been found in the database, it will default to
 the buillt in rule:
 
 patron_maxissueqty - undef
-patron_maxonsiteissueqty - undef
 
 C<$branchcode> and C<$categorycode> should contain the
 literal branch code and patron category code, respectively - no
@@ -1634,21 +1621,21 @@ wildcards.
 =cut
 
 sub GetBranchBorrowerCircRule {
-    my ( $branchcode, $categorycode ) = @_;
+    my ( $branchcode, $categorycode, $checkout_type ) = @_;
 
     # Initialize default values
     my $rules = {
         patron_maxissueqty       => undef,
-        patron_maxonsiteissueqty => undef,
     };
 
     # Search for rules!
-    foreach my $rule_name (qw( patron_maxissueqty patron_maxonsiteissueqty )) {
+    foreach my $rule_name (qw( patron_maxissueqty )) {
         my $rule = Koha::CirculationRules->get_effective_rule(
             {
                 categorycode => $categorycode,
                 itemtype     => undef,
                 branchcode   => $branchcode,
+                checkout_type => $checkout_type,
                 rule_name    => $rule_name,
             }
         );
diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm
index 4b8d8d8675..c267930bda 100644
--- a/Koha/CirculationRules.pm
+++ b/Koha/CirculationRules.pm
@@ -53,9 +53,6 @@ our $RULE_KINDS = {
     patron_maxissueqty => {
         scope => [ 'branchcode', 'categorycode', 'checkout_type' ],
     },
-    patron_maxonsiteissueqty => {
-        scope => [ 'branchcode', 'categorycode' ],
-    },
     max_holds => {
         scope => [ 'branchcode', 'categorycode' ],
     },
@@ -115,9 +112,6 @@ our $RULE_KINDS = {
     maxissueqty => {
         scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ],
     },
-    maxonsiteissueqty => {
-        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
-    },
     maxsuspensiondays => {
         scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ],
     },
diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl
index 6bea920dd9..941abdae73 100755
--- a/admin/smart-rules.pl
+++ b/admin/smart-rules.pl
@@ -84,7 +84,6 @@ if ($op eq 'delete') {
             itemtype     => $itemtype eq '*' ? undef : $itemtype,
             rules        => {
                 maxissueqty                      => undef,
-                maxonsiteissueqty                => undef,
                 rentaldiscount                   => undef,
                 fine                             => undef,
                 finedays                         => undef,
@@ -127,7 +126,6 @@ elsif ($op eq 'delete-branch-cat') {
                     rules        => {
                         max_holds                      => undef,
                         patron_maxissueqty             => undef,
-                        patron_maxonsiteissueqty       => undef,
                     }
                 }
             );
@@ -150,7 +148,6 @@ elsif ($op eq 'delete-branch-cat') {
                     rules        => {
                         max_holds                => undef,
                         patron_maxissueqty       => undef,
-                        patron_maxonsiteissueqty => undef,
                     }
                 }
             );
@@ -162,7 +159,6 @@ elsif ($op eq 'delete-branch-cat') {
                 categorycode => undef,
                 rules        => {
                     patron_maxissueqty       => undef,
-                    patron_maxonsiteissueqty => undef,
                 }
             }
         );
@@ -185,7 +181,6 @@ elsif ($op eq 'delete-branch-cat') {
                 rules        => {
                     max_holds         => undef,
                     patron_maxissueqty       => undef,
-                    patron_maxonsiteissueqty => undef,
                 }
             }
         );
@@ -260,7 +255,6 @@ elsif ($op eq 'add') {
     my $chargeperiod = $input->param('chargeperiod');
     my $chargeperiod_charge_at = $input->param('chargeperiod_charge_at');
     my $maxissueqty  = strip_non_numeric($input->param('maxissueqty'));
-    my $maxonsiteissueqty  = strip_non_numeric($input->param('maxonsiteissueqty'));
     my $renewalsallowed  = $input->param('renewalsallowed');
     my $renewalperiod    = $input->param('renewalperiod');
     my $norenewalbefore  = $input->param('norenewalbefore');
@@ -288,11 +282,10 @@ elsif ($op eq 'add') {
     my $overduefinescap = $input->param('overduefinescap') || '';
     my $cap_fine_to_replacement_price = $input->param('cap_fine_to_replacement_price') eq 'on';
     my $note = $input->param('note');
-    $debug and warn "Adding $br, $bor, $itemtype, $fine, $maxissueqty, $maxonsiteissueqty, $cap_fine_to_replacement_price";
+    $debug and warn "Adding $br, $bor, $itemtype, $fine, $maxissueqty, $cap_fine_to_replacement_price";
 
     my $rules = {
         maxissueqty                   => $maxissueqty,
-        maxonsiteissueqty             => $maxonsiteissueqty,
         rentaldiscount                => $rentaldiscount,
         fine                          => $fine,
         finedays                      => $finedays,
@@ -335,8 +328,6 @@ elsif ($op eq 'add') {
 elsif ($op eq "set-branch-defaults") {
     my $categorycode  = $input->param('categorycode');
     my $patron_maxissueqty   = strip_non_numeric($input->param('patron_maxissueqty'));
-    my $patron_maxonsiteissueqty = $input->param('patron_maxonsiteissueqty');
-    $patron_maxonsiteissueqty = strip_non_numeric($patron_maxonsiteissueqty);
     my $holdallowed   = $input->param('holdallowed');
     my $hold_fulfillment_policy = $input->param('hold_fulfillment_policy');
     my $returnbranch  = $input->param('returnbranch');
@@ -362,7 +353,6 @@ elsif ($op eq "set-branch-defaults") {
                 branchcode   => undef,
                 rules        => {
                     patron_maxissueqty             => $patron_maxissueqty,
-                    patron_maxonsiteissueqty       => $patron_maxonsiteissueqty,
                 }
             }
         );
@@ -384,7 +374,6 @@ elsif ($op eq "set-branch-defaults") {
                 branchcode   => $branch,
                 rules        => {
                     patron_maxissueqty             => $patron_maxissueqty,
-                    patron_maxonsiteissueqty       => $patron_maxonsiteissueqty,
                 }
             }
         );
@@ -401,8 +390,6 @@ elsif ($op eq "set-branch-defaults") {
 elsif ($op eq "add-branch-cat") {
     my $categorycode  = $input->param('categorycode');
     my $patron_maxissueqty = strip_non_numeric($input->param('patron_maxissueqty'));
-    my $patron_maxonsiteissueqty = $input->param('patron_maxonsiteissueqty');
-    $patron_maxonsiteissueqty = strip_non_numeric($patron_maxonsiteissueqty);
     my $max_holds = $input->param('max_holds');
     $max_holds =~ s/\s//g;
     $max_holds = undef if $max_holds !~ /^\d+/;
@@ -416,7 +403,6 @@ elsif ($op eq "add-branch-cat") {
                     rules        => {
                         max_holds         => $max_holds,
                         patron_maxissueqty       => $patron_maxissueqty,
-                        patron_maxonsiteissueqty => $patron_maxonsiteissueqty,
                     }
                 }
             );
@@ -428,7 +414,6 @@ elsif ($op eq "add-branch-cat") {
                     rules        => {
                         max_holds         => $max_holds,
                         patron_maxissueqty       => $patron_maxissueqty,
-                        patron_maxonsiteissueqty => $patron_maxonsiteissueqty,
                     }
                 }
             );
@@ -441,7 +426,6 @@ elsif ($op eq "add-branch-cat") {
                 rules        => {
                     max_holds         => $max_holds,
                     patron_maxissueqty       => $patron_maxissueqty,
-                    patron_maxonsiteissueqty => $patron_maxonsiteissueqty,
                 }
             }
         );
@@ -453,7 +437,6 @@ elsif ($op eq "add-branch-cat") {
                 rules        => {
                     max_holds         => $max_holds,
                     patron_maxissueqty       => $patron_maxissueqty,
-                    patron_maxonsiteissueqty => $patron_maxonsiteissueqty,
                 }
             }
         );
diff --git a/installer/data/mysql/atomicupdate/bug_24101_2_convert_circulation_rules.perl b/installer/data/mysql/atomicupdate/bug_24101_2_convert_circulation_rules.perl
new file mode 100644
index 0000000000..7b7df4922e
--- /dev/null
+++ b/installer/data/mysql/atomicupdate/bug_24101_2_convert_circulation_rules.perl
@@ -0,0 +1,33 @@
+$DBversion = 'XXX'; # will be replaced by the RM
+if( CheckVersion( $DBversion ) ) {
+
+    # First, convert maxonsiteissueqty to maxissueqty
+    my $update_sth = $dbh->prepare(q|
+        UPDATE circulation_rules
+        SET checkout_type = ?,
+            rule_name = ?
+        WHERE rule_name=?
+    |);
+    $update_sth->execute(
+        'OS',                       # checkout_type => 'OS' (on-site checkout)
+        'maxissueqty',
+        'maxonsiteissueqty'
+    );
+
+    # Then, convert patron_maxonsiteissueqty to patron_maxissueqty
+    $update_sth = $dbh->prepare(q|
+        UPDATE circulation_rules
+        SET checkout_type = ?,
+            rule_name = ?
+        WHERE rule_name=?
+    |);
+    $update_sth->execute(
+        'OS',                       # checkout_type => 'OS' (on-site checkout)
+        'patron_maxissueqty',
+        'patron_maxonsiteissueqty'
+    );
+
+    # Always end with this (adjust the bug info)
+    SetVersion( $DBversion );
+    print "Upgrade to $DBversion done (Bug 24101 - Convert maxonsiteissueqty to maxissueqty )\n";
+}
diff --git a/installer/onboarding.pl b/installer/onboarding.pl
index 547df29e0d..dab7980ee9 100755
--- a/installer/onboarding.pl
+++ b/installer/onboarding.pl
@@ -273,7 +273,6 @@ if ( $step == 5 ) {
                 holds_per_day                    => $holds_per_day,
                 holds_per_record                 => $holds_per_record,
                 maxissueqty                      => "",
-                maxonsiteissueqty                => "",
                 maxsuspensiondays                => "",
                 no_auto_renewal_after            => "",
                 no_auto_renewal_after_hard_limit => "",
@@ -291,7 +290,6 @@ if ( $step == 5 ) {
             categorycode => $categorycode,
             rules        => {
                 patron_maxissueqty       => "",
-                patron_maxonsiteissueqty => "",
                 max_holds                => "",
             }
         };
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 5ae3bb94e1..86f3edee0d 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
@@ -91,7 +91,6 @@
                 <th>Actions</th>
                 <th>Note</th>
                 <th>Current checkouts allowed</th>
-                <th>Current on-site checkouts allowed</th>
                 <th>Loan period</th>
                 <th>Unit</th>
                 <th>Hard due date</th>
@@ -128,7 +127,6 @@
                         [% SET i = '' UNLESS i.defined %]
                         [% SET note = all_rules.$c.$i.note %]
                         [% SET maxissueqty = all_rules.$c.$i.maxissueqty %]
-                        [% SET maxonsiteissueqty = all_rules.$c.$i.maxonsiteissueqty %]
                         [% SET issuelength = all_rules.$c.$i.issuelength %]
                         [% SET lengthunit = all_rules.$c.$i.lengthunit %]
                         [% SET hardduedate = all_rules.$c.$i.hardduedate %]
@@ -156,7 +154,7 @@
                         [% SET article_requests = all_rules.$c.$i.article_requests %]
                         [% SET rentaldiscount = all_rules.$c.$i.rentaldiscount %]
 
-                        [% SET show_rule = maxissueqty || maxonsiteissueqty || issuelength || lengthunit || hardduedate || hardduedatebefore || hardduedateexact || fine || chargeperiod || chargeperiod_charge_at || firstremind || overduefinescap || cap_fine_to_replacement_price || finedays || maxsuspensiondays || suspension_chargeperiod || renewalsallowed || renewalsallowed || norenewalbefore || auto_renew || no_auto_renewal_after || no_auto_renewal_after_hard_limit || reservesallowed || holds_per_day || holds_per_record || onshelfholds || opacitemholds || article_requests || article_requests %]
+                        [% SET show_rule = maxissueqty || issuelength || lengthunit || hardduedate || hardduedatebefore || hardduedateexact || fine || chargeperiod || chargeperiod_charge_at || firstremind || overduefinescap || cap_fine_to_replacement_price || finedays || maxsuspensiondays || suspension_chargeperiod || renewalsallowed || renewalsallowed || norenewalbefore || auto_renew || no_auto_renewal_after || no_auto_renewal_after_hard_limit || reservesallowed || holds_per_day || holds_per_record || onshelfholds || opacitemholds || article_requests || article_requests %]
                         [% IF show_rule %]
                             [% SET row_count = row_count + 1 %]
                             <tr row_countd="row_[% row_count | html %]">
@@ -190,13 +188,6 @@
                                             <span>Unlimited</span>
                                         [% END %]
                                     </td>
-                                    <td>
-                                        [% IF maxonsiteissueqty.defined && maxonsiteissueqty != ''  %]
-                                            [% maxonsiteissueqty | html %]
-                                        [% ELSE %]
-                                            <span>Unlimited</span>
-                                        [% END %]
-                                    </td>
                                     <td>[% issuelength | html %]</td>
                                     <td>
                                         [% IF ( lengthunit == 'days' ) %]
@@ -333,7 +324,6 @@
                     </td>
                     <td><input type="text" name="note" id="note" size="15" value="" maxlength="100"></td>
                     <td><input type="text" name="maxissueqty" id="maxissueqty" size="3" /></td>
-                    <td><input type="text" name="maxonsiteissueqty" id="maxonsiteissueqty" size="3" /></td>
                     <td><input type="text" name="issuelength" id="issuelength" size="3" /> </td>
                     <td>
                       <select name="lengthunit" id="lengthunit">
@@ -417,7 +407,6 @@
                       <th>&nbsp;</th>
                       <th>Note</th>
                       <th>Current checkouts allowed</th>
-                      <th>Current on-site checkouts allowed</th>
                       <th>Loan period</th>
                       <th>Unit</th>
                       <th>Hard due date</th>
@@ -460,7 +449,6 @@
                 <tr>
                     <th>&nbsp;</th>
                     <th>Total current checkouts allowed</th>
-                    <th>Total current on-site checkouts allowed</th>
                     <th>Maximum total holds allowed (count)</th>
                     <th>Hold policy</th>
                     <th>Hold pickup library match</th>
@@ -473,10 +461,6 @@
                         [% SET patron_maxissueqty = CirculationRules.Search( current_branch, undef, undef, 'patron_maxissueqty' ) %]
                         <input type="text" name="patron_maxissueqty" size="3" value="[% patron_maxissueqty | html %]"/>
                     </td>
-                    <td>
-                        [% SET patron_maxonsiteissueqty = CirculationRules.Search( current_branch, undef, undef, 'patron_maxonsiteissueqty' ) %]
-                        <input type="text" name="patron_maxonsiteissueqty" size="3" value="[% patron_maxonsiteissueqty | html %]"/>
-                    </td>
                     <td>
                         [% SET rule_value = CirculationRules.Search( current_branch, undef , undef, 'max_holds' ) %]
                         <input name="max_holds" size="3" value="[% rule_value | html %]" />
@@ -635,17 +619,15 @@
                 <tr>
                     <th>Patron category</th>
                     <th>Total current checkouts allowed</th>
-                    <th>Total current on-site checkouts allowed</th>
                     <th>Total holds allowed</th>
                     <th>&nbsp;</th>
                 </tr>
                 [% FOREACH c IN categorycodes %]
                     [% NEXT UNLESS c %]
                     [% SET patron_maxissueqty = CirculationRules.Search( branchcode, c, undef, 'patron_maxissueqty' ) %]
-                    [% SET patron_maxonsiteissueqty = CirculationRules.Search( branchcode, c, undef, 'patron_maxonsiteissueqty' ) %]
                     [% SET max_holds = CirculationRules.Search( branchcode, c, undef, 'max_holds' ) %]
 
-                    [% IF  ( patron_maxissueqty.defined && patron_maxissueqty != '' ) || ( patron_maxonsiteissueqty.defined && patron_maxonsiteissueqty != '' ) || ( max_holds.defined && max_holds != '' ) %]
+                    [% IF  ( patron_maxissueqty.defined && patron_maxissueqty != '' ) || ( max_holds.defined && max_holds != '' ) %]
                     <tr>
                         <td>
                             [% IF c == undef %]
@@ -661,13 +643,6 @@
                                 <span>Unlimited</span>
                             [% END %]
                         </td>
-                        <td>
-                            [% IF patron_maxonsiteissueqty.defined && patron_maxonsiteissueqty != '' %]
-                                [% patron_maxonsiteissueqty | html %]
-                            [% ELSE %]
-                                <span>Unlimited</span>
-                            [% END %]
-                        </td>
                         <td>
                             [% IF max_holds.defined && max_holds != '' %]
                                 [% max_holds | html %]
@@ -690,8 +665,6 @@
                         [% END %]
                         </select>
                     </td>
-                    <td><input name="patron_maxissueqty" size="3" type="text" /></td>
-                    <td><input name="patron_maxonsiteissueqty" size="3" type="text" /></td>
                     <td><input name="max_holds" size="3" type="text" /></td>
                     <td class="actions"><button type="submit" class="btn btn-default btn-xs"><i class="fa fa-plus"></i> Add</td>
                 </tr>
@@ -988,7 +961,7 @@
                         // specific processing for the Note column
                         var note = $(this).find("a[name='viewnote']").data("content");
                         $(current_column).find("input[type='text']").val(note);
-                    } else if ( i == 8 ) {
+                    } else if ( i == 7 ) {
                         // specific processing for the Hard due date column
                         var select_value = $(this).find("input[type='hidden'][name='hardduedatecomparebackup']").val();
                         var input_value = '';
@@ -999,7 +972,7 @@
                         }
                         $(current_column).find("input[type='text']").val(input_value);
                         $(current_column).find("select").val(select_value);
-                    } else if ( i == 14 ) {
+                    } else if ( i == 13 ) {
                         // specific processing for cap_fine_to_replacement_price
                         var cap_fine_to_replacement_price = $(this).find("input[type='checkbox']");
                         $('#cap_fine_to_replacement_price').prop('checked', cap_fine_to_replacement_price.is(':checked') );
@@ -1024,10 +997,9 @@
                             // Remove potential previous input added
                             $(current_column).find("input").remove();
                             $(current_column).append("<input type='hidden' name='"+name+"' value='"+val+"' />");
-                        } else if ( i == 4 || i == 5 || i == 24 || i == 25 || i == 26 ) {
+                        } else if ( i == 4 || i == 23 || i == 24 || i == 25 ) {
                             // If the value is not an integer for
                             //     - "Current checkouts allowed"
-                            //     - "Current on-site checkouts allowed"
                             //     - "Holds allowed (total)"
                             //     - "Holds allowed (daily)"
                             //     - "Holds per record (count)"
diff --git a/t/db_dependent/Circulation/Branch.t b/t/db_dependent/Circulation/Branch.t
index 16e590fcc3..2a706780a3 100644
--- a/t/db_dependent/Circulation/Branch.t
+++ b/t/db_dependent/Circulation/Branch.t
@@ -25,7 +25,7 @@ use Koha::CirculationRules;
 
 use Koha::Patrons;
 
-use Test::More tests => 14;
+use Test::More tests => 15;
 use t::lib::Mocks;
 use t::lib::TestBuilder;
 
@@ -143,29 +143,39 @@ my $borrower_id1 = Koha::Patron->new({
 
 is_deeply(
     GetBranchBorrowerCircRule(),
-    { patron_maxissueqty => undef, patron_maxonsiteissueqty => undef },
-"Without parameter, GetBranchBorrower returns undef (unilimited) for patron_maxissueqty and patron_maxonsiteissueqty if no rules defined"
+    { patron_maxissueqty => undef },
+"Without parameter, GetBranchBorrower returns undef (unilimited) for patron_maxissueqty if no rules defined"
 );
 
 Koha::CirculationRules->set_rules(
     {
         branchcode   => $samplebranch1->{branchcode},
         categorycode => $samplecat->{categorycode},
+        checkout_type => undef,
         rules        => {
             patron_maxissueqty       => 5,
-            patron_maxonsiteissueqty => 6,
         }
     }
 );
 
+Koha::CirculationRules->set_rules(
+    {
+        branchcode   => $samplebranch1->{branchcode},
+        categorycode => $samplecat->{categorycode},
+        checkout_type => $Koha::Checkouts::type->{onsite_checkout},
+        rules        => {
+            patron_maxissueqty       => 9001,
+        }
+    }
+);
 
 Koha::CirculationRules->set_rules(
     {
         branchcode   => $samplebranch2->{branchcode},
         categorycode => undef,
+        checkout_type => undef,
         rules        => {
             patron_maxissueqty       => 3,
-            patron_maxonsiteissueqty => 2,
         }
     }
 );
@@ -184,9 +194,9 @@ Koha::CirculationRules->set_rules(
     {
         branchcode   => undef,
         categorycode => undef,
+        checkout_type => undef,
         rules        => {
             patron_maxissueqty       => 4,
-            patron_maxonsiteissueqty => 5,
         }
     }
 );
@@ -235,26 +245,35 @@ Koha::CirculationRules->set_rules(
 #Test GetBranchBorrowerCircRule
 is_deeply(
     GetBranchBorrowerCircRule(),
-    { patron_maxissueqty => 4, patron_maxonsiteissueqty => 5 },
-"Without parameter, GetBranchBorrower returns the patron_maxissueqty and patron_maxonsiteissueqty of default_circ_rules"
+    { patron_maxissueqty => 4 },
+"Without parameter, GetBranchBorrower returns the patron_maxissueqty of default_circ_rules"
 );
 is_deeply(
     GetBranchBorrowerCircRule( $samplebranch2->{branchcode} ),
-    { patron_maxissueqty => 3, patron_maxonsiteissueqty => 2 },
-"Without only the branchcode specified, GetBranchBorrower returns the patron_maxissueqty and patron_maxonsiteissueqty corresponding"
+    { patron_maxissueqty => 3 },
+"Without only the branchcode specified, GetBranchBorrower returns the patron_maxissueqty corresponding"
 );
 is_deeply(
     GetBranchBorrowerCircRule(
         $samplebranch1->{branchcode},
         $samplecat->{categorycode}
     ),
-    { patron_maxissueqty => 5, patron_maxonsiteissueqty => 6 },
-    "GetBranchBorrower returns the patron_maxissueqty and patron_maxonsiteissueqty of the branch1 and the category1"
+    { patron_maxissueqty => 5 },
+    "GetBranchBorrower returns the patron_maxissueqty of the branch1 and the category1"
+);
+is_deeply(
+    GetBranchBorrowerCircRule(
+        $samplebranch1->{branchcode},
+        $samplecat->{categorycode},
+        $Koha::Checkouts::type->{onsite_checkout}
+    ),
+    { patron_maxissueqty => 9001 },
+     "GetBranchBorrower returns the patron_maxissueqty of the branch1 and the category1 and checkout_type on-site"
 );
 is_deeply(
     GetBranchBorrowerCircRule( -1, -1 ),
-    { patron_maxissueqty => 4, patron_maxonsiteissueqty => 5 },
-"GetBranchBorrower with wrong parameters returns the patron_maxissueqty and patron_maxonsiteissueqty of default_circ_rules"
+    { patron_maxissueqty => 4 },
+"GetBranchBorrower with wrong parameters returns the patron_maxissueqty of default_circ_rules"
 );
 
 #Test GetBranchItemRule
diff --git a/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
index 41e4eb8433..7989c5e5eb 100644
--- a/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
+++ b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
@@ -87,9 +87,9 @@ Koha::CirculationRules->set_rules(
         branchcode   => $branch->{branchcode},
         categorycode => undef,
         itemtype     => undef,
+        checkout_type => undef,
         rules        => {
             maxissueqty       => 2,
-            maxonsiteissueqty => 1,
             lengthunit        => 'days',
             issuelength       => 5,
             hardduedate        => undef,
@@ -97,6 +97,17 @@ Koha::CirculationRules->set_rules(
         }
     }
 );
+Koha::CirculationRules->set_rules(
+    {
+        branchcode   => $branch->{branchcode},
+        categorycode => undef,
+        itemtype     => undef,
+        checkout_type => $Koha::Checkouts::type->{onsite_checkout},
+        rules        => {
+            maxissueqty       => 1,
+        }
+    }
+);
 
 t::lib::Mocks::mock_userenv({ patron => $patron });
 
@@ -159,9 +170,20 @@ Koha::CirculationRules->set_rules(
         branchcode   => $branch->{branchcode},
         categorycode => undef,
         itemtype     => undef,
+        checkout_type => undef,
+        rules        => {
+            maxissueqty       => 2,
+        }
+    }
+);
+Koha::CirculationRules->set_rules(
+    {
+        branchcode   => $branch->{branchcode},
+        categorycode => undef,
+        itemtype     => undef,
+        checkout_type => $Koha::Checkouts::type->{onsite_checkout},
         rules        => {
             maxissueqty       => 2,
-            maxonsiteissueqty => 1,
         }
     }
 );
diff --git a/t/db_dependent/Circulation/TooMany.t b/t/db_dependent/Circulation/TooMany.t
index 31ec01563b..46f1f55531 100644
--- a/t/db_dependent/Circulation/TooMany.t
+++ b/t/db_dependent/Circulation/TooMany.t
@@ -108,9 +108,9 @@ subtest '1 Issuingrule exist 0 0: no issue allowed' => sub {
             branchcode   => $branch->{branchcode},
             categorycode => $category->{categorycode},
             itemtype     => undef,
+            checkout_type => undef,
             rules        => {
                 maxissueqty       => 0,
-                maxonsiteissueqty => 0,
             }
         },
     );
@@ -165,9 +165,20 @@ subtest '1 Issuingrule exist with onsiteissueqty=unlimited' => sub {
             branchcode   => $branch->{branchcode},
             categorycode => $category->{categorycode},
             itemtype     => undef,
+            checkout_type => undef,
             rules        => {
                 maxissueqty       => 1,
-                maxonsiteissueqty => undef,
+            }
+        },
+    );
+    Koha::CirculationRules->set_rules(
+        {
+            branchcode   => $branch->{branchcode},
+            categorycode => $category->{categorycode},
+            itemtype     => undef,
+            checkout_type => $Koha::Checkouts::type->{onsite_checkout},
+            rules        => {
+                maxissueqty       => undef,
             }
         },
     );
@@ -220,9 +231,9 @@ subtest '1 Issuingrule exist 1 1: issue is allowed' => sub {
             branchcode   => $branch->{branchcode},
             categorycode => $category->{categorycode},
             itemtype     => undef,
+            checkout_type => undef,
             rules        => {
                 maxissueqty       => 1,
-                maxonsiteissueqty => 1,
             }
         }
     );
@@ -260,9 +271,9 @@ subtest '1 Issuingrule exist: 1 CO allowed, 1 OSCO allowed. Do a CO' => sub {
             branchcode   => $branch->{branchcode},
             categorycode => $category->{categorycode},
             itemtype     => undef,
+            checkout_type => undef,
             rules        => {
                 maxissueqty       => 1,
-                maxonsiteissueqty => 1,
             }
         }
     );
@@ -316,9 +327,9 @@ subtest '1 Issuingrule exist: 1 CO allowed, 1 OSCO allowed, Do a OSCO' => sub {
             branchcode   => $branch->{branchcode},
             categorycode => $category->{categorycode},
             itemtype     => undef,
+            checkout_type => undef,
             rules        => {
                 maxissueqty       => 1,
-                maxonsiteissueqty => 1,
             }
         }
     );
@@ -375,9 +386,9 @@ subtest '1 BranchBorrowerCircRule exist: 1 CO allowed, 1 OSCO allowed' => sub {
             branchcode   => $branch->{branchcode},
             categorycode => $category->{categorycode},
             itemtype     => undef,
+            checkout_type => undef,
             rules        => {
                 maxissueqty       => 1,
-                maxonsiteissueqty => 1,
             }
         }
     );
@@ -427,9 +438,9 @@ subtest '1 BranchBorrowerCircRule exist: 1 CO allowed, 1 OSCO allowed' => sub {
             branchcode   => $branch->{branchcode},
             categorycode => $category->{categorycode},
             itemtype     => undef,
+            checkout_type => undef,
             rules        => {
                 maxissueqty       => 1,
-                maxonsiteissueqty => 1,
             }
         }
     );
@@ -505,6 +516,7 @@ subtest 'General vs specific rules limit quantity correctly' => sub {
             categorycode => '*',
             itemtype     => $itemtype->{itemtype},
             branchcode   => '*',
+            checkout_type => '*',
             rules        => {
                 issuelength => 1,
                 firstremind => 1,        # 1 day of grace
@@ -519,10 +531,10 @@ subtest 'General vs specific rules limit quantity correctly' => sub {
         {
             branchcode   => '*',
             categorycode => '*',
+            checkout_type => '*',
             itemtype     => $itemtype->{itemtype},
             rules        => {
                 maxissueqty       => 1,
-                maxonsiteissueqty => 1,
             }
         }
     );
@@ -573,9 +585,9 @@ subtest 'General vs specific rules limit quantity correctly' => sub {
             branchcode   => $branch->{branchcode},
             categorycode => $category->{categorycode},
             itemtype     => $itemtype->{itemtype},
+            checkout_type => undef,
             rules        => {
                 maxissueqty       => 1,
-                maxonsiteissueqty => 1,
             }
         }
     );
@@ -669,9 +681,9 @@ subtest 'General vs specific rules limit quantity correctly' => sub {
             branchcode   => $branch2->{branchcode},
             categorycode => $category->{categorycode},
             itemtype     => $itemtype->{itemtype},
+            checkout_type => undef,
             rules        => {
                 maxissueqty       => 1,
-                maxonsiteissueqty => 1,
             }
         }
     );
@@ -691,9 +703,9 @@ subtest 'empty string means unlimited' => sub {
             branchcode   => '*',
             categorycode => '*',
             itemtype     => '*',
+            checkout_type => '*',
             rules        => {
                 maxissueqty       => '',
-                maxonsiteissueqty => '',
             }
         },
     );
@@ -706,7 +718,7 @@ subtest 'empty string means unlimited' => sub {
     is(
         C4::Circulation::TooMany( $patron, $item_object, { onsite_checkout => 1 } ),
         undef,
-        'maxonsiteissueqty="" should mean unlimited'
+        'maxissueqty="" should mean unlimited'
     );
 
     teardown();
-- 
2.17.1