Merge branch 'diff-line-comment-vuejs' into 'master'

Diff line comments resolve

## What does this MR do?

Diff line comments can be resolved.

Part of #10325 

To do:

- [x] Backend (@DouweM)
  - [x] Fix https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5022#note_13319326. Will be made easier by https://gitlab.com/gitlab-org/gitlab-ce/issues/17237#note_13370331
  - [x] System note when all discussions are resolved
  - [x] Notification when all discussions are resolved
  - [x] Write unit tests
  - [x] Look at resolve time https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5022#note_13912743 - Fixed by 4a13aa9
- [x] Frontend (@iamphill)
  - [x] Fix bugs
  - [x] Write more feature tests 
- [x] Frontend (@connorshea)
  - [x] Address frontend feedback
  - [x] Feature specs for Jump feature
  - [x] Documentation
  - [x] Add Vue.js in a standard way

See merge request !5022

18 files changed, 165 insertions, 62 deletions

diff --git a/app/views/discussions/_diff_discussion.html.haml b/app/views/discussions/_diff_discussion.html.haml
index fa1ad9efa73..1411daeb4a6 100644
--- a/app/views/discussions/_diff_discussion.html.haml
+++ b/app/views/discussions/_diff_discussion.html.haml
@@ -1,6 +1,6 @@
-%tr.notes_holder
+- expanded = local_assigns.fetch(:expanded, true)
+%tr.notes_holder{class: ('hide' unless expanded)}
   %td.notes_line{ colspan: 2 }
   %td.notes_content
-    %ul.notes{ data: { discussion_id: discussion.id } }
-      = render partial: "projects/notes/note", collection: discussion.notes, as: :note
-    = link_to_reply_discussion(discussion)
+    .content
+      = render "discussions/notes", discussion: discussion
diff --git a/app/views/discussions/_diff_with_notes.html.haml b/app/views/discussions/_diff_with_notes.html.haml
index 02b159ffd45..b2e55f7647a 100644
--- a/app/views/discussions/_diff_with_notes.html.haml
+++ b/app/views/discussions/_diff_with_notes.html.haml
@@ -7,8 +7,11 @@
 
   .diff-content.code.js-syntax-highlight
     %table
-      - discussion.truncated_diff_lines.each do |line|
-        = render "projects/diffs/line", line: line, diff_file: diff_file, plain: true
-
-        - if discussion.for_line?(line)
-          = render "discussions/diff_discussion", discussion: discussion
+      - discussions = { discussion.line_code => discussion }
+      = render partial: "projects/diffs/line",
+        collection: discussion.truncated_diff_lines,
+        as: :line,
+        locals: { diff_file: diff_file,
+          discussions: discussions,
+          discussion_expanded: true,
+          plain: true }
diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml
index 49702e048aa..077e8e64e5f 100644
--- a/app/views/discussions/_discussion.html.haml
+++ b/app/views/discussions/_discussion.html.haml
@@ -5,8 +5,17 @@
       = link_to user_path(discussion.author) do
         = image_tag avatar_icon(discussion.author), class: "avatar s40"
     .timeline-content
-      .discussion.js-toggle-container{ class: discussion.id }
+      .discussion.js-toggle-container{ class: discussion.id, data: { discussion_id: discussion.id } }
         .discussion-header
+          .discussion-actions
+            = link_to "#", class: "note-action-button discussion-toggle-button js-toggle-button" do
+              - if expanded
+                = icon("chevron-up")
+              - else
+                = icon("chevron-down")
+
+              Toggle discussion
+
           = link_to_member(@project, discussion.author, avatar: false)
 
           .inline.discussion-headline-light
@@ -29,17 +38,11 @@
 
             = time_ago_with_tooltip(discussion.created_at, placement: "bottom", html_class: "note-created-ago")
 
