Comment Content: Show moderation message #40612
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DAreRodz
requested review from
TimothyBJacobs,
spacedmonkey and
ajitbohra
as code owners
|
Size Change: +38 B (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
DAreRodz
force-pushed
the
fix/comment-content-add-moderation-message
branch
from
0346259
to
97caf80
Compare
DAreRodz
requested review from
cbravobernal,
ockham,
SantosGuillamot,
michalczaplinski and
luisherranz
|
I've added unit tests that cover some cases where a comment needs moderation. I hope that's enough. If we still need some e2e tests, maybe we can add them in a different PR? |
DAreRodz
added
[Block] Comments
|
I've seen that PHP unit tests are failing; this is the output I got in local, though: |
|
My bad, I forgot to push some local changes. 😅 |
|
PHP unit tests are still failing in CI:
I wonder if this is because CI is using an up-to-date version of Core that already has If so, we'll have to file (and merge) a PR against Core first to carry those changes over (and merge this PR shortly after in order to update unit tests accordingly). |
|
I can see the same unit test failures locally: Runtime: PHP 7.4.27
Configuration: /var/www/html/wp-content/plugins/gutenberg/phpunit.xml.dist
............................................................... 63 / 420 ( 15%)
............................................................... 126 / 420 ( 30%)
............................................................... 189 / 420 ( 45%)
............................................................... 252 / 420 ( 60%)
......................................FF...........I.I.I.III... 315 / 420 ( 75%)
............................................................... 378 / 420 ( 90%)
.......................................... 420 / 420 (100%)
Time: 12.63 seconds, Memory: 50.50 MB
There were 2 failures:So I think this is a problem with the implementation. I'm investigating 🙂 |
|
I could not find the problem and I also could not get the debugger to work with the unit tests, unfortunately 😕 |
Good spot, that's it exactly. WP Env checks out the default WP branch. Line 2 in 7111958 |
There was a problem hiding this comment.
I've added a couple of minor things inline.
I remembered some anti-spam measures WP Core had to put in for visitors using the moderation hash, I've edited the original ticket to note them.
One I've suggested below but the other is in the comment author block and outside this diff.
@ockham I've opened a trac ticket in WP and created WordPress/wordpress-develop#2637 to address this. |
|
I've tried posting a top-level comment; the indentation seems a bit off:
|
|
Indentation aside, this LGTM! It'd be great if @peterwilsoncc could give this final approval -- you're more familiar with the problem domain 🙂 |
|
Oh my, I thought I was testing the Comments Query Loop block, but you're right of course, it was Post Comments 🤦♂️
Sounds like a plan 👍 |
DAreRodz
force-pushed
the
fix/comment-content-add-moderation-message
branch
from
78f29fe
to
fab8385
Compare
|
I think it's ready now. @ockham, @peterwilsoncc, if you do the final check and find everything OK, I think we can approve and merge this. 😊 |
adamziel
pushed a commit
that referenced
this pull request
Apr 29, 2022
Show a moderation message in Comment Content right after publishing a comment that needs moderation. Co-authored-by: Carlos Bravo <carlos.bravo@automattic.com>
|
I cherry picked this change into |
adamziel
removed
the
Backport to WP Beta/RC
@DAreRodz kindly backported the unit test change: WordPress/wordpress-develop#2649 |
22 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Show a moderation message in Comment Content right after publishing a comment that needs moderation.
Fixes #40601
Why?
Preview comments were not displayed if they needed moderation.
How?
include_unapprovedin$comment_argsif needed to fetch the corresponding unapproved comment.Testing Instructions
Follow the same steps described in #40601, but now ensure that the preview actually appears and it's similar to the images below.
Screenshots