Problem/Motivation

If one migrates the Drupal 7 migrate source database fixture from core, the "Entity translation test" and "Test content type" nodes will have invalid content_translation_source property (a zero-length string '').

Proposed resolution

@tbd, see Remaining tasks below.

Remaining tasks

  1. Triage whether this is an issue of the database fixture or the source plugin (or something else?).
  2. Fix the issue.

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#2 core-nodecomplete_wrong_source_langcode-3187419-2.patch1.32 KBhuzooka

Comments

huzooka created an issue. See original summary.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Assigned: huzooka » Unassigned
Status: Active » Needs work
StatusFileSize
new core-nodecomplete_wrong_source_langcode-3187419-2.patch1.32 KB

A (temporary?) fix.

quietone’s picture

quietone commented

On a fresh D7 I enabled entity_translation and added some nodes. The source column is empty for nodes that have not yet been translated. It is also empty when a node is translated and the language of the revision is the language of the source. And that fits with my memory when I was working on the migration of entity translation revisions.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Status: Needs work » Needs review

@quietone yes, the source column is empty. But what kind of empty?

Based on the D7 migrate source DB fixture, the schema of that source column does not allows NULL values. But since etr_created won't be empty, the current NodeComplete::prepareRow() method sets an empty string '' as source_langode for those nodes default translation row which are translated with entity_translation . And content_translation_source cannot be an empty string. It can be NULL or any kind of string which corresponds to an available language code (ID) (e.g. 'en', 'es', 'custom_langode_id'). So for the row of node default translations, content_translation_sourceshould be NULL – because they represent the default translations source.

Setting to NR.

quietone’s picture

quietone commented

@huzooka, but the kind of empty doesn't matter in the source plugin.

Let me explain, and I should have noticed this earlier. The title states that the 'd7/NodeComplete source plugin adds invalid source "source_langcode" for "content_translation_source" destination property' and that is not true by definition. The job of the source plugin is to get the data from the source as is and not modified in any way. Yes, I know we have prepareRow() which allows some tweaking but we strive to minimize the changes to the source data itself in the source plugin. Changing/transforming the data to suit the destination is the job of the process pipeline.

The process for this property is simply, content_translation_source: source_langcode so the destination property does become an empty string. The empty string is an invalid language code and is not saved, the property on the entity is NULL. And we have proof of that because d7/MigrateNodeCompleteTest specifically tests that the content_translation_source value is NULL. That provides proof that the destination entity save is handling the empty string correctly.

Based on that this would be a won't fix.

I don't think it is necessary to add a comment to the migration yml to clarify this. However, documentation is usually better than none so leaving this at NR.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

And we have proof of that because d7/MigrateNodeCompleteTest specifically tests that the content_translation_source value is NULL.

Confirmed.

Based on this, marking as needing more info from @huzooka 🤓

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Assigned: Unassigned » huzooka
Status: Needs review » Postponed (maintainer needs more info)
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Thanks, I meant to do that 🙈

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Issue tags: +Bug Smash Initiative

Thanks for reporting this issue and creating a patch. We rely on issue reports like this one to resolve bugs and improve Drupal core.

As part of the Bug Smash Initiative, we are triaging issues that are marked "Postponed (maintainer needs more info)". This issue was marked "Postponed (maintainer needs more info)" more than a year ago, and there has been no activity here since that time.

Since we need more information (see #6 and before) to move forward with this issue, I am keeping the status at Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Status: Postponed (maintainer needs more info) » Closed (cannot reproduce)

Since it's been 7 months since the last request and we still need more information to move forward with this issue, I'm marking the issue "Closed (cannot reproduce)". If anyone can provide complete steps to reproduce the issue (starting from "Install Drupal core"), document those steps in the issue summary and set the issue status back to "Active" [or "Needs Work" if it has a patch, etc.].

Thanks!

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Status: Closed (cannot reproduce) » Closed (won't fix)

Actually, changing to "won't fix" per #5.