diff options
Diffstat (limited to 'spec/services')
| -rw-r--r-- | spec/services/boards/issues/list_service_spec.rb | 26 | ||||
| -rw-r--r-- | spec/services/ci/create_pipeline_service_spec.rb | 2 | ||||
| -rw-r--r-- | spec/services/create_deployment_service_spec.rb | 8 | ||||
| -rw-r--r-- | spec/services/issues/build_service_spec.rb | 56 | ||||
| -rw-r--r-- | spec/services/issues/create_service_spec.rb | 85 | ||||
| -rw-r--r-- | spec/services/issues/resolve_discussions_spec.rb | 106 | ||||
| -rw-r--r-- | spec/services/merge_requests/get_urls_service_spec.rb | 10 | ||||
| -rw-r--r-- | spec/services/notification_service_spec.rb | 59 | ||||
| -rw-r--r-- | spec/services/todo_service_spec.rb | 24 |
9 files changed, 255 insertions, 121 deletions
diff --git a/spec/services/boards/issues/list_service_spec.rb b/spec/services/boards/issues/list_service_spec.rb index 01baedc4761..22115c6566d 100644 --- a/spec/services/boards/issues/list_service_spec.rb +++ b/spec/services/boards/issues/list_service_spec.rb @@ -43,6 +43,32 @@ describe Boards::Issues::ListService, services: true do described_class.new(project, user, params).execute end + context 'issues are ordered by priority' do + it 'returns opened issues when list_id is missing' do + params = { board_id: board.id } + + issues = described_class.new(project, user, params).execute + + expect(issues).to eq [opened_issue2, reopened_issue1, opened_issue1] + end + + it 'returns closed issues when listing issues from Done' do + params = { board_id: board.id, id: done.id } + + issues = described_class.new(project, user, params).execute + + expect(issues).to eq [closed_issue4, closed_issue2, closed_issue3, closed_issue1] + end + + it 'returns opened issues that have label list applied when listing issues from a label list' do + params = { board_id: board.id, id: list1.id } + + issues = described_class.new(project, user, params).execute + + expect(issues).to eq [list1_issue3, list1_issue1, list1_issue2] + end + end + context 'with list that does not belong to the board' do it 'raises an error' do list = create(:list) diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 8459a3d8cfb..a969829a63e 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -200,7 +200,7 @@ describe Ci::CreatePipelineService, services: true do context 'with environment' do before do - config = YAML.dump(deploy: { environment: { name: "review/$CI_BUILD_REF_NAME" }, script: 'ls' }) + config = YAML.dump(deploy: { environment: { name: "review/$CI_COMMIT_REF_NAME" }, script: 'ls' }) stub_ci_pipeline_yaml_file(config) end diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb index 18b964e2453..a883705bd45 100644 --- a/spec/services/create_deployment_service_spec.rb +++ b/spec/services/create_deployment_service_spec.rb @@ -104,16 +104,16 @@ describe CreateDeploymentService, services: true do context 'when variables are used' do let(:params) do { - environment: 'review-apps/$CI_BUILD_REF_NAME', + environment: 'review-apps/$CI_COMMIT_REF_NAME', ref: 'master', tag: false, sha: '97de212e80737a608d939f648d959671fb0a0142', options: { - name: 'review-apps/$CI_BUILD_REF_NAME', - url: 'http://$CI_BUILD_REF_NAME.review-apps.gitlab.com' + name: 'review-apps/$CI_COMMIT_REF_NAME', + url: 'http://$CI_COMMIT_REF_NAME.review-apps.gitlab.com' }, variables: [ - { key: 'CI_BUILD_REF_NAME', value: 'feature-review-apps' } + { key: 'CI_COMMIT_REF_NAME', value: 'feature-review-apps' } ] } end diff --git a/spec/services/issues/build_service_spec.rb b/spec/services/issues/build_service_spec.rb index 09807e5d35b..1dd53236fbd 100644 --- a/spec/services/issues/build_service_spec.rb +++ b/spec/services/issues/build_service_spec.rb @@ -8,24 +8,34 @@ describe Issues::BuildService, services: true do project.team << [user, :developer] end + context 'for a single discussion' do + describe '#execute' do + let(:merge_request) { create(:merge_request, title: "Hello world", source_project: project) } + let(:discussion) { Discussion.new([create(:diff_note_on_merge_request, project: project, noteable: merge_request, note: "Almost done")]) } + let(:service) { described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid, discussion_to_resolve: discussion.id) } + + it 'references the noteable title in the issue title' do + issue = service.execute + + expect(issue.title).to include('Hello world') + end + + it 'adds the note content to the description' do + 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 + let(:issue) { described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid).execute } 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) + 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_to_resolve_discussions_of: merge_request.iid) service.execute @@ -34,7 +44,7 @@ describe Issues::BuildService, services: true do end describe '#item_for_discussion' do - let(:service) { described_class.new(project, user, merge_request_for_resolving_discussions: merge_request) } + let(:service) { described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid) } it 'mentions the author of the note' do discussion = Discussion.new([create(:diff_note_on_merge_request, author: create(:user, username: 'author'))]) @@ -47,11 +57,11 @@ describe Issues::BuildService, services: true do "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" + 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>>>") + expect(service.item_for_discussion(discussion)).to include(note_result) end end @@ -66,7 +76,7 @@ describe Issues::BuildService, services: true do it 'does not assign title when a title was given' do issue = described_class.new(project, user, - merge_request_for_resolving_discussions: merge_request, + merge_request_to_resolve_discussions_of: merge_request, title: 'What an issue').execute expect(issue.title).to eq('What an issue') @@ -74,7 +84,7 @@ describe Issues::BuildService, services: true do it 'does not assign description when a description was given' do issue = described_class.new(project, user, - merge_request_for_resolving_discussions: merge_request, + merge_request_to_resolve_discussions_of: merge_request, description: 'Fix at your earliest conveignance').execute expect(issue.description).to eq('Fix at your earliest conveignance') @@ -82,7 +92,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 +109,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 @@ -112,7 +122,7 @@ describe Issues::BuildService, services: true do 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 + issue = described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid).execute expect(issue.description).to include("Review the conversation in #{merge_request.to_reference}") end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 6045d00ff09..776cbc4296b 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.id, merge_request_to_resolve_discussions_of: merge_request.iid } } - 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_to_resolve_discussions_of: 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_to_resolve_discussions_of: merge_request.iid } } + + 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_to_resolve_discussions_of: merge_request, + description: nil, + title: nil).execute + + expect(issue.description).to be_nil + expect(issue.title).to be_nil + end end end diff --git a/spec/services/issues/resolve_discussions_spec.rb b/spec/services/issues/resolve_discussions_spec.rb new file mode 100644 index 00000000000..6cc738aec08 --- /dev/null +++ b/spec/services/issues/resolve_discussions_spec.rb @@ -0,0 +1,106 @@ +require 'spec_helper.rb' + +class DummyService < Issues::BaseService + include ::Issues::ResolveDiscussions + + def initialize(*args) + super + filter_resolve_discussion_params + end +end + +describe DummyService, 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 "#merge_request_for_resolving_discussion" do + let(:service) { described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid) } + + it "finds the merge request" do + expect(service.merge_request_to_resolve_discussions_of).to eq(merge_request) + end + + it "only queries for the merge request once" do + fake_finder = double + fake_results = double + + expect(fake_finder).to receive(:execute).and_return(fake_results).exactly(1) + expect(fake_results).to receive(:find_by).exactly(1) + expect(MergeRequestsFinder).to receive(:new).and_return(fake_finder).exactly(1) + + 2.times { service.merge_request_to_resolve_discussions_of } + end + end + + describe "#discussions_to_resolve" do + it "contains a single discussion when matching merge request and discussion are passed" do + service = described_class.new( + project, + user, + discussion_to_resolve: discussion.id, + merge_request_to_resolve_discussions_of: merge_request.iid + ) + # 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) + 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_to_resolve_discussions_of: merge_request.iid + ) + # 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 "contains only unresolved discussions" do + _second_discussion = Discussion.new([create(:diff_note_on_merge_request, :resolved, + noteable: merge_request, + project: merge_request.target_project, + line_number: 15, + )]) + service = described_class.new( + project, + user, + merge_request_to_resolve_discussions_of: merge_request.iid + ) + # 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) + end + + it "is empty when a discussion and another merge request are passed" do + service = described_class.new( + project, + user, + discussion_to_resolve: discussion.id, + merge_request_to_resolve_discussions_of: other_merge_request.iid + ) + + expect(service.discussions_to_resolve).to be_empty + end + end + end +end diff --git a/spec/services/merge_requests/get_urls_service_spec.rb b/spec/services/merge_requests/get_urls_service_spec.rb index 08829e4be70..b7a05907208 100644 --- a/spec/services/merge_requests/get_urls_service_spec.rb +++ b/spec/services/merge_requests/get_urls_service_spec.rb @@ -130,5 +130,15 @@ describe MergeRequests::GetUrlsService do }]) end end + + context 'when printing_merge_request_link_enabled is false' do + it 'returns empty array' do + project.update!(printing_merge_request_link_enabled: false) + + result = service.execute(existing_branch_changes) + + expect(result).to eq([]) + end + end end end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index ebbaea4e59a..82a4ec3f581 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -146,16 +146,6 @@ describe NotificationService, services: true do should_not_email(@u_lazy_participant) end - it "emails the note author if they've opted into notifications about their activity" do - add_users_with_subscription(note.project, issue) - note.author.notified_of_own_activity = true - reset_delivered_emails! - - notification.new_note(note) - - should_email(note.author) - end - it 'filters out "mentioned in" notes' do mentioned_note = SystemNoteService.cross_reference(mentioned_issue, issue, issue.author) @@ -486,20 +476,6 @@ describe NotificationService, services: true do should_not_email(issue.assignee) end - it "emails the author if they've opted into notifications about their activity" do - issue.author.notified_of_own_activity = true - - notification.new_issue(issue, issue.author) - - should_email(issue.author) - end - - it "doesn't email the author if they haven't opted into notifications about their activity" do - notification.new_issue(issue, issue.author) - - should_not_email(issue.author) - end - it "emails subscribers of the issue's labels" do user_1 = create(:user) user_2 = create(:user) @@ -689,19 +665,6 @@ describe NotificationService, services: true do should_email(subscriber_to_label_2) end - it "emails the current user if they've opted into notifications about their activity" do - subscriber_to_label_2.notified_of_own_activity = true - notification.relabeled_issue(issue, [group_label_2, label_2], subscriber_to_label_2) - - should_email(subscriber_to_label_2) - end - - it "doesn't email the current user if they haven't opted into notifications about their activity" do - notification.relabeled_issue(issue, [group_label_2, label_2], subscriber_to_label_2) - - should_not_email(subscriber_to_label_2) - end - it "doesn't send email to anyone but subscribers of the given labels" do notification.relabeled_issue(issue, [group_label_2, label_2], @u_disabled) @@ -855,20 +818,6 @@ describe NotificationService, services: true do should_not_email(@u_lazy_participant) end - it "emails the author if they've opted into notifications about their activity" do - merge_request.author.notified_of_own_activity = true - - notification.new_merge_request(merge_request, merge_request.author) - - should_email(merge_request.author) - end - - it "doesn't email the author if they haven't opted into notifications about their activity" do - notification.new_merge_request(merge_request, merge_request.author) - - should_not_email(merge_request.author) - end - it "emails subscribers of the merge request's labels" do user_1 = create(:user) user_2 = create(:user) @@ -1064,14 +1013,6 @@ describe NotificationService, services: true do should_not_email(@u_watcher) end - it "notifies the merger when the pipeline succeeds is false but they've opted into notifications about their activity" do - merge_request.merge_when_pipeline_succeeds = false - @u_watcher.notified_of_own_activity = true - notification.merge_mr(merge_request, @u_watcher) - - should_email(@u_watcher) - end - it_behaves_like 'participating notifications' do let(:participant) { create(:user, username: 'user-participant') } let(:issuable) { merge_request } diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index a8395cb48ea..3645b73b039 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -298,6 +298,10 @@ describe TodoService, services: true do expect(second_todo.reload.state?(new_state)).to be true end + it 'returns the updated ids' do + expect(service.send(meth, collection, john_doe)).to match_array([first_todo.id, second_todo.id]) + end + describe 'cached counts' do it 'updates when todos change' do expect(john_doe.todos.where(state: new_state).count).to eq(0) @@ -706,7 +710,7 @@ describe TodoService, services: true do should_create_todo(user: admin, author: admin, target: mr_unassigned, action: Todo::UNMERGEABLE) end end - + describe '#mark_todo' do it 'creates a todo from a merge request' do service.mark_todo(mr_unassigned, author) @@ -779,29 +783,27 @@ describe TodoService, services: true do .to change { todo.reload.state }.from('pending').to('done') end - it 'returns the number of updated todos' do # Needed on API + it 'returns the ids of updated todos' do # Needed on API todo = create(:todo, :mentioned, user: john_doe, target: issue, project: project) - expect(TodoService.new.mark_todos_as_done([todo], john_doe)).to eq(1) + expect(TodoService.new.mark_todos_as_done([todo], john_doe)).to eq([todo.id]) end context 'when some of the todos are done already' do - before do - create(:todo, :mentioned, user: john_doe, target: issue, project: project) - create(:todo, :mentioned, user: john_doe, target: another_issue, project: project) - end + let!(:first_todo) { create(:todo, :mentioned, user: john_doe, target: issue, project: project) } + let!(:second_todo) { create(:todo, :mentioned, user: john_doe, target: another_issue, project: project) } - it 'returns the number of those still pending' do + it 'returns the ids of those still pending' do TodoService.new.mark_pending_todos_as_done(issue, john_doe) - expect(TodoService.new.mark_todos_as_done(Todo.all, john_doe)).to eq(1) + expect(TodoService.new.mark_todos_as_done(Todo.all, john_doe)).to eq([second_todo.id]) end - it 'returns 0 if all are done' do + it 'returns an empty array if all are done' do TodoService.new.mark_pending_todos_as_done(issue, john_doe) TodoService.new.mark_pending_todos_as_done(another_issue, john_doe) - expect(TodoService.new.mark_todos_as_done(Todo.all, john_doe)).to eq(0) + expect(TodoService.new.mark_todos_as_done(Todo.all, john_doe)).to eq([]) end end |