WordPress.org

Make WordPress Core

Opened 13 months ago

Last modified 8 months ago

#23931 new defect (bug)

wp_insert_comment should require comment_post_ID

Reported by: markoheijnen Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 2.0
Component: Comments Keywords: has-patch
Focuses: Cc:

Description (last modified by markoheijnen)

At this moment there is no check for example comment_post_ID. Not sure if there are more checks needed. Reason I asked are a few notices on the unit tests caused by WP_UnitTest_Factory_For_Comment. Those comments don't add a post ID what should change.

Currently, if a null comment_post_ID is passed, the comment isn't connected to a post. This can create confusion. Also Unit tests should run with WP_Debug on.

Attachments (4)

comment.php.1.diff (1.9 KB) - added by westonruter 9 months ago.
Simplify construction of commentdata array for insertion with defaults
tests-comment-query.1.php (1.2 KB) - added by westonruter 9 months ago.
Update unit tests to always create comments with a valid comment_post_ID
comment.php.2.diff (2.5 KB) - added by westonruter 9 months ago.
Return WP_Error from wp_insert_comment if invalid comment_post_ID inserted
tests-comment-query.2.php (2.5 KB) - added by westonruter 9 months ago.
Also add assertions for unit tests to check for returned WP_Error instance when invalid comment_post_ID is supplied, and test the wp_insert_comment_data filter

Download all attachments as: .zip

Change History (10)

comment:1 markoheijnen13 months ago

  • Description modified (diff)

westonruter9 months ago

Simplify construction of commentdata array for insertion with defaults

comment:2 westonruter9 months ago

  • Cc weston@… added
  • Keywords has-patch added; needs-patch removed

By refactoring the way that the commentdata is prepared for insertion into the DB, it now merges the passed-in data on top of a defaults array that includes a comment_post_ID, so there will be no undefined index.

Patch also includes a wp_insert_comment_data filter before insert to accompany the wp_insert_comment action which happens after insert.

See first attached patch, or see from branch on GitHub: https://github.com/x-team/WordPress/compare/87f9d11...7588a21

Version 1, edited 9 months ago by westonruter (previous) (next) (diff)

westonruter9 months ago

Update unit tests to always create comments with a valid comment_post_ID

westonruter9 months ago

Return WP_Error from wp_insert_comment if invalid comment_post_ID inserted

westonruter9 months ago

Also add assertions for unit tests to check for returned WP_Error instance when invalid comment_post_ID is supplied, and test the wp_insert_comment_data filter

comment:4 westonruter9 months ago

BTW, the need for a wp_insert_comment_data filter came up while doing work on the Liveblog plugin: https://github.com/x-team/wp-liveblog/commit/94958de51f786e4eed6389cc0c3124fcdb8c5c29#L4R458

comment:5 jdgrimes8 months ago

Something does need to happen here. At the very least, the tests should set comment_post_ID to 0 by default. That will keep it from erroring. If we're going to force comment_post_ID to be valid though, we should make WP_UnitTest_Factory_For_Comment automatically create a post if comment_post_ID isn't set. I think that makes more sense then having to create one each time like in your patch.

comment:6 jdgrimes8 months ago

  • Cc jdg@… added
Note: See TracTickets for help on using tickets.