Attachment 107281 Details for Bug 21946 – Bug 21946: Display parent-child relationship on smart-rules.pl

[patch] Bug 21946: Display parent-child relationship on smart-rules.pl

Bug-21946-Display-parent-child-relationship-on-sma.patch (text/plain), 14.06 KB, created by Alex Arnaud on 2020-07-24 08:32:22 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Alex Arnaud

Created: 2020-07-24 08:32:22 UTC

Size: 14.06 KB

patch

obsolete

>From 2aae6970f45574e6094c150fa6289c9888a1c278 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.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 21946: 87713 | 87714 | 87715 | 87716 | 87717 | 87718 | 87719 | 88392 | 88574 | 88575 | 88576 | 88577 | 88578 | 88579 | 88580 | 88581 | 88926 | 88927 | 88928 | 88929 | 88930 | 88931 | 88932 | 88933 | 88934 | 89167 | 89168 | 89169 | 89170 | 89171 | 89172 | 89173 | 89174 | 89175 | 89176 | 93075 | 93076 | 93077 | 93078 | 93079 | 93080 | 93081 | 93082 | 93083 | 93084 | 93086 | 93087 | 93088 | 93089 | 93090 | 93091 | 93092 | 93093 | 93094 | 93095 | 93096 | 93097 | 93098 | 93099 | 93100 | 93101 | 93102 | 93103 | 93104 | 93105 | 96977 | 96978 | 96979 | 96980 | 96981 | 96982 | 96983 | 96984 | 96985 | 96986 | 96987 | 99419 | 99420 | 99421 | 99422 | 99423 | 99424 | 107035 | 107036 | 107037 | 107038 | 107039 | 107040 | 107041 | 107278 | 107279 | 107280 | 107281 | 107282 | 107283 | 107284 | 107467 | 107489 | 107686 | 107687 | 107688 | 107689 | 107690 | 107691 | 107692 | 107693 | 107694 | 107701 | 107733