From d3401695ef73d6eb35c2a624d27e48cb02df91b7 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                   | 19 +++++++++++----
 admin/itemtypes.pl                                 | 22 +----------------
 .../prog/en/modules/admin/itemtypes.tt             | 28 ++++++++++------------
 3 files changed, 27 insertions(+), 42 deletions(-)

diff --git a/Koha/Template/Plugin/Branches.pm b/Koha/Template/Plugin/Branches.pm
index da615f5673..58afa04551 100644
--- a/Koha/Template/Plugin/Branches.pm
+++ b/Koha/Template/Plugin/Branches.pm
@@ -57,22 +57,31 @@ sub GetURL {
 
 sub all {
     my ( $self, $params ) = @_;
-    my $selected = $params->{selected};
+    my $selected = $params->{selected} || ();
     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;
 
-    for my $l ( @$libraries ) {
-        if (       defined $selected and $l->{branchcode} eq $selected
-            or not defined $selected and C4::Context->userenv and $l->{branchcode} eq ( C4::Context->userenv->{branch} // q{} )
-        ) {
+    for my $l (@$libraries) {
+        if ( grep { $l->{branchcode} eq $_ } @selected
+            or  not @selected
+                and not $do_not_select_my_library
+                and C4::Context->userenv
+                and $l->{branchcode} eq ( C4::Context->userenv->{branch} // q{} ) )
+        {
             $l->{selected} = 1;
         }
     }
diff --git a/admin/itemtypes.pl b/admin/itemtypes.pl
index 05f394e053..ae0d914a1d 100755
--- a/admin/itemtypes.pl
+++ b/admin/itemtypes.pl
@@ -59,18 +59,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 $imagesets = C4::Koha::getImageSets( checked => ( $itemtype ? $itemtype->imageurl : undef ) );
     my $searchcategory = GetAuthorisedValues("ITEMTYPECAT");
     my $translated_languages = C4::Languages::getTranslatedLanguages( undef , C4::Context->preference('template') );
@@ -79,7 +67,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');
@@ -196,15 +183,8 @@ if ( $op eq 'add_form' ) {
 }
 
 if ( $op eq 'list' ) {
-    my @itypes = Koha::ItemTypes->search->as_list;
-    my @itemtypes;
-    foreach my $itype (@itypes) {
-        my $itemtype = $itype->unblessed;
-        $itemtype->{branches} = $itype->library_limits ? $itype->library_limits->as_list : [];
-        push @itemtypes, $itemtype;
-    }
     $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 2330e84779..23b795b2ce 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 ColumnsSettings %]
@@ -287,13 +288,7 @@ Item types administration
                 <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( selected => itemtype.get_library_limits, unfiltered => 1, do_not_select_my_library => 1 ) %]
                     </select>
                     <span>Select 'All libraries' if this authorized value must be displayed all the time. Otherwise select libraries you want to associate with this value.</span>
                 </li>
@@ -414,20 +409,21 @@ Item types administration
             <td>[% itemtype.processfee | $Price %]</td>
             <td>[% itemtype.checkinmsg | html_line_break | $raw %]</td>
             <td>
-                [% IF itemtype.branches.size > 0 %]
-                    [% branches_str = "" %]
-                    [% FOREACH branch IN itemtype.branches %]
+                [% SET library_limits = itemtype.library_limits %]
+                [% IF library_limits.count > 0 %]
+                    [% library_str = "" %]
+                    [% FOREACH library IN library_limits %]
                         [%- IF loop.first -%]
-                        [% branches_str = branch.branchname _ " (" _ branch.branchcode _ ")" %]
+                            [% library_str = library.branchname _ " (" _ library.branchcode _ ")" %]
                         [% ELSE %]
-                        [% branches_str = branches_str _ "\n" _ branch.branchname _ " (" _ branch.branchcode _ ")" %]
+                            [% library_str = library_str _ "\n" _ library.branchname _ " (" _ library.branchcode _ ")" %]
                         [% END %]
                     [% END %]
-                    <span class="library_limitation" title="[% branches_str | html %]">
-                        [% IF itemtype.branches.size > 1 %]
-                            [% itemtype.branches.size | html %] library limitations
+                    <span class="library_limitation" title="[% library_str | html %]">
+                        [% IF library_limits.count > 1 %]
+                            [% library_limits.count | html %] library limitations
                         [% ELSE %]
-                            [% itemtype.branches.size | html %] library limitation
+                            [% library_limits.count | html %] library limitation
                         [% END %]
                 [% ELSE %]
                     No limitation
-- 
2.11.0