Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBob Van Landuyt <bob@vanlanduyt.co>2016-10-27 00:21:50 +0300
committerBob Van Landuyt <bob@vanlanduyt.co>2016-12-05 22:55:45 +0300
commit1123057ab792ac73b1611f4d3a9faf79369dd6da (patch)
treeae88332a5d2d47d5a814583fbe874582ebaf859c /spec/services
parent5fedc46343d054df28ce81a320f33d1d43d02bc8 (diff)
Feature: delegate all open discussions to Issue
When a merge request can only be merged when all discussions are resolved. This feature allows to easily delegate those discussions to a new issue, while marking them as resolved in the merge request. The user is presented with a new issue, prepared with mentions of all unresolved discussions, including the first unresolved note of the discussion, time and link to the note. When the issue is created, the discussions in the merge request will get a system note directing the user to the newly created issue.
Diffstat (limited to 'spec/services')
-rw-r--r--spec/services/discussions/resolve_service_spec.rb52
-rw-r--r--spec/services/issues/build_service_spec.rb130
-rw-r--r--spec/services/issues/create_service_spec.rb43
-rw-r--r--spec/services/system_note_service_spec.rb28
4 files changed, 253 insertions, 0 deletions
diff --git a/spec/services/discussions/resolve_service_spec.rb b/spec/services/discussions/resolve_service_spec.rb
new file mode 100644
index 00000000000..12c3cdf28c6
--- /dev/null
+++ b/spec/services/discussions/resolve_service_spec.rb
@@ -0,0 +1,52 @@
+require 'spec_helper'
+
+describe Discussions::ResolveService do
+ describe '#execute' do
+ let(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request)]).first }
+ let(:project) { merge_request.project }
+ let(:merge_request) { discussion.noteable }
+ let(:user) { create(:user) }
+ let(:service) { described_class.new(discussion.noteable.project, user, merge_request: merge_request) }
+
+ before do
+ project.team << [user, :master]
+ end
+
+ it "doesn't resolve discussions the user can't resolve" do
+ expect(discussion).to receive(:can_resolve?).with(user).and_return(false)
+
+ service.execute(discussion)
+
+ expect(discussion.resolved?).to be(false)
+ end
+
+ it 'resolves the discussion' do
+ service.execute(discussion)
+
+ expect(discussion.resolved?).to be(true)
+ end
+
+ it 'executes the notification service' do
+ expect_any_instance_of(MergeRequests::ResolvedDiscussionNotificationService).to receive(:execute).with(discussion.noteable)
+
+ service.execute(discussion)
+ end
+
+ it 'adds a system note to the discussion' do
+ issue = create(:issue, project: project)
+
+ expect(SystemNoteService).to receive(:discussion_continued_in_issue).with(discussion, project, user, issue)
+ service = described_class.new(project, user, merge_request: merge_request, follow_up_issue: issue)
+ service.execute(discussion)
+ end
+
+ it 'can resolve multiple discussions at once' do
+ other_discussion = Discussion.for_diff_notes([create(:diff_note_on_merge_request, noteable: discussion.noteable, project: discussion.noteable.source_project)]).first
+
+ service.execute([discussion, other_discussion])
+
+ expect(discussion.resolved?).to be(true)
+ expect(other_discussion.resolved?).to be(true)
+ end
+ end
+end
diff --git a/spec/services/issues/build_service_spec.rb b/spec/services/issues/build_service_spec.rb
new file mode 100644
index 00000000000..4cfba35c830
--- /dev/null
+++ b/spec/services/issues/build_service_spec.rb
@@ -0,0 +1,130 @@
+require 'spec_helper.rb'
+
+describe Issues::BuildService, services: true do
+ let(:project) { create(:project) }
+ let(:user) { create(:user) }
+
+ before do
+ project.team << [user, :developer]
+ end
+
+ context 'for discussions in a merge request' do
+ let(:merge_request) { create(:merge_request_with_diff_notes, source_project: project) }
+ let(:issue) { described_class.new(project, user, merge_request_for_resolving_discussions: merge_request).execute }
+
+ def position_on_line(line_number)
+ 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: merge_request.diff_refs
+ )
+ end
+
+ describe '#items_for_discussions' do
+ it 'has an item for each discussion' do
+ create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.source_project, position: position_on_line(13))
+ service = described_class.new(project, user, merge_request_for_resolving_discussions: merge_request)
+
+ service.execute
+
+ expect(service.items_for_discussions.size).to eq(2)
+ end
+ end
+
+ describe '#item_for_discussion' do
+ let(:service) { described_class.new(project, user, merge_request_for_resolving_discussions: merge_request) }
+
+ it 'mentions the author of the note' do
+ discussion = Discussion.new([create(:diff_note_on_merge_request, author: create(:user, username: 'author'))])
+ expect(service.item_for_discussion(discussion)).to include('@author')
+ end
+
+ it 'wraps the note in a blockquote' do
+ note_text = "This is a string\n"\
+ ">>>\n"\
+ "with a blockquote\n"\
+ "> That has a quote\n"\
+ ">>>\n"
+ note_result = "This is a string\n"\
+ "> with a blockquote\n"\
+ "> > That has a quote\n"
+ discussion = Discussion.new([create(:diff_note_on_merge_request, note: note_text)])
+ expect(service.item_for_discussion(discussion)).to include(">>>\n#{note_result}\n>>>")
+ end
+ end
+
+ describe '#execute' do
+ it 'has the merge request reference in the title' do
+ expect(issue.title).to include(merge_request.title)
+ end
+
+ it 'has the reference of the merge request in the description' do
+ expect(issue.description).to include(merge_request.to_reference)
+ end
+
+ it 'does not assign title when a title was given' do
+ issue = described_class.new(project, user,
+ merge_request_for_resolving_discussions: merge_request,
+ title: 'What an issue').execute
+
+ expect(issue.title).to eq('What an issue')
+ end
+
+ it 'does not assign description when a description was given' do
+ issue = described_class.new(project, user,
+ merge_request_for_resolving_discussions: merge_request,
+ description: 'Fix at your earliest conveignance').execute
+
+ expect(issue.description).to eq('Fix at your earliest conveignance')
+ end
+
+ describe 'with multiple discussions' do
+ before do
+ create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.target_project, position: position_on_line(15))
+ end
+
+ it 'mentions all the authors in the description' do
+ authors = merge_request.diff_discussions.map(&:author)
+
+ expect(issue.description).to include(*authors.map(&:to_reference))
+ end
+
+ it 'has a link for each unresolved discussion in the description' do
+ notes = merge_request.diff_discussions.map(&:first_note)
+ links = notes.map { |note| Gitlab::UrlBuilder.build(note) }
+
+ expect(issue.description).to include(*links)
+ end
+
+ it 'mentions additional notes' do
+ create_list(:diff_note_on_merge_request, 2, noteable: merge_request, project: merge_request.target_project, position: position_on_line(15))
+
+ expect(issue.description).to include('(+2 comments)')
+ end
+ end
+ end
+ end
+
+ context 'For a merge request without discussions' do
+ let(:merge_request) { create(:merge_request, source_project: project) }
+
+ describe '#execute' do
+ it 'mentions the merge request in the description' do
+ issue = described_class.new(project, user, merge_request_for_resolving_discussions: merge_request).execute
+
+ expect(issue.description).to include("Review the conversation in #{merge_request.to_reference}")
+ end
+ end
+ end
+
+ describe '#execute' do
+ it 'builds a new issues with given params' do
+ issue = described_class.new(project, user, title: 'Issue #1', description: 'Issue description').execute
+
+ expect(issue.title).to eq('Issue #1')
+ expect(issue.description).to eq('Issue description')
+ end
+ end
+end
diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb
index 5c0331ebe66..8bde61ee336 100644
--- a/spec/services/issues/create_service_spec.rb
+++ b/spec/services/issues/create_service_spec.rb
@@ -136,5 +136,48 @@ describe Issues::CreateService, services: true do
end
it_behaves_like 'new issuable record that supports slash commands'
+
+ context 'for a merge request' do
+ let(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request)]).first }
+ let(:merge_request) { discussion.noteable }
+ let(:project) { merge_request.source_project }
+ let(:opts) { { merge_request_for_resolving_discussions: merge_request } }
+
+ before do
+ project.team << [user, :master]
+ end
+
+ it 'resolves the discussion for the merge request' do
+ described_class.new(project, user, opts).execute
+ discussion.first_note.reload
+
+ expect(discussion.resolved?).to be(true)
+ end
+
+ it 'added a system note to the discussion' do
+ described_class.new(project, user, opts).execute
+
+ reloaded_discussion = MergeRequest.find(merge_request.id).discussions.first
+
+ expect(reloaded_discussion.last_note.system).to eq(true)
+ end
+
+ it 'assigns the title and description for the issue' do
+ issue = described_class.new(project, user, opts).execute
+
+ expect(issue.title).not_to be_nil
+ expect(issue.description).not_to be_nil
+ end
+
+ it 'can set nil explicityly to the title and description' do
+ issue = described_class.new(project, user,
+ merge_request_for_resolving_discussions: merge_request,
+ description: nil,
+ title: nil).execute
+
+ expect(issue.description).to be_nil
+ expect(issue.title).to be_nil
+ end
+ end
end
end
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index 435cfb07292..07a9d8e1997 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -712,4 +712,32 @@ describe SystemNoteService, services: true do
end
end
end
+
+ describe '.discussion_continued_in_issue' do
+ let(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request)]).first }
+ let(:merge_request) { discussion.noteable }
+ let(:project) { merge_request.source_project }
+ let(:issue) { create(:issue, project: project) }
+ let(:user) { create(:user) }
+
+ def reloaded_merge_request
+ MergeRequest.find(merge_request.id)
+ end
+
+ before do
+ project.team << [user, :developer]
+ end
+
+ it 'creates a new note in the discussion' do
+ # we need to completely rebuild the merge request object, or the `@discussions` on the merge request are not reloaded.
+ expect { SystemNoteService.discussion_continued_in_issue(discussion, project, user, issue) }.
+ to change { reloaded_merge_request.discussions.first.notes.size }.by(1)
+ end
+
+ it 'mentions the created issue in the system note' do
+ note = SystemNoteService.discussion_continued_in_issue(discussion, project, user, issue)
+
+ expect(note.note).to include(issue.to_reference)
+ end
+ end
end