From 9e133ba51ce5281cce587f862ffb3212b8ae705d Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Wed, 16 Oct 2019 14:37:28 +0200
Subject: [PATCH] Bug 23271: Improve library limitation selection for item
 types

---
 Koha/Template/Plugin/Branches.pm              |  6 ++++
 admin/itemtypes.pl                            | 16 +----------
 .../prog/en/modules/admin/itemtypes.tt        | 28 ++++++++-----------
 3 files changed, 19 insertions(+), 31 deletions(-)

diff --git a/Koha/Template/Plugin/Branches.pm b/Koha/Template/Plugin/Branches.pm
index 80ef90e83c0..c2da80d8d64 100644
--- a/Koha/Template/Plugin/Branches.pm
+++ b/Koha/Template/Plugin/Branches.pm
@@ -62,11 +62,17 @@ sub all {
     my $selecteds = $params->{selecteds};
     my $unfiltered = $params->{unfiltered} || 0;
     my $search_params = $params->{search_params} || {};
+    my $do_not_select_my_library = $params->{do_not_select_my_library} || 0; # By default we select the library of the logged in user if no selected passed
 
     if ( !$unfiltered ) {
         $search_params->{only_from_group} = $params->{only_from_group} || 0;
     }
 
+    my @selected =
+      ref $selected eq 'Koha::Libraries'
+      ? $selected->get_column('branchcode')
+      : $selected;
+
     my $libraries = $unfiltered
       ? Koha::Libraries->search( $search_params, { order_by => ['branchname'] } )->unblessed
       : Koha::Libraries->search_filtered( $search_params, { order_by => ['branchname'] } )->unblessed;
diff --git a/admin/itemtypes.pl b/admin/itemtypes.pl
index 07866f72fb6..a1c8001ce2e 100755
--- a/admin/itemtypes.pl
+++ b/admin/itemtypes.pl
@@ -58,18 +58,6 @@ undef($sip_media_type) if defined($sip_media_type) and $sip_media_type =~ /^\s*$
 if ( $op eq 'add_form' ) {
     my $itemtype = Koha::ItemTypes->find($itemtype_code);
 
-    my $selected_branches = $itemtype ? $itemtype->get_library_limits : undef;
-    my $branches = Koha::Libraries->search( {}, { order_by => ['branchname'] } )->unblessed;
-    my @branches_loop;
-    foreach my $branch ( @$branches ) {
-        my $selected = ($selected_branches && grep {$_->branchcode eq $branch->{branchcode}} @{ $selected_branches->as_list } ) ? 1 : 0;
-        push @branches_loop, {
-            branchcode => $branch->{branchcode},
-            branchname => $branch->{branchname},
-            selected   => $selected,
-        };
-    }
-
     my $parent_type = $itemtype ? $itemtype->parent_type : undef;
     my $parent_types = Koha::ItemTypes->search({parent_type=>undef,itemtype => {'!='=>$itemtype_code}});
     my $imagesets = C4::Koha::getImageSets( checked => ( $itemtype ? $itemtype->imageurl : undef ) );
@@ -83,7 +71,6 @@ if ( $op eq 'add_form' ) {
         imagesets => $imagesets,
         searchcategory => $searchcategory,
         can_be_translated => ( scalar(@$translated_languages) > 1 ? 1 : 0 ),
-        branches_loop    => \@branches_loop,
     );
 } elsif ( $op eq 'add_validate' ) {
     my $is_a_modif   = $input->param('is_a_modif');
@@ -209,9 +196,8 @@ if ( $op eq 'add_form' ) {
 }
 
 if ( $op eq 'list' ) {
-    my @itemtypes = Koha::ItemTypes->search->as_list;
     $template->param(
-        itemtypes => \@itemtypes,
+        itemtypes => scalar Koha::ItemTypes->search,
         messages  => \@messages,
     );
 }
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt
index b419619a30c..67392d31c3f 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt
@@ -1,6 +1,7 @@
 [% USE raw %]
 [% USE Asset %]
 [% USE Koha %]
+[% USE Branches %]
 [% USE AuthorisedValues %]
 [% USE Price %]
 [% USE TablesSettings %]
@@ -376,13 +377,7 @@ Data deleted
                 <li><label for="branches">Library limitation: </label>
                     <select id="branches" name="branches" multiple size="10">
                         <option value="">All libraries</option>
-                        [% FOREACH branch IN branches_loop %]
-                        [% IF ( branch.selected ) %]
-                            <option selected="selected" value="[% branch.branchcode | html %]">[% branch.branchname | html %]</option>
-                        [% ELSE %]
-                            <option value="[% branch.branchcode | html %]">[% branch.branchname | html %]</option>
-                        [% END %]
-                        [% END %]
+                        [% PROCESS options_for_libraries libraries => Branches.all( selecteds => itemtype.get_library_limits, unfiltered => 1, do_not_select_my_library => 1 ) %]
                     </select>
                     <span class="hint">Select 'All libraries' if all libraries use this item type. Otherwise, select the specific libraries that use this item type.</span>
                 </li>
@@ -529,20 +524,21 @@ Data deleted
             <td>[% itemtype.processfee | $Price %]</td>
             <td>[% itemtype.checkinmsg | html_line_break | $raw %]</td>
             <td>
-                [% IF itemtype.library_limits %]
-                    [% libraries_str = "" %]
-                    [% FOREACH library IN itemtype.library_limits %]
+                [% SET library_limits = itemtype.library_limits %]
+                [% IF library_limits.count > 0 %]
+                    [% library_str = "" %]
+                    [% FOREACH library IN library_limits %]
                         [%- IF loop.first -%]
-                        [% libraries_str = library.branchname _ " (" _ library.branchcode _ ")" %]
+                            [% library_str = library.branchname _ " (" _ library.branchcode _ ")" %]
                         [% ELSE %]
-                        [% libraries_str = libraries_str _ "\n" _ library.branchname _ " (" _ library.branchcode _ ")" %]
+                            [% library_str = library_str _ "\n" _ library.branchname _ " (" _ library.branchcode _ ")" %]
                         [% END %]
                     [% END %]
-                    <span class="library_limitation" title="[% libraries_str | html %]">
-                        [% IF itemtype.library_limits.count > 1 %]
-                            [% itemtype.library_limits.count | html %] library limitations
+                    <span class="library_limitation" title="[% library_str | html %]">
+                        [% IF library_limits.count > 1 %]
+                            [% library_limits.count | html %] library limitations
                         [% ELSE %]
-                            [% itemtype.library_limits.count | html %] library limitation
+                            [% library_limits.count | html %] library limitation
                         [% END %]
                 [% ELSE %]
                     No limitation
-- 
2.20.1