Attachment 132509 Details for Bug 24975 – Bug 24975: Keep the translations, prevent duplicates, and other fixes

[patch] Bug 24975: Keep the translations, prevent duplicates, and other fixes

Bug-24975-Keep-the-translations-prevent-duplicates.patch (text/plain), 11.26 KB, created by Martin Renvoize (ashimema) on 2022-03-30 06:37:40 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2022-03-30 06:37:40 UTC

Size: 11.26 KB

patch

obsolete

>From ed70b121cebdd09694d0c2c22415dc7404829887 Mon Sep 17 00:00:00 2001
>From: Julian Maurice <julian.maurice@biblibre.com>
>Date: Thu, 26 Mar 2020 09:13:33 +0100
>Subject: [PATCH] Bug 24975: Keep the translations, prevent duplicates, and
> other fixes
>
>Translations are no longer lost when you change an itemtype's
>description, thanks to a new `l10n_source.key` column which must be
>unique within it's "text group" ('itemtype' is a text group)
>This also prevent duplicate entries in l10n_source
>
>Columns were renamed to better match their purpose:
>* context -> group
>* source -> text
>
>Use Koha::Cache::Memory::Lite if other caching systems are not available
>
>Prepend 'l10n:' to the cache key to avoid conflicts
>
>Test plan:
>1. DROP TABLE IF EXISTS l10n_target, l10n_source
>2. Follow the test plan from previous patch
>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>
>https://bugs.koha-community.org/show_bug.cgi?id=24875
>---
> Koha/DBIx/Component/L10nSource.pm             | 50 ++++++++++++-------
> Koha/I18N.pm                                  | 23 +++++----
> Koha/Schema/Result/Itemtype.pm                |  9 ++--
> .../data/mysql/atomicupdate/bug-24975.perl    | 16 +++---
> .../prog/en/modules/admin/localization.tt     | 13 +++--
> svc/localization                              |  6 +--
> 6 files changed, 66 insertions(+), 51 deletions(-)
>
>diff --git a/Koha/DBIx/Component/L10nSource.pm b/Koha/DBIx/Component/L10nSource.pm
>index 9d4ea398d9..933638e834 100644
>--- a/Koha/DBIx/Component/L10nSource.pm
>+++ b/Koha/DBIx/Component/L10nSource.pm
>@@ -42,32 +42,46 @@ Koha::DBIx::Component::L10nSource
> 
> =head2 update_l10n_source
> 
>-    $self->update_l10n_source($context, @sources)
>+    $self->update_l10n_source($group, $key, $text)
> 
>-Ensure that all sources in C<@sources>and only them are present in l10n_source
>-for a given C<$context>
>+Update or create an entry in l10n_source
> 
> =cut
> 
> sub update_l10n_source {
>-    my ($self, $context, @sources) = @_;
>+    my ($self, $group, $key, $text) = @_;
>+
>+    my $l10n_source_rs = $self->result_source->schema->resultset('L10nSource');
>+    $l10n_source_rs->update_or_create(
>+        {
>+            group => $group,
>+            key   => $key,
>+            text  => $text,
>+        },
>+        {
>+            key => 'group_key',
>+        }
>+    );
>+}
> 
>-    my $l10n_source_rs = $self->result_source->schema->resultset('L10nSource')->search({ context => $context });
>+=head2 delete_l10n_source
> 
>-    # Insert missing l10n_source rows
>-    foreach my $source (@sources) {
>-        my $count = $l10n_source_rs->search({ source => $source })->count;
>-        if ($count == 0) {
>-            $l10n_source_rs->create({ source => $source });
>-        }
>-    }
>+    $self->delete_l10n_source($group, $key, $text)
> 
>-    # Remove l10n_source rows that do not match an existing source
>-    my %sources = map { $_ => undef } @sources;
>-    my @l10n_sources = grep { !exists $sources{$_->source} } $l10n_source_rs->all;
>-    foreach my $l10n_source (@l10n_sources) {
>-        $l10n_source->delete;
>-    }
>+Remove an entry from l10n_source
>+
>+=cut
>+
>+sub delete_l10n_source {
>+    my ($self, $group, $key) = @_;
>+
>+    my $l10n_source_rs = $self->result_source->schema->resultset('L10nSource');
>+    $l10n_source_rs->search(
>+        {
>+            group => $group,
>+            key   => $key,
>+        }
>+    )->delete();
> }
> 
> 1;
>diff --git a/Koha/I18N.pm b/Koha/I18N.pm
>index 8fc0aae957..96e9bd26d1 100644
>--- a/Koha/I18N.pm
>+++ b/Koha/I18N.pm
>@@ -228,21 +228,24 @@ sub _expand {
> 
> =head2 db_t
> 
>-    db_t($context, $source)
>+    db_t($group, $text)
>     db_t('itemtype', $itemtype->description)
> 
> Search for a translation in l10n_target table and return it if found
>-Return source string otherwise
>+Return C<$text> otherwise
> 
> =cut
> 
> sub db_t {
>-    my ($context, $source) = @_;
>+    my ($group, $text) = @_;
> 
>     my $cache = Koha::Caches->get_instance();
>-    my $language = C4::Languages::getlanguage();
>-    my $cache_key = sprintf('%s:%s', $context, $language);
>+    unless ($cache->is_cache_active) {
>+        $cache = Koha::Cache::Memory::Lite->get_instance();
>+    }
> 
>+    my $language = C4::Languages::getlanguage();
>+    my $cache_key = sprintf('l10n:%s:%s', $group, $language);
>     my $translations = $cache->get_from_cache($cache_key);
>     unless ($translations) {
>         my $schema = Koha::Database->new->schema;
>@@ -250,7 +253,7 @@ sub db_t {
> 
>         my @targets = $rs->search(
>             {
>-                'l10n_source.context' => $context,
>+                'l10n_source.group' => $group,
>                 language => $language,
>             },
>             {
>@@ -259,16 +262,16 @@ sub db_t {
>                 result_class => 'DBIx::Class::ResultClass::HashRefInflator',
>             },
>         );
>-        $translations = { map { $_->{l10n_source}->{source} => $_->{translation} } @targets };
>+        $translations = { map { $_->{l10n_source}->{text} => $_->{translation} } @targets };
> 
>         $cache->set_in_cache($cache_key, $translations);
>     }
> 
>-    if (exists $translations->{$source}) {
>-        return $translations->{$source};
>+    if (exists $translations->{$text}) {
>+        return $translations->{$text};
>     }
> 
>-    return $source;
>+    return $text;
> }
> 
> 1;
>diff --git a/Koha/Schema/Result/Itemtype.pm b/Koha/Schema/Result/Itemtype.pm
>index 9aba59a46e..c63684eaaf 100644
>--- a/Koha/Schema/Result/Itemtype.pm
>+++ b/Koha/Schema/Result/Itemtype.pm
>@@ -356,8 +356,7 @@ sub insert {
> 
>     my $result = $self->next::method(@_);
> 
>-    my @sources = $self->result_source->resultset->get_column('description')->all;
>-    $self->update_l10n_source('itemtype', @sources);
>+    $self->update_l10n_source('itemtype', $self->itemtype, $self->description);
> 
>     return $result;
> }
>@@ -370,8 +369,7 @@ sub update {
>     my $result = $self->next::method(@_);
> 
>     if ($is_description_changed) {
>-        my @sources = $self->result_source->resultset->get_column('description')->all;
>-        $self->update_l10n_source('itemtype', @sources);
>+        $self->update_l10n_source('itemtype', $self->itemtype, $self->description);
>     }
> 
>     return $result;
>@@ -382,8 +380,7 @@ sub delete {
> 
>     my $result = $self->next::method(@_);
> 
>-    my @sources = $self->result_source->resultset->get_column('description')->all;
>-    $self->update_l10n_source('itemtype', @sources);
>+    $self->delete_l10n_source('itemtype', $self->itemtype);
> 
>     return $result;
> }
>diff --git a/installer/data/mysql/atomicupdate/bug-24975.perl b/installer/data/mysql/atomicupdate/bug-24975.perl
>index 748b9b7b8f..66d0319928 100644
>--- a/installer/data/mysql/atomicupdate/bug-24975.perl
>+++ b/installer/data/mysql/atomicupdate/bug-24975.perl
>@@ -3,10 +3,12 @@ if( CheckVersion( $DBversion ) ) {
>     $dbh->do(q{
>         CREATE TABLE IF NOT EXISTS l10n_source (
>             l10n_source_id INT(11) NOT NULL AUTO_INCREMENT,
>-            context VARCHAR(100) NULL DEFAULT NULL,
>-            source TEXT NOT NULL,
>+            `group` VARCHAR(100) NULL DEFAULT NULL,
>+            `key` VARCHAR(100) NOT NULL,
>+            `text` TEXT NOT NULL,
>             PRIMARY KEY (l10n_source_id),
>-            KEY context_source (context, source(60))
>+            UNIQUE KEY group_key (`group`, `key`),
>+            KEY group_text (`group`, `text`(60))
>         ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin;
>     });
> 
>@@ -25,17 +27,17 @@ if( CheckVersion( $DBversion ) ) {
> 
>     # Populate l10n_source and l10n_target for itemtypes
>     $dbh->do(q{
>-        INSERT INTO l10n_source (context, source)
>-        SELECT DISTINCT 'itemtype', description FROM itemtypes
>+        INSERT IGNORE INTO l10n_source (`group`, `key`, `text`)
>+        SELECT DISTINCT 'itemtype', itemtype, description FROM itemtypes
>     });
> 
>     if (TableExists('localization')) {
>         $dbh->do(q{
>-            INSERT INTO l10n_target (l10n_source_id, language, translation)
>+            INSERT IGNORE INTO l10n_target (l10n_source_id, language, translation)
>             SELECT DISTINCT l10n_source_id, lang, translation
>             FROM localization
>             JOIN itemtypes ON (localization.code = itemtypes.itemtype)
>-            JOIN l10n_source ON (itemtypes.description = l10n_source.source AND l10n_source.context = 'itemtype')
>+            JOIN l10n_source ON (itemtypes.itemtype = l10n_source.`key` AND l10n_source.`group` = 'itemtype')
>             WHERE entity = 'itemtypes'
>         });
> 
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/localization.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/localization.tt
>index 1cde049e10..ead8d801d4 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/localization.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/localization.tt
>@@ -45,8 +45,8 @@
>                     <table id="localization">
>                         <thead>
>                             <tr>
>-                                <th>Context</th>
>-                                <th>Source</th>
>+                                <th>Group</th>
>+                                <th>Text</th>
>                                 <th>Translations</th>
>                             </tr>
>                         </thead>
>@@ -89,12 +89,12 @@
>                 },
>                 columns: [
>                     {
>-                        name: 'context',
>-                        data: 'context',
>+                        name: 'group',
>+                        data: 'group',
>                     },
>                     {
>-                        name: 'source',
>-                        data: 'source',
>+                        name: 'text',
>+                        data: 'text',
>                     },
>                     {
>                         name: 'targets',
>@@ -120,7 +120,6 @@
>                                 translateLink.attr('href', '#');
>                                 translateLink.attr('data-l10n-source-id', row.l10n_source_id);
>                                 translateLink.attr('data-language', language.rfc4646_subtag);
>-                                translateLink.attr('data-source', row.source);
>                                 if (translations[language.rfc4646_subtag]) {
>                                     translateLink.attr('data-translation', translations[language.rfc4646_subtag].translation);
>                                 }
>diff --git a/svc/localization b/svc/localization
>index 8972857625..f795d5724b 100755
>--- a/svc/localization
>+++ b/svc/localization
>@@ -24,7 +24,7 @@ sub get_translations {
>         my @and;
>         my @words = split /\s+/, $search;
>         foreach my $word (@words) {
>-            push @and, \['source LIKE ? COLLATE utf8mb4_unicode_ci', "%$word%"];
>+            push @and, \['text LIKE ? COLLATE utf8mb4_unicode_ci', "%$word%"];
>         }
>         $cond->{-and} = \@and;
>     }
>@@ -37,7 +37,7 @@ sub get_translations {
>         result_class => 'DBIx::Class::ResultClass::HashRefInflator',
>         rows => $length,
>         offset => $start,
>-        order_by => { -asc => ['context', 'source'] },
>+        order_by => { -asc => ['group', 'text'] },
>     });
> 
>     $response->param(
>@@ -73,7 +73,7 @@ sub update_translation {
>     }
> 
>     my $source = $schema->resultset('L10nSource')->find($id);
>-    my $cache_key = sprintf('%s:%s', $source->context, $lang);
>+    my $cache_key = sprintf('l10n:%s:%s', $source->group, $lang);
>     Koha::Caches->get_instance()->clear_from_cache($cache_key);
> 
>     C4::Service->return_success( $response );
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 24975: 101703 | 101786 | 102387 | 102388 | 102513 | 102514 | 102688 | 102701 | 132388 | 132508 | 132509 | 132510 | 132511 | 132512 | 132513 | 132514 | 132515 | 132516 | 132517 | 132518 | 132519 | 132520 | 132521 | 132522 | 132523 | 132524 | 132525 | 132526 | 132527 | 132528 | 132529 | 132530 | 132531 | 132532 | 132533 | 132546 | 132562 | 132563 | 132564 | 132565 | 132566 | 132567 | 132568 | 132569 | 132570 | 132571 | 132572 | 132573 | 132574 | 132575 | 149941 | 149942 | 149943 | 149944 | 149945 | 149946 | 149947 | 149948 | 149949 | 149950 | 149951 | 149952 | 149953 | 149954 | 149956 | 149957 | 150015 | 150033 | 150034