From e5014fa96e11763ee3d53d05c5d1184a612f7b9d Mon Sep 17 00:00:00 2001 From: Julian Maurice 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 --- 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 d896471131..bc2734587b 100644 --- a/Koha/I18N.pm +++ b/Koha/I18N.pm @@ -207,21 +207,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; @@ -229,7 +232,7 @@ sub db_t { my @targets = $rs->search( { - 'l10n_source.context' => $context, + 'l10n_source.group' => $group, language => $language, }, { @@ -238,16 +241,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 91adcf342c..651db81a97 100644 --- a/Koha/Schema/Result/Itemtype.pm +++ b/Koha/Schema/Result/Itemtype.pm @@ -244,8 +244,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; } @@ -258,8 +257,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; @@ -270,8 +268,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 2cf521ad88..49a466682b 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/localization.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/localization.tt @@ -43,8 +43,8 @@ - - + + @@ -87,12 +87,12 @@ }, columns: [ { - name: 'context', - data: 'context', + name: 'group', + data: 'group', }, { - name: 'source', - data: 'source', + name: 'text', + data: 'text', }, { name: 'targets', @@ -118,7 +118,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
ContextSourceGroupText Translations