WordPress.org

Make WordPress Core

Opened 10 months ago

Last modified 4 days ago

#41821 new enhancement

REST API: Add support for threaded comments

Reported by: rmccue Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 4.7
Component: REST API Keywords: has-patch needs-unit-tests
Focuses: rest-api Cc:

Description

(I don't think we have a tracking ticket for this already.)

We should add support for threading comments, particularly in "flat" mode. Flat mode would allow threading in frontend code relatively easily.

The only issue with this is that it will overflow per_page. We intentionally treat this as an indicator and occasionally have less items already (if they're filtered inside the get_items() loop, e.g.), so I don't think this is a huge issue. In addition, threaded mode should be opt-in in any case.

Previously: https://github.com/WP-API/WP-API/issues/1612

Attachments (2)

41821.diff (33.7 KB) - added by brgweb 5 days ago.
Enable threaded comments in rest api
41821v2.diff (2.6 KB) - added by brgweb 4 days ago.
Fix WPCS errors

Download all attachments as: .zip

Change History (8)

#1 @birgire
7 days ago

#44555 was marked as a duplicate.

#2 follow-up: @brgweb
6 days ago

I'd like to work on it. Should I fork github WP API repository or work with trac?

Last edited 6 days ago by brgweb (previous) (diff)

#3 in reply to: ↑ 2 @brgweb
6 days ago

Replying to brgweb:

I'd like to work on it. Should I fork github WP API repository or work with trac?

Nevermind... I'm using svn already.

@brgweb
5 days ago

Enable threaded comments in rest api

#4 in reply to: ↑ description ; follow-up: @brgweb
5 days ago

  • Focuses rest-api added
  • Keywords has-patch added; needs-patch removed

@birgire I've added a diff to this ticket that enables threaded comments in rest api.

It adds a boolean query parameter "hierarchical". Instead of passing "hierarchical" straight to WP_Comment_Query, it retrieves only top level comments (or just the first level if "parent" is set) and retrieve children inside prepare_items_for_response method.

I've tested with fake data and in a production environment and it worked as expected.

Replying to rmccue:

(I don't think we have a tracking ticket for this already.)

We should add support for threading comments, particularly in "flat" mode. Flat mode would allow threading in frontend code relatively easily.

The only issue with this is that it will overflow per_page. We intentionally treat this as an indicator and occasionally have less items already (if they're filtered inside the get_items() loop, e.g.), so I don't think this is a huge issue. In addition, threaded mode should be opt-in in any case.

Previously: https://github.com/WP-API/WP-API/issues/1612

#5 in reply to: ↑ 4 ; follow-up: @birgire
5 days ago

  • Keywords needs-unit-tests added

Replying to brgweb:

@birgire I've added a diff to this ticket that enables threaded comments in rest api.

It adds a boolean query parameter "hierarchical". Instead of passing "hierarchical" straight to WP_Comment_Query, it retrieves only top level comments (or just the first level if "parent" is set) and retrieve children inside prepare_items_for_response method.

I've tested with fake data and in a production environment and it worked as expected.

great, thanks for the patch

Few notes on 41821.diff:

  • It seems to change the dynamic fields support in WP_REST_Comments_Controller::prepare_item_for_response(), so the _fields parameter is not working on the comment children. See [43087]. Are you working with the latest trunk?
  • Is there any need for a check_read_permission() on the children, as seen here?
  • It seems to implement the 'hierarchical' => 'threaded' according the children tree output response, not the 'hierarchical' => true for a flat array? I might be misunderstanding the patch and the ticket's description though :)
  • We should consider unit tests for this support.
  • Current unit tests fail and need to be updated.
  • There are some issues regarding WordPress Code Standard, resulting in a larger diff file.

@brgweb
4 days ago

Fix WPCS errors

#6 in reply to: ↑ 5 @brgweb
4 days ago

Thanks for the review! I've uplodaded another diff.

Replying to birgire:

great, thanks for the patch

Few notes on 41821.diff:

  • It seems to change the dynamic fields support in WP_REST_Comments_Controller::prepare_item_for_response(), so the _fields parameter is not working on the comment children. See [43087]. Are you working with the latest trunk?

I believe that I've changed the working copy during tests and was with a stable version of WordPress. It's fixed now.

  • Is there any need for a check_read_permission() on the children, as seen here?

I've added this check in the new patch.

  • It seems to implement the 'hierarchical' => 'threaded' according the children tree output response, not the 'hierarchical' => true for a flat array? I might be misunderstanding the patch and the ticket's description though :)

It implements the threaded array in WP_Comment_Query but it outputs a flat array. Working with threaded in the query made the code smaller and the implementation easier. For REST API json responses, I don't think there's a use for the threaded output (maybe I'm wrong :) )

  • We should consider unit tests for this support.
  • Current unit tests fail and need to be updated.

I'd like to help with unit tests too. I will work on it.

  • There are some issues regarding WordPress Code Standard, resulting in a larger diff file.

Older version of the file and editor messing with spaces and tabs. Fixed in v2.

Note: See TracTickets for help on using tickets.