From 47e8379ea7085dc0862695c0d687d4dcc6bf7493 Mon Sep 17 00:00:00 2001
From: Felipe Artur <felipefac@gmail.com>
Date: Mon, 25 Jun 2018 16:46:36 -0300
Subject: [PATCH] Add draft comments for MergeRequests

Implements a new model, DraftNote, which is related to MergeRequests
only and enables to comment on a review in batches. The DraftNotes are
only visible to the author until they're published at which point they
become regular Notes.

This also includes a DraftsController and two related services:
DraftNotes::CreateService and DraftNotes::PublishService

A DraftNote behaves almost identically to a Note, except has no cached
markdown fields and needs to process quick action changes every time it
is displayed.

NOTE: THIS IS ONLY THE BACKEND CHANGES. NO FRONTEND INCLUDED
---
 app/models/concerns/diff_positionable_note.rb |  81 ++++
 app/models/diff_note.rb                       |  68 +--
 config/routes/project.rb                      |  11 +
 db/schema.rb                                  |  17 +
 .../merge_requests/drafts_controller.rb       | 127 ++++++
 ee/app/models/draft_note.rb                   | 103 +++++
 ee/app/models/ee/merge_request.rb             |   1 +
 ee/app/models/license.rb                      |   1 +
 ee/app/policies/draft_note_policy.rb          |  13 +
 ee/app/serializers/draft_note_entity.rb       |  37 ++
 ee/app/serializers/draft_note_serializer.rb   |   4 +
 ee/app/services/draft_notes/base_service.rb   |  17 +
 ee/app/services/draft_notes/create_service.rb |  52 +++
 .../services/draft_notes/publish_service.rb   |  52 +++
 ee/changelogs/unreleased/issue_1984.yml       |   5 +
 .../20180917145556_create_draft_notes.rb      |  22 +
 .../merge_requests/drafts_controller_spec.rb  | 413 ++++++++++++++++++
 ee/spec/factories/draft_note.rb               |  31 ++
 .../lib/gitlab/import_export/all_models.yml   |   1 +
 .../draft_notes/create_service_spec.rb        |  68 +++
 .../draft_notes/publish_service_spec.rb       |  72 +++
 21 files changed, 1129 insertions(+), 67 deletions(-)
 create mode 100644 app/models/concerns/diff_positionable_note.rb
 create mode 100644 ee/app/controllers/projects/merge_requests/drafts_controller.rb
 create mode 100644 ee/app/models/draft_note.rb
 create mode 100644 ee/app/policies/draft_note_policy.rb
 create mode 100644 ee/app/serializers/draft_note_entity.rb
 create mode 100644 ee/app/serializers/draft_note_serializer.rb
 create mode 100644 ee/app/services/draft_notes/base_service.rb
 create mode 100644 ee/app/services/draft_notes/create_service.rb
 create mode 100644 ee/app/services/draft_notes/publish_service.rb
 create mode 100644 ee/changelogs/unreleased/issue_1984.yml
 create mode 100644 ee/db/migrate/20180917145556_create_draft_notes.rb
 create mode 100644 ee/spec/controllers/projects/merge_requests/drafts_controller_spec.rb
 create mode 100644 ee/spec/factories/draft_note.rb
 create mode 100644 ee/spec/services/draft_notes/create_service_spec.rb
 create mode 100644 ee/spec/services/draft_notes/publish_service_spec.rb

diff --git a/app/models/concerns/diff_positionable_note.rb b/app/models/concerns/diff_positionable_note.rb
new file mode 100644
index 00000000000000..b61bf29e6adbe0
--- /dev/null
+++ b/app/models/concerns/diff_positionable_note.rb
@@ -0,0 +1,81 @@
+# frozen_string_literal: true
+module DiffPositionableNote
+  extend ActiveSupport::Concern
+
+  included do
+    before_validation :set_original_position, on: :create
+    before_validation :update_position, on: :create, if: :on_text?
+
+    serialize :original_position, Gitlab::Diff::Position # rubocop:disable Cop/ActiveRecordSerialize
+    serialize :position, Gitlab::Diff::Position # rubocop:disable Cop/ActiveRecordSerialize
+    serialize :change_position, Gitlab::Diff::Position # rubocop:disable Cop/ActiveRecordSerialize
+  end
+
+  %i(original_position position change_position).each do |meth|
+    define_method "#{meth}=" do |new_position|
+      if new_position.is_a?(String)
+        new_position = JSON.parse(new_position) rescue nil
+      end
+
+      if new_position.is_a?(Hash)
+        new_position = new_position.with_indifferent_access
+        new_position = Gitlab::Diff::Position.new(new_position)
+      end
+
+      return if new_position == read_attribute(meth)
+
+      super(new_position)
+    end
+  end
+
+  def on_text?
+    position&.position_type == "text"
+  end
+
+  def on_image?
+    position&.position_type == "image"
+  end
+
+  def supported?
+    for_commit? || self.noteable.has_complete_diff_refs?
+  end
+
+  def active?(diff_refs = nil)
+    return false unless supported?
+    return true if for_commit?
+
+    diff_refs ||= noteable.diff_refs
+
+    self.position.diff_refs == diff_refs
+  end
+
+  def set_original_position
+    return unless position
+
+    self.original_position = self.position.dup unless self.original_position&.complete?
+  end
+
+  def update_position
+    return unless supported?
+    return if for_commit?
+
+    return if active?
+    return unless position
+
+    tracer = Gitlab::Diff::PositionTracer.new(
+      project: self.project,
+      old_diff_refs: self.position.diff_refs,
+      new_diff_refs: self.noteable.diff_refs,
+      paths: self.position.paths
+    )
+
+    result = tracer.trace(self.position)
+    return unless result
+
+    if result[:outdated]
+      self.change_position = result[:position]
+    else
+      self.position = result[:position]
+    end
+  end
+end
diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb
index 047d353b4b5cc0..95694377fe3e89 100644
--- a/app/models/diff_note.rb
+++ b/app/models/diff_note.rb
@@ -5,14 +5,11 @@
 # A note of this type can be resolvable.
 class DiffNote < Note
   include NoteOnDiff
