Add additional fields to the `/wp-json/wp/v2/comments` endpoints

[hannahtinkler]

The WordPress app is currently undergoing a upgrade from the V1.1 Jetpack endpoints to the V2 WordPress JSON endpoints. There are a 3 fields missing from the V2 endpoint that are present in the V1.1 endpoints, that the team would like added.

Field 1: can_moderate
bool - whether the authenticated user can edit the comment

Field 2: post_details
array- named post in the V1.1 endpoint but needs to be renamed as there is already a post property in the V2 response. Properties:

• id (int - the post ID)
• title (string - the title of the post)
• type (string - the post type)
• link (string - like API link to the post)

Field 3: i_replied
bool - whether the authenticated user replied to the comment

[swissspidy]

can_moderate

Can't this be determined through other means, e.g. _links or HTTP headers?

post_details

Ditto here, can't this be done by embedding the up link?

i_replied

There might be a more RESTful approach for this as well. For example, there's already a children link — perhaps we could add a similar link just for children by the current user.

Trac ticket: https://core.trac.wordpress.org/ticket/63928#ticket

## Proposed changes

• Adds can_moderate field to the comment endpoint responses, to indicate if the authenticated user can edit the comment
• Adds post_details field to the comment endpoint responses, containing the post ID, title, type, and REST link
• Adds i_replied field to the comment endpoint responses, to indicate if the authenticated user replied to the comment

---
## Why are these changes being made?
The WordPress app is currently undergoing a upgrade from the V1.1 Jetpack endpoints to the V2 WordPress JSON endpoints. There are a 3 fields missing from the V2 endpoints that are ​present in the V1.1 endpoints. The WordPress Mobile team need these to be added before they can upgrade to the V2 comment endpoints.

---
## Testing steps

1. Leave a comment on a post as the bog admin
2. Make an request authenticated as the blog admin to the comments endpoint for the blog (example below)
3. Assert that the new fields are present for all returned comments
4. Assert that can_moderate is true for the comment you left in step 1
5. Assert that i_replied is false for the comment you left in step 1
6. Assert that the data in post_details is correct
7. Reply to the comment you left in step 1 as the bog admin
8. Make an request authenticated as the blog admin to the comments endpoint for the blog
9. Assert that i_replied is true for the comment you left in step 1
10. Make an unauthenticated request to the comments endpoint for the blog
11. Assert that can_moderate is false for all comments
12. Assert that i_replied is false for all comments

[hannahtinkler]

Thanks for the feedback @swissspidy!

can_moderate

Can't this be determined through other means, e.g. _links or HTTP headers?

I can't see anything in _links that would indicate the authenticated user is able to edit the given comment - have I misunderstood?

Any HTTP headers would apply to the whole request so I don't think it would be a reliable indicator that the authenticated user can edit any one specific comment returned in the response. Ability to edit a specific comment may depend on a number of factors including whether the user is an admin, the comment author, or some custom logic applied through filters like user_has_cap.

post_details

Ditto here, can't this be done by embedding the up link?

Do you mean re-using the _links.up.href value for post_details.link in the response, or using the _links.up.href value in the app to request the post information?

i_replied

There might be a more RESTful approach for this as well. For example, there's already a children link — perhaps we could add a similar link just for children by the current user.

Comments render in the WordPress app in a list; the team's intention is to use this data in a the list along side each comment. Supplying a link to retrieve the children belonging to the current user would require the app to make an extra call per comment displayed, which would not be ideal for performance or network usage.

[crazytonyli]

Hi @swissspidy, just for a little bit of background info, I'm one of the developers working on the WordPress mobile app. I asked Hannah to help add these fields to the REST API in core.

Regarding the post_details field, what the app needs is the post title, so that we can show a UI like: "<Someone> posted a comment on <Post title>: <Comment content>". We can fetch any post detail information in a separate HTTP request, but this would require one request for each comment, which isn't very performant.

[swissspidy]

I'm definitely also mindful of extra HTTP requests, which is why I suggested to use ​linking and embedding. targetHints are also a new addition in 6.7 to avoid additional requests, see #61739.

Making additional information available is fine, but it needs to be done in line with the REST API's design. The current PR does not do that.

Some input from component maintainers would be appreciated. cc @rmccue @kadamwhite @spacedmonkey @TimothyBlynJacobs

