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

Fix viewing outdated comments in pull request reviews #2249

Merged
jcansdale merged 5 commits into from
Mar 4, 2019
Merged

Fix viewing outdated comments in pull request reviews #2249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ea273a9
Fix navigation to outdated comments
jcansdale Feb 28, 2019
9b40871
Open outdated file if we can't find a line for comment
jcansdale Feb 28, 2019
1219ae1
Add logging to PullRequestReviewCommentViewModel
jcansdale Feb 28, 2019
0b483a2
Merge branch 'master' into fixes/2247-viewing-outdated-comments
jcansdale Mar 4, 2019
359275f
Update graphql submodule to point at branch net46
jcansdale Mar 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions src/GitHub.App/ViewModels/GitHubPane/PullRequestReviewCommentViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -14,6 +16,8 @@ namespace GitHub.ViewModels.GitHubPane
/// </summary>
public class PullRequestReviewCommentViewModel : IPullRequestReviewFileCommentViewModel
{
static readonly ILogger log = LogManager.ForContext<PullRequestReviewCommentViewModel>();

readonly IPullRequestEditorService editorService;
readonly IPullRequestSession session;
readonly PullRequestReviewCommentModel model;
Expand Down Expand Up @@ -52,18 +56,34 @@ async Task DoOpen()
{
if (thread == null)
{
var file = await session.GetFile(RelativePath, model.Thread.CommitSha);
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));

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));
}
}
}

if (thread != null && thread.LineNumber != -1)
{
await editorService.OpenDiff(session, RelativePath, thread);
}
}
catch (Exception)
catch (Exception e)
{
// TODO: Show error.
log.Error(e, nameof(DoOpen));
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/GitHub.InlineReviews/Services/PullRequestSessionService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public IReadOnlyList<IInlineCommentThreadModel> 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<IInlineCommentThreadModel>();
Expand Down
2 changes: 1 addition & 1 deletion submodules/octokit.graphql.net
Open in desktop
Submodule octokit.graphql.net updated 154 files