Variables to config: language.types [d7]

Here is my first try to work on the migrate project. I'm not sure if I've done it correctly. Please suggest.

Hi @siharthap, that looks pretty good. We're just waiting on this patch: https://www.drupal.org/node/2410625

Which will automate all the dumps for D7 and then we're going to start full speed on the D7 migrate path.

Thank You @benjy.

Will need to be submitted again to Core since moving from sandbox.

I've completed moving this to the language module and it's running fine locally.

But I can't find variable 'language_interface' in D7, where is it defined?

What about the other language variables; language_count, language_negotiation_language, language_negotiation_language_content and language_negotiation_language_url? Are these to be migrated as well?

Accidentally lost my previous works (too many distractions today). So here is a quick reroll, done just before going to bed. It needs more work and I'm too tired to do so now. But, let's see what testbot finds.

Change parent issue from the duplicate to the current one.

Reroll remove unused use statements

Not sure how I messed up that edit.

1. +++ b/core/modules/language/migration_templates/d7_language_types.yml
   @@ -0,0 +1,25 @@
   +  'negotioation/language_url/enabled/language_url':
   +    plugin: static_map
   +    source: language_url
   +    map:
   +      0: FALSE
   +      1: TRUE
   

   No need for static_map; IIRC the config system will cast 0 and 1 to boolean, based on the config schema.

2. +++ b/core/modules/language/migration_templates/d7_language_types.yml
   @@ -0,0 +1,25 @@
   +  'negotioation/language_url/enabled/language_content':
   +    plugin: static_map
   +    source: language_content
   +    map:
   +      0: FALSE
   +      1: TRUE
   

   Ditto.

3. +++ b/core/modules/language/src/Tests/Migrate/d7/MigrateLanguageTypesTest.php
   @@ -0,0 +1,43 @@
   +/**
   +  * Tests migration of language variables into configuration.
   

   Nit: There's an extra space here, at the beginning of the line.

4. +++ b/core/modules/language/src/Tests/Migrate/d7/MigrateLanguageTypesTest.php
   @@ -0,0 +1,43 @@
   + */
   +
   +class MigrateLanguageTypesTest extends MigrateDrupal7TestBase {
   

   Nit: There's an extra line between the doc block and the class.

5. +++ b/core/modules/language/src/Tests/Migrate/d7/MigrateLanguageTypesTest.php
   @@ -0,0 +1,43 @@
   +  /**
   +   * Modules to enable.
   +   *
   +   * @var array
   +   */
   +  public static $modules = ['language'];
   

   Nit: Need a blank line between the class declaration and this doc comment.

6. +++ b/core/modules/language/src/Tests/Migrate/d7/MigrateLanguageTypesTest.php
   @@ -0,0 +1,43 @@
   +    $config = \Drupal::config('language.settings');
   

   Should be $this->config().

Thx phenaproxima.
1-6 fixed. No interdiff because of Variable.php.

Thank you, config schema checker :) Looks like "negotiation" is misspelled in the migration...

It's more than a typo, as this failing patch will show.

+++ b/core/modules/language/src/Tests/Migrate/d7/MigrateLanguageTypesTest.php
@@ -0,0 +1,42 @@
+    $this->assertIdentical(0, $config->get('negotiation.language_url.enabled.language-url'));
+    $this->assertIdentical(0, $config->get('negotiation.language_content.enabled.language-interface'));

It looks like these are not asserting the values that are in the dump files...?

Revisiting this and there is confusion on the source and the destination.

The IS identifies 3 variables to migrate:

1. 'language_interface' => 'configurable.language_interface'. But when I grep D7 for 'language_interface' no instances are returned.
2. 'language_content' => 'configurable.language_content'. But language.types.configurable contains only one item, language_interface.
3. 'language_url' => 'configurable.language_content'. Same problem as #2.

And here is the current test data for the D7 variable, language_types.
a:3:{s:8:"language";b:1;s:16:"language_content";b:0;s:12:"language_url";b:0;}

So, what are the configuration destinations for 'language', 'language_content' and 'language url'?

Turns out these variables do not exist in Drupal 7; instead, they refer to the complex language_negotiation_* variables, which are complicated enough to need their own migrations -- possibly a set of migrations. They pack a lot of data into a single variable, which is unpacked in the Drupal 8 language.types config object.

Turns out these variables do not exist in Drupal 7

Which variables are you referring to?

