Merge branch 'new-diff-notes' into 'master'

New diff notes

Fixes #12732, #14731, #19375, #14783 

Builds on https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4110

To do:
- [x] Get it mostly working
- [x] Validate position validity
- [x] Fix: Don’t link to `#`
- [x] Fix: Base ref can be `nil`, potentially, when the MR has an oprhan source branch => Yep, doesn’t work. We need to store a `start_id`
- [x] Optimize: Fewer duplicate `git diff` compares
- [x] Optimize: Pass paths to `PositionTracer#diff` for faster diffs
- [x] Refactor: Use `head_id` in `MergeRequest`/`MergeRequestDiff` instead of `source_sha`
- [x] Refactor: Convert existing array-based diff refs to the DiffRefs model
- [x] Tweak: Use `note_type` in `Autosave` key
- [x] Tweak: Remove `line_code: note.line_code` from `link_to_reply_discussion`
- [x] Update: `SentNotifications` and reply-by-email receiver
- [x] Update: MR diff notification email
- [x] Update: API (MR, Commit note creation and entity)
- [x] Update: GitHub importer
- [x] Address any other TODO comments
- [x] Fix: Suppress "edited 4 minutes ago"
- [x] Write tests
  - [x] `LineMapper`
  - [x] `PositionTracer`
  - [x] `Position`
  - [x] `DiffPositionUpdateService`
  - [x] `DiffNote`
  - [x] `MergeRequests::RefreshService` / `MergeRequest#update_diff_notes_positions`
- [x] Make sure commits with diff notes don't get cleaned up, since this would prevent the diff notes from being rendered (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5062)

Future improvements:
- Display unresolved comments on files outside the diff, if the comment was added when that file _was_ part of the diff
- Allow commenting on sections between hunks, when expanding the diff using `...`
  - (We'd need to generate line code based on Position if we have it, even if it falls outside bounds of diff)
- `diff_hunk` on diff note API entity
- Show diff hunk in notification email
- Resolved line notes would have a boolean, and be inactive through `notes.any? { !active? || resolved? }`
- Multi line notes would store a number of positions, and do the right thing (™) in grouping and then rendering if the first item is multiline? => true
- Image diff notes could store x,y,width,height instead of old_line,new_line for similar grouping. Does it need a reference to say if it's on old or new? These can't have line_codes, clearly. Rendering would be interesting.
- Show commit line comments in the MR diff
- Comment on specific selected words
- Comment on file header
- Unfold top of discussion diff note
- New diff notes API for commits and MRs

/cc @rspeicher

See merge request !4101

1 files changed, 7 insertions, 8 deletions

diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb
index e22dce59d0f..eb57516247d 100644
--- a/app/helpers/diff_helper.rb
+++ b/app/helpers/diff_helper.rb
@@ -30,12 +30,8 @@ module DiffHelper
     options
   end
 
-  def safe_diff_files(diffs, diff_refs)
-    diffs.decorate! { |diff| Gitlab::Diff::File.new(diff, diff_refs) }
-  end
-
-  def generate_line_code(file_path, line)
-    Gitlab::Diff::LineCode.generate(file_path, line.new_pos, line.old_pos)
+  def safe_diff_files(diffs, diff_refs: nil, repository: nil)
+    diffs.decorate! { |diff| Gitlab::Diff::File.new(diff, diff_refs: diff_refs, repository: repository) }
   end
 
   def unfold_bottom_class(bottom)
@@ -93,6 +89,8 @@ module DiffHelper
   end
 
   def commit_for_diff(diff_file)
+    return diff_file.content_commit if diff_file.content_commit
+    
     if diff_file.deleted_file
       @base_commit || @commit.parent || @commit
     else
@@ -100,10 +98,11 @@ module DiffHelper
     end
   end
 
-  def diff_file_html_data(project, diff_commit, diff_file)
+  def diff_file_html_data(project, diff_file)
+    commit = commit_for_diff(diff_file)
     {
       blob_diff_path: namespace_project_blob_diff_path(project.namespace, project,
-                                                       tree_join(diff_commit.id, diff_file.file_path))
+                                                       tree_join(commit.id, diff_file.file_path))
     }
   end