Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Commit 1a47482

Browse files
grokysgrokys
authored
Merge pull request #1723 from github/fixes/correctly-using-property-helper
Missing buttons and errors when deleting or editing a comment
2 parents 165a97b + 98e2491 commit 1a47482

File tree

10 files changed

+108
-96
lines changed

10 files changed

+108
-96
lines changed

‎src/GitHub.Exports.Reactive/Services/IPullRequestSession.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,12 +109,10 @@ Task<IPullRequestReviewCommentModel> PostReviewComment(
109109
/// Posts a PR review comment reply.
110110
/// </summary>
111111
/// <param name="body">The comment body.</param>
112-
/// <param name="inReplyTo">The REST ID of the comment to reply to.</param>
113112
/// <param name="inReplyToNodeId">The GraphQL ID of the comment to reply to.</param>
114113
/// <returns>A comment model.</returns>
115114
Task<IPullRequestReviewCommentModel> PostReviewComment(
116115
string body,
117-
int inReplyTo,
118116
string inReplyToNodeId);
119117

120118
/// <summary>

‎src/GitHub.InlineReviews/Services/IPullRequestSessionService.cs‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -260,9 +260,8 @@ Task<IPullRequestReviewCommentModel> PostPendingReviewCommentReply(
260260
/// Posts a new standalone PR review comment.
261261
/// </summary>
262262
/// <param name="localRepository">The local repository.</param>
263-
/// <param name="remoteRepositoryOwner">The owner of the repository fork to post to.</param>
264263
/// <param name="user">The user posting the comment.</param>
265-
/// <param name="number">The pull request number.</param>
264+
/// <param name="pullRequestNodeId">The pull request node id.</param>
266265
/// <param name="body">The comment body.</param>
267266
/// <param name="commitId">THe SHA of the commit to comment on.</param>
268267
/// <param name="path">The relative path of the file to comment on.</param>
@@ -272,11 +271,9 @@ Task<IPullRequestReviewCommentModel> PostPendingReviewCommentReply(
272271
/// The method posts a new standalone pull request comment that is not attached to a pending
273272
/// pull request review.
274273
/// </remarks>
275-
Task<IPullRequestReviewCommentModel> PostStandaloneReviewComment(
276-
ILocalRepositoryModel localRepository,
277-
string remoteRepositoryOwner,
274+
Task<IPullRequestReviewCommentModel> PostStandaloneReviewComment(ILocalRepositoryModel localRepository,
278275
IAccount user,
279-
int number,
276+
string pullRequestNodeId,
280277
string body,
281278
string commitId,
282279
string path,
@@ -286,19 +283,16 @@ Task<IPullRequestReviewCommentModel> PostStandaloneReviewComment(
286283
/// Posts a PR review comment reply.
287284
/// </summary>
288285
/// <param name="localRepository">The local repository.</param>
289-
/// <param name="remoteRepositoryOwner">The owner of the repository fork to post to.</param>
290286
/// <param name="user">The user posting the comment.</param>
291-
/// <param name="number">The pull request number.</param>
287+
/// <param name="pullRequestNodeId">The pull request node id.</param>
292288
/// <param name="body">The comment body.</param>
293-
/// <param name="inReplyTo">The comment ID to reply to.</param>
289+
/// <param name="inReplyToNodeId">The comment node id to reply to.</param>
294290
/// <returns>A model representing the posted comment.</returns>
295-
Task<IPullRequestReviewCommentModel> PostStandaloneReviewCommentReply(
296-
ILocalRepositoryModel localRepository,
297-
string remoteRepositoryOwner,
291+
Task<IPullRequestReviewCommentModel> PostStandaloneReviewCommentReply(ILocalRepositoryModel localRepository,
298292
IAccount user,
299-
int number,
293+
string pullRequestNodeId,
300294
string body,
301-
int inReplyTo);
295+
string inReplyToNodeId);
302296

303297
/// <summary>
304298
/// Delete a PR review comment.

‎src/GitHub.InlineReviews/Services/PullRequestSession.cs‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -138,11 +138,11 @@ public async Task<IPullRequestReviewCommentModel> PostReviewComment(
138138

139139
if (!HasPendingReview)
140140
{
141+
var pullRequestNodeId = await GetPullRequestNodeId();
141142
model = await service.PostStandaloneReviewComment(
142143
LocalRepository,
143-
RepositoryOwner,
144144
User,
145-
PullRequest.Number,
145+
pullRequestNodeId,
146146
body,
147147
commitId,
148148
path,
@@ -194,20 +194,19 @@ public async Task<IPullRequestReviewCommentModel> EditComment(string commentNode
194194
/// <inheritdoc/>
195195
public async Task<IPullRequestReviewCommentModel> PostReviewComment(
196196
string body,
197-
int inReplyTo,
198197
string inReplyToNodeId)
199198
{
200199
IPullRequestReviewCommentModel model;
201200

202201
if (!HasPendingReview)
203202
{
203+
var pullRequestNodeId = await GetPullRequestNodeId();
204204
model = await service.PostStandaloneReviewCommentReply(
205205
LocalRepository,
206-
RepositoryOwner,
207206
User,
208-
PullRequest.Number,
207+
pullRequestNodeId,
209208
body,
210-
inReplyTo);
209+
inReplyToNodeId);
211210
}
212211
else
213212
{

‎src/GitHub.InlineReviews/Services/PullRequestSessionService.cs‎

Lines changed: 49 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -528,77 +528,73 @@ public async Task<IPullRequestReviewCommentModel> PostPendingReviewCommentReply(
528528
/// <inheritdoc/>
529529
public async Task<IPullRequestReviewCommentModel> PostStandaloneReviewComment(
530530
ILocalRepositoryModel localRepository,
531-
string remoteRepositoryOwner,
532531
IAccount user,
533-
int number,
532+
string pullRequestNodeId,
534533
string body,
535534
string commitId,
536535
string path,
537536
int position)
538537
{
539538
var address = HostAddress.Create(localRepository.CloneUrl.Host);
540-
var apiClient = await apiClientFactory.Create(address);
541-
542-
var result = await apiClient.CreatePullRequestReviewComment(
543-
remoteRepositoryOwner,
544-
localRepository.Name,
545-
number,
546-
body,
547-
commitId,
548-
path,
549-
position);
550-
551-
await usageTracker.IncrementCounter(x => x.NumberOfPRReviewDiffViewInlineCommentPost);
539+
var graphql = await graphqlFactory.CreateConnection(address);
552540

553-
return new PullRequestReviewCommentModel
541+
var addReview = new AddPullRequestReviewInput
554542
{
555-
Body = result.Body,
556-
CommitId = result.CommitId,
557-
DiffHunk = result.DiffHunk,
558-
Id = result.Id,
559-
OriginalCommitId = result.OriginalCommitId,
560-
OriginalPosition = result.OriginalPosition,
561-
Path = result.Path,
562-
Position = result.Position,
563-
CreatedAt = result.CreatedAt,
564-
User = user,
543+
Body = body,
544+
CommitOID = commitId,
545+
Event = Octokit.GraphQL.Model.PullRequestReviewEvent.Comment,
546+
PullRequestId = pullRequestNodeId,
547+
Comments = new[]
548+
{
549+
new DraftPullRequestReviewComment
550+
{
551+
Body = body,
552+
Path = path,
553+
Position = position,
554+
},
555+
},
565556
};
557+
558+
var mutation = new Mutation()
559+
.AddPullRequestReview(addReview)
560+
.Select(payload =>
561+
payload.PullRequestReview
562+
.Comments(1, null, null, null)
563+
.Nodes.Select(x => new PullRequestReviewCommentModel
564+
{
565+
Id = x.DatabaseId.Value,
566+
NodeId = x.Id,
567+
Body = x.Body,
568+
CommitId = x.Commit.Oid,
569+
Path = x.Path,
570+
Position = x.Position,
571+
CreatedAt = x.CreatedAt.Value,
572+
DiffHunk = x.DiffHunk,
573+
OriginalPosition = x.OriginalPosition,
574+
OriginalCommitId = x.OriginalCommit.Oid,
575+
PullRequestReviewId = x.PullRequestReview.DatabaseId.Value,
576+
User = user,
577+
IsPending = false
578+
}));
579+
580+
var result = (await graphql.Run(mutation)).First();
581+
await usageTracker.IncrementCounter(x => x.NumberOfPRReviewDiffViewInlineCommentPost);
582+
return result;
566583
}
567584

568585
/// <inheritdoc/>
569586
public async Task<IPullRequestReviewCommentModel> PostStandaloneReviewCommentReply(
570587
ILocalRepositoryModel localRepository,
571-
string remoteRepositoryOwner,
572588
IAccount user,
573-
int number,
589+
string pullRequestNodeId,
574590
string body,
575-
int inReplyTo)
591+
string inReplyToNodeId)
576592
{
577-
var address = HostAddress.Create(localRepository.CloneUrl.Host);
578-
var apiClient = await apiClientFactory.Create(address);
579-
580-
var result = await apiClient.CreatePullRequestReviewComment(
581-
remoteRepositoryOwner,
582-
localRepository.Name,
583-
number,
584-
body,
585-
inReplyTo);
586-
587-
await usageTracker.IncrementCounter(x => x.NumberOfPRReviewDiffViewInlineCommentPost);
588-
589-
return new PullRequestReviewCommentModel
590-
{
591-
Body = result.Body,
592-
CommitId = result.CommitId,
593-
DiffHunk = result.DiffHunk,
594-
Id = result.Id,
595-
OriginalCommitId = result.OriginalCommitId,
596-
OriginalPosition = result.OriginalPosition,
597-
Path = result.Path,
598-
Position = result.Position,
599-
CreatedAt = result.CreatedAt,
600-
User = user,
601-
};
593+
var review = await CreatePendingReview(localRepository, user, pullRequestNodeId);
594+
var comment = await PostPendingReviewCommentReply(localRepository, user, review.NodeId, body, inReplyToNodeId);
595+
await SubmitPendingReview(localRepository, user, review.NodeId, null, PullRequestReviewEvent.Comment);
596+
comment.IsPending = false;
597+
return comment;
602598
}
603599

604600
/// <inheritdoc/>

‎src/GitHub.InlineReviews/ViewModels/CommentViewModel.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public class CommentViewModel : ReactiveObject, ICommentViewModel
2424
CommentEditState state;
2525
DateTimeOffset updatedAt;
2626
string undoBody;
27-
bool canDelete;
27+
ObservableAsPropertyHelper<bool> canDelete;
2828

2929
/// <summary>
3030
/// Initializes a new instance of the <see cref="CommentViewModel"/> class.
@@ -61,13 +61,13 @@ protected CommentViewModel(
6161
User = user;
6262
UpdatedAt = updatedAt;
6363

64-
var canDelete = this.WhenAnyValue(
64+
var canDeleteObservable = this.WhenAnyValue(
6565
x => x.EditState,
6666
x => x == CommentEditState.None && user.Login.Equals(currentUser.Login));
6767

68-
canDelete.ToProperty(this, x => x.CanDelete);
68+
canDelete = canDeleteObservable.ToProperty(this, x => x.CanDelete);
6969

70-
Delete = ReactiveCommand.CreateAsyncTask(canDelete, DoDelete);
70+
Delete = ReactiveCommand.CreateAsyncTask(canDeleteObservable, DoDelete);
7171

7272
var canEdit = this.WhenAnyValue(
7373
x => x.EditState,
@@ -137,6 +137,7 @@ void DoBeginEdit(object unused)
137137
{
138138
if (state != CommentEditState.Editing)
139139
{
140+
ErrorMessage = null;
140141
undoBody = Body;
141142
EditState = CommentEditState.Editing;
142143
}
@@ -231,8 +232,7 @@ public bool IsSubmitting
231232

232233
public bool CanDelete
233234
{
234-
get { return canDelete; }
235-
private set { this.RaiseAndSetIfChanged(ref canDelete, value); }
235+
get { return canDelete.Value; }
236236
}
237237

238238
/// <inheritdoc/>

‎src/GitHub.InlineReviews/ViewModels/InlineCommentThreadViewModel.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,8 @@ async Task<ICommentModel> DoPostComment(object parameter)
7272
Guard.ArgumentNotNull(parameter, nameof(parameter));
7373

7474
var body = (string)parameter;
75-
var replyId = Comments[0].Id;
7675
var nodeId = Comments[0].NodeId;
77-
return await Session.PostReviewComment(body, replyId, nodeId);
76+
return await Session.PostReviewComment(body, nodeId);
7877
}
7978

8079
async Task<ICommentModel> DoEditComment(object parameter)

‎src/GitHub.InlineReviews/Views/CommentView.xaml‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,23 @@
9494
Margin="0 2 0 0"
9595
Foreground="{DynamicResource VsBrush.WindowText}"
9696
Markdown="{Binding Body}"/>
97+
98+
<DockPanel Grid.Column="1" Grid.Row="2"
99+
Margin="0 4"
100+
HorizontalAlignment="Left"
101+
TextBlock.Foreground="Red">
102+
<DockPanel.Style>
103+
<Style TargetType="FrameworkElement">
104+
<Style.Triggers>
105+
<DataTrigger Binding="{Binding ErrorMessage}" Value="{x:Null}">
106+
<Setter Property="Visibility" Value="Collapsed"/>
107+
</DataTrigger>
108+
</Style.Triggers>
109+
</Style>
110+
</DockPanel.Style>
111+
<ui:OcticonImage DockPanel.Dock="Left" Icon="alert" Margin="0 0 4 0"/>
112+
<TextBlock Text="{Binding ErrorMessage}" TextWrapping="Wrap"/>
113+
</DockPanel>
97114
</StackPanel>
98115

99116
<!-- Displays edit view or a reply placeholder-->

‎test/GitHub.InlineReviews.UnitTests/Services/PullRequestSessionTests.cs‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ namespace GitHub.InlineReviews.UnitTests.Services
2020
public class PullRequestSessionTests
2121
{
2222
const int PullRequestNumber = 5;
23+
const string PullRequestNodeId = "pull_request_id";
2324
const string RepoUrl = "https://foo.bar/owner/repo";
2425
const string FilePath = "test.cs";
2526

@@ -514,16 +515,15 @@ public class ThePostReviewCommentMethod
514515
[Test]
515516
public async Task PostsToCorrectForkWithNoPendingReview()
516517
{
517-
var service = Substitute.For<IPullRequestSessionService>();
518+
var service = CreateService();
518519
var target = CreateTarget(service, "fork", "owner", false);
519520

520521
await target.PostReviewComment("New Comment", "COMMIT_ID", "file.cs", new DiffChunk[0], 1);
521522

522523
await service.Received(1).PostStandaloneReviewComment(
523524
target.LocalRepository,
524-
"owner",
525525
target.User,
526-
PullRequestNumber,
526+
PullRequestNodeId,
527527
"New Comment",
528528
"COMMIT_ID",
529529
"file.cs",
@@ -533,24 +533,23 @@ await service.Received(1).PostStandaloneReviewComment(
533533
[Test]
534534
public async Task PostsReplyToCorrectForkWithNoPendingReview()
535535
{
536-
var service = Substitute.For<IPullRequestSessionService>();
536+
var service = CreateService();
537537
var target = CreateTarget(service, "fork", "owner", false);
538538

539-
await target.PostReviewComment("New Comment", 1, "node1");
539+
await target.PostReviewComment("New Comment", "node1");
540540

541541
await service.Received(1).PostStandaloneReviewCommentReply(
542542
target.LocalRepository,
543-
"owner",
544543
target.User,
545-
PullRequestNumber,
544+
PullRequestNodeId,
546545
"New Comment",
547-
1);
546+
"node1");
548547
}
549548

550549
[Test]
551550
public async Task PostsToCorrectForkWithPendingReview()
552551
{
553-
var service = Substitute.For<IPullRequestSessionService>();
552+
var service = CreateService();
554553
var target = CreateTarget(service, "fork", "owner", true);
555554

556555
await target.PostReviewComment("New Comment", "COMMIT_ID", "file.cs", new DiffChunk[0], 1);
@@ -568,10 +567,10 @@ await service.Received(1).PostPendingReviewComment(
568567
[Test]
569568
public async Task PostsReplyToCorrectForkWithPendingReview()
570569
{
571-
var service = Substitute.For<IPullRequestSessionService>();
570+
var service = CreateService();
572571
var target = CreateTarget(service, "fork", "owner", true);
573572

574-
await target.PostReviewComment("New Comment", 1, "node1");
573+
await target.PostReviewComment("New Comment", "node1");
575574

576575
await service.Received(1).PostPendingReviewCommentReply(
577576
target.LocalRepository,
@@ -581,6 +580,16 @@ await service.Received(1).PostPendingReviewCommentReply(
581580
"node1");
582581
}
583582

583+
static IPullRequestSessionService CreateService()
584+
{
585+
var service = Substitute.For<IPullRequestSessionService>();
586+
587+
service.GetGraphQLPullRequestId(Arg.Any<ILocalRepositoryModel>(), Arg.Any<string>(), Arg.Any<int>())
588+
.Returns(PullRequestNodeId);
589+
590+
return service;
591+
}
592+
584593
PullRequestSession CreateTarget(
585594
IPullRequestSessionService service,
586595
string localRepositoryOwner,

0 commit comments

Comments
 (0)