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 21 months ago.
26240-tests.diff (1.3 KB) - added by JanHenkG 20 months ago.
26240.2.diff (715 bytes) - added by JanHenkG 20 months ago.
26240.3.diff (756 bytes) - added by SergeyBiryukov 15 months ago.
26240.4.diff (738 bytes) - added by wonderboymusic 15 months ago.

Download all attachments as: .zip

Change History (19)

@coffee2code21 months ago

comment:1 @nofearinc21 months ago

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

@JanHenkG20 months ago

@JanHenkG20 months ago

comment:2 @JanHenkG20 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 @coffee2code15 months ago

Patches still apply cleanly. Seems like an easy win.

comment:4 @DrewAPicture15 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 @wonderboymusic15 months ago

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

[28558] didn't paste itself here.

comment:6 @ocean9015 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 @ocean9015 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:8 follow-up: @ocean9015 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 @DrewAPicture15 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 @DrewAPicture15 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 @ocean9015 months ago

In 28604:

Pass $post->ID to get_comments_number filter.

Also fixes indentation and a typo in corresponding filter docs.

see #26240.

@SergeyBiryukov15 months ago

comment:12 @SergeyBiryukov15 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 @SergeyBiryukov15 months ago

  • Keywords commit added

@wonderboymusic15 months ago

comment:14 @wonderboymusic15 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.