Fatal error in CommentEntityTranslation @MigrationSource when source site does not have comment or node module installed

@Wim Leers, yes it will. :-)

1. +++ b/core/modules/comment/src/Plugin/migrate/source/d7/CommentEntityTranslation.php
   @@ -15,6 +16,17 @@
   +  public function checkRequirements() {
   

   I really prefer having the query method as the first method in a source plugin, can we move this to the bottom of the source plugin?

2. +++ b/core/modules/comment/src/Plugin/migrate/source/d7/CommentEntityTranslation.php
   @@ -15,6 +16,17 @@
   +    if (!$this->getDatabase()->schema()->tableExists('comment')) {
   

   If we are going to report that comment is not installed then we should check if it is installed, not just for the presence of the comment table.
   if (!$this->moduleExists('comment')) {

As always, this needs a test.

Addressed #6.1 and #6.2 of comment #6, still need works for tests.

Adding test.

I've found only one minor thing:

+++ b/core/modules/comment/tests/src/Kernel/Migrate/d7/CommentEntityTranslationCheckRequirementsTest.php
@@ -0,0 +1,42 @@
+  /**
+   * Tests exception is thrown when profile_fields tables do not exist.
+   */
+  public function testCheckRequirements() {

C/P leftover

Oops. This should fix the comment.

This is RTBC when the test passes (it will pass). I see no conflicts with core 9.2.x.

Setting to RTBC based on the comment in #12.

Based on the query of this plugin (line #31 and #33), this source plugin also requires the node module. Same situation as in #3176394: Do not migrate comment related configurations if "comment" wasn't enabled on the source site. (This is because Drupal core only migrates node comments.)

So we require three source module:

1. entity_translation - reflected in the annotation
2. comment –this will be added now
3. node – this is missing.

Test(s?) still need(s) to be updated.

@Wim Leers, yes, I agree.

Re #15. \Drupal\comment\Plugin\migrate\source\d7\Comment::query does use the node module but that code won't be reached in the test, the checkRequirements will run first. The only modules that need to be installed are those that are needed to create an instance of 'd7_comment_entity_translation' and it's dependent migrations.

Edit: refer to #15

Adding a test for when the node module is not enabled.

Small improvement to remove a local variable.

The patch in #21 can be ignored.

Forgot to update the comment.

Committed 08350ae and pushed to 9.2.x. Thanks!

Fixed on commit per #23

@@ -22,7 +22,7 @@ class CommentEntityTranslationCheckRequirementsTest extends MigrateDrupal7TestBa
   ];

   /**
-   * Tests exception is thrown when comment module is not enabled in the source.
+   * Tests exception thrown when the given module is not enabled in the source.
    *
    * @dataProvider providerTestCheckRequirements
    *

Backported to 9.1.x because this is a major bug and the risk of disruption is low.

Tagging Bug Smash Initiative because I'm checking over RTBC bugs as part of the initiative.