WordPress.org

Make WordPress Core

Opened 22 months ago

Closed 2 months 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 22 months ago.
tests-comment-query.php.patch (1.1 KB) - added by westonruter 22 months ago.
24826.diff (4.1 KB) - added by westonruter 11 months ago.
Refreshed patch. PR: https://github.com/x-team/wordpress-develop/pull/24
24826.1.diff (4.2 KB) - added by morganestes 3 months ago.
Refreshed patch
24826.2.diff (6.0 KB) - added by morganestes 3 months 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 @DrewAPicture22 months ago

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

@westonruter22 months ago

comment:2 @westonruter22 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 22 months ago by westonruter (previous) (diff)

comment:3 @SergeyBiryukov18 months ago

  • Version changed from trunk to 3.5

comment:4 @wonderboymusic11 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 @westonruter11 months ago

  • Keywords needs-refresh removed

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

comment:6 @wonderboymusic4 months ago

  • Milestone changed from Future Release to 4.2

comment:7 @slackbot3 months ago

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

comment:8 @DrewAPicture3 months ago

  • Keywords needs-refresh added

24826.diff needs a refresh.

@morganestes3 months ago

Refreshed patch

comment:9 @morganestes3 months ago

  • Keywords needs-refresh removed

comment:10 @DrewAPicture3 months 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?

@morganestes3 months ago

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

comment:11 @boonebgorges3 months 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 @slackbot3 months ago

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

comment:13 @DrewAPicture3 months ago

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

comment:14 @DrewAPicture3 months ago

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

comment:15 @boonebgorges2 months 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 @DrewAPicture2 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

comment:17 @DrewAPicture2 months 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.