Problem/Motivation

Right now, block configuration translation migrations are defined in content_translation module. It seems to be obvious that they should be in config_translation.

Proposed resolution

Move d6_block_translation and d7_block_translation into config_translation module.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#12 3175729-12-9.1.5.patch1.92 KBWim Leers
#4 3187263-4.patch2.58 KBquietone
#4 interdiff-2-4.txt1.35 KBquietone
#2 core-block_translation_migration_is_config_migration-3187263-2.patch666 byteshuzooka
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

huzooka created an issue. See original summary.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
CreditAttribution: huzooka at Cheppers for Acquia commented
Assigned: huzooka » Unassigned
Status: Active » Needs review
FileSize
core-block_translation_migration_is_config_migration-3187263-2.patch666 bytes

Status: Needs review » Needs work

The last submitted patch, 2: core-block_translation_migration_is_config_migration-3187263-2.patch, failed testing. View results

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Needs work » Needs review
FileSize
interdiff-2-4.txt1.35 KB
3187263-4.patch2.58 KB

How did we all miss tha

Update the state files as needed and install the correct modules in the tests.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs review » Reviewed & tested by the community

How did we all miss tha

😄 Well there were lots of big patches in play!

Tests pass, and there's no BC impact because the migration definition IDs remain the same!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Needs review

Well there is a BC impact because something is moving modules. But in order to have a config translation you need the module so I think that is moot.

One question should core/modules/content_translation/migrations/state/content_translation.migrate_drupal.yml be updated?

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
CreditAttribution: huzooka at Cheppers for Acquia commented

@alexpott, No, it shouldn't.

We use the same data source for migrating into block configs and block content entities. In Drupal 6/7, these where the same thing.

d*_custom_block_translation migrations remains in content_translation.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs review » Reviewed & tested by the community

I believe #7 adequately answers #6 🤓

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Fixed

Committed 77480be and pushed to 9.2.x. Thanks!

Only committing this to 9.2.x because it is minor and moving something from one module to another.

  • alexpott committed 77480be on 9.2.x 
    Issue #3187263 by quietone, huzooka: Block configuration translation...

Status: Fixed » Closed (fixed)

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

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Related issues: +#3175729: Mark i18n migrations as finished
FileSize
3175729-12-9.1.5.patch1.92 KB 
+++ b/core/modules/config_translation/migrations/state/config_translation.migrate_drupal.yml
@@ -1,7 +1,9 @@
     i18nprofile: config_translation
+    i18nblocks: config_translation
   7:
+    i18n_block: config_translation
     i18n_variable: config_translation

These hunks already landed in 9.1.x in #3175729: Mark i18n migrations as finished.

Attached is a patch that applies cleanly to 9.1.5.