-          .discussion-actions
-            = link_to "#", class: "note-action-button discussion-toggle-button js-toggle-button" do
-              - if expanded
-                = icon("chevron-up")
-              - else
-                = icon("chevron-down")
-
-              Toggle discussion
+          = render "discussions/headline", discussion: discussion
 
         .discussion-body.js-toggle-content{ class: ("hide" unless expanded) }
           - if discussion.diff_discussion? && discussion.diff_file
             = render "discussions/diff_with_notes", discussion: discussion
           - else
-            = render "discussions/notes", discussion: discussion
+            .panel.panel-default
+              = render "discussions/notes", discussion: discussion
diff --git a/app/views/discussions/_headline.html.haml b/app/views/discussions/_headline.html.haml
new file mode 100644
index 00000000000..c1dabeed387
--- /dev/null
+++ b/app/views/discussions/_headline.html.haml
@@ -0,0 +1,14 @@
+- if discussion.resolved?
+  .discussion-headline-light.js-discussion-headline
+    Resolved
+    - if discussion.resolved_by
+      by
+      = link_to_member(@project, discussion.resolved_by, avatar: false)
+    = time_ago_with_tooltip(discussion.resolved_at, placement: "bottom")
+- elsif discussion.last_updated_at != discussion.created_at
+  .discussion-headline-light.js-discussion-headline
+    Last updated
+    - if discussion.last_updated_by
+      by
+      = link_to_member(@project, discussion.last_updated_by, avatar: false)
+    = time_ago_with_tooltip(discussion.last_updated_at, placement: "bottom")
diff --git a/app/views/discussions/_jump_to_next.html.haml b/app/views/discussions/_jump_to_next.html.haml
new file mode 100644
index 00000000000..69bd416c4de
--- /dev/null
+++ b/app/views/discussions/_jump_to_next.html.haml
@@ -0,0 +1,9 @@
+- discussion = local_assigns.fetch(:discussion, nil)
+- if current_user
+  %jump-to-discussion{ "inline-template" => true, ":discussion-id" => "'#{discussion.try(:id)}'" }
+    .btn-group{ role: "group", "v-show" => "!allResolved", "v-if" => "showButton" }
+      %button.btn.btn-default.discussion-next-btn.has-tooltip{ "@click" => "jumpToNextUnresolvedDiscussion",
+        title: "Jump to next unresolved discussion",
+        "aria-label" => "Jump to next unresolved discussion",
+        data: { container: "body" } }
+        = custom_icon("next_discussion")
diff --git a/app/views/discussions/_notes.html.haml b/app/views/discussions/_notes.html.haml
index a2642b839f6..fbe470bed2c 100644
--- a/app/views/discussions/_notes.html.haml
+++ b/app/views/discussions/_notes.html.haml
@@ -1,5 +1,15 @@
-.panel.panel-default
-  .notes{ data: { discussion_id: discussion.id } }
-    %ul.notes.timeline
-      = render partial: "projects/notes/note", collection: discussion.notes, as: :note
-  = link_to_reply_discussion(discussion)
+%ul.notes{ data: { discussion_id: discussion.id } }
+  = render partial: "projects/notes/note", collection: discussion.notes, as: :note
+
+- if current_user
+  .discussion-reply-holder
+    - if discussion.diff_discussion?
+      - line_type = local_assigns.fetch(:line_type, nil)
+
+      .btn-group-justified.discussion-with-resolve-btn{ role: "group" }
+        .btn-group{ role: "group" }
+          = link_to_reply_discussion(discussion, line_type)
+        = render "discussions/resolve_all", discussion: discussion
+        = render "discussions/jump_to_next", discussion: discussion
+    - else
+      = link_to_reply_discussion(discussion)
diff --git a/app/views/discussions/_parallel_diff_discussion.html.haml b/app/views/discussions/_parallel_diff_discussion.html.haml
index a798c438ea0..f1072ce0feb 100644
--- a/app/views/discussions/_parallel_diff_discussion.html.haml
+++ b/app/views/discussions/_parallel_diff_discussion.html.haml
@@ -1,22 +1,21 @@
-%tr.notes_holder
+- expanded = discussion_left.try(:expanded?) || discussion_right.try(:expanded?)
+%tr.notes_holder{class: ('hide' unless expanded)}
   - if discussion_left
     %td.notes_line.old
     %td.notes_content.parallel.old
