Module settings translation migrations should depend on the default settings migration

This patch looks great — but indeed is blocked on that other patch getting rerolled, and then changing

+    - user_settings

to

+    - d7_user_settings

Rerolled #2 on top of #3187320-9: Migrate Drupal 7 user settings.

Queued tests.

/me nudges @huzooka to go back to being on vacation 🤓

Can you confirm there are no other migrations that need to be modified?

I think for completeness I'll add the d6 versions as well, sometime this week I hope.

Actually, I'll not do the d6 ones. I'd rather stay as a reviewer on this.

Setting NW to update the failing tests to execute the dependent migrations.

Added a patch and interdiff file.

@Shashwat Purav, thanks.

To fix the failing tests, the dependent migration must be explicitly run in the Kernel test. For example for d6\MigrateSystemMaintenanceTranslationTest the 'system_maintenance' migration needs to run first. So, add this
$this->executeMigration('system_maintenance');
just before
$this->executeMigration('d6_system_maintenance_translation');

And similar for the other failing tests.

Thanks.

Added a patch and interdiff file for the changes suggested in #13

Added a patch and interdiff file for the changes suggested in #13

+++ b/core/modules/config_translation/migrations/d6_system_site_translation.yml
@@ -40,3 +40,6 @@ destination:
+migration_dependencies:
...
+    - system_site

+++ b/core/modules/config_translation/tests/src/Kernel/Migrate/d6/MigrateSystemSiteTranslationTest.php
@@ -21,6 +21,7 @@ class MigrateSystemSiteTranslationTest extends MigrateDrupal6TestBase {
+    $this->executeMigration('system_maintenance');

Not quite, the 'd6_system_site_translation' migration is not dependent on the 'system_maintenance' migration. To find what it is dependent on, look at the changes made to d6_system_site_translation.yml in this patch. That is,

+migration_dependencies:
		+  required:
		+    - system_site

Which means the dependency is the 'system_site' migration. For this test, change 'system_maintenance' to system_site and it should pass. You can test that locally and not rely on the test bot and waiting an hour for the report. Running PHPUnit tests. The same process can be used to find the dependency to add for the other tests.

Cheers

Great. tests are passing!

One more thing I only just noticed.

+++ b/core/modules/config_translation/tests/src/Kernel/Migrate/d6/MigrateUserConfigsTranslationTest.php
@@ -27,6 +27,7 @@ protected function setUp(): void {
+    $this->executeMigrations(['d6_user_mail', 'd6_user_settings']);
     $this->executeMigrations(['d6_user_mail_translation', 'd6_user_settings_translation']);

+++ b/core/modules/config_translation/tests/src/Kernel/Migrate/d7/MigrateUserConfigsTranslationTest.php
@@ -33,6 +33,7 @@ protected function setUp(): void {
+    $this->executeMigrations(['d7_user_mail', 'd7_user_settings']);

Since these two tests are already using executeMigrations then it will be easier to read if we add the two new migrations to the array. the array will need to be reformatted.

Added a patch and interdiff file for the changes suggested in #23

I think we should array elements into their own line in core/modules/config_translation/tests/src/Kernel/Migrate/d6/MigrateUserConfigsTranslationTest.php

+++ b/core/modules/config_translation/tests/src/Kernel/Migrate/d6/MigrateUserConfigsTranslationTest.php
@@ -27,7 +27,7 @@ protected function setUp(): void {
+    $this->executeMigrations(['d6_user_mail', 'd6_user_settings', 'd6_user_mail_translation', 'd6_user_settings_translation']);

The formatting of the array on this line should match what was done for d7/MigrateUserConfigsTranslationTest.php. That will keep the line under 80 char and make it easier to read.

Made the suggested changes from #26. Kindly Review

@abhisekmazumdar, thanks.

Just two changes left to do.

1. +++ b/core/modules/config_translation/tests/src/Kernel/Migrate/d6/MigrateUserConfigsTranslationTest.php
   @@ -25,9 +25,17 @@ class MigrateUserConfigsTranslationTest extends MigrateDrupal6TestBase {
   -    $this->installSchema('locale',
   -      ['locales_source', 'locales_target', 'locales_location']);
   -    $this->executeMigrations(['d6_user_mail_translation', 'd6_user_settings_translation']);
   +    $this->installSchema('locale', [
   +      'locales_source',
   +      'locales_target',
   +      'locales_location'
   +    ]);
   

   This change is out of scope, it needs to be reverted.

2. +++ b/core/modules/config_translation/tests/src/Kernel/Migrate/d6/MigrateUserConfigsTranslationTest.php
   @@ -25,9 +25,17 @@ class MigrateUserConfigsTranslationTest extends MigrateDrupal6TestBase {
   +      'd6_user_settings_translation'
   

   Missing trailing comma. 'd6_user_settings_translation',

Thanks, @quietone for the suggestion.

Kindly ignore the patch on #29

@abhisekmazumdar, thank you.

I reviewed the patch and it resolves the problem in the IS. If anyone wonders, there is no need to convert the use of 'executeMigration' to 'executeMigrations' because executeMigrations is only needed for migrations that have a deriver. These migrations do not.

Committed and pushed 5425271050 to 9.2.x and b2ff38ca14 to 9.1.x. Thanks!