From 9a98b43f3663a56a5575def9076b8d9fdf9a5d64 Mon Sep 17 00:00:00 2001
From: Nick Clemens <nick@bywatersolutions.com>
Date: Tue, 9 Apr 2019 20:00:16 +0000
Subject: [PATCH] Bug 21946: Display parent-child relationship on
 smart-rules.pl

To test:
 1 - Set some itemtypes to have a parent
 2 - Browse to Administration -> Circulation and fines rules
 3 - Note new description of parent/child relationships at top of page
 4 - Note that itemtype dropdown for circ rules shows child types under parents
 5 - Set a rule for a child type
 6 - Note it displays as Parent->Child
 7 - Have three child types under a parent
 8 - Set the parent 'Current checkouts allowed' to 3
 9 - Set the children 'Current checkouts allowed' to:
     type1 = 2
     type2 = 1
     type3 = 1
10 - Create some items of the type above
11 - Note you can checkout 2 of type 1, and not 3
12 - Note you can checkout 1 of type 2, but not 2
13 - Note that you now cannot checkout any of type3
14 - Note you cannot checkout any of the parent type
15 - Return one of the other items and note you can now checkout an item of type3
16 - Return another item and note you can checkout an item of the parent type
17 - Return all
18 - Set the parent type to 1
19 - Now note you can only checkout 1 of any of the children
20 - Set the parent to 0
21 - Note you cannot checkout any of the child types

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>
Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
---
 Koha/ItemType.pm                              |  11 ++
 Koha/Template/Plugin/ItemTypes.pm             |   7 +-
 .../prog/en/modules/admin/smart-rules.tt      |  36 +++++-
 t/db_dependent/Koha/ItemTypes.t               | 108 +++++++-----------
 4 files changed, 94 insertions(+), 68 deletions(-)

diff --git a/Koha/ItemType.pm b/Koha/ItemType.pm
index f15c00e023..939c97704e 100644
--- a/Koha/ItemType.pm
+++ b/Koha/ItemType.pm
@@ -153,6 +153,17 @@ sub parent {
 
 }
 
+=head3 children_with_localization
+
+    Returns the ItemType objects of the children of this type or undef.
+
+=cut
+
+sub children_with_localization {
+    my ( $self ) = @_;
+    return Koha::ItemTypes->search_with_localization({ parent_type => $self->itemtype });
+}
+
 =head3 type
 
 =cut
