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

Commit c1687a7

Browse files
jcansdalejcansdale
authored
Merge pull request #1198 from github/fixes/1195-comment-from-fork
Post comments to correct repository when on a fork.
2 parents 673f06e + ddeb44d commit c1687a7

File tree

7 files changed

+90
-16
lines changed

7 files changed

+90
-16
lines changed

‎src/GitHub.InlineReviews/GitHub.InlineReviews.csproj‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,10 @@
333333
<HintPath>..\..\packages\Microsoft.VisualStudio.Validation.14.1.111\lib\net45\Microsoft.VisualStudio.Validation.dll</HintPath>
334334
<Private>True</Private>
335335
</Reference>
336+
<Reference Include="NLog, Version=3.1.0.0, Culture=neutral, PublicKeyToken=5120e14c03d0593c, processorArchitecture=MSIL">
337+
<HintPath>..\..\packages\NLog.3.1.0.0\lib\net45\NLog.dll</HintPath>
338+
<Private>True</Private>
339+
</Reference>
336340
<Reference Include="PresentationCore" />
337341
<Reference Include="PresentationFramework" />
338342
<Reference Include="rothko, Version=0.0.2.0, Culture=neutral, PublicKeyToken=9f664c41f503810a, processorArchitecture=MSIL">

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,8 @@ Task<byte[]> ExtractFileFromGit(
8585
/// <summary>
8686
/// Posts a new PR review comment.
8787
/// </summary>
88-
/// <param name="repository">The repository.</param>
88+
/// <param name="localRepository">The local repository.</param>
89+
/// <param name="remoteRepositoryOwner">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>
@@ -94,7 +95,8 @@ Task<byte[]> ExtractFileFromGit(
9495
/// <param name="position">The line index in the diff to comment on.</param>
9596
/// <returns>A model representing the posted comment.</returns>
9697
Task<IPullRequestReviewCommentModel> PostReviewComment(
97-
ILocalRepositoryModel repository,
98+
ILocalRepositoryModel localRepository,
99+
string remoteRepositoryOwner,
98100
IAccount user,
99101
int number,
100102
string body,
@@ -105,14 +107,16 @@ Task<IPullRequestReviewCommentModel> PostReviewComment(
105107
/// <summary>
106108
/// Posts a PR review comment reply.
107109
/// </summary>
108-
/// <param name="repository">The repository.</param>
110+
/// <param name="localRepository">The local repository.</param>
111+
/// <param name="remoteRepositoryOwner">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(
115-
ILocalRepositoryModel repository,
118+
ILocalRepositoryModel localRepository,
119+
string remoteRepositoryOwner,
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: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using GitHub.Primitives;
1010
using GitHub.Services;
1111
using LibGit2Sharp;
12+
using NLog;
1213

1314
namespace GitHub.InlineReviews.Services
1415
{
@@ -137,20 +138,21 @@ public async Task<string> GetPullRequestMergeBase(ILocalRepositoryModel reposito
137138

138139
/// <inheritdoc/>
139140
public async Task<IPullRequestReviewCommentModel> PostReviewComment(
140-
ILocalRepositoryModel repository,
141+
ILocalRepositoryModel localRepository,
142+
string remoteRepositoryOwner,
141143
IAccount user,
142144
int number,
143145
string body,
144146
string commitId,
145147
string path,
146148
int position)
147149
{
148-
var address = HostAddress.Create(repository.CloneUrl.Host);
150+
var address = HostAddress.Create(localRepository.CloneUrl.Host);
149151
var apiClient = await apiClientFactory.Create(address);
150152

151153
var result = await apiClient.CreatePullRequestReviewComment(
152-
repository.Owner,
153-
repository.Name,
154+
remoteRepositoryOwner,
155+
localRepository.Name,
154156
number,
155157
body,
156158
commitId,
@@ -176,18 +178,19 @@ public async Task<IPullRequestReviewCommentModel> PostReviewComment(
176178

177179
/// <inheritdoc/>
178180
public async Task<IPullRequestReviewCommentModel> PostReviewComment(
179-
ILocalRepositoryModel repository,
181+
ILocalRepositoryModel localRepository,
182+
string remoteRepositoryOwner,
180183
IAccount user,
181184
int number,
182185
string body,
183186
int inReplyTo)
184187
{
185-
var address = HostAddress.Create(repository.CloneUrl.Host);
188+
var address = HostAddress.Create(localRepository.CloneUrl.Host);
186189
var apiClient = await apiClientFactory.Create(address);
187190

188191
var result = await apiClient.CreatePullRequestReviewComment(
189-
repository.Owner,
190-
repository.Name,
192+
remoteRepositoryOwner,
193+
localRepository.Name,
191194
number,
192195
body,
193196
inReplyTo);

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using GitHub.Extensions;
88
using GitHub.Models;
99
using GitHub.UI;
10+
using NLog;
1011
using Octokit;
1112
using ReactiveUI;
1213

@@ -17,6 +18,7 @@ namespace GitHub.InlineReviews.ViewModels
1718
/// </summary>
1819
public class CommentViewModel : ReactiveObject, ICommentViewModel
1920
{
21+
static readonly Logger log = LogManager.GetCurrentClassLogger();
2022
string body;
2123
string errorMessage;
2224
bool isReadOnly;
@@ -167,6 +169,7 @@ async Task DoCommitEdit(object unused)
167169
}
168170

169171
ErrorMessage = message;
172+
log.Error("Error posting inline comment.", e);
170173
}
171174
}
172175

‎src/GitHub.InlineReviews/packages.config‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
<package id="Microsoft.VisualStudio.Utilities" version="14.3.25407" targetFramework="net452" />
3131
<package id="Microsoft.VisualStudio.Validation" version="14.1.111" targetFramework="net452" />
3232
<package id="Microsoft.VSSDK.BuildTools" version="14.3.25407" targetFramework="net452" developmentDependency="true" />
33+
<package id="NLog" version="3.1.0.0" targetFramework="net461" />
3334
<package id="Rothko" version="0.0.2-ghfvs" targetFramework="net461" />
3435
<package id="Rx-Core" version="2.2.5-custom" targetFramework="net461" />
3536
<package id="Rx-Interfaces" version="2.2.5-custom" targetFramework="net461" />

‎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)