Problem/Motivation

The taxonomy translation migrations do not depend on the migrations that set the language content settings for taxonomy.

This was discovered while testing #3015369: Fix MigrateTestBase::executeMigrations() to execute migrations in dependency order.

Steps to reproduce

Proposed resolution

Modify d7_language_content_taxonomy_vocabulary_settings to run after d7_entity_translation_settings and skip the row if the language settings were created by d7_entity_translation_settings

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3371869

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

quietone created an issue. See original summary.

quietone’s picture

Status: Active » Needs review
StatusFileSize
new1.41 KB
new2.4 KB

The first step was to add a skip_on_empty if the content language settings existed.

The last submitted patch, 2: 3371869-2-fail.patch, failed testing. View results

quietone’s picture

StatusFileSize
new2.67 KB
new2.79 KB

So, that proves that the new pipeline works.

Then add an optional dependency on d7_entity_translation_settings d7_language_content_taxonomy_vocabulary_settings.yml so that it runs first. And then add a dependency on d7_language_content_taxonomy_vocabulary_settings to the taxonomy term translation migrations.

The changes to drupalci and the test are not needed, that leaves a patch with changes to 4 migrations.

Edit: s/to a 4/to 4/

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Think the fail patch does a good job showing the problem.

So the change looks good to me.

quietone’s picture

I triaged this RTBC issue sometime in the last weeks but didn't leave a comment.

I didn't find anything that needed to be done.

Leaving at RTBC

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 4: 3371869-4.patch, failed testing. View results

smustgrave’s picture

Status: Needs work » Reviewed & tested by the community

Rerunning tests but believe to be unrelated.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 4: 3371869-4.patch, failed testing. View results

smustgrave’s picture

Status: Needs work » Reviewed & tested by the community

Rerunning believe to be unrelated

needs-review-queue-bot’s picture

Status: Reviewed & tested by the community » Needs work

The Needs Review Queue Bot tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

smustgrave’s picture

Status: Needs work » Reviewed & tested by the community

Think this one was simple enough and clear which patch to use. Going to remark it.

quietone’s picture

Status: Reviewed & tested by the community » Needs review

The patch in #4 was failing and I tracked it to #3015369: Fix MigrateTestBase::executeMigrations() to execute migrations in dependency order. That issue ensures that the migrations will be ordered by dependency before execution. This resulted in the following error.

1) Drupal\Tests\node\Kernel\Migrate\d7\MigrateNodeTest::testNode
'language_content_settings' entity with ID 'node.et' already exists. (/var/www/html/core/lib/Drupal/Core/Entity/EntityStorageBase.php:519)
Failed asserting that two strings are equal.

This is happening because 'language_content_settings' was being run last in that group. It needs to be before any translation migration and other 'language_content_settings' ones. The change was to add a dependency to d7_language_content_taxonomy_vocabulary_settings.yml so that it runs after d7_language_content_settings, if it exists.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Random nightwatch failure

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x and cherry-picked to 10.2.x, thanks!

  • catch committed e671b542 on 10.2.x
    Issue #3371869 by quietone: Fix dependencies of taxonomy term...

  • catch committed 04900c3c on 11.x
    Issue #3371869 by quietone: Fix dependencies of taxonomy term...

Status: Fixed » Closed (fixed)

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