Problem/Motivation

The Drupal 7 synchronized fields need a migration. This should be same, or very similar to the Drupal 6 version. See #2754493: D6 synchronized field settings aren't migrated properly

Proposed resolution

Turn "synchronized fields" into untranslated fields in the D8 content translation settings.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quietone created an issue. See original summary.

quietone’s picture

Can't add a synchronized field to the test fixture. When I add the field and click save the results is "An illegal choice has been detected.". It has something to do with the inability to set the 'default parent item' in the menu settings of the content type. This happens for all content types. I've asked on slack about this and so far no response.

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

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.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

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

Issue summary: View changes
Status: Postponed » Active

No longer postponed

quietone’s picture

Status: Active » Needs review
FileSize
11.88 KB

And finally a patch.

Status: Needs review » Needs work

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

quietone’s picture

Status: Needs work » Needs review
FileSize
4.95 KB
7.6 KB

Ignore previous patch. This is the right one.

Gábor Hojtsy’s picture

The test looks good. Are we sure the syncronized fields get the value from the right place? (In practice default translation in my understanding.) That is not really covered here.

quietone’s picture

Status: Needs review » Needs work

Setting to NW for the question in #10

quietone’s picture

Status: Needs work » Needs review
FileSize
486 bytes
7.85 KB

Are we sure the syncronized fields get the value from the right place?

Yes, the value is only stored in the field table. Nothing is stored in locales_target or the i18n_string table. I checked by bringing up a d7 site with this fixture (from this patch), changed the field value for the english node and checked the tables. Repeated the same for the icelandic translation, with the same result. Finally, dumped the fixture and grepped for the field value. As I recall, this is the same behaviour I found with d6 synchronized fields.

This patch just enables i18n_sync in the source.

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Ok looking closer I can see the test actually proves this as well :) Yay! Looks all good to me.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: 3031727-11.patch, failed testing. View results

quietone’s picture

Status: Needs work » Needs review
FileSize
9 KB

Needed a reroll plus fix the Upgrade7Test. No interdiff because of the reroll. The fix was to add an entry in core/modules/field/migrations/state/field.migrate_drupal.yml for i18n_sync module.

Status: Needs review » Needs work

The last submitted patch, 14: 3031727-14.patch, failed testing. View results

quietone’s picture

Status: Needs work » Needs review
FileSize
606 bytes
9.78 KB

Fixing the test.

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

I like issues with a limited scope and straightforward implementation. Thanks for the reroll! :)

quietone’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.32 KB
9.77 KB

Needed a reroll

Gábor Hojtsy’s picture

Status: Needs review » Needs work

What is this new empty string meant to help with? It definitely does not align with coding standards and looks like a mistake.

   'cardinality' => '1',
   'translatable' => '0',
-  'deleted' => '0',
+  'deleted' => '0',''
 ))
quietone’s picture

Status: Needs work » Needs review
FileSize
8.67 KB
1.39 KB

Good eye. Don't know how that crept in but the trailing empty string is removed.

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Looks good again, thanks for the reroll.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed b2e2c03 and pushed to 8.8.x. Thanks!

The code look correct and has test coverage. Nice work.

  • alexpott committed b2e2c03 on 8.8.x
    Issue #3031727 by quietone, Gábor Hojtsy: Migrate D7 synchronized fields
    

Status: Fixed » Closed (fixed)

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