Problem/Motivation

In #2850085: Redirects for translation set migration path in Drupal 6 and 7, a mechanism was implemented to improve the upgrade path from Drupal 6/7 to Drupal 8. Basically, when doing a node migration of translated nodes, it stores a mapping of old node IDs to the new, translated node ids in order to provide automatic redirects. That way, internal node urls to translations can still work, even if no node exists that has the same ID as the old translation node.

In order to detect when a migration for translated nodes is running, NodeTranslationMigrateSubscriber checks if the current migration has a destination of entity:node, and if the source has a configuration key named translations. This check is too loose, in my opinion.

For example, I was writing a migration from Wordpress to Drupal and added a key named translations to my source configuration in order to be consistent with how core handles it. Suddenly I experienced errors when running this migration, caused by trying to write to a KeyValueStore without a valid key (It uses the source propery nid as key, which my wordpress migration doesn't provide, of course).

In order to prevent this, we need to better detect whether the current migrations is actually a Drupal-to-Drupal node migration, not just any migration into nodes. I see a few options, but I'm not quite sure which would be best:

  • Check the migration's migration tags - these contain "Drupal 6" or "Drupal 7" for core migrations.
  • Check if the source's Class is an instance of Drupal\node\Plugin\migrate\source\d6\Node or Drupal\node\Plugin\migrate\source\d7\Node
  • Check if the migrated row has a source property named nid that is numeric.

Steps to reproduce

TBA

Proposed resolution

Check if the nid is on the row - it will only be there for Drupal -> Drupal migrations.

Remaining tasks

Write a test
Add documentation for #2916356: Add a documentation page for upgrading multilingual sites
Review
Commit

User interface changes

N/A

API changes

N/A

Data model changes

N/A

Release notes snippet

N/A

CommentFileSizeAuthor
#5 2943575-5.patch949 bytesmaximpodorov
#2 2943575-node-migrate-subscriber-collide-with-other-migrations-2.patch1.01 KBMirroar
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mirroar created an issue. See original summary.

Mirroar’s picture

Mirroar CreditAttribution: Mirroar at werk21 commented
Status: Active » Needs review
Issue tags: +Needs tests
FileSize
2943575-node-migrate-subscriber-collide-with-other-migrations-2.patch1.01 KB

I've created a small patch that checks the migration tags to work around this problem. I feel the check for the migration's source class is probably the most stable verstion to fix this, though.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented
Status: Needs review » Needs work

Why can't the source migration not include a config option of 'translations'?

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.

maximpodorov’s picture

maximpodorov CreditAttribution: maximpodorov commented
Status: Needs work » Needs review
FileSize
2943575-5.patch949 bytes

I think at least nid emptiness must be checked, or redirects to /node/null can be created (if the row migration is failed to determine to destination nid).

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented
Status: Needs review » Needs work

Why not just NOT add 'translations' to your source migration? Hmm... it seems though, this is a concern for more than one person. In which case let's do what we have in #5, but let's check the source and destination value existence across the board.

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.

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

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

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

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

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.

quietone credited monymirza.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Issue summary: View changes
Issue tags: +Bug Smash Initiative
Parent issue: #2850085: Redirects for translation set migration path in Drupal 6 and 7 »

This issue was the random daily bug to triage for the Bug Smash Initiative on 2021-10-28.

Closed #3220279: Notice: Undefined index: nid in NodeTranslationMigrateSubscriber->onPostRowSave as s duplicate, adding credit.

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.

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.

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

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.