Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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
Comment | File | Size | Author |
---|---|---|---|
#17 | interdiff.txt | 1.82 KB | benjy |
#17 | 2281553-17.patch | 10.16 KB | benjy |
#15 | interdiff.txt | 523 bytes | benjy |
#15 | 2281553-15.patch | 9.94 KB | benjy |
#14 | interdiff.txt | 4.78 KB | benjy |
Comments
Comment #1
Sutharsan CreditAttribution: Sutharsan commentedComment #2
Sutharsan CreditAttribution: Sutharsan commentedAlso found the timestamp to be missing. Updated issue summary.
Comment #4
Sutharsan CreditAttribution: Sutharsan commentedExtending 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.
Comment #6
Sutharsan CreditAttribution: Sutharsan commentedThis should fix the NodeTest.
Comment #7
Sutharsan CreditAttribution: Sutharsan commentedWhile working on this issue I had some observations which should probably be fixed in a different issue:
node_revision.revision_timestamp
andnode_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 D6node_revisions.timestamp
value.Comment #8
Sutharsan CreditAttribution: Sutharsan commentedI 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.
Comment #9
chx CreditAttribution: chx commentedthis can be written as
Check https://www.drupal.org/node/2129651 "Created by one plugin".
Comment #10
Sutharsan CreditAttribution: Sutharsan commented#9 comment processed. Could even remove the "
strict
".And got
revision_timestamp
andrevision_log
of the current revision working too. (2nd bullet in #7)Comment #11
chx CreditAttribution: chx commentedThis 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?
Comment #12
Sutharsan CreditAttribution: Sutharsan commentedI 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:
D6Manifest-User.yml
andD6Manifest-Node.yml
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.Comment #13
ultimikeBased on comments 11 and 12, changing this back to "Needs work".
-mike
Comment #14
benjy CreditAttribution: benjy commentedI'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.
Comment #15
benjy CreditAttribution: benjy commentedGit merged this automatically and we ended up with revision_timestamp twice since it's already now in core.
Comment #16
ultimikeStill getting comfortable reviewing other people's patches, but here's what I found...
Shouldn't the "'body/summary': teaser" mapping be in d6_node.yml as well?
Isn't this duplicating the 'timestamp' that appears 2 lines below?
Thanks,
-mike
Comment #17
benjy CreditAttribution: benjy commentedThanks for the review.
Comment #18
ultimikeRock and roll - looks good to go!
Thanks,
-mike
Comment #19
webchickCommitted and pushed to 8.x. Thanks!