+  include DiffPositionableNote
   include Gitlab::Utils::StrongMemoize
 
   NOTEABLE_TYPES = %w(MergeRequest Commit).freeze
 
-  serialize :original_position, Gitlab::Diff::Position # rubocop:disable Cop/ActiveRecordSerialize
-  serialize :position, Gitlab::Diff::Position # rubocop:disable Cop/ActiveRecordSerialize
-  serialize :change_position, Gitlab::Diff::Position # rubocop:disable Cop/ActiveRecordSerialize
-
   validates :original_position, presence: true
   validates :position, presence: true
   validates :line_code, presence: true, line_code: true, if: :on_text?
@@ -21,8 +18,6 @@ class DiffNote < Note
   validate :verify_supported
   validate :diff_refs_match_commit, if: :for_commit?
 
-  before_validation :set_original_position, on: :create
-  before_validation :update_position, on: :create, if: :on_text?
   before_validation :set_line_code, if: :on_text?
   after_save :keep_around_commits
   after_commit :create_diff_file, on: :create
@@ -31,31 +26,6 @@ def discussion_class(*)
     DiffDiscussion
   end
 
-  %i(original_position position change_position).each do |meth|
-    define_method "#{meth}=" do |new_position|
-      if new_position.is_a?(String)
-        new_position = JSON.parse(new_position) rescue nil
-      end
-
-      if new_position.is_a?(Hash)
-        new_position = new_position.with_indifferent_access
-        new_position = Gitlab::Diff::Position.new(new_position)
-      end
-
-      return if new_position == read_attribute(meth)
-
-      super(new_position)
-    end
-  end
-
-  def on_text?
-    position.position_type == "text"
-  end
-
-  def on_image?
-    position.position_type == "image"
-  end
-
   def create_diff_file
     return unless should_create_diff_file?
 
@@ -87,15 +57,6 @@ def original_line_code
     self.diff_file.line_code(self.diff_line)
   end
 
-  def active?(diff_refs = nil)
-    return false unless supported?
-    return true if for_commit?
-
-    diff_refs ||= noteable.diff_refs
-
-    self.position.diff_refs == diff_refs
-  end
-
   def created_at_diff?(diff_refs)
     return false unless supported?
     return true if for_commit?
@@ -141,37 +102,10 @@ def supported?
     for_commit? || self.noteable.has_complete_diff_refs?
   end
 
-  def set_original_position
-    self.original_position = self.position.dup unless self.original_position&.complete?
-  end
-
   def set_line_code
     self.line_code = self.position.line_code(self.project.repository)
   end
 
-  def update_position
-    return unless supported?
-    return if for_commit?
-
-    return if active?
-
-    tracer = Gitlab::Diff::PositionTracer.new(
-      project: self.project,
-      old_diff_refs: self.position.diff_refs,
-      new_diff_refs: self.noteable.diff_refs,
-      paths: self.position.paths
-    )
-
-    result = tracer.trace(self.position)
-    return unless result
-
-    if result[:outdated]
-      self.change_position = result[:position]
-    else
-      self.position = result[:position]
-    end
-  end
-
   def verify_supported
     return if supported?
 
diff --git a/config/routes/project.rb b/config/routes/project.rb
index fd12bb8694672d..79e93c04947a5e 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -158,6 +158,17 @@
         resources :approver_groups, only: :destroy
         ## EE-specific
 
+        ## EE-specific
+        scope module: :merge_requests do
+          resources :drafts, only: [:index, :update, :create, :destroy] do
+            collection do
+              post :publish
+              delete :discard
+            end
+          end
+        end
+        ## EE-specific
+
         resources :discussions, only: [:show], constraints: { id: /\h{40}/ } do
           member do
             post :resolve
diff --git a/db/schema.rb b/db/schema.rb
index 0f0a5280ae4c93..8f872568ee3c3b 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -904,6 +904,21 @@
   add_index "deployments", ["environment_id", "iid", "project_id"], name: "index_deployments_on_environment_id_and_iid_and_project_id", using: :btree
   add_index "deployments", ["project_id", "iid"], name: "index_deployments_on_project_id_and_iid", unique: true, using: :btree
 
+  create_table "draft_notes", id: :bigserial, force: :cascade do |t|
+    t.integer "merge_request_id", null: false
+    t.integer "author_id", null: false
+    t.boolean "resolve_discussion", default: false, null: false
+    t.string "discussion_id"
+    t.text "note", null: false
+    t.text "position"
+    t.text "original_position"
+    t.text "change_position"
+  end
+
+  add_index "draft_notes", ["author_id"], name: "index_draft_notes_on_author_id", using: :btree
+  add_index "draft_notes", ["discussion_id"], name: "index_draft_notes_on_discussion_id", using: :btree
+  add_index "draft_notes", ["merge_request_id"], name: "index_draft_notes_on_merge_request_id", using: :btree
+
   create_table "emails", force: :cascade do |t|
     t.integer "user_id", null: false
     t.string "email", null: false
@@ -3158,6 +3173,8 @@
   add_foreign_key "container_repositories", "projects"
   add_foreign_key "deploy_keys_projects", "projects", name: "fk_58a901ca7e", on_delete: :cascade
   add_foreign_key "deployments", "projects", name: "fk_b9a3851b82", on_delete: :cascade
+  add_foreign_key "draft_notes", "merge_requests", on_delete: :cascade
+  add_foreign_key "draft_notes", "users", column: "author_id", on_delete: :cascade
   add_foreign_key "environments", "projects", name: "fk_d1c8c1da6a", on_delete: :cascade
   add_foreign_key "epic_issues", "epics", on_delete: :cascade
   add_foreign_key "epic_issues", "issues", on_delete: :cascade
