Context Navigation

← Previous Ticket
Next Ticket →

#23931 new defect (bug)

wp_insert_comment should require comment_post_ID

Reported by:	markoheijnen	Owned by:
Milestone:	4.2	Priority:	normal
Severity:	normal	Version:	2.0
Component:	Comments	Keywords:	has-patch needs-refresh
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 (5)

comment.php.1.diff (1.9 KB) - added by westonruter 19 months ago.: Simplify construction of commentdata array for insertion with defaults
tests-comment-query.1.php (1.2 KB) - added by westonruter 19 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 19 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 19 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
23931.diff (5.2 KB) - added by westonruter 8 months ago.: Refreshed patch. PR: https://github.com/x-team/wordpress-develop/pull/25

Download all attachments as: .zip

Change History (17)

comment:1 @markoheijnen — 2 years ago

Description modified (diff)

@westonruter — 19 months ago

Attachment comment.php.1.diff added

Simplify construction of commentdata array for insertion with defaults

comment:2 @westonruter — 19 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

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

@westonruter — 19 months ago

Attachment tests-comment-query.1.php added

Update unit tests to always create comments with a valid comment_post_ID

@westonruter — 19 months ago

Attachment comment.php.2.diff added

Return WP_Error from wp_insert_comment if invalid comment_post_ID inserted

@westonruter — 19 months ago

Attachment tests-comment-query.2.php added

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:3 @westonruter — 19 months ago

Version set to 2.0

All patches pushed to branch on GitHub: https://github.com/x-team/WordPress/compare/87f9d11...23931-require-comment_post_ID

comment:4 @westonruter — 19 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 @jdgrimes — 18 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.