diff options
author | Bob Van Landuyt <bob@vanlanduyt.co> | 2016-12-22 15:31:12 +0300 |
---|---|---|
committer | Bob Van Landuyt <bob@gitlab.com> | 2017-03-13 10:27:51 +0300 |
commit | 0267b83898d604181e70c5ea8ac4a84108d2e6d6 (patch) | |
tree | 3ce8cc81ba90185c976f3b19b7e548c67856370f /spec/services/issues | |
parent | 9ed3db915026c6e0cd266a1c276386e3e96d2151 (diff) |
Delegate a single discussion to a new issue
Delegate a discussion in a merge request into a new issue.
The discussion wil be marked as resolved and a system note will be
added linking to the newly created issue.
Diffstat (limited to 'spec/services/issues')
-rw-r--r-- | spec/services/issues/base_service_spec.rb | 112 | ||||
-rw-r--r-- | spec/services/issues/build_service_spec.rb | 39 | ||||
-rw-r--r-- | spec/services/issues/create_service_spec.rb | 85 |
3 files changed, 200 insertions, 36 deletions
diff --git a/spec/services/issues/base_service_spec.rb b/spec/services/issues/base_service_spec.rb new file mode 100644 index 00000000000..bfeefe3bcc5 --- /dev/null +++ b/spec/services/issues/base_service_spec.rb @@ -0,0 +1,112 @@ +require 'spec_helper.rb' + +describe Issues::BaseService, services: true do + let(:project) { create(:project) } + let(:user) { create(:user) } + + before do + project.team << [user, :developer] + end + + describe "for resolving discussions" do + let(:discussion) { Discussion.new([create(:diff_note_on_merge_request, project: project, note: "Almost done")]) } + let(:merge_request) { discussion.noteable } + let(:other_merge_request) { create(:merge_request, source_project: project, source_branch: "other") } + + describe "#for_single_discussion" do + it "is true when only a discussion is passed" do + service = described_class.new(project, user, discussion_to_resolve: discussion) + + expect(service.for_single_discussion?).to be_truthy + end + + it "is true when matching merge request and discussion are passed" do + service = described_class.new( + project, + user, + discussion_to_resolve: discussion, + merge_request_for_resolving_discussions: merge_request + ) + + expect(service.for_single_discussion?).to be_truthy + end + + it "is false when a discussion and another merge request are passed" do + service = described_class.new( + project, + user, + discussion_to_resolve: discussion, + merge_request_for_resolving_discussions: other_merge_request + ) + + expect(service.for_single_discussion?).to be_falsy + end + end + + describe "#for_all_discussions_in_a_merge_request" do + it "is true when only a merge request is passed" do + service = described_class.new(project, user, merge_request_for_resolving_discussions: merge_request) + + expect(service.for_all_discussions_in_a_merge_request?).to be_truthy + end + + it "is false when matching merge request and discussion are passed" do + service = described_class.new( + project, + user, + discussion_to_resolve: discussion, + merge_request_for_resolving_discussions: merge_request + ) + + expect(service.for_all_discussions_in_a_merge_request?).to be_falsy + end + end + + describe "#discussions_to_resolve" do + it "only contains a single discussion when only a discussion is passed" do + service = described_class.new(project, user, discussion_to_resolve: discussion) + + expect(service.discussions_to_resolve).to contain_exactly(discussion) + end + + it "is contains a single discussion when matching merge request and discussion are passed" do + service = described_class.new( + project, + user, + discussion_to_resolve: discussion, + merge_request_for_resolving_discussions: merge_request + ) + + expect(service.discussions_to_resolve).to contain_exactly(discussion) + end + + it "contains all discussions when only a merge request is passed" do + second_discussion = Discussion.new([create(:diff_note_on_merge_request, + noteable: merge_request, + project: merge_request.target_project, + line_number: 15)]) + service = described_class.new( + project, + user, + merge_request_for_resolving_discussions: merge_request + ) + # We need to compare discussion id's because the Discussion-objects are rebuilt + # which causes the object-id's not to be different. + discussion_ids = service.discussions_to_resolve.map(&:id) + + expect(discussion_ids).to contain_exactly(discussion.id, second_discussion.id) + end + + it "is empty when a discussion and another merge request are passed" do + service = described_class.new( + project, + user, + discussion_to_resolve: discussion, + merge_request_for_resolving_discussions: other_merge_request + ) + + expect(service.discussions_to_resolve).to be_empty + end + end + end +end diff --git a/spec/services/issues/build_service_spec.rb b/spec/services/issues/build_service_spec.rb index 09807e5d35b..cca7cdbdd2d 100644 --- a/spec/services/issues/build_service_spec.rb +++ b/spec/services/issues/build_service_spec.rb @@ -8,23 +8,36 @@ describe Issues::BuildService, services: true do project.team << [user, :developer] end + context 'for a single discussion' do + describe '#execute' do + it 'references the noteable title in the issue title' do + merge_request = create(:merge_request, title: "Hello world", source_project: project) + discussion = Discussion.new([create(:note_on_merge_request, project: project, noteable: merge_request)]) + service = described_class.new(project, user, discussion_to_resolve: discussion) + + issue = service.execute + + expect(issue.title).to include('Hello world') + end + + it 'adds the note content to the description' do + discussion = Discussion.new([create(:note_on_merge_request, project: project, note: "Almost done")]) + service = described_class.new(project, user, discussion_to_resolve: discussion) + + issue = service.execute + + expect(issue.description).to include('Almost done') + end + end + 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)) + create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.source_project, line_number: 13) service = described_class.new(project, user, merge_request_for_resolving_discussions: merge_request) service.execute @@ -82,7 +95,7 @@ describe Issues::BuildService, services: true do 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)) + create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.target_project, line_number: 15) end it 'mentions all the authors in the description' do @@ -99,7 +112,7 @@ describe Issues::BuildService, services: true do 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)) + create_list(:diff_note_on_merge_request, 2, noteable: merge_request, project: merge_request.target_project, line_number: 15) expect(issue.description).to include('(+2 comments)') end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 6045d00ff09..4a5780f257a 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -140,46 +140,85 @@ describe Issues::CreateService, services: true do it_behaves_like 'new issuable record that supports slash commands' - context 'for a merge request' do + context 'resolving discussions' 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 + describe 'for a single discussion' do + let(:opts) { { discussion_to_resolve: discussion } } - expect(discussion.resolved?).to be(true) - end + it 'resolves the discussion' do + described_class.new(project, user, opts).execute + discussion.first_note.reload - it 'added a system note to the discussion' do - described_class.new(project, user, opts).execute + expect(discussion.resolved?).to be(true) + end - reloaded_discussion = MergeRequest.find(merge_request.id).discussions.first + it 'added a system note to the discussion' do + described_class.new(project, user, opts).execute - expect(reloaded_discussion.last_note.system).to eq(true) - end + 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 'assigns the title and description for the issue' do - issue = described_class.new(project, user, opts).execute + it 'can set nil explicitly 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.title).not_to be_nil - expect(issue.description).not_to be_nil + expect(issue.description).to be_nil + expect(issue.title).to be_nil + end 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 + describe 'for a merge request' do + let(:opts) { { merge_request_for_resolving_discussions: merge_request } } + + it 'resolves the discussion' do + described_class.new(project, user, opts).execute + discussion.first_note.reload - expect(issue.description).to be_nil - expect(issue.title).to be_nil + 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 explicitly 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 |