Problem/Motivation

Variables can be set as multilingual using i18n module.

Proposed resolution

Create a source and destination plugin to migrate the data in the D6 {i18n_variable} table (name, language, value).

Remaining tasks

Write the source plugin and tests
Write the config destination plugin and tests

See roadmap section in #2208401: [META] Multilingual migrations / i18n meta issue

Original report by Ryan Weal

i18n variables (D6/D7 i18n contrib -> D8 translatable config). In D6 & D7 the i18n module provided an "override" table of any settings the user specified in their settings.php file (or in the DB in d7). Virtually any D6 or D7 variable could have a mirror of itself in the i18n table. Best case scenario, we detect which settings have the override set, and then pull all the language versions for those config and apply them to the translatable config entities in D8.

Comments

penyaskito’s picture

AFAIK, we can only override labels in D8.

Gábor Hojtsy’s picture

So long as we have a mapping of a Drupal 6 variable to its Drupal 8 equivalent, we should be easily able to migrate it to config translation, basically we migrate it into a config file with the same structure but only the translation. Work for any translatable variable that is still marked translatable in Drupal 8 config.

phenaproxima’s picture

Project: IMP » Drupal core
Version: » 8.0.x-dev
Component: Code » migration system
phenaproxima’s picture

Gábor Hojtsy’s picture

quietone’s picture

I've enabled the internationalization, content type translation and profile translation and 3 languages. Made some test content played with configuration but have not been able to populate the i18n_variable table. How does one generate that data? Please be specific, I've never done any multilingual work before.

quietone’s picture

What exactly is "D8 translatable config" mentioned in the IS?

Gábor Hojtsy’s picture

@quietone: almost all configuration in Drupal 8 is translatable, enable the configuration translation module and see the list of translatable configuration in admin > configuration > regional and language > configuration translation.

The i18n_variable table seems to be originated from i18n_variable module which does not even use it anymore. It uses a generic storage system of the variable module instead. See http://cgit.drupalcode.org/i18n/tree/i18n_variable/i18n_variable.install... update functions :) That combination allows you to translate things like content type names, the site name, etc. Those are now in translatable config in Drupal 8.

Hope this helps.

quietone’s picture

@Gábor Hojtsy, thanks I do see the D8 translations in config. But, so far, I can only make content translations for D6.

How do I create a translation for things like site name and site maintenance msg in D6?

Gábor Hojtsy’s picture

Right, I quoted Drupal 7 code. Sorry. So in Drupal 6:

To translate site name, etc you'll need to set the global i18n_variables array. See https://www.drupal.org/node/313272. Note that you'll be editing translations on the config page itself by switching language.. For some higher level config objects like content types, that is much easier:

- Enable he content type translation module (from i18n). It will enable all its dependencies. No external dependencies.
- Add a language other than English.
- Go to /admin/build/translate/search and limit search to content type. Voila!

quietone’s picture

Thx, I've got some data in the i18n_variable table! The bit I missed was changing settings.php.

quietone’s picture

Thinking this through a bit with a simple example.

Drupal 6

+-------------+----------+-------------------------+
| name        | language | value                   |
+-------------+----------+-------------------------+
| site_slogan | mi       | s:15:"tihei mauri ora"; |

Drupal 8

| collection  | name        | data
|             | system.site | a:11:{s:4:"uuid";s:36:"a3fb04cd-efdf-4032-9000-c545f81edfcb";s:4:"name";s:3:"md8";s:4:"mail";s:20:"vaspagnolo@gmail.com";s:6:"slogan";s:14:"english slogan";s:4:"page";<more stuff here |
| language.es | system.site | a:2:{s:4:"name";s:11:"md8 spanish";s:6:"slogan";s:19:"spanish site slogan";}                                                                                                                                                                        

So, for all site_slogans in D6 the following applies.
language ==> collection field as language.XX
value ==> system.site/slogan

A source plugin that produces rows identical to the source should be sufficient. And allow for processing of the value, if needed, in more complex inputs.

Before doing that I want to know more about writing language data to config. Specifically, can the existing config destination plugins be sufficient? Will they handle the collection field?

Gábor Hojtsy’s picture

That I don't know any better than you unfortunately. Wanna try? :)

quietone’s picture

Status: Active » Needs review
FileSize
6.53 KB

First go at this. It is modeled off the existing Variable.php. This could be the wrong approach but have to start somewhere. And will know more when using it in a real migration.

The plugin will return an array of the following form.

'id' => 'the first input variable name'
'site_name' => [
   'fr' => 'site name in French'
  'es' => 'site name in Spanish'
  ]
'site_logon' => [
   'fr' => 'site logo in French'
  'es' => 'site logo in Spanish'
  ]

For now, let's see what the bot finds,

quietone’s picture

+++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/i18nVariable.php
@@ -0,0 +1,101 @@
+      ->select('variable', 'v')

s/variable/i18n_variable/

Oops, will fix soon.

penyaskito’s picture

Great progress :)
Thanks for working on this :)

quietone’s picture

Issue summary: View changes

Started working on a config destination plugin. Can't use the existing one because, I think, the language must be set to the language of the data being migrated first. Correct me if I am wrong on that.

It looks like there are several setLanguage methods. Which one to use and how?

quietone’s picture

Status: Needs review » Needs work

Needs the destination plugin.

quietone’s picture

Status: Needs work » Needs review
FileSize
11.06 KB

penyaskito, thanks for the help on IRC.

Added a destination plugin. It is in migrate, just like the current Variable destination plugin. Not sure if that is the right module for it. And the changes to config schema need to be checked. I'm not very familiar with that system.

quietone’s picture

Changed the destination plugin to sort the destination by language so the saving can be done by language.

Even though I had questions about how to handle multiple languages I wanted to create this so I had a better understanding of the issue. Now thats done, time to return to the questions.

1) Is using i18n to precede the filenames ok?
2) Should the destination plugin be in migrate or somewhere else?
3) Is calculateDependencies correct?
4) Should the source plugin be smarter and allow for selecting one or more languages?

quietone’s picture

Title: Migrate D6 i18n variables » [META] Migrate D6 i18n variables

I'm going to start making child issues for other i18n variables that need a migration. I'll start by looking at the existing D6 migrations that use the variable source plugin. These are:

d6_book_settings.yml
d6_system_image_gd.yml
d6_date_formats.yml
d6_system_maintenance.yml
d6_system_cron.yml
d6_system_rss.yml
d6_system_date.yml
d6_system_performance.yml
d6_system_file.yml
d6_system_image.yml
d6_system_logging.yml
d6_system_site.yml
d6_action_settings.yml
d6_dblog_settings.yml
d6_syslog_settings.yml
d6_node_settings.yml
d6_aggregator_settings.yml
d6_file_settings.yml
d6_search_settings.yml
d6_simpletest_settings.yml
d6_statistics_settings.yml
d6_forum_settings.yml
d6_comment_entity_form_display_subject.yml
d6_comment_field_instance.yml
d6_comment_field.yml
d6_comment_entity_display.yml
d6_comment_entity_form_display.yml
d6_comment_type.yml
d6_user_mail.yml
d6_user_settings.yml

Therefore, changing the title to include [META].

quietone’s picture

Talked to chx on IRC and he suggested that it would be better to not have two destination plugins for Config. Makes sense and it was a simple once I stopped over thinking it.

chx’s picture

Nifty!

quietone’s picture

Issue tags: +blocker

This is a blocker for any issue that migrates i18n variables to configuration. Note that the variable to configuration migrations are usually suitable for tagging Novice.

Gábor Hojtsy’s picture

