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\NodeorDrupal\node\Plugin\migrate\source\d7\Node - Check if the migrated row has a source property named
nidthat 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
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 2943575-5.patch | 949 bytes | maximpodorov |
| #2 | 2943575-node-migrate-subscriber-collide-with-other-migrations-2.patch | 1.01 KB | Mirroar |
Comments
Comment #2
Mirroar commentedI'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.
Comment #3
heddnWhy can't the source migration not include a config option of 'translations'?
Comment #5
maximpodorov commentedI 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).
Comment #6
heddnWhy 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.
Comment #13
quietone commentedThis 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.
Comment #19
quietone commentedThe Migrate Drupal Module and Migrate Drupal UI are deprecated and they are not in Drupal 12.0.0.
Issues for these modules should now be on the 11.x branch. And the changes are limited to critical and major bug fixes. Other changes are allowed at the discretion of the core Release Managers in consultation with the Migrate subsystem maintainers.