Opened 11 years ago
Last modified 10 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 (24)
#2
@
10 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
@
10 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
@
10 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
@
10 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
@
10 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
@
9 years ago
- Keywords needs-refresh added
- Milestone changed from Awaiting Review to Future Release
Please combine/refresh the patches for src/tests.
#13
@
9 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
@
9 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
@
8 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.
#16
@
8 years ago
- Keywords close added
There is no reason why a Comment must have a relationship with a Post. I see this as a wontfix
.
#17
@
8 years ago
Currently comments have a relationship with a post. All functions are there to retrieve comments based on the post ID. So closing this ticket without doing nothing feels wrong to me.
#18
@
10 months ago
I'm inclined to agree with @rachelbaker here. While most cases involve a comment with a relationship to a post, it's entirely possible that the desired behaviour is that a comment could be inserted without having a relationship with a post.
Some thoughts:
- "Comments" is its own menu item. It's not nested under "Posts", "Pages", or both.
- Most comment functions that deal with a post ID are within
comment-template.php
, which is specifically for use within the loop. - For comments that are post-independent, a relationship to a post is not desired. If a relationship to a post was required by WP, deleting that post would also delete its comments.
- As an example, a contact form can be created by adding a form to a page that submits comments. This doesn't need a plugin that uses custom post types and such, as the Comments API provides all that's needed (email, custom comment types, etc). For this type of available usage, there's no need for a relationship to a post.
Simplify construction of commentdata array for insertion with defaults