diff options
Diffstat (limited to 'spec/services/merge_requests')
6 files changed, 99 insertions, 54 deletions
diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb index 5a6a9df3f44..d10f82289bd 100644 --- a/spec/services/merge_requests/build_service_spec.rb +++ b/spec/services/merge_requests/build_service_spec.rb @@ -251,22 +251,22 @@ RSpec.describe MergeRequests::BuildService do end context 'when the source branch matches an issue' do - where(:issue_tracker, :source_branch, :closing_message) do - :jira | 'FOO-123-fix-issue' | 'Closes FOO-123' - :jira | 'fix-issue' | nil - :custom_issue_tracker | '123-fix-issue' | 'Closes #123' - :custom_issue_tracker | 'fix-issue' | nil - :internal | '123-fix-issue' | 'Closes #123' - :internal | 'fix-issue' | nil + where(:factory, :source_branch, :closing_message) do + :jira_service | 'FOO-123-fix-issue' | 'Closes FOO-123' + :jira_service | 'fix-issue' | nil + :custom_issue_tracker_integration | '123-fix-issue' | 'Closes #123' + :custom_issue_tracker_integration | 'fix-issue' | nil + nil | '123-fix-issue' | 'Closes #123' + nil | 'fix-issue' | nil end with_them do before do - if issue_tracker == :internal - issue.update!(iid: 123) - else - create(:"#{issue_tracker}_service", project: project) + if factory + create(factory, project: project) project.reload + else + issue.update!(iid: 123) end end @@ -350,23 +350,23 @@ RSpec.describe MergeRequests::BuildService do end context 'when the source branch matches an issue' do - where(:issue_tracker, :source_branch, :title, :closing_message) do - :jira | 'FOO-123-fix-issue' | 'Resolve FOO-123 "Fix issue"' | 'Closes FOO-123' - :jira | 'fix-issue' | 'Fix issue' | nil - :custom_issue_tracker | '123-fix-issue' | 'Resolve #123 "Fix issue"' | 'Closes #123' - :custom_issue_tracker | 'fix-issue' | 'Fix issue' | nil - :internal | '123-fix-issue' | 'Resolve "A bug"' | 'Closes #123' - :internal | 'fix-issue' | 'Fix issue' | nil - :internal | '124-fix-issue' | '124 fix issue' | nil + where(:factory, :source_branch, :title, :closing_message) do + :jira_service | 'FOO-123-fix-issue' | 'Resolve FOO-123 "Fix issue"' | 'Closes FOO-123' + :jira_service | 'fix-issue' | 'Fix issue' | nil + :custom_issue_tracker_integration | '123-fix-issue' | 'Resolve #123 "Fix issue"' | 'Closes #123' + :custom_issue_tracker_integration | 'fix-issue' | 'Fix issue' | nil + nil | '123-fix-issue' | 'Resolve "A bug"' | 'Closes #123' + nil | 'fix-issue' | 'Fix issue' | nil + nil | '124-fix-issue' | '124 fix issue' | nil end with_them do before do - if issue_tracker == :internal - issue.update!(iid: 123) - else - create(:"#{issue_tracker}_service", project: project) + if factory + create(factory, project: project) project.reload + else + issue.update!(iid: 123) end end @@ -399,23 +399,23 @@ RSpec.describe MergeRequests::BuildService do end context 'when the source branch matches an issue' do - where(:issue_tracker, :source_branch, :title, :closing_message) do - :jira | 'FOO-123-fix-issue' | 'Resolve FOO-123 "Fix issue"' | 'Closes FOO-123' - :jira | 'fix-issue' | 'Fix issue' | nil - :custom_issue_tracker | '123-fix-issue' | 'Resolve #123 "Fix issue"' | 'Closes #123' - :custom_issue_tracker | 'fix-issue' | 'Fix issue' | nil - :internal | '123-fix-issue' | 'Resolve "A bug"' | 'Closes #123' - :internal | 'fix-issue' | 'Fix issue' | nil - :internal | '124-fix-issue' | '124 fix issue' | nil + where(:factory, :source_branch, :title, :closing_message) do + :jira_service | 'FOO-123-fix-issue' | 'Resolve FOO-123 "Fix issue"' | 'Closes FOO-123' + :jira_service | 'fix-issue' | 'Fix issue' | nil + :custom_issue_tracker_integration | '123-fix-issue' | 'Resolve #123 "Fix issue"' | 'Closes #123' + :custom_issue_tracker_integration | 'fix-issue' | 'Fix issue' | nil + nil | '123-fix-issue' | 'Resolve "A bug"' | 'Closes #123' + nil | 'fix-issue' | 'Fix issue' | nil + nil | '124-fix-issue' | '124 fix issue' | nil end with_them do before do - if issue_tracker == :internal - issue.update!(iid: 123) - else - create(:"#{issue_tracker}_service", project: project) + if factory + create(factory, project: project) project.reload + else + issue.update!(iid: 123) end end diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index b2351ab53bd..da547716e1e 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -82,7 +82,7 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do let(:opts) do { title: 'Awesome merge_request', - description: "well this is not done yet\n/wip", + description: "well this is not done yet\n/draft", source_branch: 'feature', target_branch: 'master', assignees: [user2] diff --git a/spec/services/merge_requests/handle_assignees_change_service_spec.rb b/spec/services/merge_requests/handle_assignees_change_service_spec.rb index 0bf18f16abb..f9eed6eea2d 100644 --- a/spec/services/merge_requests/handle_assignees_change_service_spec.rb +++ b/spec/services/merge_requests/handle_assignees_change_service_spec.rb @@ -6,7 +6,7 @@ RSpec.describe MergeRequests::HandleAssigneesChangeService do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user) } let_it_be(:assignee) { create(:user) } - let_it_be(:merge_request) { create(:merge_request, author: user, source_project: project, assignees: [assignee]) } + let_it_be_with_reload(:merge_request) { create(:merge_request, author: user, source_project: project, assignees: [assignee]) } let_it_be(:old_assignees) { create_list(:user, 3) } let(:options) { {} } @@ -45,13 +45,27 @@ RSpec.describe MergeRequests::HandleAssigneesChangeService do service.execute(merge_request, old_assignees, options) end + let(:note) { merge_request.notes.system.last } + let(:removed_note) { "unassigned #{old_assignees.map(&:to_reference).to_sentence}" } + + context 'when unassigning all users' do + before do + merge_request.update!(assignee_ids: []) + end + + it 'creates assignee note' do + execute + + expect(note).not_to be_nil + expect(note.note).to eq removed_note + end + end + it 'creates assignee note' do execute - note = merge_request.notes.last - expect(note).not_to be_nil - expect(note.note).to include "assigned to #{assignee.to_reference} and unassigned #{old_assignees.map(&:to_reference).to_sentence}" + expect(note.note).to include "assigned to #{assignee.to_reference} and #{removed_note}" end it 'sends email notifications to old and new assignees', :mailer, :sidekiq_inline do diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index ac39fb59c62..503c0282bd6 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -181,7 +181,7 @@ RSpec.describe MergeRequests::MergeService do commit = double('commit', safe_message: "Fixes #{jira_issue.to_reference}") allow(merge_request).to receive(:commits).and_return([commit]) - expect_any_instance_of(JiraService).to receive(:close_issue).with(merge_request, jira_issue, user).once + expect_any_instance_of(Integrations::Jira).to receive(:close_issue).with(merge_request, jira_issue, user).once service.execute(merge_request) end @@ -193,7 +193,7 @@ RSpec.describe MergeRequests::MergeService do commit = double('commit', safe_message: "Fixes #{jira_issue.to_reference}") allow(merge_request).to receive(:commits).and_return([commit]) - expect_any_instance_of(JiraService).not_to receive(:close_issue) + expect_any_instance_of(Integrations::Jira).not_to receive(:close_issue) service.execute(merge_request) end diff --git a/spec/services/merge_requests/update_assignees_service_spec.rb b/spec/services/merge_requests/update_assignees_service_spec.rb index 076161c9029..3a0b17c2768 100644 --- a/spec/services/merge_requests/update_assignees_service_spec.rb +++ b/spec/services/merge_requests/update_assignees_service_spec.rb @@ -17,6 +17,7 @@ RSpec.describe MergeRequests::UpdateAssigneesService do description: "FYI #{user2.to_reference}", assignee_ids: [user3.id], source_project: project, + target_project: project, author: create(:user)) end @@ -24,6 +25,7 @@ RSpec.describe MergeRequests::UpdateAssigneesService do project.add_maintainer(user) project.add_developer(user2) project.add_developer(user3) + merge_request.errors.clear end let(:service) { described_class.new(project: project, current_user: user, params: opts) } @@ -32,35 +34,53 @@ RSpec.describe MergeRequests::UpdateAssigneesService do describe 'execute' do def update_merge_request service.execute(merge_request) - merge_request.reload + end + + shared_examples 'removing all assignees' do + it 'removes all assignees' do + expect(update_merge_request).to have_attributes(assignees: be_empty, errors: be_none) + end + + it 'enqueues the correct background work' do + expect_next(MergeRequests::HandleAssigneesChangeService, project: project, current_user: user) do |service| + expect(service) + .to receive(:async_execute) + .with(merge_request, [user3], execute_hooks: true) + end + + update_merge_request + end end context 'when the parameters are valid' do context 'when using sentinel values' do - let(:opts) { { assignee_ids: [0] } } + context 'when using assignee_ids' do + let(:opts) { { assignee_ids: [0] } } + + it_behaves_like 'removing all assignees' + end - it 'removes all assignees' do - expect { update_merge_request }.to change(merge_request, :assignees).to([]) + context 'when using assignee_id' do + let(:opts) { { assignee_id: 0 } } + + it_behaves_like 'removing all assignees' end end - context 'the assignee_ids parameter is the empty list' do + context 'when the assignee_ids parameter is the empty list' do let(:opts) { { assignee_ids: [] } } - it 'removes all assignees' do - expect { update_merge_request }.to change(merge_request, :assignees).to([]) - end + it_behaves_like 'removing all assignees' end it 'updates the MR, and queues the more expensive work for later' do expect_next(MergeRequests::HandleAssigneesChangeService, project: project, current_user: user) do |service| expect(service) - .to receive(:async_execute) - .with(merge_request, [user3], execute_hooks: true) + .to receive(:async_execute).with(merge_request, [user3], execute_hooks: true) end expect { update_merge_request } - .to change(merge_request, :assignees).to([user2]) + .to change { merge_request.reload.assignees }.from([user3]).to([user2]) .and change(merge_request, :updated_at) .and change(merge_request, :updated_by).to(user) end @@ -68,7 +88,10 @@ RSpec.describe MergeRequests::UpdateAssigneesService do it 'does not update the assignees if they do not have access' do opts[:assignee_ids] = [create(:user).id] - expect { update_merge_request }.not_to change(merge_request, :assignee_ids) + expect(update_merge_request).to have_attributes( + assignees: [user3], + errors: be_any + ) end it 'is more efficient than using the full update-service' do diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index a85fbd77d70..6ec2b158d30 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -297,6 +297,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do reviewers: [], milestone: nil, total_time_spent: 0, + time_change: 0, description: "FYI #{user2.to_reference}" } ) @@ -768,6 +769,13 @@ RSpec.describe MergeRequests::UpdateService, :mailer do update_merge_request({ target_branch: 'target' }) end + + it "does not try to mark as unchecked if it's already unchecked" do + expect(merge_request).to receive(:unchecked?).and_return(true) + expect(merge_request).not_to receive(:mark_as_unchecked) + + update_merge_request({ target_branch: "target" }) + end end context 'when auto merge is enabled and target branch changed' do |