• Douwe Maan's avatar
    Merge branch 'new-diff-notes' into 'master' · 86d238e4
    Douwe Maan authored
    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
    86d238e4
repository.rb 25.3 KB