Opened 8 years ago
Last modified 23 months ago
#23931 new defect (bug)
wp_insert_comment should require comment_post_ID
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 2.0 |
Component: | Comments | Keywords: | needs-patch close |
Focuses: | Cc: |
Description (last modified by )
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 (6)
Change History (23)
#2
@
8 years 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
@
8 years 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
#3
@
8 years 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
#4
@
8 years 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
#5
@
8 years 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.
#7
@
7 years ago
- Keywords needs-refresh added
- Milestone changed from Awaiting Review to Future Release
Please combine/refresh the patches for src/tests.
#13
@
6 years ago
- Keywords needs-testing dev-feedback added; needs-refresh removed
I've refreshed the patch, but there were some test failures that needed to be addressed after the update. 3 tests failed due to a WP_Error
being returned and 1 failed asserting that an array is not empty, because you can't create a comment without a comment_post_ID
anymore. I added the ID and all tests pass again.
#14
@
6 years ago
- Milestone changed from 4.2 to Future Release
Punting to future release, this needs more discussion before being ready for a commit.
#15
@
6 years ago
- Keywords needs-patch added; has-patch needs-testing dev-feedback removed
We can't start returning WP_Error
objects from wp_insert_comment()
. It'll break any plugin doing something like
$comment_id = wp_insert_comment( $args ); if ( ! $comment_id ) { $weep = true; }
If we want to have some more elegant and informative error handling, we'll need to do it in a new function, and turn wp_insert_comment()
into a wrapper for it.
Two other possible courses of action: (1) Simply return false when no comment_post_ID is present. (2) Do nothing, and leave it as-is, since it doesn't look like there's a functional bug here.
Simplify construction of commentdata array for insertion with defaults