From c0aa3439f1f2e79ca9a88b073c3bd6498bfbe903 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Wed, 13 Mar 2019 22:36:35 -0300
Subject: [PATCH] Bug 20307: Some more fixes

---
 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 +-
 .../prog/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 a4ac0e970e..3ba0f36eb0 100644
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -1096,8 +1096,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 f198f5b1dc..c768572f6a 100644
--- a/C4/Koha.pm
+++ b/C4/Koha.pm
@@ -547,7 +547,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 ) : () ),
@@ -558,8 +557,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 ef81e19e7c..da42059456 100644
--- a/C4/Search.pm
+++ b/C4/Search.pm
@@ -583,15 +583,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's 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 b60c57d67d..ce19d9d8bb 100644
--- a/Koha/AuthorisedValue.pm
+++ b/Koha/AuthorisedValue.pm
@@ -145,6 +145,32 @@ sub opac_description {
     return $self->lib_opac() || $self->lib();
 }
 
+=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
@@ -163,7 +189,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..35bad0c8c5 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,     # Rename with opac_translated_description to avoid confusion
+        opac_description => $av->opac_translated_description # Same with 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 4fa66bf33c..c63113eb80 100755
--- a/admin/authorised_values.pl
+++ b/admin/authorised_values.pl
@@ -224,7 +224,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 04c25bf536..5163e84ebe 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 dbb6d99e5c..4fa1f202aa 100755
--- a/opac/opac-memberentry.pl
+++ b/opac/opac-memberentry.pl
@@ -620,7 +620,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 aeb88ca036..90482b1b51 100755
--- a/opac/opac-reserve.pl
+++ b/opac/opac-reserve.pl
@@ -435,7 +435,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 cf562fb97e..4c60382f8b 100755
--- a/opac/opac-suggestions.pl
+++ b/opac/opac-suggestions.pl
@@ -188,7 +188,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.11.0