WordPress.org

Make WordPress Core

Opened 21 months ago

Closed 6 weeks ago

#24826 closed enhancement (fixed)

Add __construct function to the "WP_Comment_Query" Class

Reported by: ramiy Owned by: boonebgorges
Milestone: 4.2 Priority: normal
Severity: normal Version: 3.5
Component: Comments Keywords: has-patch commit 4.2-beta
Focuses: Cc:

Description

Add PHP5 construct function to the comments query class, just like we have constructor functions on WP_Query and WP_User_Query classes.

Attachments (5)

comment.php.diff (3.1 KB) - added by westonruter 21 months ago.
tests-comment-query.php.patch (1.1 KB) - added by westonruter 21 months ago.
24826.diff (4.1 KB) - added by westonruter 10 months ago.
Refreshed patch. PR: https://github.com/x-team/wordpress-develop/pull/24
24826.1.diff (4.2 KB) - added by morganestes 7 weeks ago.
Refreshed patch
24826.2.diff (6.0 KB) - added by morganestes 7 weeks ago.
Cleaned up and re-added docblocks to class methods that were split in 4.2.

Download all attachments as: .zip

Change History (22)

comment:1 @DrewAPicture21 months ago

  • Cc xoodrew@… added
  • Version changed from trunk to 3.1

@westonruter21 months ago

comment:2 @westonruter21 months ago

  • Cc weston@… added
  • Keywords has-patch added; needs-patch removed
  • Severity changed from major to normal
  • Version changed from 3.1 to trunk

Add WP_Comment_Query::__construct() for parity with WP_Query::__construct()

Also:
Add WP_Comment_Query::get_comments() for parity with WP_Query::get_posts()
Add WP_Comment_Query::$comments for parity with WP_Query::$posts
Declare WP_Comment_Query::$query_vars which was previously implicit
Add parse_comment_query action for parity with parse_query in WP_Query.

Relates to #15032

Patches are also on GitHub at:
https://github.com/x-team/WordPress/tree/24826
https://github.com/x-team/WordPress/commit/db6105cbaf60ca20501fafea29b0f8277ed03cd3

Passes unit tests for comment group; also added a new test for the changes to WP_Comment_Query (attached).

Last edited 21 months ago by westonruter (previous) (diff)

comment:3 @SergeyBiryukov17 months ago

  • Version changed from trunk to 3.5

comment:4 @wonderboymusic10 months ago

  • Keywords needs-refresh added; 2nd-opinion dev-feedback removed
  • Milestone changed from Awaiting Review to Future Release

This does improve the class, but the patch currently explodes

comment:5 @westonruter10 months ago

  • Keywords needs-refresh removed

In 24826.diff merged core patch and unit test patch into one, and fixed conflicts.

comment:6 @wonderboymusic3 months ago

  • Milestone changed from Future Release to 4.2

comment:7 @slackbot7 weeks ago

This ticket was mentioned in Slack in #core by drew. View the logs.

comment:8 @DrewAPicture7 weeks ago

  • Keywords needs-refresh added

24826.diff needs a refresh.

@morganestes7 weeks ago

Refreshed patch

comment:9 @morganestes7 weeks ago

  • Keywords needs-refresh removed

comment:10 @DrewAPicture7 weeks ago

  • Keywords commit added
  • Owner set to boonebgorges
  • Status changed from new to reviewing

@morganestes: Thanks for the refresh. We should probably update the @since versions on WP_Comment_Query::get_comments() and the new constructor to 4.2.0. Barring that, I think we can move this for commit consideration.

@boonebgorges: Any chance you could review this?

@morganestes7 weeks ago

Cleaned up and re-added docblocks to class methods that were split in 4.2.

comment:11 @boonebgorges7 weeks ago

Yes, I can review. I would only feel comfortable moving forward if I had some time to review the documentation and some backward compatibility issues. If this needs to be committed today to make 4.2.0, let's punt. If it can go in sometime in the next week, let's keep it in 4.2.

comment:12 @slackbot7 weeks ago

This ticket was mentioned in Slack in #core by drew. View the logs.

comment:13 @DrewAPicture7 weeks ago

  • Keywords 4.2-beta added
  • Owner changed from boonebgorges to DrewAPicture
  • Status changed from reviewing to accepted

comment:14 @DrewAPicture7 weeks ago

  • Owner changed from DrewAPicture to boonebgorges
  • Status changed from accepted to assigned

comment:15 @boonebgorges6 weeks ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 31793:

Improve method consistency in WP_Comment_Query.

  • Introduce a __construct() method, which can accept an array of query vars.
  • Move query logic out of query() method and into a new get_comments() method.
  • Ensure that $this->comments is set whenever get_comments() returns a value.
  • Introduce a parse_query() method, where query vars are parsed with default values and the 'parse_comment_query' action is fired.

These changes bring WP_Comment_Query syntax closer to that of WP_Query.

Props westonruter, morganestes, boonebgorges.
Fixes #24826.

comment:16 @DrewAPicture6 weeks ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

The hash notation should've been moved to the constructor as well.

comment:17 @DrewAPicture6 weeks ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 31795:

Move the default arguments hash notation for WP_Comment_Query to the new constructor, where the argument definitions were moved in [31793].

Core style dictates that the default arguments should be documented in the same function or method where they are defined.

See [31793].
Fixes #24826.

Note: See TracTickets for help on using tickets.