wp_insert_comment should require comment_post_ID

[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.

[westonruter]

Simplify construction of commentdata array for insertion with defaults

[westonruter]

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

[westonruter]

Update unit tests to always create comments with a valid comment_post_ID

[westonruter]

Return WP_Error from wp_insert_comment if invalid comment_post_ID inserted

[westonruter]

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

[westonruter]

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

[westonruter]

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

[jdgrimes]

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.

[wonderboymusic]

Please combine/refresh the patches for src/tests.

[westonruter]

Refreshed patch. PR: ​https://github.com/x-team/wordpress-develop/pull/25

[SergeyBiryukov]

#30944 was marked as a duplicate.

[SergeyBiryukov]

Related: #30946

[DrewAPicture]

Latest patch needs a refresh.

[valendesigns]

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.

[rachelbaker]

Punting to future release, this needs more discussion before being ready for a commit.

[boonebgorges]

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.

[rachelbaker]

There is no reason why a Comment must have a relationship with a Post. I see this as a wontfix.

[markoheijnen]

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.

[costdev]

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.

[callumbw95]

Hi @markoheijnen,

As there hasn't been much movement on this ticket in a good while now, I am going to mark this ticket as wontfix. However please feel free to reopen and continue the conversation further if there is more conversation to be had here. 😃