From ded2a159724001a9da1125fdb04da0df293f881f Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Fri, 19 Jul 2019 12:56:53 +0100
Subject: [PATCH] Bug 20307: Some more fixes

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 C4/Items.pm                                   |  4 +--
 C4/Koha.pm                                    |  6 ++--
 C4/Search.pm                                  |  8 +++---
 Koha/AuthorisedValue.pm                       | 28 ++++++++++++++++++-
 Koha/AuthorisedValues.pm                      | 26 +++++++++++------
 Koha/Template/Plugin/AuthorisedValues.pm      |  8 +++---
 admin/authorised_values.pl                    |  2 +-
 .../en/modules/admin/authorised_values.tt     | 25 +++++++++--------
 opac/opac-memberentry.pl                      |  2 +-
 opac/opac-reserve.pl                          |  2 +-
 opac/opac-suggestions.pl                      |  2 +-
 11 files changed, 75 insertions(+), 38 deletions(-)

diff --git a/C4/Items.pm b/C4/Items.pm
index b31fe1f5a6..4b109cf9d5 100644
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -1086,8 +1086,8 @@ sub GetItemsLocationInfo {
         while ( my $data = $sth->fetchrow_hashref ) {
              my $av = Koha::AuthorisedValues->search({ category => 'LOC', authorised_value => $data->{location} });
              $av = $av->count ? $av->next : undef;
-             $data->{location_intranet} = $av ? $av->lib : '';
-             $data->{location_opac}     = $av ? $av->opac_description : '';
+             $data->{location_intranet} = $av ? $av->translated_description : '';
+             $data->{location_opac}     = $av ? $av->opac_translated_description : '';
 	     push @results, $data;
 	}
 	return @results;
diff --git a/C4/Koha.pm b/C4/Koha.pm
index 74073ca478..6a2ed1e0d1 100644
--- a/C4/Koha.pm
+++ b/C4/Koha.pm
@@ -540,7 +540,6 @@ sub GetAuthorisedValues {
     my $result = $cache->get_from_cache($cache_key);
     return $result if $result;
 
-    # FIXME Need to deal with $opac
     my $avs = Koha::AuthorisedValues->search_with_localization(
         {
             ( $category ? ( category => $category ) : () ),
@@ -551,8 +550,9 @@ sub GetAuthorisedValues {
     while ( my $av = $avs->next ) {
         my $av_unblessed = $av->unblessed;
         $av_unblessed->{translated_description} = $av->translated_description;
-        $av_unblessed->{lib} = $av->translated_description;
-        $av_unblessed->{lib_opac} = $av->translated_description;
+        $av_unblessed->{opac_translated_description} = $av->opac_translated_description;
+        $av_unblessed->{lib} = $av->lib;
+        $av_unblessed->{lib_opac} = $av->lib_opac;
         push @results, $av_unblessed;
 
     }
diff --git a/C4/Search.pm b/C4/Search.pm
index 5d68165574..b4ee7b5a84 100644
--- a/C4/Search.pm
+++ b/C4/Search.pm
@@ -573,15 +573,15 @@ sub getRecords {
            # also, if it's a location code, use the name instead of the code
                             if ( $link_value =~ /location/ ) {
                                 # TODO Retrieve all authorised values at once, instead of 1 query per entry
-                                my $av = Koha::AuthorisedValues->search({ category => 'LOC', authorised_value => $one_facet });
-                                $facet_label_value = $av->count ? $av->next->opac_description : '';
+                                my $av = Koha::AuthorisedValues->search_with_localization({ category => 'LOC', authorised_value => $one_facet });
+                                $facet_label_value = $av->count ? $av->next->opac_translated_description : ''; # FIXME: Why are we displaying the opac description for staff?
                             }
 
                             # also, if it's a collection code, use the name instead of the code
                             if ( $link_value =~ /ccode/ ) {
                                 # TODO Retrieve all authorised values at once, instead of 1 query per entry
-                                my $av = Koha::AuthorisedValues->search({ category => 'CCODE', authorised_value => $one_facet });
-                                $facet_label_value = $av->count ? $av->next->opac_description : '';
+                                my $av = Koha::AuthorisedValues->search_with_localization({ category => 'CCODE', authorised_value => $one_facet });
+                                $facet_label_value = $av->count ? $av->next->opac_translated_description : '';
                             }
 
             # but we're down with the whole label being in the link's title.
diff --git a/Koha/AuthorisedValue.pm b/Koha/AuthorisedValue.pm
index 997ededbb7..2c2e4cd051 100644
--- a/Koha/AuthorisedValue.pm
+++ b/Koha/AuthorisedValue.pm
@@ -52,6 +52,32 @@ sub opac_description {
 
 =head2 Internal methods
 
+=head3 opac_translated_description
+
+=cut
+
+sub opac_translated_description {
+    my ( $self, $lang ) = @_;
+    if ( my $translated_description = eval { $self->get_column('opac_translated_description') } ) {
+        # If the value has already been fetched (eg. from sarch_with_localization),
+        # do not search for it again
+        # Note: This is a bit hacky but should be fast
+        return $translated_description
+             ? $translated_description
+             : $self->lib;
+    }
+    $lang ||= C4::Languages::getlanguage;
+    my $translated_description = Koha::Localizations->search({
+        code => $self->authorised_value,
+        entity => 'avs',
+        lang => $lang,
+        interface => 'opac',
+    })->next;
+    return $translated_description
+         ? $translated_description->translation
+         : $self->translated_description; # FIXME Is that what we really want?
+}
+
 =head3 translated_description
 
 =cut
@@ -70,7 +96,7 @@ sub translated_description {
     my $translated_description = Koha::Localizations->search({
         code => $self->authorised_value,
         entity => 'avs',
-        lang => $lang
+        lang => $lang,
     })->next;
     return $translated_description
          ? $translated_description->translation
diff --git a/Koha/AuthorisedValues.pm b/Koha/AuthorisedValues.pm
index bab3e0ce18..475423f977 100644
--- a/Koha/AuthorisedValues.pm
+++ b/Koha/AuthorisedValues.pm
@@ -77,7 +77,7 @@ sub search_by_marc_field {
 
     return unless $tagfield or $tagsubfield;
 
-    return $self->SUPER::search(
+    return $self->search_with_localization(
         {   'marc_subfield_structures.frameworkcode' => $frameworkcode,
             ( defined $tagfield    ? ( 'marc_subfield_structures.tagfield'    => $tagfield )    : () ),
             ( defined $tagsubfield ? ( 'marc_subfield_structures.tagsubfield' => $tagsubfield ) : () ),
@@ -94,7 +94,7 @@ sub search_by_koha_field {
 
     return unless $kohafield;
 
-    return $self->SUPER::search(
+    return $self->search_with_localization(
         {   'marc_subfield_structures.frameworkcode' => $frameworkcode,
             'marc_subfield_structures.kohafield'     => $kohafield,
             ( defined $category ? ( category_name    => $category )         : () ),
@@ -111,7 +111,7 @@ sub find_by_koha_field {
     my $kohafield        = $params->{kohafield};
     my $authorised_value = $params->{authorised_value};
 
-    my $av = $self->search(
+    my $av = $self->search_with_localization(
         {   'marc_subfield_structures.frameworkcode' => $frameworkcode,
             'marc_subfield_structures.kohafield'     => $kohafield,
             'me.authorised_value'                    => $authorised_value,
@@ -132,13 +132,17 @@ sub get_description_by_koha_field {
     return {} unless defined $authorised_value;
 
     my $memory_cache = Koha::Cache::Memory::Lite->get_instance;
-    my $cache_key    = "AV_descriptions:$frameworkcode:$kohafield:$authorised_value";
+    my $language     = C4::Languages::getlanguage();
+    my $cache_key    = "AV_descriptions:$language:$frameworkcode:$kohafield:$authorised_value";
     my $cached       = $memory_cache->get_from_cache($cache_key);
     return $cached if $cached;
 
     my $av = $self->find_by_koha_field($params);
     return {} unless defined $av;
-    my $descriptions = { lib => $av->lib, opac_description => $av->opac_description };
+    my $descriptions = {
+        lib              => $av->translated_description,
+        opac_description => $av->opac_translated_description,
+    };
     $memory_cache->set_in_cache( $cache_key, $descriptions );
     return $descriptions;
 }
@@ -149,7 +153,8 @@ sub get_descriptions_by_koha_field {
     my $kohafield = $params->{kohafield};
 
     my $memory_cache = Koha::Cache::Memory::Lite->get_instance;
-    my $cache_key    = "AV_descriptions:$frameworkcode:$kohafield";
+    my $language     = C4::Languages::getlanguage();
+    my $cache_key    = "AV_descriptions:$language:$frameworkcode:$kohafield";
     my $cached       = $memory_cache->get_from_cache($cache_key);
     return @$cached if $cached;
 
@@ -157,8 +162,8 @@ sub get_descriptions_by_koha_field {
     my @descriptions = map {
         {
             authorised_value => $_->authorised_value,
-            lib              => $_->lib,
-            opac_description => $_->opac_description
+            lib              => $_->translated_description,
+            opac_description => $_->opac_translated_description
         }
     } @avs;
     $memory_cache->set_in_cache( $cache_key, \@descriptions );
@@ -190,13 +195,16 @@ sub search_with_localization {
     my $language = C4::Languages::getlanguage();
     $Koha::Schema::Result::AuthorisedValue::LANGUAGE = $language;
     $attributes->{order_by} = 'translated_description' unless exists $attributes->{order_by};
-    $attributes->{join} = 'localization';
+    my @join = $attributes->{join} || ();
+    push @join, 'localization';
+    my $join = { join => \@join };
     $attributes->{'+select'} = [
         {
             coalesce => [qw( localization.translation me.lib )],
             -as      => 'translated_description'
         }
     ];
+    $attributes = { %$attributes, %$join };
     $self->search( $params, $attributes );
 }
 
diff --git a/Koha/Template/Plugin/AuthorisedValues.pm b/Koha/Template/Plugin/AuthorisedValues.pm
index 95b0c4c939..bd9a640999 100644
--- a/Koha/Template/Plugin/AuthorisedValues.pm
+++ b/Koha/Template/Plugin/AuthorisedValues.pm
@@ -29,8 +29,8 @@ sub GetByCode {
     my $av = Koha::AuthorisedValues->search({ category => $category, authorised_value => $code });
     return $av->count
             ? $opac
-                ? $av->next->opac_description
-                : $av->next->lib
+                ? $av->next->opac_translated_description
+                : $av->next->translated_description
             : $code;
 }
 
@@ -83,8 +83,8 @@ sub GetDescriptionByKohaField {
     );
     return %$av
             ? $params->{opac}
-                ? $av->{opac_description}
-                : $av->{lib}
+                ? $av->{opac_description} # Rename with opac_translated_description to avoid confusion
+                : $av->{lib}              # Same with translated_description
             : ''; # Maybe we should return $params->{authorised_value}?
 
 }
diff --git a/admin/authorised_values.pl b/admin/authorised_values.pl
index 8a6133890a..c3bdf5470c 100755
--- a/admin/authorised_values.pl
+++ b/admin/authorised_values.pl
@@ -225,7 +225,7 @@ if ( $op eq 'list' ) {
 
     $searchfield ||= $category_list[0];
 
-    my $avs_by_category = Koha::AuthorisedValues->search_with_localization( { category => $searchfield } );
+    my $avs_by_category = Koha::AuthorisedValues->search( { category => $searchfield } );
 
     $template->param(
         authorised_values => $avs_by_category,
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/authorised_values.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/authorised_values.tt
index 4ffc70ce5c..411fb39e50 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/authorised_values.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/authorised_values.tt
@@ -74,6 +74,7 @@
         </li>
         <li>
             <label for="lib">Description: </label>
+            <input type="text" name="lib" id="lib" value="[% lib | html %]" maxlength="200" />
             [% IF can_be_translated %]
                 <a href="/cgi-bin/koha/admin/localization.pl?entity=avs&code=[% authorised_value | uri %]&interface=intranet"" title="Translate description" rel="gb_page_center[600,500]"><i class="fa fa-pencil"></i> Translate into other languages</a>
             [% END %]
@@ -227,19 +228,21 @@
     [% FOREACH av IN authorised_values %]
     <tr>
         <td>[% av.authorised_value | html %]</td>
-        [% IF av.translated_descriptions.size %]
-            [% av.lib | html %] (default)<br/>
-            [% FOR description IN av.translated_descriptions %]
-                [% IF description.translation == av.translated_description %]
-                    <b>[% description.translation | html %]</b>
-                [% ELSE %]
-                    [% description.translation | html %] ([% description.lang | html %])
+        <td>
+            [% IF av.translated_descriptions.size %]
+                [% av.lib | html %] (default)<br/>
+                [% FOR description IN av.translated_descriptions %]
+                    [% IF description.translation == av.translated_description %]
+                        <b>[% description.translation | html %]</b>
+                    [% ELSE %]
+                        [% description.translation | html %] ([% description.lang | html %])
+                    [% END %]
+                    <br/>
                 [% END %]
-                <br/>
+            [% ELSE %]
+                [% av.lib |html %]
             [% END %]
-        [% ELSE %]
-            [% av.lib |html %]
-        [% END %]
+        </td>
 
         <td>[% av.lib_opac | html %]</td>
         <td>[% IF ( av.imageurl ) %]<img src="[% av.imageurl | url %]" alt=""/>[% ELSE %]&nbsp;[% END %]</td>
diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl
index 2272f16921..40ab844c4a 100755
--- a/opac/opac-memberentry.pl
+++ b/opac/opac-memberentry.pl
@@ -628,7 +628,7 @@ sub GeneratePatronAttributesForm {
         my $av = Koha::AuthorisedValues->search(
             { category => 'PA_CLASS', authorised_value => $class } );
 
-        my $lib = $av->count ? $av->next->opac_description : $class;
+        my $lib = $av->count ? $av->next->opac_translated_description : $class;
 
         push @class_loop,
             {
diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
index 96834cb6cd..a6374a30ad 100755
--- a/opac/opac-reserve.pl
+++ b/opac/opac-reserve.pl
@@ -460,7 +460,7 @@ foreach my $biblioNum (@biblionumbers) {
     }
 
     my @notforloan_avs = Koha::AuthorisedValues->search_by_koha_field({ kohafield => 'items.notforloan', frameworkcode => $frameworkcode });
-    my $notforloan_label_of = { map { $_->authorised_value => $_->opac_description } @notforloan_avs };
+    my $notforloan_label_of = { map { $_->authorised_value => $_->opac_translated_description } @notforloan_avs };
 
     $biblioLoopIter{itemLoop} = [];
     my $numCopiesAvailable = 0;
diff --git a/opac/opac-suggestions.pl b/opac/opac-suggestions.pl
index 1ac168c0f2..54d7869e5b 100755
--- a/opac/opac-suggestions.pl
+++ b/opac/opac-suggestions.pl
@@ -200,7 +200,7 @@ foreach my $suggestion(@$suggestions_loop) {
     }
     if($suggestion->{'patronreason'}){
         my $av = Koha::AuthorisedValues->search({ category => 'OPAC_SUG', authorised_value => $suggestion->{patronreason} });
-        $suggestion->{'patronreason'} = $av->count ? $av->next->opac_description : '';
+        $suggestion->{'patronreason'} = $av->count ? $av->next->opac_translated_description : '';
     }
 }
 
-- 
2.20.1