Problem/Motivation

Node revision migration from D6 to D8 does not migrate: revision log messages, timestamp, summary, revision uid and revision language.

Proposed resolution

  • Extend migration to include the missing items.
  • Extend the tests to cover the missing items.

Remaining tasks

  • DONE Migration of: log, timestamp, summary, uid
  • DONE Migration of revision language
  • DONE Extend node migration test coverage

User interface changes

n/a

API changes

n/a

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Sutharsan’s picture

Assigned: Unassigned » Sutharsan
Sutharsan’s picture

Assigned: Sutharsan » Unassigned
Issue summary: View changes
Status: Active » Needs review
FileSize
2.74 KB

Also found the timestamp to be missing. Updated issue summary.

Status: Needs review » Needs work

The last submitted patch, 2: drupal-migrate-node-revision-log-2281553-1.patch, failed testing.

Sutharsan’s picture

Title: D6->D8 Node Revision Log Messages not being migrated. » D6->D8 Node Revision migration is incomplete
Issue summary: View changes
Status: Needs work » Needs review
FileSize
4.05 KB
3.96 KB

Extending the scope once more. summary, uid and language are also missing in node revision migrations.

This patch adds the teaser and uid to the migration and adds test coverage for revision titel, uid and summary.

Have not been able to migrate the revision language yet. Both the node and the node revision have a 'langcode' field. I have not been able to figure out how to get the language from source into the revision destination.

Status: Needs review » Needs work

The last submitted patch, 4: drupal-migrate-node-revision-log-2281553-4.patch, failed testing.

Sutharsan’s picture

Status: Needs work » Needs review
FileSize
5.63 KB
1.72 KB

This should fix the NodeTest.

Sutharsan’s picture

While working on this issue I had some observations which should probably be fixed in a different issue:

  • Node language in Drupal 6 is by default an empty string. This is currently migrated as an empty string in D8. Several IMP issues target at D6->D8 language migration.
  • node_revision.revision_timestamp and node_revision.revision_log of the current revision has the wrong value. 'revision_log' is always empty (NULL) 'revision_timestamp' is (probably) the creation time of the D8 node, but not the D6 node_revisions.timestamp value.
Sutharsan’s picture

I discussed the node revision language issue with @GaborHojtsy in IRC. The empty language sting in D6 was replaced by "und" in D7. It only occurs when Locale module is disable. When enabled, the language string will get the value of the appropriate node (revision) language. The empty language string in D6 should be migrated to "und" in D8. Any other D6 language string value should be migrated as-is to D8.

The attached patch includes changes for the default language migration.

chx’s picture

Status: Needs review » Needs work
+  langcode:
+    -
+      plugin: get
+      source: language
+    -
+      plugin: default_value
+      default_value: "und"
+      strict: false

this can be written as

+  langcode:
+    plugin: default_value
+    source: language
+    default_value: "und"
+    strict: false

Check https://www.drupal.org/node/2129651 "Created by one plugin".

Sutharsan’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
8.93 KB
5.04 KB

#9 comment processed. Could even remove the "strict".
And got revision_timestamp and revision_log of the current revision working too. (2nd bullet in #7)

chx’s picture

This is coming along very nicely, thanks!

Testing for an empty string in the log, however, is not a great thing to test for -- it is by default empty, after all. Could we put something in the dump and the assert?

Sutharsan’s picture

Testing for an empty string in the log, however, is not a great thing to test for -- it is by default empty, after all

I agree. I came to do this while trying to fix a problem in the real world. But the problem does not occur during the test, and (apparently) I did not come back to it after making the test work. The problem is that the revision log message of the current revision (the last revision that was created) is copied to all revisions of that node that did not have a log message in the source.
To reproduce the problem:

  • In a fresh D6 site, create a node with title and body content.
  • Edit the node, create a revision and enter a log message, save the node.
  • Migrate the D6 site to D8 using D6Manifest-User.yml and D6Manifest-Node.yml
  • Check the D8 node revision page at node/1/revisions and see that the first revision of the node now has the same log message as the last revision.

The problem may be related to the fact that in Migrate an entity revision is created based on the current entity. But I have debugged as far as EntityRevision::save(). In there, the log content in $entity->revision_log->value is correct.

ultimike’s picture

Status: Needs review » Needs work

Based on comments 11 and 12, changing this back to "Needs work".

-mike

benjy’s picture

Status: Needs work » Needs review
FileSize
9.81 KB
4.78 KB

I've re-rolled this patch, removed some duplicate timestamp data in the unit test which has since been added by another patch and then updated the dump/assertion as suggested in #11 plus a few test clean-ups.

benjy’s picture

FileSize
9.94 KB
523 bytes

Git merged this automatically and we ended up with revision_timestamp twice since it's already now in core.

ultimike’s picture

Status: Needs review » Needs work

Still getting comfortable reviewing other people's patches, but here's what I found...

  1. +++ b/core/modules/migrate_drupal/config/install/migrate.migration.d6_node_revision.yml
    @@ -21,6 +24,10 @@ process:
    +  'body/summary': teaser
    

    Shouldn't the "'body/summary': teaser" mapping be in d6_node.yml as well?

  2. +++ b/core/modules/migrate_drupal/src/Plugin/migrate/source/d6/Node.php
    @@ -50,6 +50,8 @@ public function query() {
    +        'timestamp',
    

    Isn't this duplicating the 'timestamp' that appears 2 lines below?

Thanks,
-mike

benjy’s picture

Status: Needs work » Needs review
FileSize
10.16 KB
1.82 KB

Thanks for the review.

ultimike’s picture

Status: Needs review » Reviewed & tested by the community

Rock and roll - looks good to go!

Thanks,
-mike

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

  • webchick committed dfecbdd on 8.0.x
    Issue #2281553 by benjy, Sutharsan, ultimate: D6->D8 Node Revision...

Status: Fixed » Closed (fixed)

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