diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-07-07 20:45:03 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2016-07-07 20:45:03 +0000 |
commit | 86d238e4bda6424a79eb9d8ea7cfe41af714f49f (patch) | |
tree | e8d266bf7b22516ccfc2f83819ad858c08d1b0ad /app/assets/javascripts | |
parent | 91cf0387dd91b380647457c41edd948a357b620c (diff) | |
parent | c66bcf34dd2ede698a784f55bd17346e85aeaf92 (diff) | |
download | gitlab-ce-86d238e4bda6424a79eb9d8ea7cfe41af714f49f.tar.gz |
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
Diffstat (limited to 'app/assets/javascripts')
-rw-r--r-- | app/assets/javascripts/notes.js.coffee | 13 |
1 files changed, 11 insertions, 2 deletions
diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index 7c1d943667b..0b7d8f64456 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -240,12 +240,16 @@ class @Notes @note_ids.push(note.id) form = $("#new-discussion-note-form-#{note.discussion_id}") + if note.original_discussion_id? and form.length is 0 + form = $("#new-discussion-note-form-#{note.original_discussion_id}") row = form.closest("tr") note_html = $(note.html) note_html.syntaxHighlight() # is this the first note of discussion? discussionContainer = $(".notes[data-discussion-id='" + note.discussion_id + "']") + if note.original_discussion_id? and discussionContainer.length is 0 + discussionContainer = $(".notes[data-discussion-id='" + note.original_discussion_id + "']") if discussionContainer.length is 0 # insert the note and the reply button after the temp row row.after note.discussion_html @@ -318,6 +322,7 @@ class @Notes form.addClass "js-main-target-form" form.find("#note_line_code").remove() + form.find("#note_position").remove() form.find("#note_type").remove() ### @@ -335,10 +340,12 @@ class @Notes new Autosave textarea, [ "Note" - form.find("#note_commit_id").val() - form.find("#note_line_code").val() form.find("#note_noteable_type").val() form.find("#note_noteable_id").val() + form.find("#note_commit_id").val() + form.find("#note_type").val() + form.find("#note_line_code").val() + form.find("#note_position").val() ] ### @@ -512,10 +519,12 @@ class @Notes setupDiscussionNoteForm: (dataHolder, form) => # setup note target form.attr 'id', "new-discussion-note-form-#{dataHolder.data("discussionId")}" + form.attr "data-line-code", dataHolder.data("lineCode") form.find("#note_type").val dataHolder.data("noteType") form.find("#line_type").val dataHolder.data("lineType") form.find("#note_commit_id").val dataHolder.data("commitId") form.find("#note_line_code").val dataHolder.data("lineCode") + form.find("#note_position").val dataHolder.attr("data-position") form.find("#note_noteable_type").val dataHolder.data("noteableType") form.find("#note_noteable_id").val dataHolder.data("noteableId") form.find('.js-note-discard') |