Add/rest api for block commenting#65181
Add/rest api for block commenting#65181poojabhimani12 wants to merge 27 commits intoWordPress:trunkfrom
Conversation
…add/rest-api-for-block-commenting
…commenting Add Rest api filters for block commenting
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @poojabhimani12! In case you missed it, we'd love to have you join us in our Slack community. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
|
Thanks for splitting the PRs, that's really helpful. I'm adding @TimothyBJacobs here for his expertise on the REST API. I know @tyxla had some comments on the other thread as well about the REST API. It would be cool to add some php unit tests to clarify what these changes are about, and maybe consider adding some context to the PR description. |
tyxla
left a comment
There was a problem hiding this comment.
Thanks for splitting this one out to a separate PR 🙌
From my perspective, this mostly looks good and is close enough!
Passing the latest feedback over to this PR, and adding some more let me know what you think.
|
|
||
| return $response; | ||
| } | ||
| add_filter( 'rest_prepare_comment', 'add_user_avatar_urls_in_rest_response_6_7', 10, 1 ); |
There was a problem hiding this comment.
As also mentioned in #60622:
This will only work when retrieving comments via the REST API. If we want to retrieve them on the backend, it won't work.
I'm trying to understand why get_comment will set the avatar URL to null (as mentioned here). A few possibilities come to mind:
- is it because the
show_avatarsoption is not enabled? - is it because the
author_avatar_urlsfield is not enabled when performing the REST API request?
Can you speak more about where this assignment to null happens?
In any case, to achieve what you're doing it should be enough to just use the rest_avatar_sizes filter and the rest_get_avatar_urls() function would build the avatar URLs properly. If we're just doing it for the REST API response, we're doing it in an inconsistent way which is likely to introduce unexpected bugs and missing data in unexpected places.
There was a problem hiding this comment.
The issue has been resolved. We found that the REST API was returning a null avatar URL because it only supported the default 'comment' type while we were using a custom type.
To fix this, we applied the get_avatar_comment_types filter to include our custom comment type, 'block_comment'. This allowed us to remove the rest_prepare_comment filter, and the issue is now resolved.
|
|
||
| return $prepared_comment; | ||
| } | ||
| add_filter( 'rest_pre_insert_comment', 'update_comment_type_in_rest_api_6_7', 10, 2 ); |
There was a problem hiding this comment.
Thanks for reconsidering the previous approach of updating the comment during a REST request 🙌
Using rest_pre_insert_comment works OK if you always want to do it only through the REST API.
I acknowledge it doesn't make sense to use rest_preprocess_comment because the comment_type and comment_approved fields will be overridden automatically after that.
However, if you want this to work through any interface or flow that updates the comments, you might want to use the wp_update_comment_data filter instead.
There was a problem hiding this comment.
When a comment is submitted, whether it's a custom comment type or the default, comments from registered users will automatically be marked as "approved." For comments that need moderation, set their status to "on hold" initially. After review, you can update the status from "on hold" to "approved" as needed.
There was a problem hiding this comment.
Thanks for clarifying that part @poojabhimani12.
I don't understand why we're doing it only when inserting a comment via the REST API. Wouldn't it be a better idea to do it consistently when a comment is inserted/updated?
There was a problem hiding this comment.
The Comment REST API currently only allows inserting default comment types. If you try to use a custom comment type, it returns an incorrect comment type error.
To resolve this, a filter must be applied to include custom comment types in the REST API. However, no filter is needed when using the comment insert function, as it already supports the insertion of custom comment types.
There was a problem hiding this comment.
This makes sense; thanks a lot for elaborating 🙌
| if ( ! function_exists( 'update_comment_type_in_rest_api_6_7' ) && gutenberg_is_experiment_enabled( 'gutenberg-block-comment' ) ) { | ||
| function update_comment_type_in_rest_api_6_7( $prepared_comment, $request ) { | ||
| if ( ! empty( $request['comment_type'] ) && 'block_comment' === $request['comment_type'] ) { | ||
| $prepared_comment['comment_type'] = $request['comment_type']; |
There was a problem hiding this comment.
You might want to extend the admin_comment_types_dropdown filter to allow users to filter in the admin interface by the new comment_type that you're introducing. Unless you want to intentionally hide those comments from the interface.
There was a problem hiding this comment.
Why not always allow there properties to be set and avoid hardcoding the comment type?
| 'gutenberg-experiments', | ||
| 'gutenberg_experiments_section', | ||
| array( | ||
| 'label' => __( 'Enable multi-user commenting on blocks in post editor', 'gutenberg' ), |
There was a problem hiding this comment.
You might want to consider this prior feedback:
Perhaps it can be reflected in the experiment title?
There was a problem hiding this comment.
We’ve made this update based on the feedback provided here: #60622 (comment).
There was a problem hiding this comment.
I don't think we should be adding this setting here, and hardcode the comment type. We should generalise it to work either with all comment types, or add an (experimental) option/setting to the comment type registration and use that setting (instead of hardcoding anything).
|
|
||
| return $response; | ||
| } | ||
| add_filter( 'rest_prepare_comment', 'add_user_avatar_urls_in_rest_response_6_7', 10, 1 ); |
There was a problem hiding this comment.
Priority 10 and 1 accepted arguments are the defaults for add_filter(), so they can be omitted.
| add_filter( 'rest_prepare_comment', 'add_user_avatar_urls_in_rest_response_6_7', 10, 1 ); | |
| add_filter( 'rest_prepare_comment', 'add_user_avatar_urls_in_rest_response_6_7' ); |
…k-commenting update get avatar url rest-api filter
| return $comment_type; | ||
|
|
||
| } | ||
| add_filter( 'get_avatar_comment_types', 'update_get_avatar_comment_type', 10, 1 ); |
There was a problem hiding this comment.
Aside from the spacing issues which I'm assuming you're addressing separately, the trailing arguments aren't necessary because they're the defaults:
| add_filter( 'get_avatar_comment_types', 'update_get_avatar_comment_type', 10, 1 ); | |
| add_filter( 'get_avatar_comment_types', 'update_get_avatar_comment_type' ); |
I also think some basic unit tests would be useful to have here. The logic is straightforward, so I don't assume the tests to be complicated. |
…k-commenting Extend the admin_comment_types_dropdown filter
…k-commenting added unit test cases for filter functions
…k-commenting remove spacing from unit testing file
…k-commenting resolve phpcs error from unit test file
…k-commenting resolve phpcs error in unit test file
|
Using the existing comments controller like this is I think sufficient for an experiment. But we most likely are going to want to treat this more like post types, where different comment types are at different routes. |
I am not sure if we need a new endpoint here. Adding support for this comment type for the existing endpoint makes sense to me. |
lib/experimental/editor-settings.php
Outdated
| wp_add_inline_script( 'wp-block-library', 'window.__experimentalFullPageClientSideNavigation = true', 'before' ); | ||
| } | ||
| if ( $gutenberg_experiments && array_key_exists( 'gutenberg-block-comment', $gutenberg_experiments ) ) { | ||
| wp_add_inline_script( 'wp-block-editor', 'window.__experimentalEnableBlockComment = true', 'before' ); |
There was a problem hiding this comment.
Why do we need this flag in this PR?
| 'block_comment' => __( 'Block Comments' ), | ||
| ); | ||
| } | ||
| add_filter( 'admin_comment_types_dropdown', 'update_comment_type_filter_dropdown' ); |
There was a problem hiding this comment.
Why do we need to add this comment type to the comment management list? Comments are managed in the editor?
There was a problem hiding this comment.
We have incorporated the comment type into the backend comment page filter based on the feedback received: #65181 (comment).
There was a problem hiding this comment.
How is this related to the REST API?
…hah-multidots/gutenberg into add/rest-api-for-block-commenting
…k-commenting remove experimental flag from this PR
…k-commenting remove filters from rest API PR
…k-commenting-conflict-resolve Add/rest api for block commenting conflict resolve
|
Hi @poojabhimani12, I'd like to confirm the status of this PR. Are there any blockers to moving forward with this PR? Thank you! |
@t-hamano - From my side no such blockers just need to update with latest version, @karthick-murugan has created new PR with latest WordPress Version. @ellatrix - you can share your thought here and review it. |
|
@poojabhimani12 thanks for the reply! Now, let's close this PR and move on to the new one: #71660 Thank you to everyone who reviewed this PR, and I hope you'll come back and check out the new one. |
What?
Add Rest API for Block Level Comment
Why?
#59445
#60622 (comment)
This PR updates the REST API to support our custom comment type, 'block_comment.' Previously, only the default 'comment' type was supported. We've developed the comment API endpoint in Gutenberg to enable block-based commenting, providing greater flexibility for managing comments within the block editor.