-      %ul.notes{ data: { discussion_id: discussion_left.id } }
-        = render partial: "projects/notes/note", collection: discussion_left.notes, as: :note
-
-      = link_to_reply_discussion(discussion_left, 'old')
+      .content{class: ('hide' unless discussion_left.expanded?)}
+        = render "discussions/notes", discussion: discussion_left, line_type: 'old'
   - else
     %td.notes_line.old= ""
-    %td.notes_content.parallel.old= ""
+    %td.notes_content.parallel.old
+      .content
 
   - if discussion_right
     %td.notes_line.new
     %td.notes_content.parallel.new
-      %ul.notes{ data: { discussion_id: discussion_right.id } }
-        = render partial: "projects/notes/note", collection: discussion_right.notes, as: :note
-
-      = link_to_reply_discussion(discussion_right, 'new')
+      .content{class: ('hide' unless discussion_right.expanded?)}
+        = render "discussions/notes", discussion: discussion_right, line_type: 'new'
   - else
     %td.notes_line.new= ""
-    %td.notes_content.parallel.new= ""
+    %td.notes_content.parallel.new
+      .content
diff --git a/app/views/discussions/_resolve_all.html.haml b/app/views/discussions/_resolve_all.html.haml
new file mode 100644
index 00000000000..7a8767ddba0
--- /dev/null
+++ b/app/views/discussions/_resolve_all.html.haml
@@ -0,0 +1,11 @@
+- if discussion.for_merge_request?
+  %resolve-discussion-btn{ ":namespace-path" => "'#{discussion.project.namespace.path}'",
+      ":project-path" => "'#{discussion.project.path}'",
+      ":discussion-id" => "'#{discussion.id}'",
+      ":merge-request-id" => discussion.noteable.iid,
+      ":can-resolve" => discussion.can_resolve?(current_user),
+      "inline-template" => true }
+    .btn-group{ role: "group", "v-if" => "showButton" }
+      %button.btn.btn-default{ type: "button", "@click" => "resolve", ":disabled" => "loading" }
+        = icon("spinner spin", "v-show" => "loading")
+        {{ buttonText }}
diff --git a/app/views/notify/repository_push_email.html.haml b/app/views/notify/repository_push_email.html.haml
index 4dc39a72225..c0c07d65daa 100644
--- a/app/views/notify/repository_push_email.html.haml
+++ b/app/views/notify/repository_push_email.html.haml
@@ -75,8 +75,7 @@
             - blob = diff_file.blob
             - if blob && blob.respond_to?(:text?) && blob_text_viewable?(blob)
               %table.code.white
-                - diff_file.highlighted_diff_lines.each do |line|
-                  = render "projects/diffs/line", line: line, diff_file: diff_file, plain: true, email: true
+                = render partial: "projects/diffs/line", collection: diff_file.highlighted_diff_lines, as: :line, locals: { diff_file: diff_file, plain: true, email: true }
             - else
               No preview for this file type
           %br
diff --git a/app/views/notify/resolved_all_discussions_email.html.haml b/app/views/notify/resolved_all_discussions_email.html.haml
new file mode 100644
index 00000000000..522421b7cc3
--- /dev/null
+++ b/app/views/notify/resolved_all_discussions_email.html.haml
@@ -0,0 +1,2 @@
+%p
+  All discussions on Merge Request #{@merge_request.to_reference} were resolved by #{@resolved_by.name}
diff --git a/app/views/notify/resolved_all_discussions_email.text.erb b/app/views/notify/resolved_all_discussions_email.text.erb
new file mode 100644
index 00000000000..b0d380af8fc
--- /dev/null
+++ b/app/views/notify/resolved_all_discussions_email.text.erb
@@ -0,0 +1,3 @@
+All discussions on Merge Request <%= @merge_request.to_reference %> were resolved by <%= @resolved_by.name %>
+
+<%= url_for(namespace_project_merge_request_url(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request)) %>
diff --git a/app/views/projects/diffs/_line.html.haml b/app/views/projects/diffs/_line.html.haml
index 891b2bd9802..7042e9f1fc9 100644
--- a/app/views/projects/diffs/_line.html.haml
+++ b/app/views/projects/diffs/_line.html.haml
@@ -1,7 +1,7 @@
 - email = local_assigns.fetch(:email, false)
 - plain = local_assigns.fetch(:plain, false)
 - type = line.type
