Problem/Motivation

Revisited this and discovered that the destination property content_translation_update_definitions is no longer used anywhere in core. I used grep -ir content_translation_update_definitions core and found it declared in 3 migration yml files but not anywhere else.

Further exploration shows that content_translation_update_definitions was added in #2225775: Migrate Drupal 6 core node translation to Drupal 8 and later removed in #2599228: Programmatically created translatable content type returns SQL error on content creation.

I don't think any of the three migrations were committed at the time that the property was removed. I suspect these migrations started as a copy paste of the migrations added in #2225775: Migrate Drupal 6 core node translation to Drupal 8 and the use of the property,or lack thereof, was never questioned or tested.


Original IS

d6_language_content_settings.yml, d7_language_content_settings.yml, and d7_node_translation.yml all have a property content_translation_update_definitions on the destination, which set to node. I'm not sure what the purpose of the setting is but it implies that it should be changed for other entities. It seems like for other entities one would copy the migration and change that destination property. Would it not be more flexible to have this as a property in the pipeline?

destination:
  content_translation_update_definitions:
    - node

And note that the source uses a constant value of node for the entity type. While that is a similar problem, it is a much more common pattern and the entity type property is in the pipeline and therefor easier to modify.

Proposed resolution

Remove usage of content_translation_update_definitions in destination plugins.

CommentFileSizeAuthor
#7 2930050-7.patch1.86 KBquietone
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quietone created an issue. See original summary.

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

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

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.

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.

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

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

quietone’s picture

Title: Move destination property content_translation_update_definitions to the pipeline » Remove destination property content_translation_update_definitions from migrations
Issue summary: View changes
quietone’s picture

Status: Active » Needs review
FileSize
1.86 KB

Should be not failures.

quietone’s picture

Issue summary: View changes
heddn’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +needs backport to 8.8.x

Based on the IS, we don't use these values any more. Let's get rid of them. Ideally should be backported to 8.8.

alison’s picture

Assigned: Unassigned » alison

I'll work on a backport -- just so I know, is there a fancy/automatic process for doing this, or is it just a straightforward/manual thing, "make the same changes against that other version of the code"?

alison’s picture

Assigned: alison » Unassigned

...actually, I think I'm misunderstanding something -- the patch in #7 applies fine on 8.8.x, so does that mean there's no need for backporting, it's just something the committer will need to do when they commit? (Or was it just that we needed someone to make sure the patch at #7 applies fine on 8.8.x? If so -- done.)

quietone’s picture

This doesn't need backporting, it is removing what is effectively dead code.

alexpott’s picture

Version: 8.9.x-dev » 8.8.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed and pushed 44416f1c03 to 9.0.x and 4669dbe1f8 to 8.9.x. Thanks!

Going to see about getting this into 8.8.x.

  • alexpott committed 44416f1 on 9.0.x
    Issue #2930050 by quietone: Remove destination property...

  • alexpott committed 4669dbe on 8.9.x
    Issue #2930050 by quietone: Remove destination property...
alexpott’s picture

Status: Patch (to be ported) » Fixed

Discussed with @catch and we agreed to backport.

  • alexpott committed 03f175b on 8.8.x
    Issue #2930050 by quietone: Remove destination property...
xjm’s picture

We don't have a process for using tags like "needs backport to 8.8.x"; it's just an extra committer +1 or sometimes a release management decision during the alpha. Just for future reference. :)

This does indeed appear to be dead code that can be removed non-disruptively so the backport makes sense.

Status: Fixed » Closed (fixed)

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