Add latest changes from gitlab-org/gitlab@13-1-stable-ee

3 files changed, 407 insertions, 0 deletions

diff --git a/spec/services/draft_notes/create_service_spec.rb b/spec/services/draft_notes/create_service_spec.rb
new file mode 100644
index 00000000000..8f244ed386b
--- /dev/null
+++ b/spec/services/draft_notes/create_service_spec.rb
@@ -0,0 +1,94 @@
+# 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 thread')
+    expect(draft).not_to be_persisted
+  end
+
+  context 'in a thread' 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 thread' 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
+
+  context 'diff highlight cache clearing' do
+    context 'when diff file is unfolded and it is not a reply' do
+      it 'clears diff highlighting cache' do
+        expect_next_instance_of(DraftNote) do |draft|
+          allow(draft).to receive_message_chain(:diff_file, :unfolded?) { true }
+        end
+
+        expect(merge_request).to receive_message_chain(:diffs, :clear_cache)
+
+        create_draft(note: 'This is a test')
+      end
+    end
+
+    context 'when diff file is not unfolded and it is not a reply' do
+      it 'clears diff highlighting cache' do
+        expect_next_instance_of(DraftNote) do |draft|
+          allow(draft).to receive_message_chain(:diff_file, :unfolded?) { false }
+        end
+
+        expect(merge_request).not_to receive(:diffs)
+
+        create_draft(note: 'This is a test')
+      end
+    end
+  end
+end
diff --git a/spec/services/draft_notes/destroy_service_spec.rb b/spec/services/draft_notes/destroy_service_spec.rb
new file mode 100644
index 00000000000..d0bf88dcdbe
--- /dev/null
+++ b/spec/services/draft_notes/destroy_service_spec.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe DraftNotes::DestroyService do
+  let(:merge_request) { create(:merge_request) }
+  let(:project) { merge_request.target_project }
+  let(:user) { merge_request.author }
+
+  def destroy(draft_note = nil)
+    DraftNotes::DestroyService.new(merge_request, user).execute(draft_note)
+  end
+
+  it 'destroys a single draft note' do
+    drafts = create_list(:draft_note, 2, merge_request: merge_request, author: user)
+
+    expect { destroy(drafts.first) }
+      .to change { DraftNote.count }.by(-1)
+
+    expect(DraftNote.count).to eq(1)
+  end
+
+  it 'destroys all draft notes for a user in a merge request' do
+    create_list(:draft_note, 2, merge_request: merge_request, author: user)
+
+    expect { destroy }.to change { DraftNote.count }.by(-2)
+    expect(DraftNote.count).to eq(0)
+  end
+
+  context 'diff highlight cache clearing' do
+    context 'when destroying all draft notes of a user' do
+      it 'clears highlighting cache if unfold required for any' do
+        drafts = create_list(:draft_note, 2, merge_request: merge_request, author: user)
+
+        allow_any_instance_of(DraftNote).to receive_message_chain(:diff_file, :unfolded?) { true }
+        expect(merge_request).to receive_message_chain(:diffs, :clear_cache)
+
+        destroy(drafts.first)
+      end
+    end
+
+    context 'when destroying one draft note' do
+      it 'clears highlighting cache if unfold required' do
+        create_list(:draft_note, 2, merge_request: merge_request, author: user)
+
+        allow_any_instance_of(DraftNote).to receive_message_chain(:diff_file, :unfolded?) { true }
+        expect(merge_request).to receive_message_chain(:diffs, :clear_cache)
+
+        destroy
+      end
+    end
+  end
+end
diff --git a/spec/services/draft_notes/publish_service_spec.rb b/spec/services/draft_notes/publish_service_spec.rb
new file mode 100644
index 00000000000..4ebae2f9aa2
--- /dev/null
+++ b/spec/services/draft_notes/publish_service_spec.rb
@@ -0,0 +1,261 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe DraftNotes::PublishService do
+  include RepoHelpers
+
+  let(:merge_request) { create(:merge_request) }
+  let(:project) { merge_request.target_project }
+  let(:user) { merge_request.author }
+  let(:commit) { project.commit(sample_commit.id) }
+
+  let(:position) do
+    Gitlab::Diff::Position.new(
+      old_path: "files/ruby/popen.rb",
+      new_path: "files/ruby/popen.rb",
+      old_line: nil,
+      new_line: 14,
+      diff_refs: commit.diff_refs
+    )
+  end
+
+  def publish(draft: nil)
+    DraftNotes::PublishService.new(merge_request, user).execute(draft)
+  end
+
+  context 'single draft note' do
+    let(:commit_id) { nil }
+    let!(:drafts) { create_list(:draft_note, 2, merge_request: merge_request, author: user, commit_id: commit_id, position: position) }
+
+    it 'publishes' do
+      expect { publish(draft: drafts.first) }.to change { DraftNote.count }.by(-1).and change { Note.count }.by(1)
+      expect(DraftNote.count).to eq(1)
+    end
+
+    it 'does not skip notification', :sidekiq_might_not_need_inline do
+      expect(Notes::CreateService).to receive(:new).with(project, user, drafts.first.publish_params).and_call_original
+      expect_next_instance_of(NotificationService) do |notification_service|
+        expect(notification_service).to receive(:new_note)
+      end
+
+      result = publish(draft: drafts.first)
+
+      expect(result[:status]).to eq(:success)
+    end
+
+    context 'commit_id is set' do
+      let(:commit_id) { commit.id }
+
+      it 'creates note from draft with commit_id' do
+        result = publish(draft: drafts.first)
+
+        expect(result[:status]).to eq(:success)
+        expect(merge_request.notes.first.commit_id).to eq(commit_id)
+      end
+    end
+  end
+
+  context 'multiple draft notes' do
+    let(:commit_id) { nil }
+
+    before do
+      create(:draft_note, merge_request: merge_request, author: user, note: 'first note', commit_id: commit_id, position: position)
+      create(:draft_note, merge_request: merge_request, author: user, note: 'second note', commit_id: commit_id, position: position)
+    end
+
+    context 'when review fails to create' do
+      before do
+        expect_next_instance_of(Review) do |review|
+          allow(review).to receive(:save!).and_raise(ActiveRecord::RecordInvalid.new(review))
+        end
+      end
+
+      it 'does not publish any draft note' do
+        expect { publish }.not_to change { DraftNote.count }
+      end
+
+      it 'returns an error' do
+        result = publish
+
+        expect(result[:status]).to eq(:error)
+        expect(result[:message]).to match(/Unable to save Review/)
+      end
+    end
+
+    it 'returns success' do
+      result = publish
+
+      expect(result[:status]).to eq(:success)
+    end
+
+    it 'publishes all draft notes for a user in a merge request' do
+      expect { publish }.to change { DraftNote.count }.by(-2).and change { Note.count }.by(2).and change { Review.count }.by(1)
+      expect(DraftNote.count).to eq(0)
+
+      notes = merge_request.notes.order(id: :asc)
+      expect(notes.first.note).to eq('first note')
+      expect(notes.last.note).to eq('second note')
+    end
+
+    it 'sends batch notification' do
+      expect_next_instance_of(NotificationService) do |notification_service|
+        expect(notification_service).to receive_message_chain(:async, :new_review).with(kind_of(Review))
+      end
+
+      publish
+    end
+
+    context 'commit_id is set' do
+      let(:commit_id) { commit.id }
+
+      it 'creates note from draft with commit_id' do
+        result = publish
+
+        expect(result[:status]).to eq(:success)
+
+        merge_request.notes.each do |note|
+          expect(note.commit_id).to eq(commit_id)
+        end
+      end
+    end
+  end
+
+  context 'draft notes with suggestions' do
+    let(:project) { create(:project, :repository) }
+    let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
+
+    let(:suggestion_note) do
+      <<-MARKDOWN.strip_heredoc
+        ```suggestion
+          foo
+        ```
+      MARKDOWN
+    end
+
+    let!(:draft) { create(:draft_note_on_text_diff, note: suggestion_note, merge_request: merge_request, author: user) }
+
+    it 'creates a suggestion with correct content' do
+      expect { publish(draft: draft) }.to change { Suggestion.count }.by(1)
+        .and change { DiffNote.count }.from(0).to(1)
+
+      suggestion = Suggestion.last
+
+      expect(suggestion.from_line).to eq(14)
+      expect(suggestion.to_line).to eq(14)
+      expect(suggestion.from_content).to eq("    vars = {\n")
+      expect(suggestion.to_content).to eq("  foo\n")
+    end
+
+    context 'when the diff is changed' do
+      let(:file_path) { 'files/ruby/popen.rb' }
+      let(:branch_name) { project.default_branch }
+      let(:commit) { project.repository.commit }
+
+      def update_file(file_path, new_content)
+        params = {
+          file_path: file_path,
+          commit_message: "Update File",
+          file_content: new_content,
+          start_project: project,
+          start_branch: project.default_branch,
+          branch_name: branch_name
+        }
+
+        Files::UpdateService.new(project, user, params).execute
+      end
+
+      before do
+        project.add_developer(user)
+      end
+
+      it 'creates a suggestion based on the latest diff content and positions' do
+        diff_file = merge_request.diffs(paths: [file_path]).diff_files.first
+        raw_data = diff_file.new_blob.data
+
+        # Add a line break to the beginning of the file
+        result = update_file(file_path, raw_data.prepend("\n"))
+        oldrev = merge_request.diff_head_sha
+        newrev = result[:result]
+
+        expect(newrev).to be_present
+
+        # Generates new MR revision at DB level
+        refresh = MergeRequests::RefreshService.new(project, user)
+        refresh.execute(oldrev, newrev, merge_request.source_branch_ref)
+
+        expect { publish(draft: draft) }.to change { Suggestion.count }.by(1)
+          .and change { DiffNote.count }.from(0).to(1)
+
+        suggestion = Suggestion.last
+
+        expect(suggestion.from_line).to eq(15)
+        expect(suggestion.to_line).to eq(15)
+        expect(suggestion.from_content).to eq("    vars = {\n")
+        expect(suggestion.to_content).to eq("  foo\n")
+      end
+    end
+  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
+      other_user = create(:user)
+      project.add_developer(other_user)
+
+      create(:draft_note, merge_request: merge_request,
+                          author: user,
+                          note: "thanks\n/assign #{other_user.to_reference}")
+
+      expect { publish }.to change { DraftNote.count }.by(-1).and change { Note.count }.by(2)
+      expect(merge_request.reload.assignees).to match_array([other_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.assignees).to eq([user])
+      expect(merge_request.notes.last).to be_system
+    end
+  end
+
+  context 'with drafts that resolve threads' 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 thread' do
+      publish(draft: draft_note)
+
+      expect(note.discussion.resolved?).to be true
+    end
+
+    it 'sends notifications if all threads are resolved' do
+      expect_next_instance_of(MergeRequests::ResolvedDiscussionNotificationService) do |instance|
+        expect(instance).to receive(:execute).with(merge_request)
+      end
+
+      publish
+    end
+  end
+
+  context 'user cannot create notes' do
+    before do
+      allow(Ability).to receive(:allowed?).with(user, :create_note, merge_request).and_return(false)
+    end
+
+    it 'returns an error' do
+      expect(publish[:status]).to eq(:error)
+    end
+  end
+end