Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comments: add comment counts endpoint #8202

Merged
merged 2 commits into from
Nov 27, 2017
Merged

Conversation

vindl
Copy link
Member

@vindl vindl commented Nov 17, 2017

Depends on D8370-code.
Closes: Automattic/wp-calypso#19477

Add endpoint that will return comment counts for each available status on a given site. Optionally, these results are filterable by post_id parameter.

Testing instructions:

  1. Apply D8370-code on your sandbox.
  2. Point Jetpack to your sandbox using define( ‘JETPACK__API_BASE’, ‘https://hostname.wordpress.com/jetpack.’ ).
  3. Set GET request to https://public-api.wordpress.com/rest/v1/sites/{site_slug}/comment-counts
  4. Verify that the comment counts are correct.
  5. Try passing in post_id parameter with the request and verify that the results are filtered by post.

Add endpoint that will return comment counts for each available
status on a given site. Optionally, these results are filterable
by post_id parameter.
@vindl vindl requested a review from a team as a code owner November 17, 2017 20:03
@vindl vindl self-assigned this Nov 17, 2017
@vindl vindl added [Feature] Comments [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Nov 17, 2017
@vindl vindl added this to the 5.6 milestone Nov 17, 2017
@vindl vindl requested a review from a team November 17, 2017 20:04
return new WP_Error( 'authorization_required', 'An active access token must be used to retrieve comment counts.', 403 );
}

if ( ! current_user_can_for_blog( $blog_id, 'moderate_comments' ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So moderate_comments is sadly not used anyplace but an obscure ajax endpoint for wp-admin. Automattic/wp-calypso#17687

The corner case to check for is an Author who has a post with some comments. In wp-admin I should be able to see full counts, but only be able to moderate comments on their posts. In this case I think we need to switch this check to 'edit_posts'

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in bb1606d.

@vindl vindl requested a review from oskosk November 27, 2017 13:04
Copy link
Member

@dereksmart dereksmart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with D8370 -- works and looks great!

@dereksmart dereksmart added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Nov 27, 2017
@dereksmart dereksmart merged commit 2186e93 into master Nov 27, 2017
@dereksmart dereksmart deleted the add/comment-counts-endpoint branch November 27, 2017 19:44
@dereksmart dereksmart removed the [Status] Ready to Merge Go ahead, you can push that green button! label Nov 27, 2017
jeherve added a commit that referenced this pull request Nov 28, 2017
oskosk pushed a commit that referenced this pull request Nov 28, 2017
* Changelog 5.6: create base for changelog.

* Update changelog with 5.5.1 info.

* Changelog: add #7930 and #8238

* Changelog: add #8076

* Changelog: add #8100

* Changelog: add #8117

* Changelog: add #8141

* Changelog: add #8143

* Changelog: add #8147

* Changelog: add #8149

* Changelog: add #8153

* Changelog: add #8173

* Changelog: add #8184

* Changelog: add #8196

* Changelog: add #8199

* Changelog: add #8093

* Changelog: add #8171

* Changelog: add #8182

* Changelog: add #8202, #8222

* Changelog: add #8228

* Changelog: add #8240

* Changelog: add #8251

* remove AL card change
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants