Hide reply link for unpublished comments

The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Picking up again.
- Update the PR destination and base to 10.1.x
- Add additional test case from past patches. Both looks same to me.
Happy to weigh in other opinions and refactor those.
- All the test cases are passing on local now.

Hiding other patches as the one in #43 seems valid.
PR contains all the changes done in #43

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.

So following the steps in the issue summary with the changes from MR 879 I'm still seeing reply button for unpublished comment.

Don't show the link, or allow admins to reply to unpublished comments?

This is a question so is this the approach that was taken?

Since this is changing the UI before/after screenshots in the issue summary will be needed

Thanks.

Tested manually and this does solve the problem, admins can reply to unpublished comments with the patch.

Before:

After:

Took the gifs from #76 and added to the issue summary user interface section

Ran the test-only feature too

1) Drupal\Tests\comment\Functional\CommentAccessTest::testUnpublishedCommentReplyForCommentAdministrators
Behat\Mink\Exception\ExpectationException: Current response status code is 403, but 200 expected.
/builds/issue/drupal-221761/vendor/behat/mink/src/WebAssert.php:888
/builds/issue/drupal-221761/vendor/behat/mink/src/WebAssert.php:145
/builds/issue/drupal-221761/core/modules/comment/tests/src/Functional/CommentAccessTest.php:152
/builds/issue/drupal-221761/vendor/phpunit/phpunit/src/Framework/TestResult.php:729
ERRORS!
Tests: 3, Assertions: 54, Errors: 1.
PHPUnit 9.6.19 by Sebastian Bergmann and contributors.
Testing Drupal\Tests\comment\Functional\CommentInterfaceTest

Coverage is definitely there

MR does appear to address the problem described in the summary.

Believe this is good.

Thanks for your work on this issue! Thanks especially @pameeela for the very clear manual testing. I've updated the issue summary with those images.

There are a few coding standards and best practices issues with the current MR (see my comments therein). I do understand that in some cases these are being copied from existing tests and/or surrounding code, but we should not do the wrong thing for the sake of consistency. (If desired, a followup can be filed to improve the best practices in the rest of these tests.)

I think this is pretty close.

Saving issue credits.

I also just noticed no one actually ever answered @alexpott's question in #50 and #51. NW for that also.

FWIW, I think there is a usecase here, but we should articulate specifically why this is the right approach (vs. removing the link in this case).

If we need to escalate it for further feedback once that reply is given, I think it should go to the usability team and/or comment subsystem maintainers; this is not really a product-level decision. Thanks!

I was a bit unsure about the solution myself, in testing it seemed a bit odd to reply to an unpublished comment. I am not super clear on the use case for replying before approving.

If an admin replies to an unpublished comment, the reply is unpublished as well and it has to be approved separately (even if they have permission to comment without approval). This makes sense, but you can then approve it meaning you could have a published reply to an unpublished comment. And if the original comment is deleted rather than approved, the reply is also deleted. (Why would you reply to a comment you were going to delete ? Well, exactly. But also why wouldn't you just approve the comment before replying?)

Having said all that, the stakes here seem extremely low. But it makes more sense to me to hide the reply link until the comment is approved.

Updated based on the latest suggested approach.

Agreed with the new proposed solution, hiding the link seems right.

Put up an MR with test coverage.

catch → changed the visibility of the branch 221761-comment-admins-cannot to hidden.

Woohoo! Manual test passed, added updated screenshots to the IS.

Solution and test code look good, could maybe check for the reply link to exist and not just the text to be a little more specific but this works.

Committed and pushed 6067ed426c to 11.x and 3b1e465274 to 11.0.x and f096a302e8 to 10.4.x and 71ac91d7ce to 10.3.x. Thanks!

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