WordPress.org

Make WordPress Core

#26240 closed defect (bug) (fixed)

Allow get_comments_number() to accept post object

Reported by: coffee2code Owned by: DrewAPicture
Milestone: 4.0 Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-patch commit
Focuses: docs Cc:

Description

get_comments_number() unnecessarily enforces an int value for its $post_id argument. This is contrary to expectations whereby most template functions accept either a post object or ID, and it doesn't match up with the function's phpDocs which indicate either value is acceptable.

Ultimately, the $post_id argument gets sent to get_post() to obtain the post, so the argument can be directly passed along and left to be handled by get_post().

Attached is a patch which removes the unnecessary typecasting. The patch also adds braces to the if conditional to match the new guideline change.

Also included are some unit tests for it. Since there doesn't appear to be any unit tests for comment-template.php functions, I created tests/comment/output.php to mirror how posts template functions are tested in tests/post/output.php (though post template tests are pretty meager).

Attachments (5)

26240.diff (2.0 KB) - added by coffee2code 18 months ago.
26240-tests.diff (1.3 KB) - added by JanHenkG 16 months ago.
26240.2.diff (715 bytes) - added by JanHenkG 16 months ago.
26240.3.diff (756 bytes) - added by SergeyBiryukov 11 months ago.
26240.4.diff (738 bytes) - added by wonderboymusic 11 months ago.

Download all attachments as: .zip

Change History (19)

@coffee2code18 months ago

comment:1 @nofearinc17 months ago

Looks good and works for me, although I think that the test should be in a separate diff.

@JanHenkG16 months ago

@JanHenkG16 months ago

comment:2 @JanHenkG16 months ago

The fix of coffee2code also worked for me, I kept this fix unchanged in the files I attached. The changes I made are:

  • Created separate patch files for the fix itself and the tests.
  • Refactored the tests a bit, for example I removed the use of the $post_id member variable.

comment:3 @coffee2code12 months ago

Patches still apply cleanly. Seems like an easy win.

comment:4 @DrewAPicture12 months ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 4.0

The docs are already in-line with accepting either so +1 for 26240.2.diff minus the unnecessary brace change(s).

comment:5 @wonderboymusic12 months ago

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

[28558] didn't paste itself here.

comment:6 @ocean9011 months ago

  • Keywords needs-patch added; has-patch commit removed

There is a typo in the get_comments_number hook doc for the $post_id arg: Nnumber.

Also the value of $post_id has changed. Previously if $post_id was 0 $post_id was set to get_the_ID(). It seems like $post_id was never a WP_Post object so we can replace $post_id with $post->ID in the get_comments_number filter.

comment:7 @ocean9011 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:8 follow-up: @ocean9011 months ago

  • Focuses docs added

[27156] removed the "Optional" part for $post_id, we should add it back since it's optional.

comment:9 in reply to: ↑ 8 @DrewAPicture11 months ago

Replying to ocean90:

[27156] removed the "Optional" part for $post_id, we should add it back since it's optional.

Opened #28388

comment:10 @DrewAPicture11 months ago

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

In 28602:

Fix parameter description for the $post_id argument in get_comments_number() to note that it is optional.

Also fixes the corresponding filter docs, as $post_id, not $post is passed to the filter.

See [27156]. Fixes #26240.

comment:11 @ocean9011 months ago

In 28604:

Pass $post->ID to get_comments_number filter.

Also fixes indentation and a typo in corresponding filter docs.

see #26240.

@SergeyBiryukov11 months ago

comment:12 @SergeyBiryukov11 months ago

  • Keywords has-patch added; needs-patch removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

This test currently fails:

1) Tests_Comment_Template::test_get_comments_number
Trying to get property of non-object

S:\home\wordpress\develop\src\wp-includes\comment-template.php:701
S:\home\wordpress\develop\tests\phpunit\tests\comment\template.php:10

26240.3.diff fixes it.

comment:13 @SergeyBiryukov11 months ago

  • Keywords commit added

@wonderboymusic11 months ago

comment:14 @wonderboymusic11 months ago

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

In 28638:

Fix an error caused by [28604] in get_comments_number() unit tests.

Props SergeyBiryukov.
Fixes #26240.

Note: See TracTickets for help on using tickets.