[TimothyBlynJacobs]

Yeah, the PR as is does not follow the WordPress design patterns for the REST API. Pascal is spot on here. in his suggestions for a more Core RESTful approach.

[rmccue]

Replying to hannahtinkler:

I can't see anything in _links that would indicate the authenticated user is able to edit the given comment - have I misunderstood?

The idiomatic approach to this for the basic CRUD operations is to check the Allow header for an item - eg if it contains PUT, then edits are allowed. This is ​generated using the permission callbacks, combined with whether the callback is actually registered.

Those headers are embedded for self via targetHints, so you can check _links.self.0.targetHints to see if it contains PUT. This is new as of 6.7 as @swissspidy mentions (#61739); older sites will only have it available on the individual endpoint Allow headers.

For custom actions beyond CRUD/REST verbs, the pattern we use in other places (eg the posts controller) is ​a self link with custom relations like https://api.w.org/action-{name} - I don't think that's necessary here, but could be if eg core had a separate cap for spamming a comment vs editing it.

Ditto here, can't this be done by embedding the up link?

Do you mean re-using the _links.up.href value for post_details.link in the response, or using the _links.up.href value in the app to request the post information?

The up link contains the parent post, so setting ?_embed (or ?_embed[]=up) will bring the post's details in too, eg ​https://wordpress.org/news/wp-json/wp/v2/comments?_embed[]=up has _embedded.up.0 for the post's details.

Comments render in the WordPress app in a list; the team's intention is to use this data in a the list along side each comment. Supplying a link to retrieve the children belonging to the current user would require the app to make an extra call per comment displayed, which would not be ideal for performance or network usage.

Generally, we try and keep the data as a clean representation of the objects stored in WP, and non-client-specific. This data might make sense for the Automattic app, but would cause extra database queries for every client, even those that don't need this data.

We could potentially add a custom link relation here - it's a combination of replies and me so I don't think there's a registered one - with targetHints including X-WP-Total. You could then detect if there's a reply by checking if X-WP-Total > 0. (We actually don't have a replies link at all right now which we probably should.)

That said, it's relatively expensive to look up and it's very specific to a particular design, so again I'm not sure it's appropriate to include.

You could potentially pull the comments list + batch request for /comments?parent={id}&author={userid} as a way to do this in only two HTTP requests.

[crazytonyli]

The up link contains the parent post, so setting ?_embed (or ?_embed[]=up) will bring the post's details in too

That's a nice solution. I was not aware of the _embed query. However, in practice, using that query would dramatically increase the response body size: 2x for short posts, and it could be worse for relatively long posts. That probably won't be the first solution that I'd choose for the mobile app.

[hannahtinkler]

Thanks all for your patience and the additional context - I appreciate it! 🙏

targetHints are also a new addition in 6.7

This will be super useful; I can see it maps back to the controller permissions callbacks so would be more robust than just a current_user_can call anyway.

I'm definitely also mindful of extra HTTP requests, which is why I suggested to use linking and embedding.

TIL! The embedding feature is really cool, I hadn't seen that in WordPress before. I share Tony's concerns about response size though - although the post content is excluded, my requests are almost double the size using ?_embed=up.

@swisspiddy / @rmccue / anyone else: Looking at the code, there doesn't seem to be a way to customise the embed fields that are returned (e.g. like you would for a direct request with ?fields=) except for providing a different context via the _links hrefs - am I reading that correctly?

I'm assuming that adding a _links entry either for just the post title, or for an additional up context, wouldn't be encouraged. Likewise for adding a reduced, app-specific embed context (even if there was some way to pass a context to the embed code without changing the _links href). My only other thought was around being able to specify which embed fields should be returned, but that would obviously defeat the point of contexts, and would be a very non-trivial change in how the REST API works.

I'd be interested to hear any other thoughts on how we might be able to solve this without additional request(s) or increasing the response size, if anyone has any - I'm not seeing anything else at the moment 😕

The app team are revisiting how the app uses the existing fields and will investigate use of targetHints and embeds.

[hannahtinkler]

As mentioned in the PR, the app team are revisiting how the app uses the existing fields and will investigate use of targetHints and embeds. We'll revisit Core changes if needed, but expect they won't be 🙂 Many thanks to the folks who commented to add suggestions and context!

[swissspidy]

Removing milestone from closed ticket.