Problem/Motivation

We need an upgrade path for the Drupal 7 Comment module.

Remaining Tasks

Write migrations (with tests) to handle the following:

  • Variables related to comments (e.g., comment_anonymous_page and its ilk; see comment_uninstall() for more) need to be moved into configuration as needed. It looks like D8 Comment maintains no configuration, so this is not needed.
  • Comment entities need to be migrated.
  • Comment types need to be migrated, with relevant configuration.
CommentFileSizeAuthor
#14 interdiff-2532534-11-14.txt302 bytesphenaproxima
#14 2532534-14.patch36.91 KBphenaproxima
#11 interdiff-2532534-10-11.txt416 bytesphenaproxima
#11 2532534-11.patch36.48 KBphenaproxima
#10 2532534-10.patch36.45 KBphenaproxima
#9 2532534-9.patch32.83 KBphenaproxima
#7 2532534-7.patch11.01 KBphenaproxima
#1 migration_files_for-2532534-1.patch5.86 KBmiguelc303
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

miguelc303’s picture

miguelc303 CreditAttribution: miguelc303 at Anexus commented
Related issues: +#2532496: Migration Files for Drupal 7 Comment Type, +#2532526: Migration Files for Drupal 7 Comment Entity Display, +#2532524: Migration Files for Drupal 7 Comment Entity Form Display, +#2384567: Migration Files for Drupal 7 Text Formats & Filters, +#2414651: Migration Files for Drupal 7 Users, +#2423103: Migration Files for Drupal 7 Content
FileSize
migration_files_for-2532534-1.patch5.86 KB

I did a patch to enable end user to migrate comments from Drupal 7 to Drupal 8

Status: Needs review » Needs work

The last submitted patch, 1: migration_files_for-2532534-1.patch, failed testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented
Priority: Normal » Major
Issue summary: View changes
Issue tags: +Migrate critical

Merging #2500465: Upgrade path for Comment 7.x into this one. Since this is one of the "big 4" that people won't consider migration complete without, bumping to a Migrate critical.

quietone’s picture

quietone CreditAttribution: quietone commented
Parent issue: » #2456259: [META] Drupal 7 to Drupal 8 Migration path

Add parent task

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Needs work » Postponed

Comment migration depends on nodes, so this is blocked by #2423103: Migration Files for Drupal 7 Content.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Issue summary: View changes

Fixing the IS.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
FileSize
2532534-7.patch11.01 KB

Re-rolled and merged all related/child issues into the patch.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Postponed » Needs work

Node migrations are in, so this be unblocked.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Needs work » Needs review
FileSize
2532534-9.patch32.83 KB

I done made a patch with tests!

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
FileSize
2532534-10.patch36.45 KB

Added a test of the CommentType source plugin.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
FileSize
2532534-11.patch36.48 KB
interdiff-2532534-10-11.txt416 bytes

Sigh...reverted a couple of non-crucial changes to the D6 comment migrations.

The last submitted patch, 9: 2532534-9.patch, failed testing.

The last submitted patch, 10: 2532534-10.patch, failed testing.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
FileSize
2532534-14.patch36.91 KB
interdiff-2532534-11-14.txt302 bytes

Fixed missing dependencies in d7_field_instance which cause exceptions during real-world migrations.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented
Issue tags: -Migrate critical +Migrate D7 critical

Just some house-keeping, ignore me.

quietone’s picture

quietone CreditAttribution: quietone commented

It looks like the test data has only one comment. If that is true, is that really sufficient?

+++ b/core/modules/comment/migration_templates/d6_comment.yml
@@ -39,7 +39,7 @@ migration_dependencies:
-    - d6_user
     - d6_comment_entity_display
     - d6_comment_entity_form_display
+    - d6_user
     - d6_filter_format

What is the significance of the order change?

quietone’s picture

quietone CreditAttribution: quietone commented

Using a site with only 50 nodes of devel generated content, this worked really well. Looks like all the content is there in the right display order and the correct authors.

I did get one error:

Importing d7_comment_entity_form_display
Importing d7_comment_entity_form_display_subject
array_key_exists(): The first argument should be either a string or an integer NestedArray.php:77                                                                                                       [warning]
Importing d7_field
ultimike’s picture

ultimike
Primary language English
Location Florida, USA
CreditAttribution: ultimike at DrupalEasy commented
Status: Needs review » Reviewed & tested by the community

This looks good to me. I walked through it while peppering phenaproxima with questions on IRC, I'm satisfied.

-mike

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented

Regarding #17 -- since it's a non-fatal error that didn't completely hose the migration, let's deal with it in a follow-up issue.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented
Status: Reviewed & tested by the community » Fixed

Tested this manually yesterday myself with a few of the other patches that went in today and it worked GREAT. Proof: https://twitter.com/webchick/status/639189072838168576

Committed and pushed to 8.0.x. YEAH! :D

  • webchick committed 5fb22f4 on 8.0.x 
    Issue #2532534 by phenaproxima, miguelc303: Migration Files for Drupal 7...

Status: Fixed » Closed (fixed)

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