From 44c860a61af36502d0b75062c9e201ed2ab11ba8 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Thu, 9 Jan 2020 13:53:41 +0100
Subject: [PATCH] Bug 20307: Fix logic in search_with_localization and add
 tests

Maybe we will want to replace this method at some point. There was
something wrong in the logic.

I did not manage to implement what I really needed here.

The real SQL query we need is:
SELECT SELECT *,
            COALESCE( `localization_intranet`.`translation`, `me`.`lib` ) AS `translated_description`,
            COALESCE( `localization_opac`.`translation`, `me`.`lib` ) AS `opac_translated_description`
        FROM `authorised_values` `me`
        LEFT JOIN (SELECT localization_id, code, lang, translation FROM localization WHERE entity='authorised_values' AND interface='intranet') `localization_intranet` ON ( `localization_intranet`.`lang` = 'es-ES' AND CONCAT(me.category, '_', `me`.`authorised_value`) = `localization_intranet`.`code` )
        LEFT JOIN (SELECT localization_id, code, lang, translation FROM localization WHERE entity='authorised_values' AND interface='opac') `localization_opac` ON ( `localization_opac`.`lang` = 'es-ES' AND CONCAT(me.category, '_', `me`.`authorised_value`) = `localization_opac`.`code` )
        WHERE me.category="?" ORDER BY `translated_description`;

So 2 left join on the same table, with different on clause (to deal with
the 2 interfaces)
That would give us something like:
| id  | category | authorised_value | lib  | lib_opac | imageurl | translated_description | opac_translated_description |

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 C4/Koha.pm                                    |   1 -
 Koha/AuthorisedValues.pm                      |  27 +++-
 Koha/Schema/Result/AuthorisedValue.pm         |   8 +-
 .../Result/AuthorisedValueLocalization.pm     |   4 +-
 t/db_dependent/AuthorisedValues.t             | 130 +++++++++++++++++-
 5 files changed, 159 insertions(+), 11 deletions(-)

diff --git a/C4/Koha.pm b/C4/Koha.pm
index 6a2ed1e0d1..46896b729b 100644
--- a/C4/Koha.pm
+++ b/C4/Koha.pm
@@ -554,7 +554,6 @@ sub GetAuthorisedValues {
         $av_unblessed->{lib} = $av->lib;
         $av_unblessed->{lib_opac} = $av->lib_opac;
         push @results, $av_unblessed;
-
     }
 
     $cache->set_in_cache( $cache_key, \@results, { expiry => 5 } );