diff --git a/ee/app/controllers/projects/merge_requests/drafts_controller.rb b/ee/app/controllers/projects/merge_requests/drafts_controller.rb
new file mode 100644
index 00000000000000..ccfa8f2a92b5df
--- /dev/null
+++ b/ee/app/controllers/projects/merge_requests/drafts_controller.rb
@@ -0,0 +1,127 @@
+# frozen_string_literal: true
+
+class Projects::MergeRequests::DraftsController < Projects::MergeRequests::ApplicationController
+  include Gitlab::Utils::StrongMemoize
+
+  respond_to :json
+
+  before_action :check_draft_notes_available!, except: [:index]
+  before_action :authorize_create_draft!, only: [:create]
+  before_action :authorize_admin_draft!, only: [:update, :destroy]
+
+  def index
+    drafts = prepare_notes_for_rendering(draft_notes)
+    render json: DraftNoteSerializer.new(current_user: current_user).represent(drafts)
+  end
+
+  def create
+    create_params = draft_note_params.merge(in_reply_to_discussion_id: params[:in_reply_to_discussion_id])
+    create_service = DraftNotes::CreateService.new(merge_request, current_user, create_params)
+
+    draft_note = create_service.execute
+
+    prepare_notes_for_rendering(draft_note)
+
+    render json: DraftNoteSerializer.new(current_user: current_user).represent(draft_note)
+  end
+
+  def update
+    draft_note.update!(draft_note_params)
+
+    prepare_notes_for_rendering(draft_note)
+
+    render json: DraftNoteSerializer.new(current_user: current_user).represent(draft_note)
+  end
+
+  def destroy
+    draft_note.destroy!
+
+    head :ok
+  end
+
+  def publish
+    DraftNotes::PublishService.new(merge_request, current_user).execute(params[:id])
+
+    head :ok
+  end
+
+  def discard
+    draft_notes.delete_all
+
+    head :ok
+  end
+
+  private
+
+  def draft_note
+    strong_memoize(:draft_note) do
+      draft_notes.try(:find, params[:id])
+    end
+  end
+
+  def draft_notes
+    return unless current_user && draft_notes_available?
+
+    strong_memoize(:draft_notes) do
+      merge_request.draft_notes.authored_by(current_user)
+    end
+  end
+
+  # rubocop: disable CodeReuse/ActiveRecord
+  def merge_request
+    @merge_request ||= MergeRequestsFinder.new(current_user, project_id: @project.id).find_by!(iid: params[:merge_request_id])
+  end
+  # rubocop: enable CodeReuse/ActiveRecord
+
+  def draft_note_params
+    params.require(:draft_note).permit(
+      :note,
+      :position,
+      :resolve_discussion
+    )
+  end
+
+  def prepare_notes_for_rendering(notes)
+    return [] unless notes
+
+    notes = Array.wrap(notes)
+
+    # Preload author and access-level information
+    DraftNote.preload_author(notes)
+    user_ids = notes.map(&:author_id)
+    project.team.max_member_access_for_user_ids(user_ids)
+
+    notes.map(&method(:render_draft_note))
+  end
+
+  def render_draft_note(note)
+    params = { quick_actions_target_id: merge_request.id, quick_actions_target_type: 'MergeRequest', text: note.note }
+    result = PreviewMarkdownService.new(@project, current_user, params).execute
+    markdown_params = { markdown_engine: result[:markdown_engine], issuable_state_filter_enabled: true }
+
+    note.rendered_note = view_context.markdown(result[:text], markdown_params)
+    note.users_referenced = result[:users]
+    note.commands_changes = view_context.markdown(result[:commands])
+
+    note
+  end
+
+  def draft_notes_available?
+    @project.feature_available?(:batch_comments) &&
+      ::Feature.enabled?(:batch_comments, current_user, default_enabled: false)
+  end
+
+  def authorize_admin_draft!
+    access_denied! unless can?(current_user, :admin_note, draft_note)
+  end
+
+  def authorize_create_draft!
+    access_denied! unless can?(current_user, :create_note, merge_request)
+  end
+
+  def check_draft_notes_available!
+    return if draft_notes_available?
+
+    access_denied!('Draft Notes are not available with your current License')
+  end
+end
diff --git a/ee/app/models/draft_note.rb b/ee/app/models/draft_note.rb
new file mode 100644
index 00000000000000..b36586a6532625
--- /dev/null
+++ b/ee/app/models/draft_note.rb
@@ -0,0 +1,103 @@
+# frozen_string_literal: true
+class DraftNote < ActiveRecord::Base
+  include DiffPositionableNote
+  include Gitlab::Utils::StrongMemoize
+
+  PUBLISH_ATTRS = %i(noteable_id noteable_type type note).freeze
+  DIFF_ATTRS = %i(position original_position change_position).freeze
+
+  # Attribute used to store quick actions changes and users referenced.
+  attr_accessor :commands_changes
+  attr_accessor :users_referenced
+
+  # Text with quick actions filtered out
+  attr_accessor :rendered_note
+
+  belongs_to :author, class_name: 'User'
+  belongs_to :merge_request
+
+  validates :merge_request_id, presence: true
+  validates :author_id, presence: true, uniqueness: { scope: [:merge_request_id, :discussion_id] }, if: :discussion_id?
+  validates :discussion_id, allow_nil: true, format: { with: /\A\h{40}\z/ }
+
+  scope :authored_by, ->(u) { where(author_id: u.id) }
+
+  def project
+    merge_request.target_project
+  end
+
+  # noteable_id and noteable_type methods
+  # are used to generate discussion_id on Discussion.discussion_id
+  def noteable_id
+    merge_request_id
+  end
+
+  def noteable
+    merge_request
+  end
+
+  def noteable_type
+    "MergeRequest"
+  end
+
+  def for_commit?
+    false
+  end
+
+  def resolvable?
+    false
+  end
+
+  def emoji_awardable?
+    false
+  end
+
+  def on_diff?
+    position&.complete?
+  end
+
+  def type
+    return 'DiffNote' if  on_diff?
+    return 'DiscussionNote' if discussion_id.present?
+
+    'Note'
+  end
+
+  def references
+    {
+      users: users_referenced,
+      commands: commands_changes
+    }
+  end
+
+  def line_code
+    @line_code ||= original_position&.line_code(project.repository)
+  end
+
+  def file_hash
+    return unless diff_file
+
+    Digest::SHA1.hexdigest(diff_file.file_path)
+  end
+
+  def publish_params
+    attrs = PUBLISH_ATTRS.dup
+    attrs.concat(DIFF_ATTRS) if on_diff?
+    params = slice(*attrs)
+    params[:in_reply_to_discussion_id] = discussion_id if discussion_id.present?
+
+    params
+  end
+
+  def self.preload_author(draft_notes)
+    ActiveRecord::Associations::Preloader.new.preload(draft_notes, { author: :status })
+  end
+
+  private
+
+  def diff_file
+    strong_memoize(:diff_file) do
+      original_position&.diff_file(project.repository)
+    end
+  end
+end
diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb
index cb23f24cbe8a1d..c75f2e0af0374a 100644
--- a/ee/app/models/ee/merge_request.rb
+++ b/ee/app/models/ee/merge_request.rb
@@ -9,6 +9,7 @@ module MergeRequest
       has_many :approved_by_users, through: :approvals, source: :user
       has_many :approvers, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
       has_many :approver_groups, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
