Problem/Motivation

The migration tests show that the wrong data is migrated on a D7 site.

In drupal7.php node 1 has the following in the "node" table:

->values(array(
  'nid' => '1',
  'vid' => '1',
  'type' => 'test_content_type',
  'language' => 'en',
  'title' => 'A Node',
  'uid' => '2',
  'status' => '1',
  'created' => '1421727515',
  'changed' => '1441032132',
  'comment' => '2',
  'promote' => '1',
  'sticky' => '0',
  'tnid' => '0',
  'translate' => '0',
))

The "node_revisions" record for the same node has the following:

->values(array(
  'nid' => '1',
  'vid' => '1',
  'uid' => '1',
  'title' => 'A Node',
  'log' => '',
  'timestamp' => '1441032132',
  'status' => '1',
  'comment' => '2',
  'promote' => '1',
  'sticky' => '0',
))

This data is consistent - the original creation was 1421727515, which corresponds to values in both "file_managed" and "taxonomy_index", and it was then updated at 1441032132.

However, after the migration runs the "created" and "changed" values are set to 1529615790, which come from "entity_translation" and "entity_translation_revision".

Why is the core migration not using the values taken from the "node" and "node_revision" tables?

Proposed resolution

Update the dates in the database, see #6.

Other options suggested:
Either:
1. Update the tests so that the "created" and "updated" values are consistent across all relevant tables.
2. Update the migration logic to use the "created" and "updated" values from the "node" and "node_revision" tables in preference to values obtainable elsewhere, e.g. in "entity_translation" and "entity_translation_revision".

Remaining tasks

Commit

User interface changes

n/a

API changes

n/a

Data model changes

n/a

Release notes snippet

TBD

CommentFileSizeAuthor
#6 3303925-6.patch2.97 KBquietone

Issue fork drupal-3303925

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:

About issue forks

Comments

DamienMcKenna created an issue. See original summary.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Issue summary: View changes
quietone’s picture

quietone commented
Issue tags: +migrate-d7-d8

Just updating tags,

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.

quietone’s picture

quietone commented
StatusFileSize
new 3303925-6.patch2.97 KB

If I understand this correctly I actually think this is an error in the database. The database fixture has not always been created via the UI and sometimes this are not quite right.

However, I think the logic is correct here. That node has been translated so getting the dates from the entity translation table seems to be correct. To verify this, I tested on Drupal 7 starting with the current test fixture. I then added a new node of the same content type (one that uses entity translation) and added a French translation. I then looked at the dates in the node/node_revision and entity/entity_revision tables. Sorry, I didn't output that data. I concluded that the values for created/changed in the entity/entity_revision tables should be the same as the English node in the node/node_revision tables.

I have made a patch that changes those dates and MigrateNodeCompleteTest.

quietone’s picture

quietone commented
Status: Active » Needs review
smustgrave’s picture

smustgrave commented

What's a good way to test this?

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

Then maybe we need test coverage for the data without translations? IMHO that's why the existing test coverage hasn't failed for the revision timestamp value.

quietone’s picture

quietone commented

I don't see how that will help. The incorrect data was in the entity_translation tables. And the node complete migration is designed to get everything. To run d7_node_complete without translations defeats its purpose.

@smustgrave, the only way I can think of to test this is to use a Drupal 7 site. Add entity_translation and i18n. Enable entity translation for a for a content type and another language. Then add a node of that content type and add a translation. Then examine the timestamps for the node in the node/node_revision and entity_translation/entity_translation_revision tables. You want to make sure that the pattern of changes matches what is in the patch and node tables for drupal7.php.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

English
Created - 1690313326
Changed - 1690313326

Spanish
Created - 1690313497
Changed - 1690313497

Ran the fixture DB dump to drupal7.php
Verified the nodes created and changed dates are matching.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: 3303925-6.patch, failed testing. View results

quietone’s picture

quietone commented
Status: Needs work » Needs review

There was an unrelated failure.

1) Drupal\Tests\user\Functional\UserCancelTest::testMassUserCancelByAdmin
Behat\Mink\Exception\ElementNotFoundException: Button with id|name|label|value "Apply to selected items" not found.

I retested and tests are passing, so restoring RTBC.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Restoring to RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: 3303925-6.patch, failed testing. View results

quietone’s picture

quietone commented

Unrelated fail, retesting.

1) Drupal\Tests\system\Functional\System\CronRunTest::testAutomatedCron
Failed asserting that 1690508392 is less than 1690508392.
quietone’s picture

quietone commented
Status: Needs work » Reviewed & tested by the community

Tests passing again, back to rtbc

quietone’s picture

quietone commented
Issue summary: View changes

I'm triaging RTBC issues.

I've read the issue summary, comments and patch. All I see is that the proposed resolution does not match what is implemented. I have updated that now.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: 3303925-6.patch, failed testing. View results

quietone’s picture

quietone commented
Status: Needs work » Needs review

Unrelated failure, retesting

1) Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testViewMode with data set "without alignment" (false)
Behat\Mink\Exception\ElementNotFoundException: Element matching css "article.media--view-mode-view-mode-1" not found.
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Does seem to be random

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: 3303925-6.patch, failed testing. View results

smustgrave’s picture

smustgrave commented
Status: Needs work » Reviewed & tested by the community

Seems to be unrelated.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
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 opened merge request !5339

smustgrave’s picture

smustgrave commented
Status: Needs work » Reviewed & tested by the community

Hiding files and converted #6 to an MR.

Please do not credit me if I did not other wise earn it as all I did was convert this.

Restoring status

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Reviewed & tested by the community » Needs work

I'm going to go back to the point that the problem is the "changed" value is not migrated correctly when translations are not used,

To run d7_node_complete without translations defeats its purpose.

I've used d7_node_complete as a base for several migrations, none of them had translations. When you run "migrate:import --configure-only" it uses d7_node_complete, even if the source site didn't use translations.

Putting this back to "needs work" as I strongly feel the actual problem is not being fixed.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

In practice I find that using the "timestamp" source property gives the correct value migrations, not the "changed" value as it results in the wrong value if the migration is updated (via Drush and migrate_tools).

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

What's weird is that sometimes using changed:timestamp results in incorrect values, but running the migration again corrects it. Go figure.

quietone’s picture

quietone commented
Status: Needs work » Postponed

The Migrate Drupal Module was approved for removal in #3371229: [Policy] Migrate Drupal and Migrate Drupal UI after Drupal 7 EOL.

This is Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.

The deprecation work is in #3522602: [meta] Tasks to remove Migrate Drupal module and the removal work in #3522602: [meta] Tasks to remove Migrate Drupal module.

Migrate Drupal will not be moved to a contributed project. It will be removed from core after the Drupal 12.x branch is open.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

quietone’s picture

quietone commented
Status: Postponed » Closed (won't fix)

The 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.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.