Merge pull request #2249 from github/fixes/2247-viewing-outdated-comments

Fix viewing outdated comments in pull request reviews

Author: jcansdale

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
     /// </summary>
     public class PullRequestReviewCommentViewModel : IPullRequestReviewFileCommentViewModel
     {
+        static readonly ILogger log = LogManager.ForContext<PullRequestReviewCommentViewModel>();
+
         readonly IPullRequestEditorService editorService;
         readonly IPullRequestSession session;
         readonly PullRequestReviewCommentModel model;
@@ -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));
             }
         }
     }

src/GitHub.InlineReviews/Services/PullRequestSessionService.cs

@@ -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>();