Problem/Motivation

The way we migrate comment types from Drupal 6 to Drupal 8 is no longer going to cut it, as we have discovered over in #2853872: Migration for forum and article comments: duplicate comment types and incorrect comment_entity_statistics. That issue is against the D7 migration path, but D6 is affected too because its comment type migration only creates two comment types -- comment and comment_no_subject. This can result in conflicts and problems when the Forum module is introduced, or the Standard profile is installed.

Proposed resolution

The d6_comment_type migration should behave exactly like the d7_comment_type migration and create a new comment type for each node type. To facilitate this, it would be a LOT easier and cleaner to remove the comment type-related source plugins from the D6 migration path, and have the D6 NodeType source plugin include the comment settings for each node type. This will be a million times simpler and cleaner!

Remaining tasks

Write a patch, bikeshed, review, and commit. Not necessarily in that order.

User interface changes

None.

API changes

A few source plugins from Migrate Drupal (which is experimental!) will probably be removed. Therefore, this constitutes a BC break, although it's in an experimental meta-module and therefore within policy.

Data model changes

None.

Comments

phenaproxima created an issue. See original summary.

phenaproxima’s picture

Status: Active » Needs review
FileSize
38.08 KB

Here's a first attempt to ice the variable-based comment settings plugins in favor of merging that information into the d6_node_type plugin...

Status: Needs review » Needs work

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

phenaproxima’s picture

Status: Needs work » Needs review
FileSize
42.9 KB
6.09 KB

This oughta do the trick.

Status: Needs review » Needs work

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

phenaproxima’s picture

Status: Needs work » Needs review
FileSize
43.48 KB
1006 bytes

Okay, THIS should fix all the entity counts. Man...that test takes forever.

maxocub’s picture

Status: Needs review » Needs work
Issue tags: +Needs change record

I think that this makes a lot of sense and that it is much easier to understand what is going on.

Nevertheless, I tested it manually and it doesn't seem to be working properly yet. When I go to a migrated node which is supposed to have comments, I don't see them. It seems like there's no comment fields on the node types.

Maybe the tests should try to reproduce that, like load a node and see if the comments are there.

phenaproxima’s picture

Issue tags: +Needs tests

Well...dammit. Thanks for testing, @maxocub! I will try to reproduce the problem, and expand the test coverage to account for the hole, when I trace it.