Skip to content

Fix type of parent ID when creating a response to a parent comment#2195

Closed
mike-lei wants to merge 1 commit intowp-graphql:developfrom
mike-lei:comment-parent-id-type
Closed

Fix type of parent ID when creating a response to a parent comment#2195
mike-lei wants to merge 1 commit intowp-graphql:developfrom
mike-lei:comment-parent-id-type

Conversation

@mike-lei
Copy link
Copy Markdown

What does this implement/fix? Explain your changes.

I was trying to create a comment under another comment (i.e. reply to a comment), but it looks like setting parent in createComment's input field does not work at all, the comment is posted directly under the post, not a response to another comment.
Changed the type of parent to Int and the issue was gone.
According to https://developer.wordpress.org/reference/functions/wp_new_comment/, when creating a new comment with parent ID, comment_parent field should be integer, not ID/string. Currently we're taking parent argument as ID.

Does this close any currently open issues?

No

Any relevant logs, error output, GraphiQL screenshots, etc?

There's no error message for this bug.
The following command would post the comment directly under post 3538 (parent does not work):

mutation CREATE_COMMENT {
  createComment(
    input: {commentOn: 3538 (Post ID), content: "This is a test comment, yo", author: "Jason", authorEmail: "xyz@gmail.com", parent: "Y29tbWVudDoxMDQ1"  (Parent Comment ID)}
  ) {
    success
  }
}

The following command would post a response under comment "Y29tbWVudDoxMDQ1" / 1045 after I changed type of parent to Int:

mutation CREATE_COMMENT {
  createComment(
    input: {commentOn: 3538 (Post ID), content: "This is a test comment, yo", author: "Jason", authorEmail: "xyz@gmail.com", parent: 1045  (Parent Comment ID)}
  ) {
    success
  }
}

Any other comments?

First time creating a PR to an open source project, please let me know if I need to change anything, thanks!

Where has this been tested?

Operating System: Windows 11

WordPress Version: 5.8.2

@qlty-cloud-legacy
Copy link
Copy Markdown

Code Climate has analyzed commit ce3db83 and detected 0 issues on this pull request.

View more on Code Climate.

@mike-lei mike-lei changed the title Fix type of parent ID when creating a child to a comment Fix type of parent ID when creating a response to a parent comment Dec 29, 2021
@spencersmb
Copy link
Copy Markdown

spencersmb commented Jan 1, 2022

+1 Having an issue with trying to post a comment as a reply to a parent comment. I'm trying to pass the comment ID (string) as the parent, so the current comment I'm trying to submit will be set as a "reply" instead of its own comment but it isn't working....

@justlevine justlevine added the needs: tests Tests should be added to be sure this works as expected label Apr 2, 2022
@justlevine
Copy link
Copy Markdown
Collaborator

This is working locally for me, but we should probably add a wpunit test for creating child comments.

@justlevine
Copy link
Copy Markdown
Collaborator

On further review (#997), this seems to be the wrong approach.
The type should be ID, and you should be feeding it the WP database ID (as either a String or an Int).

Rather, we need to (plugin wide) either indicate that most where args expect a databaseId and not a relay ID or ideally create a helper function that accepts either a global id or db id and resolve it to the int db id.

@justlevine justlevine added invalid? Something is not right here compat: breaking change This is a breaking change to existing functionality needs: reviewer response This needs the attention of a codeowner or maintainer component: connections Relating to GraphQL Connections labels Apr 2, 2022
@jasonbahl
Copy link
Copy Markdown
Collaborator

closing in favor of #2328

@jasonbahl jasonbahl closed this May 3, 2022
@mike-lei mike-lei deleted the comment-parent-id-type branch May 3, 2022 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

compat: breaking change This is a breaking change to existing functionality component: connections Relating to GraphQL Connections invalid? Something is not right here needs: reviewer response This needs the attention of a codeowner or maintainer needs: tests Tests should be added to be sure this works as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants