Problem/Motivation

Same problem as in #2313241: d6_taxonomy_term migrate fails when the parent term has not been already migrated.

The bug was reintroduced in #2348875: Improving our dump files.

git show ec9a3ec27da1c9f2cb6a4568e8fae5f4b1b54c32 -- core/modules/migrate_drupal/src/Tests/d6/MigrateTaxonomyTermTest.php shows the bad change.

Proposed resolution

Add explicit test coverage for stubbed taxonomy terms.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#12 2590215-12.patch5.03 KBwebflo
#10 2590215-10.interdiff.txt1.98 KBwebflo
#10 2590215-10.patch4.84 KBwebflo
#6 2590215-6.patch2.86 KBwebflo
#4 2590215-4-test-only.patch1.92 KBwebflo
#4 2590215-4.patch2.92 KBwebflo
#3 2590215-3.patch1.04 KBwebflo
#2 2590215-2.patch778 byteswebflo
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

webflo created an issue. See original summary.

webflo’s picture

webflo CreditAttribution: webflo at UEBERBIT GmbH commented
Status: Active » Needs review
FileSize
2590215-2.patch778 bytes
webflo’s picture

webflo CreditAttribution: webflo at UEBERBIT GmbH commented
FileSize
2590215-3.patch1.04 KB
webflo’s picture

webflo CreditAttribution: webflo at UEBERBIT GmbH commented
FileSize
2590215-4.patch2.92 KB
2590215-4-test-only.patch1.92 KB

The test in #3 didn't fail because migrate catches all exceptions in destination and process plugins.

Status: Needs review » Needs work

The last submitted patch, 4: 2590215-4-test-only.patch, failed testing.

webflo’s picture

webflo CreditAttribution: webflo at UEBERBIT GmbH commented
Status: Needs work » Needs review
FileSize
2590215-6.patch2.86 KB 
+++ b/core/modules/taxonomy/src/Plugin/migrate/destination/EntityTaxonomyTerm.php
@@ -20,11 +20,15 @@ class EntityTaxonomyTerm extends EntityContentBase {
+    $row->setDestinationProperty('default_langcode', TRUE);

The default_langcode is not required.

webflo’s picture

webflo CreditAttribution: webflo at UEBERBIT GmbH commented
Title: Term migration is broken » d6_taxonomy_term migrate fails when the parent term has not been already migrated
Issue summary: View changes
webflo’s picture

webflo CreditAttribution: webflo at UEBERBIT GmbH commented
Title: d6_taxonomy_term migrate fails when the parent term has not been already migrated » Regression: d6_taxonomy_term migrate fails when the parent term has not been already migrated
Issue summary: View changes
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Needs review » Needs work
Issue tags: +Needs tests

This looks good to me but I'd like to see a dedicated test of the EntityTaxonomyTerm destination. It could be a kernel test that passes a stub row to import() and ensure that it's created as expected, without exceptions.

webflo’s picture

webflo CreditAttribution: webflo at UEBERBIT GmbH commented
Status: Needs work » Needs review
FileSize
2590215-10.patch4.84 KB
2590215-10.interdiff.txt1.98 KB
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Needs review » Needs work
Issue tags: -Needs tests 
  1. +++ b/core/modules/taxonomy/tests/src/Kernel/Plugin/migrate/destination/EntityTaxonomyTermTest.php
@@ -0,0 +1,63 @@
+<?php
+
+namespace Drupal\Tests\taxonomy\Kernel\Plugin\migrate\destination;

    Missing @file comment. 

  2. +++ b/core/modules/taxonomy/tests/src/Kernel/Plugin/migrate/destination/EntityTaxonomyTermTest.php
@@ -0,0 +1,63 @@
+    $migration = Migration::create($config);
+    $destination = $migration->getDestinationPlugin();

    Nit: This can be a single line. Migration::create($config)->getDestinationPlugin() 

  3. +++ b/core/modules/taxonomy/tests/src/Kernel/Plugin/migrate/destination/EntityTaxonomyTermTest.php
@@ -0,0 +1,63 @@
+    // Stub without name.

    Can this be a little more verbose? 

  4. +++ b/core/modules/taxonomy/tests/src/Kernel/Plugin/migrate/destination/EntityTaxonomyTermTest.php
@@ -0,0 +1,63 @@
+    /**
+     * @var TermInterface $term
+     */

    Nit: Should be /** @var ... */, all on one line. And TermInterface should be the fully-qualified name of the interface. 

  5. +++ b/core/modules/taxonomy/tests/src/Kernel/Plugin/migrate/destination/EntityTaxonomyTermTest.php
@@ -0,0 +1,63 @@
+    $this->assertEquals('Stub name for source tid:1', $term->getName());
+    $this->assertEquals(0, $term->getWeight());

    assertSame() is preferred over assertEquals(). 

  6. +++ b/core/modules/taxonomy/tests/src/Kernel/Plugin/migrate/destination/EntityTaxonomyTermTest.php
@@ -0,0 +1,63 @@
+    // Stub with name.

    As with before, can this be better explained?

webflo’s picture

webflo CreditAttribution: webflo at UEBERBIT GmbH commented
Status: Needs work » Needs review
FileSize
2590215-12.patch5.03 KB
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Needs review » Reviewed & tested by the community

Prime.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Acquia commented
Status: Reviewed & tested by the community » Postponed

Hold on now - #2590993: Create stub entities with proper default values supercedes this...

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Murphysboro, IL, USA
CreditAttribution: mikeryan at Acquia commented
Status: Postponed » Closed (duplicate)

Fixed by #2590993: Create stub entities with proper default values.