+      has_many :draft_notes
 
       # codeclimate_artifact is deprecated and replaced with code_quality_artifact (#5779)
       delegate :codeclimate_artifact, to: :head_pipeline, prefix: :head, allow_nil: true
diff --git a/ee/app/models/license.rb b/ee/app/models/license.rb
index 1a2dc3480a279f..6c00c7e6b0da5a 100644
--- a/ee/app/models/license.rb
+++ b/ee/app/models/license.rb
@@ -68,6 +68,7 @@ class License < ActiveRecord::Base
     packages
     code_owner_as_approver_suggestion
     feature_flags
+    batch_comments
   ].freeze
 
   EEU_FEATURES = EEP_FEATURES + %i[
diff --git a/ee/app/policies/draft_note_policy.rb b/ee/app/policies/draft_note_policy.rb
new file mode 100644
index 00000000000000..be99d12c5f8c8d
--- /dev/null
+++ b/ee/app/policies/draft_note_policy.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class DraftNotePolicy < BasePolicy
+  delegate { @subject.merge_request }
+
+  condition(:is_author) { @user && @subject.author == @user }
+
+  rule { is_author }.policy do
+    enable :read_note
+    enable :admin_note
+    enable :resolve_note
+  end
+end
diff --git a/ee/app/serializers/draft_note_entity.rb b/ee/app/serializers/draft_note_entity.rb
new file mode 100644
index 00000000000000..7b38d01db7bc85
--- /dev/null
+++ b/ee/app/serializers/draft_note_entity.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+class DraftNoteEntity < Grape::Entity
+  include RequestAwareEntity
+
+  expose :id
+  expose :author, using: NoteUserEntity
+  expose :merge_request_id
+  expose :position, if: -> (note, _) { note.on_diff? }
+  expose :line_code
+  expose :file_hash
+  expose :note
+  expose :rendered_note, as: :note_html
+  expose :references
+  expose :discussion_id
+  expose :resolve_discussion
+  expose :noteable_type
+
+  expose :current_user do
+    expose :can_edit do |note|
+      can?(current_user, :admin_note, note)
+    end
+
+    expose :can_award_emoji do |note|
+      note.emoji_awardable?
+    end
+
+    expose :can_resolve do |note|
+      note.resolvable? && can?(current_user, :resolve_note, note)
+    end
+  end
+
+  private
+
+  def current_user
+    request.current_user
+  end
+end
diff --git a/ee/app/serializers/draft_note_serializer.rb b/ee/app/serializers/draft_note_serializer.rb
new file mode 100644
index 00000000000000..282d7f9bddafad
--- /dev/null
+++ b/ee/app/serializers/draft_note_serializer.rb
@@ -0,0 +1,4 @@
+# frozen_string_literal: true
+class DraftNoteSerializer < BaseSerializer
+  entity DraftNoteEntity
+end
diff --git a/ee/app/services/draft_notes/base_service.rb b/ee/app/services/draft_notes/base_service.rb
new file mode 100644
index 00000000000000..139fd34be9b80b
--- /dev/null
+++ b/ee/app/services/draft_notes/base_service.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module DraftNotes
+  class BaseService
+    attr_accessor :merge_request, :current_user, :params
+
+    def initialize(merge_request, current_user, params = nil)
+      @merge_request, @current_user, @params = merge_request, current_user, params.dup
+    end
+
+    private
+
+    def project
+      merge_request.target_project
+    end
+  end
+end
diff --git a/ee/app/services/draft_notes/create_service.rb b/ee/app/services/draft_notes/create_service.rb
new file mode 100644
index 00000000000000..4d20e1f78e2889
--- /dev/null
+++ b/ee/app/services/draft_notes/create_service.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+module DraftNotes
+  class CreateService < DraftNotes::BaseService
+    attr_accessor :in_draft_mode, :in_reply_to_discussion_id
+
+    def initialize(merge_request, current_user, params = nil)
+      @in_reply_to_discussion_id = params.delete(:in_reply_to_discussion_id)
+      super
+    end
+
+    def execute
+      if in_reply_to_discussion_id.present?
+        unless discussion
+          return base_error('Discussion to reply to cannot be found')
+        end
+
+        params[:discussion_id] = discussion.reply_id
+      end
+
+      if params[:resolve_discussion] && !can_resolve_discussion?
+        return base_error('User is not allowed to resolve discussion')
+      end
+
+      draft_note = DraftNote.new(params)
+      draft_note.merge_request = merge_request
+      draft_note.author = current_user
+      draft_note.save
+
+      draft_note
+    end
+
+    private
+
+    def base_error(text)
+      DraftNote.new.tap do |draft|
+        draft.errors.add(:base, text)
+      end
+    end
+
+    def discussion
+      @discussion ||= merge_request.notes.find_discussion(in_reply_to_discussion_id)
+    end
+
+    def can_resolve_discussion?
+      note = discussion&.notes&.first
+      return false unless note
+
+      current_user && Ability.allowed?(current_user, :resolve_note, note)
+    end
+  end
+end
diff --git a/ee/app/services/draft_notes/publish_service.rb b/ee/app/services/draft_notes/publish_service.rb
new file mode 100644
index 00000000000000..f2a77ae4fe82fa
--- /dev/null
+++ b/ee/app/services/draft_notes/publish_service.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+module DraftNotes
+  class PublishService < DraftNotes::BaseService
+    def execute(draft_id = nil)
+      if draft_id
+        publish_draft_note(draft_id)
+      else
+        publish_draft_notes
+      end
+    end
+
+    private
+
+    def publish_draft_note(draft_id)
+      draft = DraftNote.find(draft_id)
+
+      create_note_from_draft(draft)
+      draft.delete
+
+      MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(merge_request)
+    end
+
+    def publish_draft_notes
+      draft_notes.each(&method(:create_note_from_draft))
+      draft_notes.delete_all
+
+      MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(merge_request)
+    end
+
+    def create_note_from_draft(draft)
+      note = Notes::CreateService.new(draft.project, draft.author, draft.publish_params).execute
+      set_discussion_resolve_status(note, draft)
+    end
+
+    def set_discussion_resolve_status(note, draft_note)
+      return unless draft_note.discussion_id.present?
+
+      discussion = note.discussion
+
+      if draft_note.resolve_discussion && discussion.can_resolve?(current_user)
+        discussion.resolve!(current_user)
+      else
+        discussion.unresolve!
+      end
+    end
+
+    def draft_notes
+      @draft_notes ||= merge_request.draft_notes.authored_by(current_user)
+    end
+  end
+end
diff --git a/ee/changelogs/unreleased/issue_1984.yml b/ee/changelogs/unreleased/issue_1984.yml
new file mode 100644
index 00000000000000..eb2ec9b7e3e72a
--- /dev/null
+++ b/ee/changelogs/unreleased/issue_1984.yml
@@ -0,0 +1,5 @@
+---
+title: Batch comments on merge requests
+merge_request: 4213
+author:
+type: added
diff --git a/ee/db/migrate/20180917145556_create_draft_notes.rb b/ee/db/migrate/20180917145556_create_draft_notes.rb
new file mode 100644
index 00000000000000..66ba767b53c876
--- /dev/null
+++ b/ee/db/migrate/20180917145556_create_draft_notes.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+class CreateDraftNotes < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    create_table :draft_notes, id: :bigserial do |t|
+      t.references :merge_request, null: false, index: true, foreign_key: { on_delete: :cascade }
+      t.foreign_key :users, null: false, column: :author_id, on_delete: :cascade
+      t.integer :author_id, null: false, index: true
+      t.boolean :resolve_discussion, default: false, null: false
+      t.string :discussion_id
+      t.text :note, null: false
+      t.text :position
+      t.text :original_position
+      t.text :change_position
+    end
+
+    add_index :draft_notes, :discussion_id
+  end
+end
diff --git a/ee/spec/controllers/projects/merge_requests/drafts_controller_spec.rb b/ee/spec/controllers/projects/merge_requests/drafts_controller_spec.rb
new file mode 100644
index 00000000000000..30c91adc294597
--- /dev/null
+++ b/ee/spec/controllers/projects/merge_requests/drafts_controller_spec.rb
@@ -0,0 +1,413 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Projects::MergeRequests::DraftsController do
+  let(:project)       { create(:project, :repository) }
+  let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) }
+  let(:user)          { project.owner }
+  let(:user2)         { create(:user) }
+
+  let(:params) do
+    {
+      namespace_id: project.namespace.to_param,
+      project_id: project.to_param,
+      merge_request_id: merge_request.iid
+    }
+  end
+
+  before do
+    sign_in(user)
+    stub_licensed_features(batch_comments: true)
+  end
+
+  describe 'GET #index' do
+    let!(:draft_note) { create(:draft_note, merge_request: merge_request, author: user) }
+
+    it 'list merge request draft notes for current user' do
+      get :index, params
+
+      expect(json_response.first['merge_request_id']).to eq(merge_request.id)
+      expect(json_response.first['author']['id']).to eq(user.id)
+      expect(json_response.first['note_html']).not_to be_empty
+    end
+  end
+
+  describe 'POST #create' do
+    def create_draft_note(draft_overrides: {}, overrides: {})
+      post_params = params.merge({
+        draft_note: {
+          note: 'This is a unpublished comment'
+        }.merge(draft_overrides)
+      }.merge(overrides))
+
+      post :create, post_params
+    end
+
+    context 'without permissions' do
+      let(:project) { create(:project, :private) }
+
+      before do
+        sign_in(user2)
+      end
+
+      it 'does not allow draft note creation' do
+        expect { create_draft_note }.to change { DraftNote.count }.by(0)
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
+    it 'creates a draft note' do
+      expect { create_draft_note }.to change { DraftNote.count }.by(1)
+    end
+
+    it 'creates draft note with position' do
+      diff_refs = project.commit(RepoHelpers.sample_commit.id).try(:diff_refs)
+
+      position = Gitlab::Diff::Position.new(
+        old_path: "files/ruby/popen.rb",
+        new_path: "files/ruby/popen.rb",
+        old_line: nil,
+        new_line: 14,
+        diff_refs: diff_refs
+      )
+
+      create_draft_note(draft_overrides: { position: position.to_json })
+
+      expect(response).to have_gitlab_http_status(200)
+      expect(json_response['position']).to be_present
+      expect(json_response['file_hash']).to be_present
+      expect(json_response['line_code']).to match(/\w+_\d+_\d+/)
+      expect(json_response['note_html']).to eq('<p dir="auto">This is a unpublished comment</p>')
+    end
+
+    it 'creates a draft note with quick actions' do
+      create_draft_note(draft_overrides: { note: "#{user2.to_reference}\n/assign #{user.to_reference}" })
+
+      expect(response).to have_gitlab_http_status(200)
+      expect(json_response['note_html']).to match(/#{user2.to_reference}/)
+      expect(json_response['references']['commands']).to match(/Assigns/)
+      expect(json_response['references']['users']).to include(user2.username)
+    end
+
+    context 'in a discussion' do
+      let(:discussion) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project).discussion }
+
+      it 'creates draft note as a reply' do
+        expect do
+          create_draft_note(overrides: { in_reply_to_discussion_id: discussion.reply_id })
+        end.to change { DraftNote.count }.by(1)
+
+        draft_note = DraftNote.last
+
+        expect(draft_note).to be_valid
+        expect(draft_note.discussion_id).to eq(discussion.reply_id)
+      end
+
+      it 'creates a draft note that will resolve a discussion' do
+        expect do
+          create_draft_note(
+            overrides: { in_reply_to_discussion_id: discussion.reply_id },
+            draft_overrides: { resolve_discussion: true }
+          )
+        end.to change { DraftNote.count }.by(1)
+
+        draft_note = DraftNote.last
+
+        expect(draft_note).to be_valid
+        expect(draft_note.discussion_id).to eq(discussion.reply_id)
+        expect(draft_note.resolve_discussion).to eq(true)
+      end
+
+      it 'cannot create more than one draft note per discussion' do
+        expect do
+          create_draft_note(
+            overrides: { in_reply_to_discussion_id: discussion.reply_id },
+            draft_overrides: { resolve_discussion: true }
+          )
+        end.to change { DraftNote.count }.by(1)
+
+        expect do
+          create_draft_note(
+            overrides: { in_reply_to_discussion_id: discussion.reply_id },
+            draft_overrides: { resolve_discussion: true, note: 'A note' }
+          )
+        end.to change { DraftNote.count }.by(0)
+      end
+    end
+  end
+
+  describe 'PUT #update' do
+    let(:draft) { create(:draft_note, merge_request: merge_request, author: user) }
+
+    def update_draft_note(overrides = {})
+      put_params = params.merge({
+        id: draft.id,
+        draft_note: {
+          note: 'This is an updated unpublished comment'
+        }.merge(overrides)
+      })
+
+      put :update, put_params
+    end
+
+    context 'without permissions' do
+      before do
+        sign_in(user2)
+      end
+
+      it 'does not allow editing draft note belonging to someone else' do
+        update_draft_note
+
+        expect(response).to have_gitlab_http_status(404)
+        expect(draft.reload.note).not_to eq('This is an updated unpublished comment')
+      end
+    end
+
+    it 'updates the draft' do
+      expect(draft.note).not_to be_empty
+
+      expect { update_draft_note }.not_to change { DraftNote.count }
+
+      draft.reload
+
+      expect(draft.note).to eq('This is an updated unpublished comment')
+      expect(json_response['note_html']).not_to be_empty
+    end
+  end
+
+  describe 'POST #publish' do
+    it 'publishes draft notes with position' do
+      diff_refs = project.commit(RepoHelpers.sample_commit.id).try(:diff_refs)
+
+      position = Gitlab::Diff::Position.new(
+        old_path: "files/ruby/popen.rb",
+        new_path: "files/ruby/popen.rb",
+        old_line: nil,
+        new_line: 14,
+        diff_refs: diff_refs
+      )
+
+      draft = create(:draft_note_on_text_diff, merge_request: merge_request, author: user, position: position)
+
+      expect { post :publish, params }.to change { Note.count }.by(1)
+        .and change { DraftNote.count }.by(-1)
+
+      note = merge_request.notes.reload.last
+
+      expect(note.note).to eq(draft.note)
+      expect(note.position).to eq(draft.position)
+    end
+
+    it 'does nothing if there are no draft notes' do
+      expect { post :publish, params }.to change { Note.count }.by(0).and change { DraftNote.count }.by(0)
+    end
+
+    it 'publishes a draft note with quick actions and applies them' do
+      create(:draft_note, merge_request: merge_request, author: user, note: "/assign #{user.to_reference}")
+
+      expect(merge_request.assignee_id).to be_nil
+
+      expect { post :publish, params }.to change { Note.count }.by(1)
+        .and change { DraftNote.count }.by(-1)
+
+      expect(response).to have_gitlab_http_status(200)
+      expect(merge_request.reload.assignee_id).to eq(user.id)
+      expect(Note.last.system?).to be true
+    end
+
+    it 'publishes all draft notes for an MR' do
+      draft_params = { merge_request: merge_request, author: user }
+
+      drafts = create_list(:draft_note, 4, draft_params)
+
+      note = create(:discussion_note_on_merge_request, noteable: merge_request, project: project)
+      draft_reply = create(:draft_note, draft_params.merge(discussion_id: note.discussion_id))
+
+      diff_note = create(:diff_note_on_merge_request, noteable: merge_request, project: project)
+      diff_draft_reply = create(:draft_note, draft_params.merge(discussion_id: diff_note.discussion_id))
+
+      expect { post :publish, params }.to change { Note.count }.by(6)
+        .and change { DraftNote.count }.by(-6)
+
+      expect(response).to have_gitlab_http_status(200)
+
+      notes = merge_request.notes.reload
+
+      expect(notes.pluck(:note)).to include(*drafts.map(&:note))
+      expect(note.discussion.notes.last.note).to eq(draft_reply.note)
+      expect(diff_note.discussion.notes.last.note).to eq(diff_draft_reply.note)
+    end
+
+    it 'can publish just a single draft note' do
+      draft_params = { merge_request: merge_request, author: user }
+
+      drafts = create_list(:draft_note, 4, draft_params)
+
+      expect { post :publish, params.merge(id: drafts.first.id) }.to change { Note.count }.by(1)
+        .and change { DraftNote.count }.by(-1)
+    end
+
+    context 'when publishing drafts in a discussion' do
+      let(:note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) }
+
+      def create_reply(discussion_id, resolves: false)
+        create(:draft_note,
+               merge_request: merge_request,
+               author: user,
+               discussion_id: discussion_id,
+               resolve_discussion: resolves
+              )
+      end
+
+      it 'resolves a discussion if the draft note resolves it' do
+        draft_reply = create_reply(note.discussion_id, resolves: true)
+
+        post :publish, params
+
+        discussion = note.discussion
+
+        expect(discussion.notes.last.note).to eq(draft_reply.note)
+        expect(discussion.resolved?).to eq(true)
+        expect(discussion.resolved_by.id).to eq(user.id)
+      end
+
+      it 'unresolves a discussion if the draft note unresolves it' do
+        note.discussion.resolve!(user)
+        expect(note.discussion.resolved?).to eq(true)
+
+        draft_reply = create_reply(note.discussion_id, resolves: false)
+
+        post :publish, params
+
+        discussion = note.discussion
+
+        expect(discussion.notes.last.note).to eq(draft_reply.note)
+        expect(discussion.resolved?).to eq(false)
+      end
+    end
+  end
+
+  describe 'DELETE #destroy' do
+    let(:draft) { create(:draft_note, merge_request: merge_request, author: user) }
+
+    def create_draft
+      create(:draft_note, merge_request: merge_request, author: user)
+    end
+
+    it 'destroys the draft note' do
+      draft = create_draft
+
+      expect { delete :destroy, params.merge(id: draft.id) }.to change { DraftNote.count }.by(-1)
+      expect(response).to have_gitlab_http_status(200)
+    end
+
+    context 'without permissions' do
+      before do
+        sign_in(user2)
+      end
+
+      it 'does not allow editing draft note belonging to someone else' do
+        draft = create_draft
+
+        expect { delete :destroy, params.merge(id: draft.id) }.to change { DraftNote.count }.by(0)
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+  end
+
+  describe 'DELETE #discard' do
+    it 'deletes all DraftNotes belonging to a user in a Merge Request' do
+      create_list(:draft_note, 6, merge_request: merge_request, author: user)
+
+      expect { delete :discard, params }.to change { DraftNote.count }.by(-6)
+      expect(response).to have_gitlab_http_status(200)
+    end
+  end
+
+  shared_examples_for 'batch comments feature disabled' do
+    context 'GET #index' do
+      it 'does not return existing drafts' do
+        create_list(:draft_note, 4, merge_request: merge_request, author: user)
+
+        get :index, params
+
+        expect(json_response).to eq([])
+      end
+    end
+
+    context 'POST #create' do
+      it 'errors out' do
+        expect do
+          post :create, params.merge(draft_note: { note: 'comment' })
+        end.to change { DraftNote.count }.by(0)
+
+        expect(response).to have_gitlab_http_status(403)
+      end
+    end
+
+    context 'PUT #update' do
+      it 'errors out' do
+        draft = create(:draft_note, merge_request: merge_request, author: user)
+
+        expect do
+          put :update, params.merge(id: draft.id, draft_note: { note: 'comment' })
+        end.to change { DraftNote.count }.by(0)
+
+        expect(response).to have_gitlab_http_status(403)
+      end
+    end
+
+    context 'DELETE #destroy' do
+      it 'errors out' do
+        draft = create(:draft_note, merge_request: merge_request, author: user)
+
+        expect { delete :destroy, params.merge(id: draft.id) }.to change { DraftNote.count }.by(0)
+        expect(response).to have_gitlab_http_status(403)
+      end
+    end
+
+    context 'collection endpoints' do
+      before do
+        create_list(:draft_note, 5, merge_request: merge_request, author: user)
+      end
+
+      context 'POST #publish' do
+        it 'errors out' do
+          expect do
+            post :publish, params
+          end.to change { DraftNote.count }.by(0).and change { Note.count }.by(0)
+
+          expect(response).to have_gitlab_http_status(403)
+          expect(DraftNote.count).to eq(5)
+        end
+      end
+
+      context 'DELETE #discard' do
+        it 'errors out' do
+          expect do
+            delete :discard, params
+          end.to change { DraftNote.count }.by(0)
+
+          expect(response).to have_gitlab_http_status(403)
+          expect(DraftNote.count).to eq(5)
+        end
+      end
+    end
+  end
+
+  context 'disabled due to license' do
+    before do
+      stub_licensed_features(batch_comments: false)
+    end
+
+    it_behaves_like 'batch comments feature disabled'
+  end
+
+  context 'disabled via feature flag' do
+    before do
+      stub_feature_flags(batch_comments: false)
+    end
+
+    it_behaves_like 'batch comments feature disabled'
+  end
+end
diff --git a/ee/spec/factories/draft_note.rb b/ee/spec/factories/draft_note.rb
new file mode 100644
index 00000000000000..5be6d883a1ecc1
--- /dev/null
+++ b/ee/spec/factories/draft_note.rb
@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+FactoryBot.define do
+  factory :draft_note do
+    note { generate(:title) }
+    association :author, factory: :user
+    association :merge_request, factory: :merge_request
+
+    factory :draft_note_on_text_diff do
+      transient do
+        line_number 14
+        diff_refs { merge_request.try(:diff_refs) }
+      end
+
+      position do
+        Gitlab::Diff::Position.new(
+          old_path: "files/ruby/popen.rb",
+          new_path: "files/ruby/popen.rb",
+          old_line: nil,
+          new_line: line_number,
+          diff_refs: diff_refs
+        )
+      end
+    end
+
+    factory :draft_note_on_discussion, traits: [:on_discussion]
+
+    trait :on_discussion do
+      discussion_id { create(:discussion_note_on_merge_request, noteable: merge_request, project: project).discussion_id }
+    end
+  end
+end
diff --git a/ee/spec/lib/gitlab/import_export/all_models.yml b/ee/spec/lib/gitlab/import_export/all_models.yml
index 70f1b13db4bb43..b31640ed19fc82 100644
--- a/ee/spec/lib/gitlab/import_export/all_models.yml
+++ b/ee/spec/lib/gitlab/import_export/all_models.yml
@@ -9,6 +9,7 @@ merge_requests:
 - approvers
 - approver_groups
 - approved_by_users