-- line_code = diff_file.line_code(line) unless plain
+- line_code = diff_file.line_code(line)
 %tr.line_holder{ plain ? { class: type} : { class: type, id: line_code } }
   - case type
   - when 'match'
@@ -28,3 +28,10 @@
         %pre= diff_line_content(line.text, type)
       - else
         = diff_line_content(line.text, type)
+
+- discussions = local_assigns.fetch(:discussions, nil)
+- if discussions && !line.meta?
+  - discussion = discussions[line_code]
+  - if discussion
+    - discussion_expanded = local_assigns.fetch(:discussion_expanded, discussion.expanded?)
+    = render "discussions/diff_discussion", discussion: discussion, expanded: discussion_expanded
diff --git a/app/views/projects/diffs/_text_file.html.haml b/app/views/projects/diffs/_text_file.html.haml
index ab5463ba89d..f1d2d4bf268 100644
--- a/app/views/projects/diffs/_text_file.html.haml
+++ b/app/views/projects/diffs/_text_file.html.haml
@@ -5,15 +5,12 @@
 
 %table.text-file.code.js-syntax-highlight{ data: diff_view_data, class: too_big ? 'hide' : '' }
   - last_line = 0
-  - diff_file.highlighted_diff_lines.each do |line|
-    - last_line = line.new_pos
-    = render "projects/diffs/line", line: line, diff_file: diff_file
-
-    - unless @diff_notes_disabled
-      - line_code = diff_file.line_code(line)
-      - discussion = @grouped_diff_discussions[line_code] if line_code
-      - if discussion
-        = render "discussions/diff_discussion", discussion: discussion
+  - discussions = @grouped_diff_discussions unless @diff_notes_disabled
+  = render partial: "projects/diffs/line",
+    collection: diff_file.highlighted_diff_lines,
+    as: :line,
+    locals: { diff_file: diff_file, discussions: discussions }
 
+  - last_line = diff_file.highlighted_diff_lines.last.new_pos
   - if !diff_file.new_file && last_line > 0
     = diff_match_line last_line, last_line, bottom: true
diff --git a/app/views/projects/merge_requests/_discussion.html.haml b/app/views/projects/merge_requests/_discussion.html.haml
index 53dd300c35c..d070979bcfe 100644
--- a/app/views/projects/merge_requests/_discussion.html.haml
+++ b/app/views/projects/merge_requests/_discussion.html.haml
@@ -4,5 +4,8 @@
       = link_to 'Close merge request', merge_request_path(@merge_request, merge_request: {state_event: :close }), method: :put, class: "btn btn-nr btn-comment btn-close close-mr-link js-note-target-close", title: "Close merge request", data: {original_text: "Close merge request", alternative_text: "Comment & close merge request"}
     - if @merge_request.closed?
       = link_to 'Reopen merge request', merge_request_path(@merge_request, merge_request: {state_event: :reopen }), method: :put, class: "btn btn-nr btn-comment btn-reopen reopen-mr-link js-note-target-reopen", title: "Reopen merge request", data: {original_text: "Reopen merge request", alternative_text: "Comment & reopen merge request"}
+  %comment-and-resolve-btn{ "inline-template" => true, ":discussion-id" => "" }
+    %button.btn.btn-nr.btn-default.append-right-10.js-comment-resolve-button{ "v-if" => "showButton", type: "submit", data: { namespace_path: "#{@merge_request.project.namespace.path}", project_path: "#{@merge_request.project.path}" } }
+      {{ buttonText }}
 
 #notes= render "projects/notes/notes_with_form"
diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml
index a1313064725..f8025fc1dbe 100644
--- a/app/views/projects/merge_requests/_show.html.haml
+++ b/app/views/projects/merge_requests/_show.html.haml
@@ -1,6 +1,8 @@
 - page_title           "#{@merge_request.title} (#{@merge_request.to_reference})", "Merge Requests"
 - page_description     @merge_request.description
 - page_card_attributes @merge_request.card_attributes
+- content_for :page_specific_javascripts do
+  = page_specific_javascript_tag('diff_notes/diff_notes_bundle.js')
 
 - if diff_view == :parallel
   - fluid_layout true
@@ -65,8 +67,18 @@
           = link_to diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#diffs', action: 'diffs', toggle: 'tab' } do
             Changes
             %span.badge= @merge_request.diff_size
+        %li#resolve-count-app.line-resolve-all-container.pull-right.prepend-top-10.hidden-xs{ "v-cloak" => true }
+          %resolve-count{ "inline-template" => true, ":logged-out" => "#{current_user.nil?}"  }
+            .line-resolve-all{ "v-show" => "discussionCount > 0",
+              ":class" => "{ 'has-next-btn': !loggedOut && resolvedDiscussionCount !== discussionCount }" }
+              %span.line-resolve-btn.is-disabled{ type: "button",
+                  ":class" => "{ 'is-active': resolvedDiscussionCount === discussionCount }" }
+                = render "shared/icons/icon_status_success.svg"
+              %span.line-resolve-text
+                {{ resolvedDiscussionCount }}/{{ discussionCount }} {{ discussionCount | pluralize 'discussion' }} resolved
+            = render "discussions/jump_to_next"
 
-      .tab-content
+      .tab-content#diff-notes-app
         #notes.notes.tab-pane.voting_notes
           .content-block.content-block-small.oneline-block
             = render 'award_emoji/awards_block', awardable: @merge_request, inline: true
diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml
index 759c72b2477..402f5b52f5e 100644
--- a/app/views/projects/notes/_form.html.haml
+++ b/app/views/projects/notes/_form.html.haml
@@ -1,4 +1,4 @@
-= form_for [@project.namespace.becomes(Namespace), @project, @note], remote: true, html: { :'data-type' => 'json', multipart: true, id: nil, class: "new-note js-new-note-form js-quick-submit common-note-form" }, authenticity_token: true do |f|
+= form_for [@project.namespace.becomes(Namespace), @project, @note], remote: true, html: { :'data-type' => 'json', multipart: true, id: nil, class: "new-note js-new-note-form js-quick-submit common-note-form", "data-noteable-iid" => @note.noteable.try(:iid), }, authenticity_token: true do |f|
   = hidden_field_tag :view, diff_view
   = hidden_field_tag :line_type
   = note_target_fields(@note)
diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml
index 71da8ac9d7c..d2ac1ce2b9a 100644
--- a/app/views/projects/notes/_note.html.haml
+++ b/app/views/projects/notes/_note.html.haml
@@ -1,5 +1,6 @@
 - return unless note.author
 - return if note.cross_reference_not_visible_for?(current_user)
+- can_resolve = can?(current_user, :resolve_note, note)
 
 - note_editable = note_editable?(note)
 %li.timeline-entry{ id: dom_id(note), class: ["note", "note-row-#{note.id}", ('system-note' if note.system)], data: {author_id: note.author.id, editable: note_editable} }
@@ -16,19 +17,48 @@
             commented
           %a{ href: "##{dom_id(note)}" }
             = time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago')
