From eeb41c759e246bf96bda8d8f02478860cc6448bb Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Mon, 25 Jul 2016 22:43:47 -0600
Subject: [PATCH] Add endpoints to resolve diff notes and discussions

---
 .../projects/discussions_controller.rb        | 40 +++++++++++++++++++
 .../projects/merge_requests_controller.rb     |  7 +---
 app/controllers/projects/notes_controller.rb  | 17 ++++++--
 app/models/merge_request.rb                   |  7 ++++
 config/routes.rb                              | 12 ++++--
 5 files changed, 71 insertions(+), 12 deletions(-)
 create mode 100644 app/controllers/projects/discussions_controller.rb

diff --git a/app/controllers/projects/discussions_controller.rb b/app/controllers/projects/discussions_controller.rb
new file mode 100644
index 00000000000..40c321e7a6c
--- /dev/null
+++ b/app/controllers/projects/discussions_controller.rb
@@ -0,0 +1,40 @@
+class Projects::DiscussionsController < Projects::ApplicationController
+  before_action :module_enabled
+  before_action :merge_request
+  before_action :discussion
+  before_action :authorize_resolve_discussion!
+
+  def resolve
+    return render_404 unless discussion.resolvable?
+
+    discussion.resolve!(current_user)
+
+    head :ok
+  end
+
+  def unresolve
+    return render_404 unless discussion.resolvable?
+
+    discussion.unresolve!
+
+    head :ok
+  end
+
+  private
+
+  def merge_request
+    @merge_request ||= @project.merge_requests.find_by!(iid: params[:merge_request_id])
+  end
+
+  def discussion
+    @discussion ||= @merge_request.discussions.find { |d| d.id == params[:id] } || render_404
+  end
+
+  def authorize_resolve_discussion!
+    access_denied! unless discussion.can_resolve?(current_user)
+  end
+
+  def module_enabled
+    render_404 unless @project.merge_requests_enabled
+  end
+end
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 594a61464b9..72ab7cff90c 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -369,12 +369,9 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   # :show, :diff, :commits, :builds. but not when request the data through AJAX
   def define_discussion_vars
     # Build a note object for comment form
-    @note = @project.notes.new(noteable: @noteable)
+    @note = @project.notes.new(noteable: @merge_request)
 
-    @discussions = @noteable.mr_and_commit_notes.
-      inc_author_project_award_emoji.
-      fresh.
-      discussions
+    @discussions = @merge_request.discussions
 
     # This is not executed lazily
     @notes = Banzai::NoteRenderer.render(
diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb
index 0b93dc31ac7..fb9d027ce03 100644
--- a/app/controllers/projects/notes_controller.rb
+++ b/app/controllers/projects/notes_controller.rb
@@ -5,6 +5,7 @@ class Projects::NotesController < Projects::ApplicationController
   before_action :authorize_read_note!
   before_action :authorize_create_note!, only: [:create]
   before_action :authorize_admin_note!, only: [:update, :destroy]
+  before_action :authorize_resolve_note!, only: [:resolve]
   before_action :find_current_user_notes, only: [:index]
 
   def index
@@ -67,12 +68,18 @@ class Projects::NotesController < Projects::ApplicationController
   end
 
   def resolve
-    sleep 2
+    return render_404 unless note.resolvable?
+
+    note.resolve!(current_user)
+
     head :ok
   end
 
-  def resolve_all
-    sleep 2
+  def unresolve
+    return render_404 unless note.resolvable?
+
+    note.unresolve!
+
     head :ok
   end
 
@@ -185,6 +192,10 @@ class Projects::NotesController < Projects::ApplicationController
     return access_denied! unless can?(current_user, :admin_note, note)
   end
 
+  def authorize_resolve_note!
+    return access_denied! unless can?(current_user, :resolve_note, note)
+  end
+
   def note_params
     params.require(:note).permit(
       :note, :noteable, :noteable_id, :noteable_type, :project_id,
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 471e32f3b60..04cec923f2e 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -394,6 +394,13 @@ class MergeRequest < ActiveRecord::Base
     )
   end
 
+  def discussions
+    self.mr_and_commit_notes.
+      inc_author_project_award_emoji.
+      fresh.
+      discussions
+  end
+
   def hook_attrs
     attrs = {
       source: source_project.try(:hook_attrs),
diff --git a/config/routes.rb b/config/routes.rb
index cf2f96c74dd..40dce1745db 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -720,6 +720,13 @@ Rails.application.routes.draw do
             get :update_branches
             get :diff_for_path
           end
+
+          resources :discussions, only: [], constraints: { id: /\h{40}/ } do
+            member do
+              post :resolve
+              delete :resolve, action: :unresolve
+            end
+          end
         end
 
         resources :branches, only: [:index, :new, :create, :destroy], constraints: { id: Gitlab::Regex.git_reference_regex }
@@ -825,14 +832,11 @@ Rails.application.routes.draw do
         resources :group_links, only: [:index, :create, :destroy], constraints: { id: /\d+/ }
 
         resources :notes, only: [:index, :create, :destroy, :update], constraints: { id: /\d+/ } do
-          collection do
-            post :resolve_all
-          end
-
           member do
             post :toggle_award_emoji
             delete :delete_attachment
             post :resolve
+            delete :resolve, action: :unresolve
           end
         end