+- draft_notes
 pipelines:
 - source_pipeline
 - sourced_pipelines
diff --git a/ee/spec/services/draft_notes/create_service_spec.rb b/ee/spec/services/draft_notes/create_service_spec.rb
new file mode 100644
index 00000000000000..dac14e9b6acce5
--- /dev/null
+++ b/ee/spec/services/draft_notes/create_service_spec.rb
@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe DraftNotes::CreateService do
+  let(:merge_request) { create(:merge_request) }
+  let(:project) { merge_request.target_project }
+  let(:user) { merge_request.author }
+
+  def create_draft(params)
+    described_class.new(merge_request, user, params).execute
+  end
+
+  it 'creates a simple draft note' do
+    draft = create_draft(note: 'This is a test')
+
+    expect(draft).to be_an_instance_of(DraftNote)
+    expect(draft.note).to eq('This is a test')
+    expect(draft.author).to eq(user)
+    expect(draft.project).to eq(merge_request.target_project)
+    expect(draft.discussion_id).to be_nil
+  end
+
+  it 'cannot resolve when there is nothing to resolve' do
+    draft = create_draft(note: 'Not a reply!', resolve_discussion: true)
+
+    expect(draft.errors[:base]).to include('User is not allowed to resolve discussion')
+    expect(draft).not_to be_persisted
+  end
+
+  context 'in a discussion' do
+    it 'creates a draft note with discussion_id' do
+      discussion = create(:discussion_note_on_merge_request, noteable: merge_request, project: project).discussion
+
+      draft = create_draft(note: 'A reply!', in_reply_to_discussion_id: discussion.reply_id)
+
+      expect(draft.note).to eq('A reply!')
+      expect(draft.discussion_id).to eq(discussion.reply_id)
+      expect(draft.resolve_discussion).to be_falsey
+    end
+
+    it 'creates a draft that resolves the discussion' do
+      discussion = create(:discussion_note_on_merge_request, noteable: merge_request, project: project).discussion
+
+      draft = create_draft(note: 'A reply!', in_reply_to_discussion_id: discussion.reply_id, resolve_discussion: true)
+
+      expect(draft.note).to eq('A reply!')
+      expect(draft.discussion_id).to eq(discussion.reply_id)
+      expect(draft.resolve_discussion).to be true
+    end
+  end
+
+  it 'creates a draft note with a position in a diff' do
+    diff_refs = project.commit(RepoHelpers.sample_commit.id).try(:diff_refs)
+
+    position = Gitlab::Diff::Position.new(
+      old_path: "files/ruby/popen.rb",
+      new_path: "files/ruby/popen.rb",
+      old_line: nil,
+      new_line: 14,
+      diff_refs: diff_refs
+    )
+
+    draft = create_draft(note: 'Comment on diff', position: position.to_json)
+
+    expect(draft.note).to eq('Comment on diff')
+    expect(draft.original_position.to_json).to eq(position.to_json)
+  end
+end
diff --git a/ee/spec/services/draft_notes/publish_service_spec.rb b/ee/spec/services/draft_notes/publish_service_spec.rb
new file mode 100644
index 00000000000000..c2942013d9b8e9
--- /dev/null
+++ b/ee/spec/services/draft_notes/publish_service_spec.rb
@@ -0,0 +1,72 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe DraftNotes::PublishService do
+  let(:merge_request) { create(:merge_request) }
+  let(:project) { merge_request.target_project }
+  let(:user) { merge_request.author }
+
+  def publish(id: nil)
+    DraftNotes::PublishService.new(merge_request, user).execute(id)
+  end
+
+  it 'publishes a single draft note' do
+    drafts = create_list(:draft_note, 2, merge_request: merge_request, author: user)
+
+    expect { publish(id: drafts.first.id) }.to change { DraftNote.count }.by(-1).and change { Note.count }.by(1)
+    expect(DraftNote.count).to eq(1)
+  end
+
+  it 'publishes all draft notes for a user in a merge request' do
+    create_list(:draft_note, 2, merge_request: merge_request, author: user)
+
+    expect { publish }.to change { DraftNote.count }.by(-2).and change { Note.count }.by(2)
+    expect(DraftNote.count).to eq(0)
+  end
+
+  it 'only publishes the draft notes belonging to the current user' do
+    other_user = create(:user)
+    project.add_maintainer(other_user)
+
+    create_list(:draft_note, 2, merge_request: merge_request, author: user)
+    create_list(:draft_note, 2, merge_request: merge_request, author: other_user)
+
+    expect { publish }.to change { DraftNote.count }.by(-2).and change { Note.count }.by(2)
+    expect(DraftNote.count).to eq(2)
+  end
+
+  context 'with quick actions' do
+    it 'performs quick actions' do
+      create(:draft_note, merge_request: merge_request, author: user, note: "thanks\n/assign #{user.to_reference}")
+
+      expect { publish }.to change { DraftNote.count }.by(-1).and change { Note.count }.by(2)
+      expect(merge_request.reload.assignee).to eq(user)
+      expect(merge_request.notes.last).to be_system
+    end
+
+    it 'does not create a note if it only contains quick actions' do
+      create(:draft_note, merge_request: merge_request, author: user, note: "/assign #{user.to_reference}")
+
+      expect { publish }.to change { DraftNote.count }.by(-1).and change { Note.count }.by(1)
+      expect(merge_request.reload.assignee).to eq(user)
+      expect(merge_request.notes.last).to be_system
+    end
+  end
+
+  context 'with drafts that resolve discussions' do
+    let(:note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) }
+    let(:draft_note) { create(:draft_note, merge_request: merge_request, author: user, resolve_discussion: true, discussion_id: note.discussion.reply_id) }
+
+    it 'resolves the discussion' do
+      publish(id: draft_note.id)
+
+      expect(note.discussion.resolved?).to be true
+    end
+
+    it 'sends notifications if all discussions are resolved' do
+      expect_any_instance_of(MergeRequests::ResolvedDiscussionNotificationService).to receive(:execute).with(merge_request)
+
+      publish
+    end
+  end
+end
-- 
GitLab