New diff notes

Douwe Maanrequested to merge
new-diff-notes into master

Fixes #12732 (closed), #14731 (closed), #19375 (closed), #14783 (closed)

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

To do:

  • Get it mostly working
  • Validate position validity
  • Fix: Don’t link to #
  • 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
  • Optimize: Fewer duplicate git diff compares
  • Optimize: Pass paths to PositionTracer#diff for faster diffs
  • Refactor: Use head_id in MergeRequest/MergeRequestDiff instead of source_sha
  • Refactor: Convert existing array-based diff refs to the DiffRefs model
  • Tweak: Use note_type in Autosave key
  • Tweak: Remove line_code: note.line_code from link_to_reply_discussion
  • Update: SentNotifications and reply-by-email receiver
  • Update: MR diff notification email
  • Update: API (MR, Commit note creation and entity)
  • Update: GitHub importer
  • Address any other TODO comments
  • Fix: Suppress "edited 4 minutes ago"
  • Write tests
    • LineMapper
    • PositionTracer
    • Position
    • DiffPositionUpdateService
    • DiffNote
    • MergeRequests::RefreshService / MergeRequest#update_diff_notes_positions
  • 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

Merge request reports