Missing buttons and errors when deleting or editing a comment

[StanleyGoldman]

• Correctly utilizing an ObservableAsPropertyHelper
  • As part of a last minute fix to Adding Edit/Delete functions to inline comments #1701 I attempted to use ToProperty but I completely failed on the follow through.
• Standalone comments made by the rest api do not return a node Id, the lack of this data results in the deletion or editing of standalone comments fail in error, in some cases.
  • We need to use Graphql to make standalone comments. In order to fix that some code from Use GraphQL for Pull Request Models. #1712 was ported over

[meaghanlewis]

I'm not able to update comments

and see this error in the log:

2018-06-08 14:56:50.896 [04640] EROR [01] CommentViewModel          Error posting comment
System.AggregateException: One or more errors occurred. ---> Octokit.GraphQL.Core.GraphQLQueryException: Argument 'input' on Field 'updatePullRequestReviewComment' has an invalid value. Expected type 'UpdatePullRequestReviewCommentInput!'.
   --- End of inner exception stack trace ---
   at Octokit.GraphQL.Core.Deserializers.ResponseDeserializer.Deserialize[TResult](CompiledQuery`1 query, JObject data)
   at Octokit.GraphQL.Core.Deserializers.ResponseDeserializer.Deserialize[TResult](CompiledQuery`1 query, String data)
   at Octokit.GraphQL.Connection.<Run>d__14`1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task)
   at GitHub.InlineReviews.Services.PullRequestSessionService.<EditComment>d__31.MoveNext() in C:\projects\visualstudio\src\GitHub.InlineReviews\Services\PullRequestSessionService.cs:line 656
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at GitHub.InlineReviews.Services.PullRequestSession.<EditComment>d__21.MoveNext() in C:\projects\visualstudio\src\GitHub.InlineReviews\Services\PullRequestSession.cs:line 183
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at GitHub.InlineReviews.ViewModels.InlineCommentThreadViewModel.<DoEditComment>d__6.MoveNext() in C:\projects\visualstudio\src\GitHub.InlineReviews\ViewModels\InlineCommentThreadViewModel.cs:line 85
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at GitHub.InlineReviews.ViewModels.CommentViewModel.<DoCommitEdit>d__15.MoveNext() in C:\projects\visualstudio\src\GitHub.InlineReviews\ViewModels\CommentViewModel.cs:line 171
---> (Inner Exception #0) Octokit.GraphQL.Core.GraphQLQueryException: Argument 'input' on Field 'updatePullRequestReviewComment' has an invalid value. Expected type 'UpdatePullRequestReviewCommentInput!'.<---

---> (Inner Exception #1) Octokit.GraphQL.Core.GraphQLQueryException: Argument 'pullRequestReviewCommentId' on InputObject 'UpdatePullRequestReviewCommentInput' has an invalid value. Expected type 'ID!'.<---

[shana]

Could you elaborate on why this needs to be created and why the TODO comment (I'm totally clueless on this part of the code)?

[grokys]

I think this is where Stanley got stuck. The problem is that we're currently mixing GraphQL and REST queries and each of them has a different concept of ID, so we currently need both. #1712 will fix this properly by moving to GQL for all this stuff, but in the interim I think we can convert the two "Add Comment" methods that currently use REST, to using GQL; though will be a bit messy because there's currently no way to actually do this in a single GQL mutation (this is what #1712 has to do too)

[StanleyGoldman]

Apologies for the lack of communication here. I left my status with this in Slack.

@grokys is correct. I migrated some of his code from #1712 in order to fill the gaps.

[meaghanlewis]

@StanleyGoldman I can edit/delete new comments I've made right away, but I noticed a few other things:

• I am unable to edit existing comments - no edit/delete icons shows
  

• New single comments are added as Pending
  

• And I think because they are pending, when I try to to reply to single comments, that fails:
  

[grokys]

As regards @meaghanlewis issues:

I am unable to edit existing comments - no edit/delete icons shows

I'm not seeing this. Not sure what is needed to repro.

New single comments are added as Pending

This is quite simple: IsPending is erroneously being set to true here and needs to be set to false after this call

when I try to to reply to single comments, that fails:

This is because Database IDs are being used as Node IDs here

TIP: Don't just call ToString() on IDs, this is never the right thing to do.

[StanleyGoldman]

Thanks @grokys for finding the errors. Yea, the ToString() was a straggler from after the first time I learned the lesson.

[StanleyGoldman]

@meaghanlewis I also am unable to repro the unable to missing delete button issue, I can pair with you on it today.

[meaghanlewis]

So for the comment I complained about with the missing edit/delete button that was my fault 😞 i was using a test account ghfvs-tester and trying to edit a comment made by meaghanlewis. My bad.

[meaghanlewis]

@StanleyGoldman this is looking good, but 2 more things.

1. Replies made to comments are listed as Pending temporarily.
   

2. I'm unable to delete Pending comments.
   

I'm not sure if this is getting to be out of scope for this particular fix, so I'm okay with opening separate issues for these as well. What do you think?

[StanleyGoldman]

@meaghanlewis I replicated and fixed the first issue, replying to standalone comments should not show as pending anymore.

As far as your second issue goes, I added functionality to display an error directly in that view. But I do not know how to fix the error you found. I'm going to try again tomorrow morning with a fresh brain.

Also it seems you were able to identify another bug with @jcansdale in Slack last night. Would you happen to know how to reproduce the error.

[grokys]

I suspect the inability to delete pending comments is a problem with the REST API, in that it refuses to accept the existence of pending comments. This is a problem because there's no way in GraphQL to delete comments.

We spoke to someone on the API ecosystem team last week who suggested that the REST API not working with pending comments was in error, so my feeling is that we should work with them to get this fixed.

[grokys]

Are you sure this is right? This method is PostPendingReviewCommentReply, so doesn't this need to be set to pending?

[StanleyGoldman]

Thanks