+++ b/core/modules/migrate/src/Plugin/migrate/destination/Config.php
@@ -74,6 +86,10 @@ public static function create(ContainerInterface $container, array $configuratio
   public function import(Row $row, array $old_destination_id_values = array()) {
+    if ($row->hasDestinationProperty('langcode')) {
+      $this->config = $this->language_manager->getLanguageConfigOverride($row->getDestinationProperty('langcode'), $this->config->getName());
+    }

So this is basically the meat of the patch right? :)

chx’s picture

That and i18nVariable::values and ::query

Gábor Hojtsy’s picture

While I have limited understanding of the migration system at best, the reuse of the config migration looks good. What would ensure that this is only attempted to be run if there was i18n_variable table/data?

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +D8MI, +sprint, +language-config
08:09 <chx> GaborHojtsy: what an excellent quesiton
08:10 <chx> GaborHojtsy: easy
08:10 <chx> GaborHojtsy: this doesnt run at all :D
08:10 <chx> GaborHojtsy: this is a source and a destination
08:10 <chx> GaborHojtsy: https://www.drupal.org/node/2225477#comment-10778504 the real meat will be from these files.
08:10 <Druplicon> https://www.drupal.org/node/2225477 => [META] Migrate D6 i18n variables #2225477: Add migrate sources and destinations for D6 i18n variables => 27 comments, 21 IRC mentions
08:10 <chx> GaborHojtsy: and those, those have a requirements handling
08:10 <chx> GaborHojtsy: same as term migrations running only if taxonomy is on

All right, let's get this baseline in then, I see nothing wrong here. And then build on top of it :)

catch’s picture

Title: [META] Migrate D6 i18n variables » Migrate D6 i18n variables

Not properly reviewed yet, but got confused by the title so updating.

catch’s picture

Title: Migrate D6 i18n variables » Add migrate sources and destinations for D6 i18n variables
quietone’s picture

Issue tags: +migrate-d6-d8
catch’s picture

Status: Reviewed & tested by the community » Needs review

Shouldn't this use @link?

quietone’s picture

@catch, thx. But can you be more explicit? Where do you think @link is needed?

catch’s picture

Status: Needs review » Reviewed & tested by the community

Sorry I posted that comment to the wrong tab :(

Back to RTBC, I need to actually review this one again, which is why I had the tab open, but have not done that yet.

Kristen Pol’s picture

Thanks for the patch! I scanned the code for syntax and found some nitpicks that don't affect functionality. Leaving RTBC since it's unclear if any of these should be addressed.

  1. +++ b/core/modules/migrate/src/Plugin/migrate/destination/Config.php
    @@ -51,10 +59,13 @@ class Config extends DestinationBase implements ContainerFactoryPluginInterface,
    +   * @param \Drupal\Core\Language\ConfigurableLanguageManagerInterface $language_manager
    

    Nitpick: More than 80 characters.

  2. +++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/i18nVariable.php
    @@ -0,0 +1,105 @@
    +   *   A key/value pair is added for the language code.  Only those values are
    

    Nitpick: Use single space between sentences.

  3. +++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/i18nVariable.php
    @@ -0,0 +1,105 @@
    +    foreach ($result as $i18nvariable) {
    +      $values[]['language'] = $i18nvariable->language;
    +    }
    +    $result = $this->prepareQuery()->execute()->FetchAll();
    +    foreach ($result as $i18nvariable) {
    +      foreach ($values as $key => $value) {
    +        if ($values[$key]['language'] === $i18nvariable->language ) {
    +          $values[$key][$i18nvariable->name] = unserialize($i18nvariable->value);
    +          break;
    

    Nitpick: Use $i18n_variable or $i18nVariable.

    Nitpick2: Extra space after $i18nvariable->language and before ) {.

quietone’s picture

Thx, Kristen Pol. Yea, nitpicks should be addressed.
1. Sorry, don't know how to 'properly' fix that.
2. Fixed
3. Name changed, extra space removed.

Kristen Pol’s picture

Changes look good! Yeah... not sure what the policy is for the first nitpick so keeping it as is seems fine. Thanks!

  • catch committed 054d153 on 8.2.x
    Issue #2225477 by quietone: Add migrate sources and destinations for D6...

  • catch committed 39b9846 on 8.1.x
    Issue #2225477 by quietone: Add migrate sources and destinations for D6...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to all three 8.x branches. Thanks!

Added commit credit post-commit.

Generally it's really nice to see support being implemented for migrating modules like i18n_variables that have been replaced by core functionality, this is one of the things it was near-impossible to do properly with hook_update_N().

  • catch committed e87744a on 8.0.x
    Issue #2225477 by quietone: Add migrate sources and destinations for D6...
Gábor Hojtsy’s picture

Amazing, thanks all, especially @quietone! This now allows so much to make progress! Woot!

Gábor Hojtsy’s picture

Issue tags: -sprint

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.