This definitely exists in D7:
language_types : a:3:{s:8:"language";b:1;s:16:"language_content";b:0;s:12:"language_url";b:0;}

Removed tag.

I think there's five D7 variables that we need here:

1. language_negotiation_language who lists negotiation methods for the interface (language_interface in D8)
2. language_negotiation_language_content who list negotiation methods for the content
3. language_negotiation_language_url who lists negotiation methods for the url
4. language_types who specifies which of the three language types above are configurable
5. locale_language_providers_weight_language who contains the weight of the negotiation methods

I've been working on the D6 version of this issue (#2225293: Migrate D6 and D7 language negotiation settings), so I might give it a try.

@maxocub: yeah that analysis seems fine, the D7 negotiation settings are similar but a bit more limited than Drupal 8, so the migration is more 1-1 with Drupal 8 than Drupal 6, where you need to generate a whole bunch of things for D8.

@Gábor Hojtsy: In D7, the negotiation methods for language_content and language_url are fixed and non-configurable. Could we just assume they still are and use default values in the migration (like we did with D6) or do we migrate them in case someone altered them of added new ones?

I ask because it's a bit complicated to migrate them since they don't have the same names (locale-url instead of language-url, language-default instead of language-selected, etc.) and are spread across a few variables. I I haven't found a way to use the existing plugins.

Also, if someone altered and/or added new language types, should we try to see if the variables "locale_language_providers_weight_$type" exist?

The file core/modules/migrate_drupal/src/Tests/Table/d7/Variable.php is removed at following commit. Reroll is not trivial.

commit 00d4deb86f8b56c3f8a2287e3f8cc17319d47cf6
Author: webchick <drupal@webchick.net>
Date:   Fri Oct 2 18:15:03 2015 -0700

    Issue #2568203 by phenaproxima, mikeryan: Remove migrate-db.sh in favor of core tools

I'm working on a reroll, and I'm starting from scratch.

1. Migrating contrib types, I don't think we can do in the core migration, so I would not try to do that.
2. language-default was not configurable in drupal 7, so we can assume that it is a fixed thing and migrate to language-selected with a setting tied to default language
3. language_content is definitely not fixed, if you had entity_translation installed in Drupal 7
4. Re language_url, I don't know if there is a contrib module that makes it configurable, I don't think not migrating it from the variables is easier since we need to migrate the two other types, and language_url can consist of settings for the same types, so can use the same migrations(?)

I think those were all the questions? Hopefuly helpful :)

Here's a new patch. I haven't included an interdiff since I started from scratch.

I haven't found a way to use existing process plugins.

I will wait for reviews before adding unit tests for the new plugins.

Assigning to myself for review.

This is looking pretty good, but the new process plugins need tests.

1. +++ b/core/modules/language/src/Plugin/migrate/process/LanguageNegotiation.php
   @@ -0,0 +1,88 @@
   +    $methods = array_keys($value[0]);
   +    $weights = $value[1];
   

   Can we check that $value is an array and throw a MigrateException if it isn't?

2. +++ b/core/modules/language/src/Plugin/migrate/process/LanguageNegotiation.php
   @@ -0,0 +1,88 @@
   +        $new_value = 'language-selected';
   +        break;
   

   For each of these cases, can we just return the value directly instead of setting $new_value and breaking? I think it would be a little cleaner that way.

3. +++ b/core/modules/language/src/Plugin/migrate/process/LanguageTypes.php
   @@ -0,0 +1,35 @@
   +    if (array_key_exists('language', $value)) {
   

   Before this, let's check that $value is an array and throw MigrateException if not.

4. +++ b/core/modules/language/src/Plugin/migrate/process/LanguageTypes.php
   @@ -0,0 +1,35 @@
   +    if (isset($this->configuration['filter_configurable']) && $this->configuration['filter_configurable']) {
   

   Should be if (!empty($this->configuration['filter_configurable'])).

Made the changes from #40 and added unit tests.

Reviewed for the migration mappings, it all looks good for what it needs to do. Also confirmed that the Drupal 6 part of this is covered in #2225293: Migrate D6 and D7 language negotiation settings and the missing elements for Drupal 7 language negotiation are there too, so these two issues are complimentary to solve all the core language negotiation setting migrations from both Drupal 6 and 7.

Committed and pushed 635e4ee to 8.3.x and 641e529 to 8.2.x. Thanks!

Committed to 8.2.x because migrate_drupal is still experimental.

Yay, thanks!