From ea273a98f60207f075977c5a9ebc919cf73d7016 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Thu, 28 Feb 2019 14:42:03 +0000 Subject: [PATCH 1/4] Fix navigation to outdated comments Navigate to file at commit where comment was orignally added. --- .../ViewModels/GitHubPane/PullRequestReviewCommentViewModel.cs | 3 ++- src/GitHub.InlineReviews/Services/PullRequestSessionService.cs | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/GitHub.App/ViewModels/GitHubPane/PullRequestReviewCommentViewModel.cs b/src/GitHub.App/ViewModels/GitHubPane/PullRequestReviewCommentViewModel.cs index 9a9a473161..e303d7470a 100644 --- a/src/GitHub.App/ViewModels/GitHubPane/PullRequestReviewCommentViewModel.cs +++ b/src/GitHub.App/ViewModels/GitHubPane/PullRequestReviewCommentViewModel.cs @@ -52,7 +52,8 @@ async Task DoOpen() { if (thread == null) { - var file = await session.GetFile(RelativePath, model.Thread.CommitSha); + var sha = model.Thread.IsOutdated ? model.Thread.OriginalCommitSha : model.Thread.CommitSha; + var file = await session.GetFile(RelativePath, sha); thread = file.InlineCommentThreads.FirstOrDefault(t => t.Comments.Any(c => c.Comment.Id == model.Id)); } diff --git a/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs b/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs index e470647330..08e816830c 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs @@ -116,7 +116,7 @@ public IReadOnlyList BuildCommentThreads( relativePath = relativePath.Replace("\\", "/"); var threadsByPosition = pullRequest.Threads - .Where(x => x.Path == relativePath && !x.IsOutdated) + .Where(x => x.Path == relativePath) .OrderBy(x => x.Id) .GroupBy(x => Tuple.Create(x.OriginalCommitSha, x.OriginalPosition)); var threads = new List(); From 9b40871e96e9b673e23dd29f66589f5e488669f2 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Thu, 28 Feb 2019 15:08:23 +0000 Subject: [PATCH 2/4] Open outdated file if we can't find a line for comment In case the line matching algorithms catch us out, fall back to viewing comment on file revision when comment was made. --- .../PullRequestReviewCommentViewModel.cs | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/GitHub.App/ViewModels/GitHubPane/PullRequestReviewCommentViewModel.cs b/src/GitHub.App/ViewModels/GitHubPane/PullRequestReviewCommentViewModel.cs index e303d7470a..dce223e42f 100644 --- a/src/GitHub.App/ViewModels/GitHubPane/PullRequestReviewCommentViewModel.cs +++ b/src/GitHub.App/ViewModels/GitHubPane/PullRequestReviewCommentViewModel.cs @@ -52,9 +52,23 @@ async Task DoOpen() { if (thread == null) { - var sha = model.Thread.IsOutdated ? model.Thread.OriginalCommitSha : model.Thread.CommitSha; - var file = await session.GetFile(RelativePath, sha); - thread = file.InlineCommentThreads.FirstOrDefault(t => t.Comments.Any(c => c.Comment.Id == model.Id)); + if(model.Thread.IsOutdated) + { + var file = await session.GetFile(RelativePath, model.Thread.OriginalCommitSha); + thread = file.InlineCommentThreads.FirstOrDefault(t => t.Comments.Any(c => c.Comment.Id == model.Id)); + } + else + { + var file = await session.GetFile(RelativePath, model.Thread.CommitSha); + thread = file.InlineCommentThreads.FirstOrDefault(t => t.Comments.Any(c => c.Comment.Id == model.Id)); + + // Fall back to opening outdated file if we can't find a line number for the comment + if(thread?.LineNumber == -1) + { + file = await session.GetFile(RelativePath, model.Thread.OriginalCommitSha); + thread = file.InlineCommentThreads.FirstOrDefault(t => t.Comments.Any(c => c.Comment.Id == model.Id)); + } + } } if (thread != null && thread.LineNumber != -1) From 1219ae14eb1b5c1da6618e785e956e0c77aa932c Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Thu, 28 Feb 2019 15:14:54 +0000 Subject: [PATCH 3/4] Add logging to PullRequestReviewCommentViewModel --- .../GitHubPane/PullRequestReviewCommentViewModel.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/GitHub.App/ViewModels/GitHubPane/PullRequestReviewCommentViewModel.cs b/src/GitHub.App/ViewModels/GitHubPane/PullRequestReviewCommentViewModel.cs index dce223e42f..7c960baaa5 100644 --- a/src/GitHub.App/ViewModels/GitHubPane/PullRequestReviewCommentViewModel.cs +++ b/src/GitHub.App/ViewModels/GitHubPane/PullRequestReviewCommentViewModel.cs @@ -3,9 +3,11 @@ using System.Reactive; using System.Threading.Tasks; using GitHub.Extensions; +using GitHub.Logging; using GitHub.Models; using GitHub.Services; using ReactiveUI; +using Serilog; namespace GitHub.ViewModels.GitHubPane { @@ -14,6 +16,8 @@ namespace GitHub.ViewModels.GitHubPane /// public class PullRequestReviewCommentViewModel : IPullRequestReviewFileCommentViewModel { + static readonly ILogger log = LogManager.ForContext(); + readonly IPullRequestEditorService editorService; readonly IPullRequestSession session; readonly PullRequestReviewCommentModel model; @@ -62,9 +66,10 @@ async Task DoOpen() var file = await session.GetFile(RelativePath, model.Thread.CommitSha); thread = file.InlineCommentThreads.FirstOrDefault(t => t.Comments.Any(c => c.Comment.Id == model.Id)); - // Fall back to opening outdated file if we can't find a line number for the comment if(thread?.LineNumber == -1) { + log.Warning("Couldn't find line number for comment on {RelativePath} @ {CommitSha}", RelativePath, model.Thread.CommitSha); + // Fall back to opening outdated file if we can't find a line number for the comment file = await session.GetFile(RelativePath, model.Thread.OriginalCommitSha); thread = file.InlineCommentThreads.FirstOrDefault(t => t.Comments.Any(c => c.Comment.Id == model.Id)); } @@ -76,9 +81,9 @@ async Task DoOpen() await editorService.OpenDiff(session, RelativePath, thread); } } - catch (Exception) + catch (Exception e) { - // TODO: Show error. + log.Error(e, nameof(DoOpen)); } } } From 359275f0997357b94906c35e2c028dd302e2d873 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 4 Mar 2019 10:26:29 +0000 Subject: [PATCH 4/4] Update graphql submodule to point at branch net46 --- submodules/octokit.graphql.net | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/submodules/octokit.graphql.net b/submodules/octokit.graphql.net index 4d2b083bd8..ff20b6e9de 160000 --- a/submodules/octokit.graphql.net +++ b/submodules/octokit.graphql.net @@ -1 +1 @@ -Subproject commit 4d2b083bd8eaeb05b7f089e4aedc9d0e0015b61d +Subproject commit ff20b6e9de3d016112de8787ec8ade080214db2b