Rollback of configuration translations clears the map table but does not delete the translation. And the reported rollback count is incorrect.

v@dev2 {/opt/sites/d8} (tmp2)$ drush mi upgrade_system_site
Processed 1 item (1 created, 0 updated, 0 failed, 0 ignored) - done with 'upgrade_system_site'                                                                                                          [status]
v@dev2 {/opt/sites/d8} (tmp2)$ drush mi upgrade_d6_i18n_system_site
Processed 2 items (2 created, 0 updated, 0 failed, 0 ignored) - done with 'upgrade_d6_i18n_system_site'                                                                                                 [status]
v@dev2 {/opt/sites/d8} (tmp2)$ drush mr upgrade_d6_i18n_system_site
Rolled back 1 item - done with 'upgrade_d6_i18n_system_site'                                                                                                                                            [status]

First noticed problems with the rollback of translations in #2225717: Add config translation support to migrations and implement for Drupal 6 user profile fields, where the problem exists with fields.

CommentFileSizeAuthor
#8 interdiff.txt498 bytesquietone
#8 2825603-8.patch12.55 KBquietone
#8 2825603-8-fail.patch9.52 KBquietone
#4 interdiff.txt1.64 KBquietone
#4 2825603-4.patch12.56 KBquietone
#2 2825603-2.patch10.78 KBquietone
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quietone created an issue. See original summary.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Active » Needs review
FileSize
2825603-2.patch10.78 KB

In order to rollback the translations, the langcode needs to be in the ids. And to do that I took a page from the D6 node translation migration and added a 'translations' key to the destination and a method isTranslationDestination(). This changes the test for a translation from checking $row for a language key to checking that key. Are there or will there be problems with this new approach?

Status: Needs review » Needs work

The last submitted patch, 2: 2825603-2.patch, failed testing.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Needs work » Needs review
FileSize
2825603-4.patch12.56 KB
interdiff.txt1.64 KB

Forgot to update ConfigTest for the changes to Config.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan as a volunteer and at Virtuoso Performance commented
Assigned: Unassigned » mikeryan
mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan as a volunteer and at Virtuoso Performance commented 
+++ b/core/modules/migrate/src/Plugin/migrate/destination/Config.php
@@ -91,7 +94,12 @@ public function import(Row $row, array $old_destination_id_values = array()) {
+    $ids = [];
+    $ids[] = $this->config->getName();

Nit: we could save a *whole line* with $ids = [$this->config->getName()];

Apart from that, how about running a test-only patch through the testbot?

Thanks.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan as a volunteer and at Virtuoso Performance commented
Assigned: mikeryan » Unassigned
quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
FileSize
2825603-8-fail.patch9.52 KB
2825603-8.patch12.55 KB
interdiff.txt498 bytes

OK, one line saved and fail patch.

The last submitted patch, 8: 2825603-8-fail.patch, failed testing.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan as a volunteer and at Virtuoso Performance commented
Assigned: Unassigned » mikeryan
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Issue tags: +D8MI, +sprint, +i18n-migrate
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented

Looks good to me :) Since this is still assigned to Mike for a review from the migrate point of view, not marking RTBC.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan as a volunteer and at Virtuoso Performance commented
Status: Needs review » Reviewed & tested by the community

Ayup.

alexpott’s picture

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

Committed and pushed 7b76b16 to 8.3.x and 759bcab to 8.2.x. Thanks!

  • alexpott committed 7b76b16 on 8.3.x 
    Issue #2825603 by quietone, mikeryan: Fix rollback of configuration...

  • alexpott committed 759bcab on 8.2.x 
    Issue #2825603 by quietone, mikeryan: Fix rollback of configuration...
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Issue tags: -sprint

Yay, thanks!

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan as a volunteer and at Virtuoso Performance commented
Assigned: mikeryan » Unassigned

  • alexpott committed 7b76b16 on 8.4.x 
    Issue #2825603 by quietone, mikeryan: Fix rollback of configuration...

  • alexpott committed 7b76b16 on 8.4.x 
    Issue #2825603 by quietone, mikeryan: Fix rollback of configuration...

Status: Fixed » Closed (fixed)

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