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

Commit 81b3a12

Browse files
committed
Post comments to correct repo when on a fork.
If the user had a forked repo checked out and was viewing the parent repo's PRs, posting a comment was erroneously trying to post the comment to the _fork_ repo, resulting in a "Not Found" exception. Fixes #1195.
1 parent 673f06e commit 81b3a12

File tree

4 files changed

+71
-6
lines changed

4 files changed

+71
-6
lines changed

src/GitHub.InlineReviews/Services/IPullRequestSessionService.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ Task<byte[]> ExtractFileFromGit(
8686
/// Posts a new PR review comment.
8787
/// </summary>
8888
/// <param name="repository">The repository.</param>
89+
/// <param name="repositoryOwner">The owner of the repository fork to post to.</param>
8990
/// <param name="user">The user posting the comment.</param>
9091
/// <param name="number">The pull request number.</param>
9192
/// <param name="body">The comment body.</param>
@@ -95,6 +96,7 @@ Task<byte[]> ExtractFileFromGit(
9596
/// <returns>A model representing the posted comment.</returns>
9697
Task<IPullRequestReviewCommentModel> PostReviewComment(
9798
ILocalRepositoryModel repository,
99+
string repositoryOwner,
98100
IAccount user,
99101
int number,
100102
string body,
@@ -106,13 +108,15 @@ Task<IPullRequestReviewCommentModel> PostReviewComment(
106108
/// Posts a PR review comment reply.
107109
/// </summary>
108110
/// <param name="repository">The repository.</param>
111+
/// <param name="repositoryOwner">The owner of the repository fork to post to.</param>
109112
/// <param name="user">The user posting the comment.</param>
110113
/// <param name="number">The pull request number.</param>
111114
/// <param name="body">The comment body.</param>
112115
/// <param name="inReplyTo">The comment ID to reply to.</param>
113116
/// <returns>A model representing the posted comment.</returns>
114117
Task<IPullRequestReviewCommentModel> PostReviewComment(
115118
ILocalRepositoryModel repository,
119+
string repositoryOwner,
116120
IAccount user,
117121
int number,
118122
string body,

src/GitHub.InlineReviews/Services/PullRequestSession.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ public async Task<IPullRequestReviewCommentModel> PostReviewComment(string body,
121121
{
122122
var model = await service.PostReviewComment(
123123
LocalRepository,
124+
RepositoryOwner,
124125
User,
125126
PullRequest.Number,
126127
body,
@@ -136,6 +137,7 @@ public async Task<IPullRequestReviewCommentModel> PostReviewComment(string body,
136137
{
137138
var model = await service.PostReviewComment(
138139
LocalRepository,
140+
RepositoryOwner,
139141
User,
140142
PullRequest.Number,
141143
body,

src/GitHub.InlineReviews/Services/PullRequestSessionService.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ public async Task<string> GetPullRequestMergeBase(ILocalRepositoryModel reposito
138138
/// <inheritdoc/>
139139
public async Task<IPullRequestReviewCommentModel> PostReviewComment(
140140
ILocalRepositoryModel repository,
141+
string repositoryOwner,
141142
IAccount user,
142143
int number,
143144
string body,
@@ -149,7 +150,7 @@ public async Task<IPullRequestReviewCommentModel> PostReviewComment(
149150
var apiClient = await apiClientFactory.Create(address);
150151

151152
var result = await apiClient.CreatePullRequestReviewComment(
152-
repository.Owner,
153+
repositoryOwner,
153154
repository.Name,
154155
number,
155156
body,
@@ -177,6 +178,7 @@ public async Task<IPullRequestReviewCommentModel> PostReviewComment(
177178
/// <inheritdoc/>
178179
public async Task<IPullRequestReviewCommentModel> PostReviewComment(
179180
ILocalRepositoryModel repository,
181+
string repositoryOwner,
180182
IAccount user,
181183
int number,
182184
string body,
@@ -186,7 +188,7 @@ public async Task<IPullRequestReviewCommentModel> PostReviewComment(
186188
var apiClient = await apiClientFactory.Create(address);
187189

188190
var result = await apiClient.CreatePullRequestReviewComment(
189-
repository.Owner,
191+
repositoryOwner,
190192
repository.Name,
191193
number,
192194
body,

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

Lines changed: 61 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using GitHub.InlineReviews.Services;
77
using GitHub.InlineReviews.UnitTests.TestDoubles;
88
using GitHub.Models;
9+
using GitHub.Primitives;
910
using LibGit2Sharp;
1011
using NSubstitute;
1112
using Xunit;
@@ -511,6 +512,42 @@ Line 3 with comment
511512

512513
public class ThePostReviewCommentMethod
513514
{
515+
[Fact]
516+
public async Task PostsToCorrectFork()
517+
{
518+
var service = Substitute.For<IPullRequestSessionService>();
519+
var target = CreateTarget(service, "fork", "owner");
520+
521+
await target.PostReviewComment("New Comment", "COMMIT_ID", "file.cs", 1);
522+
523+
await service.Received(1).PostReviewComment(
524+
Arg.Any<ILocalRepositoryModel>(),
525+
"owner",
526+
Arg.Any<IAccount>(),
527+
PullRequestNumber,
528+
"New Comment",
529+
"COMMIT_ID",
530+
"file.cs",
531+
1);
532+
}
533+
534+
[Fact]
535+
public async Task PostsReplyToCorrectFork()
536+
{
537+
var service = Substitute.For<IPullRequestSessionService>();
538+
var target = CreateTarget(service, "fork", "owner");
539+
540+
await target.PostReviewComment("New Comment", 1);
541+
542+
await service.Received(1).PostReviewComment(
543+
Arg.Any<ILocalRepositoryModel>(),
544+
"owner",
545+
Arg.Any<IAccount>(),
546+
PullRequestNumber,
547+
"New Comment",
548+
1);
549+
}
550+
514551
[Fact]
515552
public async Task UpdatesFileWithNewThread()
516553
{
@@ -529,6 +566,26 @@ public async Task UpdatesFileWithNewThread()
529566
}
530567
}
531568

569+
PullRequestSession CreateTarget(
570+
IPullRequestSessionService service,
571+
string localRepositoryOwner,
572+
string remoteRepositoryOwner)
573+
{
574+
var repository = Substitute.For<ILocalRepositoryModel>();
575+
576+
repository.CloneUrl.Returns(new UriString($"https://github.com/{localRepositoryOwner}/reop"));
577+
repository.Owner.Returns(localRepositoryOwner);
578+
repository.Name.Returns("repo");
579+
580+
return new PullRequestSession(
581+
service,
582+
Substitute.For<IAccount>(),
583+
CreatePullRequest(),
584+
repository,
585+
remoteRepositoryOwner,
586+
true);
587+
}
588+
532589
async Task<PullRequestSession> CreateTarget(FakeDiffService diffService)
533590
{
534591
var baseContents = @"Line 1
@@ -652,10 +709,10 @@ static IPullRequestSessionService CreateService(FakeDiffService diffService)
652709
result.GetTipSha(Arg.Any<ILocalRepositoryModel>()).Returns("BRANCH_TIP");
653710

654711
var diffChunk = "@@ -1,4 +1,4 @@";
655-
result.PostReviewComment(null, null, 0, null, 0).ReturnsForAnyArgs(i =>
656-
CreateComment(diffChunk, i.ArgAt<string>(3)));
657-
result.PostReviewComment(null, null, 0, null, null, null, 0).ReturnsForAnyArgs(i =>
658-
CreateComment(diffChunk, i.ArgAt<string>(3)));
712+
result.PostReviewComment(null, null, null, 0, null, 0).ReturnsForAnyArgs(i =>
713+
CreateComment(diffChunk, i.ArgAt<string>(4)));
714+
result.PostReviewComment(null, null, null, 0, null, null, null, 0).ReturnsForAnyArgs(i =>
715+
CreateComment(diffChunk, i.ArgAt<string>(4)));
659716
return result;
660717
}
661718
}

0 commit comments

Comments
 (0)