diff --git a/Koha/Template/Plugin/ItemTypes.pm b/Koha/Template/Plugin/ItemTypes.pm
index 20910562b0..5b3cf1edeb 100644
--- a/Koha/Template/Plugin/ItemTypes.pm
+++ b/Koha/Template/Plugin/ItemTypes.pm
@@ -25,9 +25,12 @@ use base qw( Template::Plugin );
 use Koha::ItemTypes;
 
 sub GetDescription {
-    my ( $self, $itemtypecode ) = @_;
+    my ( $self, $itemtypecode, $want_parent ) = @_;
     my $itemtype = Koha::ItemTypes->find( $itemtypecode );
-    return $itemtype ? $itemtype->translated_description : q{};
+    return q{} unless $itemtype;
+    my $parent;
+    $parent = $itemtype->parent if $want_parent;
+    return $parent ? $parent->translated_description . "->" . $itemtype->translated_description : $itemtype->translated_description;
 }
 
 sub Get {
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 2b9f4513b2..a36daf5f4a 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
@@ -57,6 +57,10 @@
             <li>default (all libraries), all patron categories, same item type</li>
             <li>default (all libraries), all patron categories, all item types</li>
         </ul>
+
+        <p>Where an itemtype has a parent, the rule will display as "Parent->Child" and the number of
+        current checkouts allowed will be limited to either the maximum for the parent (counting sibling types)
+        or the specific rule's type, whichever is less.</p>
         <p>To modify a rule, create a new one with the same patron category and item type.</p>
     </div>
     <div>
@@ -86,7 +90,9 @@
             <table id="default-circulation-rules">
             <thead>
             <tr>
+                <th>&nbsp;</th>
                 <th>Patron category</th>
+                <th>&nbsp;</th>
                 <th>Item type</th>
                 <th>Actions</th>
                 <th>Note</th>
@@ -162,6 +168,7 @@
                         [% IF show_rule %]
                             [% SET row_count = row_count + 1 %]
                             <tr row_countd="row_[% row_count | html %]">
+                                    <td>[% IF ( c == undef ) %]1[% ELSE %]0[% END %]</td>
                                     <td>
                                         [% IF c == undef %]
                                             <em>All</em>
@@ -169,11 +176,12 @@
                                             [% Categories.GetName(c) | html %]
                                         [% END %]
                                     </td>
+                                    <td>[% IF ( i == undef ) %]1[% ELSE %]0[% END %]</td>
                                     <td>
                                         [% IF i == undef %]
                                             <em>All</em>
                                         [% ELSE %]
-                                            [% ItemTypes.GetDescription(i) | html %]
+                                            [% ItemTypes.GetDescription(i,1) | html %]
                                         [% END %]
                                     </td>
                                     <td class="actions">
@@ -321,6 +329,7 @@
                     [% END %]
                 [% END %]
                 <tr id="edit_row">
+                    <td>2</td>
                     <td>
                         <select name="categorycode" id="categorycode">
                             <option value="*">All</option>
@@ -329,11 +338,21 @@
                         [% END %]
                         </select>
                     </td>
+                    <td>0</td>
                     <td>
                         <select name="itemtype" id="matrixitemtype" style="width:13em;">
                             <option value="*">All</option>
                         [% FOREACH itemtypeloo IN itemtypeloop %]
+                            [% NEXT IF itemtypeloo.parent_type %]
                             <option value="[% itemtypeloo.itemtype | html %]">[% itemtypeloo.translated_description | html %]</option>
+                            [% SET children = itemtypeloo.children_with_localization %]
+                            [% IF children %]
+                                <optgroup>
+                                [% FOREACH child IN children %]
+                                    <option value="[% child.itemtype | html %]">[% child.translated_description | html %]</option>
+                                [% END %]
+                                </optgroup>
+                            [% END %]
                         [% END %]
                         </select>
                     </td>
@@ -432,7 +451,9 @@
                 </tr>
                 <tfoot>
                     <tr>
+                      <th>&nbsp;</th>
                       <th>Patron category</th>
+                      <th>&nbsp;</th>
                       <th>Item type</th>
                       <th>&nbsp;</th>
                       <th>Note</th>
@@ -943,8 +964,20 @@
 
 [% MACRO jsinclude BLOCK %]
     [% Asset.js("js/admin-menu.js") | $raw %]
+    [% INCLUDE 'datatables.inc' %]
     [% INCLUDE 'calendar.inc' %]
     <script>
+        $(document).ready(function() {
+            $("#default-circulation-rules").dataTable($.extend(true,{},dataTablesDefaults, {
+                "aoColumnDefs": [
+                    { "bVisible": false, "aTargets": [ 0,2 ] },
+                    { "bSortable": false, "aTargets": ["_all"] }
+                ],
+                "aaSortingFixed": [ [0,'asc'], [1,'asc'], [2,'asc'], [3,'asc'] ],
+                "bPaginate": false,
+                "bAutoWidth": false
+            }));
+        });
 
         function clear_edit(){
             var cancel = confirm(_("Are you sure you want to cancel your changes?"));
@@ -1030,6 +1063,7 @@
                         // select the corresponding option
                         $(current_column).find("select option").each(function(){
                             opt = $(this).text().toLowerCase();
+                            itm = itm.replace(/.*->(.*)/,"$1"); //If item type is part of a group we need to clear the parent description
                             opt = opt.replace(/^\s*|\s*$/g,'');
                             if ( opt == itm.toLowerCase() ) {
                                 $(this).attr('selected', 'selected');
diff --git a/t/db_dependent/Koha/ItemTypes.t b/t/db_dependent/Koha/ItemTypes.t
index 8645229db4..cbb16f953d 100755
--- a/t/db_dependent/Koha/ItemTypes.t
+++ b/t/db_dependent/Koha/ItemTypes.t
@@ -20,7 +20,7 @@
 use Modern::Perl;
 
 use Data::Dumper;
-use Test::More tests => 24;
+use Test::More tests => 13;
 
 use t::lib::Mocks;
 use t::lib::TestBuilder;
@@ -40,54 +40,37 @@ BEGIN {
 my $database = Koha::Database->new();
 my $schema   = $database->schema();
 $schema->txn_begin;
-Koha::ItemTypes->delete;
 
-Koha::ItemType->new(
-    {
-        itemtype       => 'type1',
-        description    => 'description',
-        rentalcharge   => '0.00',
-        imageurl       => 'imageurl',
-        summary        => 'summary',
-        checkinmsg     => 'checkinmsg',
-        checkinmsgtype => 'checkinmsgtype',
-        processfee         => '0.00',
-        defaultreplacecost => '0.00',
-    }
-)->store;
-
-Koha::ItemType->new(
-    {
-        itemtype       => 'type2',
-        description    => 'description',
-        rentalcharge   => '0.00',
-        imageurl       => 'imageurl',
-        summary        => 'summary',
-        checkinmsg     => 'checkinmsg',
-        checkinmsgtype => 'checkinmsgtype',
-        processfee         => '0.00',
-        defaultreplacecost => '0.00',
-    }
-)->store;
-
-Koha::ItemType->new(
-    {
-        itemtype       => 'type3',
-        description    => 'description',
-        rentalcharge   => '0.00',
-        imageurl       => 'imageurl',
-        summary        => 'summary',
-        checkinmsg     => 'checkinmsg',
-        checkinmsgtype => 'checkinmsgtype',
-        processfee         => '0.00',
-        defaultreplacecost => '0.00',
-    }
-)->store;
+my $builder     = t::lib::TestBuilder->new;
+my $initial_count = Koha::ItemTypes->search->count;
+
+my $parent1 = $builder->build_object({ class => 'Koha::ItemTypes', value => { description => 'description' } });
+my $child1  = $builder->build_object({
+        class => 'Koha::ItemTypes',
+        value => {
+            parent_type => $parent1->itemtype,
+            description => 'description',
+        }
+    });
+my $child2  = $builder->build_object({
+        class => 'Koha::ItemTypes',
+        value => {
+            parent_type => $parent1->itemtype,
+            description => 'description',
+        }
+    });
+my $child3  = $builder->build_object({
+        class => 'Koha::ItemTypes',
+        value => {
+            parent_type => $parent1->itemtype,
+            description => 'description',
+        }
+    });
 
 Koha::Localization->new(
     {
         entity      => 'itemtypes',
-        code        => 'type1',
+        code        => $child1->itemtype,
         lang        => 'en',
         translation => 'b translated itemtype desc'
     }
@@ -95,7 +78,7 @@ Koha::Localization->new(
 Koha::Localization->new(
     {
         entity      => 'itemtypes',
-        code        => 'type2',
+        code        => $child2->itemtype,
         lang        => 'en',
         translation => 'a translated itemtype desc'
     }
@@ -103,36 +86,24 @@ Koha::Localization->new(
 Koha::Localization->new(
     {
         entity      => 'something_else',
-        code        => 'type2',
+        code        => $child2->itemtype,
         lang        => 'en',
         translation => 'another thing'
     }
 )->store;
 
-my $type = Koha::ItemTypes->find('type1');
+my $type = Koha::ItemTypes->find($child1->itemtype);
 ok( defined($type), 'first result' );
-is( $type->itemtype,       'type1',          'itemtype/code' );
-is( $type->description,    'description',    'description' );
-is( $type->rentalcharge+0, 0,                'rentalcharge' );
-is( $type->imageurl,       'imageurl',       'imageurl' );
-is( $type->summary,        'summary',        'summary' );
-is( $type->checkinmsg,     'checkinmsg',     'checkinmsg' );
-is( $type->checkinmsgtype, 'checkinmsgtype', 'checkinmsgtype' );
-
-$type = Koha::ItemTypes->find('type2');
+is_deeply( $type->unblessed, $child1->unblessed, "We got back the same object" );
+
+$type = Koha::ItemTypes->find($child2->itemtype);
 ok( defined($type), 'second result' );
-is( $type->itemtype,       'type2',          'itemtype/code' );
-is( $type->description,    'description',    'description' );
-is( $type->rentalcharge+0, 0,                'rentalcharge' );
-is( $type->imageurl,       'imageurl',       'imageurl' );
-is( $type->summary,        'summary',        'summary' );
-is( $type->checkinmsg,     'checkinmsg',     'checkinmsg' );
-is( $type->checkinmsgtype, 'checkinmsgtype', 'checkinmsgtype' );
+is_deeply( $type->unblessed, $child2->unblessed, "We got back the same object" );
 
 t::lib::Mocks::mock_preference('language', 'en');
 t::lib::Mocks::mock_preference('opaclanguages', 'en');
 my $itemtypes = Koha::ItemTypes->search_with_localization;
-is( $itemtypes->count, 3, 'There are 3 item types' );
+is( $itemtypes->count, $initial_count + 4, 'We added 4 item types' );
 my $first_itemtype = $itemtypes->next;
 is(
     $first_itemtype->translated_description,
@@ -140,7 +111,14 @@ is(
     'item types should be sorted by translated description'
 );
 
-my $builder = t::lib::TestBuilder->new;
+my $children = $parent1->children_with_localization;
+my $first_child = $children->next;
+is(
+    $first_child->translated_description,
+    'a translated itemtype desc',
+    'item types should be sorted by translated description'
+);
+
 my $item_type = $builder->build_object({ class => 'Koha::ItemTypes' });
 
 is( $item_type->can_be_deleted, 1, 'An item type that is not used can be deleted');
-- 
2.20.1