From 000ba5cd318154d6daa71bc178d01d56b31bc13a Mon Sep 17 00:00:00 2001
From: Lari Taskula <lari.taskula@hypernova.fi>
Date: Mon, 20 Apr 2020 12:16:34 +0000
Subject: [PATCH] Bug 25089: Change sorting order to branchcode, checkout_type,
 categorycode, itemtype

Otherwise we need to duplicate a rule with renewalsallowed=0 every rule
if we don't want on-site checkouts to be renewalable.

To test:
1. prove t/db_dependent/Koha/CirculationRules.t
---
 Koha/CirculationRules.pm                      |  2 +-
 .../prog/en/modules/admin/smart-rules.tt      | 46 +++++++++----------
 t/db_dependent/Koha/CirculationRules.t        |  2 +-
 3 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm
index 546d72fa56..7a951a84c8 100644
--- a/Koha/CirculationRules.pm
+++ b/Koha/CirculationRules.pm
@@ -190,7 +190,7 @@ sub get_effective_rule {
     }
 
     my $order_by = $params->{order_by}
-      // { -desc => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ] };
+      // { -desc => [ 'branchcode', 'checkout_type', 'categorycode', 'itemtype', ] };
 
     my $search_params;
     $search_params->{rule_name} = $rule_name;
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 d73e7e0f4c..358573a387 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
@@ -93,9 +93,9 @@
             <table id="default-circulation-rules">
             <thead>
             <tr>
+                <th>Checkout type</th>
                 <th>Patron category</th>
                 <th>Item type</th>
-                <th>Checkout type</th>
                 <th>Actions</th>
                 <th>Note</th>
                 <th>Current checkouts allowed</th>
@@ -129,11 +129,11 @@
             </thead>
             <tbody>
                 [% SET row_count = 0 %]
-                [% FOREACH c IN categorycodes %]
-                    [% SET c = '' UNLESS c.defined %]
-                    [% FOREACH i IN itemtypes %]
+                [% FOREACH ct IN checkout_types %]
+                    [% FOREACH c IN categorycodes %]
+                        [% SET c = '' UNLESS c.defined %]
+                        [% FOREACH i IN itemtypes %]
                         [% SET i = '' UNLESS i.defined %]
-                        [% FOREACH ct IN checkout_types %]
                             [% SET ct = '' UNLESS ct.defined %]
                             [% SET note = all_rules.$c.$i.$ct.note %]
                             [% SET maxissueqty = all_rules.$c.$i.$ct.maxissueqty %]
@@ -168,28 +168,28 @@
                                 [% SET row_count = row_count + 1 %]
                                 <tr row_countd="row_[% row_count | html %]">
                                         <td>
-                                            [% IF c == undef %]
+                                            [% IF ct == undef %]
                                                 <em>All</em>
                                             [% ELSE %]
-                                                [% Categories.GetName(c) | html %]
+                                                [% IF ct == 'CHECKOUT' %]
+                                                    <em>Normal checkout</em>
+                                                [% ELSIF ct == 'ONSITE' %]
+                                                    <em>On-site checkout</em>
+                                                [% END %]
                                             [% END %]
                                         </td>
                                         <td>
-                                            [% IF i == undef %]
+                                            [% IF c == undef %]
                                                 <em>All</em>
                                             [% ELSE %]
-                                                [% ItemTypes.GetDescription(i) | html %]
+                                                [% Categories.GetName(c) | html %]
                                             [% END %]
                                         </td>
                                         <td>
-                                            [% IF ct == undef %]
+                                            [% IF i == undef %]
                                                 <em>All</em>
                                             [% ELSE %]
-                                                [% IF ct == 'CHECKOUT' %]
-                                                    <em>Normal checkout</em>
-                                                [% ELSIF ct == 'ONSITE' %]
-                                                    <em>On-site checkout</em>
-                                                [% END %]
+                                                [% ItemTypes.GetDescription(i) | html %]
                                             [% END %]
                                         </td>
                                         <td class="actions">
@@ -322,6 +322,13 @@
                     [% END %]
                 [% END %]
                 <tr id="edit_row">
+                    <td>
+                        <select name="checkout_type" id="matrixcheckout_type" style="width:13em;">
+                            <option value="*">All</option>
+                            <option value="CHECKOUT">Normal checkout</option>
+                            <option value="ONSITE">On-site checkout</option>
+                        </select>
+                    </td>
                     <td>
                         <select name="categorycode" id="categorycode">
                             <option value="*">All</option>
@@ -338,13 +345,6 @@
                         [% END %]
                         </select>
                     </td>
-                    <td>
-                        <select name="checkout_type" id="matrixcheckout_type" style="width:13em;">
-                            <option value="*">All</option>
-                            <option value="CHECKOUT">Normal checkout</option>
-                            <option value="ONSITE">On-site checkout</option>
-                        </select>
-                    </td>
                     <td class="actions">
                         <input type="hidden" name="branch" value="[% current_branch | html %]"/>
                         <button type="submit" class="btn btn-default btn-xs"><i class="fa fa-save"></i> Save</button>
@@ -430,9 +430,9 @@
                 </tr>
                 <tfoot>
                     <tr>
+                      <th>Checkout type</th>
                       <th>Patron category</th>
                       <th>Item type</th>
-                      <th>Checkout type</th>
                       <th>&nbsp;</th>
                       <th>Note</th>
                       <th>Current checkouts allowed</th>
diff --git a/t/db_dependent/Koha/CirculationRules.t b/t/db_dependent/Koha/CirculationRules.t
index 95e063a6da..ee8b342b6e 100644
--- a/t/db_dependent/Koha/CirculationRules.t
+++ b/t/db_dependent/Koha/CirculationRules.t
@@ -288,7 +288,7 @@ sub prepare_tests_for_rule_scope_combinations {
     # We must maintain the order in order to keep the test valid. This should be
     # equal to Koha/CirculationRules.pm "order_by" of C<get_effective_rule> sub.
     # Let's explicitly define the order and fail test if we are missing a scope:
-    my $order = [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ];
+    my $order = [ 'branchcode', 'checkout_type', 'categorycode', 'itemtype' ];
     is( join(", ", sort keys %$scope),
        join(", ", sort @$order), 'Missing a scope!' ) if keys %$scope ne scalar @$order;
 
-- 
2.17.1