-        .note-actions
-          - access = note_max_access_for_user(note)
-          - if access and not note.system
-            %span.note-role.hidden-xs= access
-          - if current_user and not note.system
-            = link_to '#', title: 'Award Emoji', class: 'note-action-button note-emoji-button js-add-award js-note-emoji', data: { position: 'right' } do
-              = icon('spinner spin')
-              = icon('smile-o')
-          - if note_editable
-            = link_to '#', title: 'Edit comment', class: 'note-action-button js-note-edit' do
-              = icon('pencil')
-            = link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'note-action-button hidden-xs js-note-delete danger' do
-              = icon('trash-o')
+        - unless note.system?
+          .note-actions
+            - access = note_max_access_for_user(note)
+            - if access
+              %span.note-role.hidden-xs= access
+
+            - if note.resolvable?
+              %resolve-btn{ ":namespace-path" => "'#{note.project.namespace.path}'",
+                  ":project-path" => "'#{note.project.path}'",
+                  ":discussion-id" => "'#{note.discussion_id}'",
+                  ":note-id" => note.id,
+                  ":resolved" => note.resolved?,
+                  ":can-resolve" => can_resolve,
+                  ":resolved-by" => "'#{note.resolved_by.try(:name)}'",
+                  "v-show" => "#{can_resolve || note.resolved?}",
+                  "inline-template" => true,
+                  "v-ref:note_#{note.id}" => true }
+
+                .note-action-button
+                  = icon("spin spinner", "v-show" => "loading")
+                  %button.line-resolve-btn{ type: "button",
+                      class: ("is-disabled" unless can_resolve),
+                      ":class" => "{ 'is-active': isResolved }",
+                      ":aria-label" => "buttonText",
+                      "@click" => "resolve",
+                      ":title" => "buttonText",
+                      "v-show" => "!loading",
+                      "v-el:button" => true }
+
+                    = render "shared/icons/icon_status_success.svg"
+
+            - if current_user
+              - if note.emoji_awardable?
+                = link_to '#', title: 'Award Emoji', class: 'note-action-button note-emoji-button js-add-award js-note-emoji', data: { position: 'right' } do
+                  = icon('spinner spin')
+                  = icon('smile-o')
+
+              - if note_editable
+                = link_to '#', title: 'Edit comment', class: 'note-action-button js-note-edit' do
+                  = icon('pencil')
+                = link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'note-action-button hidden-xs js-note-delete danger' do
+                  = icon('trash-o')
       .note-body{class: note_editable ? 'js-task-list-container' : ''}
         .note-text.md
           = preserve do
diff --git a/app/views/shared/icons/_next_discussion.svg b/app/views/shared/icons/_next_discussion.svg
new file mode 100644
index 00000000000..43559a60cb0
--- /dev/null
+++ b/app/views/shared/icons/_next_discussion.svg
@@ -0,0 +1 @@
+<svg viewBox="0 0 20 19" ><path d="M15.21 7.783h-3.317c-.268 0-.472.218-.472.486v.953c0 .28.212.486.473.486h3.318v1.575c0 .36.233.452.52.23l3.06-2.37c.274-.213.286-.582 0-.804l-3.06-2.37c-.275-.213-.52-.12-.52.23v1.583zm.57-3.66c-1.558-1.22-3.783-1.98-6.254-1.98C4.816 2.143 1 4.91 1 8.333c0 1.964 1.256 3.715 3.216 4.846-.447 1.615-1.132 2.195-1.732 2.882-.142.174-.304.32-.256.56v.01c.047.213.218.368.41.368h.046c.37-.048.743-.116 1.085-.213 1.645-.425 3.13-1.22 4.377-2.34.447.048.913.077 1.38.077 2.092 0 4.01-.546 5.492-1.454-.416-.208-.798-.475-1.134-.792-1.227.63-2.743 1.008-4.36 1.008-.41 0-.828-.03-1.237-.078l-.543-.058-.41.368c-.78.696-1.655 1.248-2.616 1.654.248-.445.486-.977.667-1.664l.257-.928-.828-.484c-1.646-.948-2.598-2.32-2.598-3.763 0-2.69 3.35-4.952 7.308-4.952 1.893 0 3.647.518 4.962 1.353.393-.266.827-.473 1.29-.61z" /></svg>