Migrate system site i18n variables to config:

  • site_name
  • site_mail
  • site_slogan
  • site_frontpage
  • site_403
  • site_404
  • drupal_weight_select_max
  • admin_compact_mode
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quietone created an issue. See original summary.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

quietone’s picture

Here is a migration and test outline. This needs the source plugin in #2970848: i18n Variable to config: site offline message [d7] .

TODO:
Add data to fixture and update the test.

quietone’s picture

Issue summary: View changes
Status: Postponed » Needs work
quietone’s picture

Status: Needs work » Needs review
FileSize
5.12 KB

Here is the system site multilingual translation migration. No interdiff since this is the first patch with source data and worth running with the testbot.

Status: Needs review » Needs work

The last submitted patch, 5: 2970847-5.patch, failed testing. View results

quietone’s picture

Status: Needs work » Needs review
FileSize
5.02 KB
573 bytes

Accidentally changed an existing entry in the source fixture.

phenaproxima’s picture

Status: Needs review » Needs work

This is as beautiful and simple as it gets. Two nits, then we're ready here. Please kick it directly to RTBC once both are fixed.

  1. +++ b/core/modules/config_translation/tests/src/Kernel/Migrate/d7/MigrateSystemSiteTranslationTest.php
    @@ -0,0 +1,51 @@
    +  public static $modules = [
    +    'language',
    +    'config_translation',
    +    // Required for translation migrations.
    +    'migrate_drupal_multilingual',
    +  ];
    

    This needs @inheritdoc.

  2. +++ b/core/modules/config_translation/tests/src/Kernel/Migrate/d7/MigrateSystemSiteTranslationTest.php
    @@ -0,0 +1,51 @@
    +    $this->assertSame(NULL, $config_translation->get('admin_compact_mode'));
    

    Should be assertNull().

quietone’s picture

Status: Needs work » Needs review
FileSize
5.05 KB
1.06 KB

All fixed!

quietone’s picture

Status: Needs review » Reviewed & tested by the community

Fixes made, no coding standard errors and tests are passing so setting to RTBC as suggested by phenaproxima in #8.

The last submitted patch, 3: 2970847-3.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • Gábor Hojtsy committed 09021c5 on 8.6.x
    Issue #2970847 by quietone, phenaproxima: i18n Variable to config:...

  • Gábor Hojtsy committed 7e120d9 on 8.7.x
    Issue #2970847 by quietone, phenaproxima: i18n Variable to config:...
Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Looks great, thanks!

Status: Fixed » Closed (fixed)

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

Wim Leers’s picture

This contains significant bugs that pretend more things can be migrated/translated than D8|9 actually allows.

See #3187418-4: System site translation shouldn't migrate properties which are not translatable.