Comment form should redirect to the current page when the form is in a block

Just a re-roll against Drupal 10.2.x

@sylus, curious to know, which patch did you reroll from? #87 ?

Re-queuing tests for 10.2.x , I did a dry run myself, should be no issue applying. We'll see what the results give.

The test failures appear to be a result of some issue with the ci test runner.

Error found: No space left on device

Perhaps turn this into a merge request and use the gitlab pipeline instead

9 years, we've been using this for likely at least 6 years.

The WxT distribution is still using this fix, between 200 and 300 installs (likely many more installs that have uninstalled the update module and aren't reporting), multiple public sector organizations, multiple installs in each organization.

Left some comments on the MR.

Removed some unused services that were being added. Tests are green thoughts on the change?

@joseph.olstad mentioned you've been using this patch for a while. Did the changes in the MR continue to work for you?

@smustgrave, I am unable to test MR 6622
This merge request is way behind head of any current releases and has conflicts with

• the HEAD of 10.1.x
• the HEAD of 10.2.x
• the HEAD of 10.3.x
• the HEAD of 11.x

 ╭─◀▶ 🔀▶
 ╰❯ $ patch -p1 --dry-run < 6622.patch

checking file core/modules/comment/comment.services.yml
checking file core/modules/comment/src/CommentForm.php
checking file core/modules/comment/src/CommentLazyBuilders.php
checking file core/modules/comment/tests/src/Functional/CommentBlockTest.php
Hunk #1 succeeded at 4 (offset 2 lines).
Hunk #2 succeeded at 24 (offset 2 lines).
Hunk #3 succeeded at 102 (offset 2 lines).
checking file core/modules/comment/tests/src/Functional/CommentTestBase.php
Hunk #1 succeeded at 188 (offset 2 lines).
checking file core/modules/comment/src/CommentForm.php
Hunk #1 FAILED at 7.
Hunk #2 FAILED at 49.
Hunk #3 FAILED at 101.
Hunk #4 FAILED at 113.
Hunk #5 FAILED at 160.
Hunk #6 FAILED at 458.
6 out of 6 hunks FAILED
checking file core/modules/comment/src/CommentLazyBuilders.php
Reversed (or previously applied) patch detected!  Assume -R? [n]

ah, nevermind, I tried git apply and it went in clean.

The merge request applies cleanly against 11.x and 10.3.x but not 10.2.x.

Reviewed the code, it's functionally the same as patch 100 except rerolled.

Test coverage is added and all tests are passing.

Also, phpcs seems to be happy with this.

If I revert all the changes to core/modules/comment/src/CommentForm.php the tests still pass and reading the code I'm not sure why we're making those changes.

@alexpott, I responded to your concern and pushed up a new commit. Pipeline is running.

LOL, reverting my last commit, that code in question actually does do something.

Hmm, so on "17 February 2024" , the merge request was passing.

I made which seems like a reasonable change to make, the tests fail the failure actually might look like a test bot or ci issue not the actual MR.

Then I reverted the "reasonable" change, same result.

So, it looks like something outside of this merge request changed between February 17th 2024 and today that is affecting the unit tests.

May also need a rebase? With latest 11.x

Rebased with the latest 10.3.x

I had a look at patch 100, this is the patch that we've been using for several years.

The code in patch 100 is as follows:

@@ -411,7 +448,11 @@ public function save(array $form, FormStateInterface $form_state) {
       $this->messenger()->addError($this->t('Comment: unauthorized comment submitted or comment submitted to a closed post %subject.', ['%subject' => $comment->getSubject()]));
       // Redirect the user to the entity they are commenting on.
     }
-    $form_state->setRedirectUrl($uri);
+    $entity_url = $entity->toUrl();
+    if ($entity_url->getRouteName() === $this->routeMatch->getRouteName() &&
+      $entity_url->getRouteParameters() === $this->routeMatch->getRawParameters()) {
+      $form_state->setRedirectUrl($uri);
+    }
   }
 
 }

but the code in the MR is like this:

-    $form_state->setRedirectUrl($uri);
+    $entity_url = $entity->toUrl();
+    if ($entity_url->getRouteName() === $this->routeMatch->getRouteName() &&
+      $entity_url->getRouteParameters() === $this->routeMatch->getRawParameters()) {
+      $form_state->setRedirectUrl($uri);
+    }
+    $form_state->setRedirectUrl($uri);

Somewhere between patch 100 and the current MR, things went weird.

Specifically according to git blame it is the feb 17th commit by smustgrave f522c159ac3a that brought in this change.

So I pushed up dropping the last setRedirectUrl so that the preceding condition becomes once again meaningful.

ah crap, I made a mistake using 10.3.x , sorry folks

I should have checked that the MR was against 11.x not 10.3.x
Sorry about this. what a mess I just made!

ok @smustgrave, I did my best to recuperate your work into the MR 7177 , cherry-picking commits into it.

MR 7177 is based from 11.x

10.3.x was accidentally merged into 6922 by me, sorry.

Trying to fix this up with 7177.

The current failure is completely unrelated to what we're doing here.

Suspect something is going on with the ci system or maybe a patch that was pushed into 11.x

core/tests/Drupal/Tests/Component/Scaffold/Functional/ManageGitIgnoreTest.php

joseph.olstad → closed merge request !6622

Tried rebasing but still getting multiple test failures :(

Current test failures:

1. related to MR 7171

Exception Other      phpunit-162.xml      0 Drupal\Tests\comment\Functional\Vie
    PHPUnit Test failed to complete; Error: PHPUnit 9.6.15 by Sebastian
    Bergmann and contributors.
    
    Testing Drupal\Tests\comment\Functional\Views\CommentAdminTest
    E.                                                                  2 / 2
    (100%)
    
    Time: 00:24.360, Memory: 4.00 MB
    
    There was 1 error:
    
    1)
    Drupal\Tests\comment\Functional\Views\CommentAdminTest::testApprovalAdminInterface
    Behat\Mink\Exception\ResponseTextException: The text "zsjfmfvf" was not
    found anywhere in the text of the current page.

2. This one possibly unrelated(?)

---- Drupal\Tests\filter\Functional\FilterHtmlImageSecureTest ----
Status    Group      Filename          Line Function                            
--------------------------------------------------------------------------------
Fail      Other      phpunit-172.xml      0 Drupal\Tests\filter\Functional\Filt
    PHPUnit Test failed to complete; Error: PHPUnit 9.6.15 by Sebastian
    Bergmann and contributors.
    
    Testing Drupal\Tests\filter\Functional\FilterHtmlImageSecureTest
    F                                                                   1 / 1
    (100%)
    
    Time: 00:06.514, Memory: 4.00 MB
    
    There was 1 failure:
    
    1)
    Drupal\Tests\filter\Functional\FilterHtmlImageSecureTest::testImageSource
    http://localhost/subdirectory/core/misc/druplicon.png was found.
    Failed asserting that false is true.

Rerolled the above patch for 10.3.10