diff --git a/Koha/AuthorisedValues.pm b/Koha/AuthorisedValues.pm
index dbdfcf4c04..c855057b35 100644
--- a/Koha/AuthorisedValues.pm
+++ b/Koha/AuthorisedValues.pm
@@ -192,18 +192,35 @@ my $avs = Koha::AuthorisedValues->search_with_localization
 sub search_with_localization {
     my ( $self, $params, $attributes ) = @_;
 
-    my $language = C4::Languages::getlanguage();
-    $Koha::Schema::Result::AuthorisedValue::LANGUAGE = $language;
-    $attributes->{order_by} = 'translated_description' unless exists $attributes->{order_by};
+    my $language  = C4::Languages::getlanguage();
+    my $interface = C4::Context->interface;
+    $Koha::Schema::Result::AuthorisedValue::LANGUAGE  = $language;
+    $Koha::Schema::Result::AuthorisedValue::INTERFACE = $interface;
+
+    unless ( exists $attributes->{order_by} ) {
+        $attributes->{order_by} =
+          $interface eq 'intranet'
+          ? 'translated_description'
+          : 'opac_translated_description';
+    }
+
     my @join = $attributes->{join} || ();
     push @join, 'localization';
     $attributes->{join} = \@join;
-    $attributes->{'+select'} = [
+    my $select = $interface eq 'intranet'
+      ? [
         {
             coalesce => [qw( localization.translation me.lib )],
             -as      => 'translated_description'
         }
-    ];
+      ]
+      : [
+        {
+            coalesce => [qw( localization.translation me.lib_opac )],
+            -as      => 'opac_translated_description'
+        }
+      ];
+    $attributes->{'+select'} = $select;
     if(defined $params->{branchcode}) {
         my $branchcode = delete $params->{branchcode};
         $self->search_with_library_limits( $params, $attributes, $branchcode );
diff --git a/Koha/Schema/Result/AuthorisedValue.pm b/Koha/Schema/Result/AuthorisedValue.pm
index 09acafc5e4..e6a9b1bfa0 100644
--- a/Koha/Schema/Result/AuthorisedValue.pm
+++ b/Koha/Schema/Result/AuthorisedValue.pm
@@ -148,18 +148,20 @@ __PACKAGE__->has_many(
 # Created by DBIx::Class::Schema::Loader v0.07046 @ 2019-02-22 14:32:48
 # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:hDlebhEn+f+thqwBo/LOqQ
 
-# Use the ItemtypeLocalization view to create the join on localization
-our $LANGUAGE;
+# Use the AuthorisedValueLocalization view to create the join on localization
+our ( $LANGUAGE, $INTERFACE );
 __PACKAGE__->has_many(
   "localization" => "Koha::Schema::Result::AuthorisedValueLocalization",
     sub {
         my $args = shift;
 
         die "no lang specified!" unless $LANGUAGE;
+        die "no interface specified!" unless $INTERFACE;
 
         return ({
-            "$args->{self_alias}.authorised_value" => { -ident => "$args->{foreign_alias}.code" },
+            "$args->{foreign_alias}.code" => \["= CONCAT(me.category, '_', me.authorised_value)"],
             "$args->{foreign_alias}.lang" => $LANGUAGE,
+            "$args->{foreign_alias}.interface" => $INTERFACE,
         });
 
     }
diff --git a/Koha/Schema/Result/AuthorisedValueLocalization.pm b/Koha/Schema/Result/AuthorisedValueLocalization.pm
index b283172b1e..be7026db1b 100644
--- a/Koha/Schema/Result/AuthorisedValueLocalization.pm
+++ b/Koha/Schema/Result/AuthorisedValueLocalization.pm
@@ -9,7 +9,7 @@ __PACKAGE__->table_class('DBIx::Class::ResultSource::View');
 __PACKAGE__->table('authorised_value_localizations');
 __PACKAGE__->result_source_instance->is_virtual(1);
 __PACKAGE__->result_source_instance->view_definition(
-    "SELECT localization_id, code, lang, translation FROM localization WHERE entity='authorised_values'"
+    "SELECT localization_id, code, lang, translation, interface FROM localization WHERE entity='authorised_values'"
 );
 
 __PACKAGE__->add_columns(
@@ -21,6 +21,8 @@ __PACKAGE__->add_columns(
   { data_type => "varchar", is_nullable => 0, size => 25 },
   "translation",
   { data_type => "text", is_nullable => 1 },
+  "interface",
+  { data_type => "text", is_nullable => 1 },
 );
 
 __PACKAGE__->belongs_to(
diff --git a/t/db_dependent/AuthorisedValues.t b/t/db_dependent/AuthorisedValues.t
index 44ae89ca09..a8c29341f7 100644
--- a/t/db_dependent/AuthorisedValues.t
+++ b/t/db_dependent/AuthorisedValues.t
@@ -1,7 +1,8 @@
 #!/usr/bin/perl
 
 use Modern::Perl;
-use Test::More tests => 15;
+use Test::More tests => 16;
+use Test::MockModule;
 
 use t::lib::TestBuilder;
 use t::lib::Mocks;
@@ -12,6 +13,7 @@ use Koha::AuthorisedValue;
 use Koha::AuthorisedValues;
 use Koha::AuthorisedValueCategories;
 use Koha::MarcSubfieldStructures;
+use utf8;
 
 my $schema  = Koha::Database->new->schema;
 $schema->storage->txn_begin;
@@ -229,4 +231,130 @@ subtest 'search_by_*_field + find_by_koha_field + get_description' => sub {
     };
 };
 
+subtest "localization" => sub {
+    plan tests => 14;
+
+    Koha::AuthorisedValues->delete;
+
+    my $category =
+      Koha::AuthorisedValueCategory->new( { category_name => 'av_for_t' } );
+    my $av1 = $builder->build_object(
+        {
+            class => 'Koha::AuthorisedValues',
+            value => { category => $category->category_name, authorised_value => 'av1' }
+        }
+    );    # translated in es-ES and fr-FR
+    my $av2 = $builder->build_object(
+        {
+            class => 'Koha::AuthorisedValues',
+            value => { category => $category->category_name, authorised_value => 'av2' }
+        }
+    );    # translated in es-ES and de-DE
+    my $av3 = $builder->build_object(
+        {
+            class => 'Koha::AuthorisedValues',
+            value => { category => $category->category_name, authorised_value => 'av3' }
+        }
+    );    # no translation
+    Koha::Localization->new(
+        {
+            entity => 'authorised_values',
+            code => sprintf( "%s_%s", $av1->category, $av1->authorised_value ),
+            lang => 'es-ES',
+            interface => 'intranet',
+            translation => 'traducción 1'
+        }
+    )->store;
+    Koha::Localization->new(
+        {
+            entity => 'authorised_values',
+            code => sprintf( "%s_%s", $av1->category, $av1->authorised_value ),
+            lang => 'es-ES',
+            interface => 'opac',
+            translation => 'opac traducción 1'
+        }
+    )->store;
+    Koha::Localization->new(
+        {
+            entity => 'authorised_values',
+            code => sprintf( "%s_%s", $av1->category, $av2->authorised_value ),
+            lang => 'es-ES',
+            interface => 'intranet',
+            translation => 'traducción 2'
+        }
+    )->store;
+
+    Koha::Localization->new(
+        {
+            entity => 'authorised_values',
+            code => sprintf( "%s_%s", $av1->category, $av1->authorised_value ),
+            lang => 'fr-FR',
+            interface => 'intranet',
+            translation => 'traduction 1'
+        }
+    )->store;
+    Koha::Localization->new(
+        {
+            entity => 'authorised_values',
+            code => sprintf( "%s_%s", $av2->category, $av2->authorised_value ),
+            lang => 'de-DE',
+            interface => 'intranet',
+            translation => 'Übersetzung 2'
+        }
+    )->store;
+
+    my $c4_languages = Test::MockModule->new('C4::Languages');
+    $c4_languages->mock('getlanguage', sub { 'es-ES' });
+    my $c4_context = Test::MockModule->new('C4::Context');
+    $c4_context->mock('interface', 'intranet');
+
+    my $avs = Koha::AuthorisedValues->search_with_localization;
+    is( $avs->count, 3);
+
+    my $av1_loc = $avs->search( { authorised_value => $av1->authorised_value } )->next;
+    is( $av1_loc->translated_description, 'traducción 1', 'av1 has a translated description fetched from search_with_localization' );
+
+    my $av2_loc = $avs->search( { authorised_value => $av2->authorised_value } )->next;
+    is( $av2_loc->translated_description, 'traducción 2', 'av2 has a translated description fetched from search_with_localization' );
+
+    my $av3_loc = $avs->search( { authorised_value => $av3->authorised_value } )->next;
+    is( $av3_loc->translated_description, $av3->lib, 'av3 does not have a translated description for intranet, lib is returned' );
+
+    is( $av1_loc->opac_translated_description, 'opac traducción 1', 'opac_translated_description is not fetched from search_with localization, but accessor is available' );
+
+    is( $av2_loc->opac_translated_description, $av2->lib_opac, 'av2 does not have a translated description for opac, lib_opac is returned' );
+
+
+    $c4_context->mock('opac', 'intranet');
+
+    $avs = Koha::AuthorisedValues->search_with_localization;
+    is( $avs->count, 3);
+
+    $av1_loc = $avs->search( { authorised_value => $av1->authorised_value } )->next;
+    is( $av1_loc->opac_translated_description, 'opac traducción 1', 'opac_translated_description is fetched from search_with_localization' );
+
+    $av2_loc = $avs->search( { authorised_value => $av2->authorised_value } )->next;
+    is( $av2_loc->opac_translated_description, $av2->lib_opac, 'av2 does not have a translated description, lib_opac is returned' );
+
+    $av3_loc = $avs->search( { authorised_value => $av3->authorised_value } )->next;
+    is( $av3_loc->opac_translated_description, $av3->lib_opac, 'same than av2 for av3' );
+
+    is( $av1_loc->translated_description, 'traducción 1', 'translated description for intranet is not fetched from search_with_localization but accessor is available' );
+
+    is( $av2_loc->translated_description, 'traducción 2', 'translated description for intranet is not fetched from search_with_localization but accessor is available' );
+
+    is( $av3_loc->translated_description, $av3->lib, 'av3 does not have a translated description, lib is returned' );
+
+    my $translated_descriptions = $av1->translated_descriptions;
+    is_deeply(
+        $translated_descriptions,
+        [
+            { lang => "es-ES", translation => "traducción 1" },
+            { lang => "fr-FR", translation => "traduction 1" },
+            { lang => "es-ES", translation => "opac traducción 1" }
+        ]
+    , 'translated_descriptions return all the translations for a given av');
+
+};
+
 $schema->storage->txn_rollback;
-- 
2.20.1