summaryrefslogtreecommitdiff
path: root/spec/services/merge_requests
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2021-04-20 14:36:54 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2021-04-20 14:36:54 +0000
commitf61bb2a16a514b71bf33aabbbb999d6732016a24 (patch)
tree9548caa89e60b4f40b99bbd1dac030420b812aa8 /spec/services/merge_requests
parent35fc54e5d261f8898e390aea7c2f5ec5fdf0539d (diff)
downloadgitlab-ce-f61bb2a16a514b71bf33aabbbb999d6732016a24.tar.gz
Add latest changes from gitlab-org/gitlab@13-11-stable-eev13.11.0-rc42
Diffstat (limited to 'spec/services/merge_requests')
-rw-r--r--spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb6
-rw-r--r--spec/services/merge_requests/after_create_service_spec.rb104
-rw-r--r--spec/services/merge_requests/base_service_spec.rb1
-rw-r--r--spec/services/merge_requests/create_pipeline_service_spec.rb2
-rw-r--r--spec/services/merge_requests/create_service_spec.rb85
-rw-r--r--spec/services/merge_requests/export_csv_service_spec.rb2
-rw-r--r--spec/services/merge_requests/handle_assignees_change_service_spec.rb114
-rw-r--r--spec/services/merge_requests/merge_orchestration_service_spec.rb1
-rw-r--r--spec/services/merge_requests/merge_service_spec.rb15
-rw-r--r--spec/services/merge_requests/merge_to_ref_service_spec.rb7
-rw-r--r--spec/services/merge_requests/push_options_handler_service_spec.rb208
-rw-r--r--spec/services/merge_requests/refresh_service_spec.rb16
-rw-r--r--spec/services/merge_requests/resolve_todos_service_spec.rb49
-rw-r--r--spec/services/merge_requests/update_assignees_service_spec.rb79
-rw-r--r--spec/services/merge_requests/update_service_spec.rb244
15 files changed, 589 insertions, 344 deletions
diff --git a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb
index 3c81ad6722d..6edaa91b8b2 100644
--- a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb
+++ b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe MergeRequests::AddTodoWhenBuildFailsService do
+RSpec.describe ::MergeRequests::AddTodoWhenBuildFailsService do
let(:user) { create(:user) }
let(:project) { create(:project, :repository) }
let(:sha) { '1234567890abcdef1234567890abcdef12345678' }
@@ -24,8 +24,8 @@ RSpec.describe MergeRequests::AddTodoWhenBuildFailsService do
before do
allow_any_instance_of(MergeRequest)
- .to receive(:head_pipeline)
- .and_return(pipeline)
+ .to receive(:head_pipeline_id)
+ .and_return(pipeline.id)
allow(service).to receive(:todo_service).and_return(todo_service)
end
diff --git a/spec/services/merge_requests/after_create_service_spec.rb b/spec/services/merge_requests/after_create_service_spec.rb
index dce351d8a31..e1f28e32164 100644
--- a/spec/services/merge_requests/after_create_service_spec.rb
+++ b/spec/services/merge_requests/after_create_service_spec.rb
@@ -93,5 +93,109 @@ RSpec.describe MergeRequests::AfterCreateService do
expect(merge_request.reload).to be_unchecked
end
end
+
+ it 'increments the usage data counter of create event' do
+ counter = Gitlab::UsageDataCounters::MergeRequestCounter
+
+ expect { execute_service }.to change { counter.read(:create) }.by(1)
+ end
+
+ context 'with a milestone' do
+ let(:milestone) { create(:milestone, project: merge_request.target_project) }
+
+ before do
+ merge_request.update!(milestone_id: milestone.id)
+ end
+
+ it 'deletes the cache key for milestone merge request counter', :use_clean_rails_memory_store_caching do
+ expect_next_instance_of(Milestones::MergeRequestsCountService, milestone) do |service|
+ expect(service).to receive(:delete_cache).and_call_original
+ end
+
+ execute_service
+ end
+ end
+
+ context 'todos' do
+ it 'does not creates todos' do
+ attributes = {
+ project: merge_request.target_project,
+ target_id: merge_request.id,
+ target_type: merge_request.class.name
+ }
+
+ expect { execute_service }.not_to change { Todo.where(attributes).count }
+ end
+
+ context 'when merge request is assigned to someone' do
+ let_it_be(:assignee) { create(:user) }
+ let_it_be(:merge_request) { create(:merge_request, assignees: [assignee]) }
+
+ it 'creates a todo for new assignee' do
+ attributes = {
+ project: merge_request.target_project,
+ author: merge_request.author,
+ user: assignee,
+ target_id: merge_request.id,
+ target_type: merge_request.class.name,
+ action: Todo::ASSIGNED,
+ state: :pending
+ }
+
+ expect { execute_service }.to change { Todo.where(attributes).count }.by(1)
+ end
+ end
+
+ context 'when reviewer is assigned' do
+ let_it_be(:reviewer) { create(:user) }
+ let_it_be(:merge_request) { create(:merge_request, reviewers: [reviewer]) }
+
+ it 'creates a todo for new reviewer' do
+ attributes = {
+ project: merge_request.target_project,
+ author: merge_request.author,
+ user: reviewer,
+ target_id: merge_request.id,
+ target_type: merge_request.class.name,
+ action: Todo::REVIEW_REQUESTED,
+ state: :pending
+ }
+
+ expect { execute_service }.to change { Todo.where(attributes).count }.by(1)
+ end
+ end
+ end
+
+ context 'when saving references to issues that the created merge request closes' do
+ let_it_be(:first_issue) { create(:issue, project: merge_request.target_project) }
+ let_it_be(:second_issue) { create(:issue, project: merge_request.target_project) }
+
+ it 'creates a `MergeRequestsClosingIssues` record for each issue' do
+ merge_request.description = "Closes #{first_issue.to_reference} and #{second_issue.to_reference}"
+ merge_request.source_branch = "feature"
+ merge_request.target_branch = merge_request.target_project.default_branch
+ merge_request.save!
+
+ execute_service
+
+ issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id)
+ expect(issue_ids).to match_array([first_issue.id, second_issue.id])
+ end
+ end
+
+ it 'tracks merge request creation in usage data' do
+ expect(Gitlab::UsageDataCounters::MergeRequestCounter).to receive(:count).with(:create)
+
+ execute_service
+ end
+
+ it 'calls MergeRequests::LinkLfsObjectsService#execute' do
+ service = instance_spy(MergeRequests::LinkLfsObjectsService)
+ allow(MergeRequests::LinkLfsObjectsService).to receive(:new).with(merge_request.target_project).and_return(service)
+
+ execute_service
+
+ expect(service).to have_received(:execute).with(merge_request)
+ end
end
end
diff --git a/spec/services/merge_requests/base_service_spec.rb b/spec/services/merge_requests/base_service_spec.rb
index 83431105545..d8ba2bc43fb 100644
--- a/spec/services/merge_requests/base_service_spec.rb
+++ b/spec/services/merge_requests/base_service_spec.rb
@@ -6,6 +6,7 @@ RSpec.describe MergeRequests::BaseService do
include ProjectForksHelper
let_it_be(:project) { create(:project, :repository) }
+
let(:title) { 'Awesome merge_request' }
let(:params) do
{
diff --git a/spec/services/merge_requests/create_pipeline_service_spec.rb b/spec/services/merge_requests/create_pipeline_service_spec.rb
index 4dd70627977..3e2e940dc24 100644
--- a/spec/services/merge_requests/create_pipeline_service_spec.rb
+++ b/spec/services/merge_requests/create_pipeline_service_spec.rb
@@ -7,6 +7,7 @@ RSpec.describe MergeRequests::CreatePipelineService do
let_it_be(:project, reload: true) { create(:project, :repository) }
let_it_be(:user) { create(:user) }
+
let(:service) { described_class.new(project, actor, params) }
let(:actor) { user }
let(:params) { {} }
@@ -50,6 +51,7 @@ RSpec.describe MergeRequests::CreatePipelineService do
context 'with fork merge request' do
let_it_be(:forked_project) { fork_project(project, nil, repository: true, target_project: create(:project, :private, :repository)) }
+
let(:source_project) { forked_project }
context 'when actor has permission to create pipelines in target project' do
diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb
index 4f47a22b07c..f2bc55103f0 100644
--- a/spec/services/merge_requests/create_service_spec.rb
+++ b/spec/services/merge_requests/create_service_spec.rb
@@ -47,16 +47,6 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
.to change { project.open_merge_requests_count }.from(0).to(1)
end
- it 'does not creates todos' do
- attributes = {
- project: project,
- target_id: merge_request.id,
- target_type: merge_request.class.name
- }
-
- expect(Todo.where(attributes).count).to be_zero
- end
-
it 'creates exactly 1 create MR event', :sidekiq_might_not_need_inline do
attributes = {
action: :created,
@@ -67,6 +57,10 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
expect(Event.where(attributes).count).to eq(1)
end
+ it 'sets the merge_status to preparing' do
+ expect(merge_request.reload).to be_preparing
+ end
+
describe 'when marked with /wip' do
context 'in title and in description' do
let(:opts) do
@@ -113,20 +107,6 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
end
it { expect(merge_request.assignees).to eq([user2]) }
-
- it 'creates a todo for new assignee' do
- attributes = {
- project: project,
- author: user,
- user: user2,
- target_id: merge_request.id,
- target_type: merge_request.class.name,
- action: Todo::ASSIGNED,
- state: :pending
- }
-
- expect(Todo.where(attributes).count).to eq 1
- end
end
context 'when reviewer is assigned' do
@@ -142,20 +122,6 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
it { expect(merge_request.reviewers).to eq([user2]) }
- it 'creates a todo for new reviewer' do
- attributes = {
- project: project,
- author: user,
- user: user2,
- target_id: merge_request.id,
- target_type: merge_request.class.name,
- action: Todo::REVIEW_REQUESTED,
- state: :pending
- }
-
- expect(Todo.where(attributes).count).to eq 1
- end
-
it 'invalidates counter cache for reviewers', :use_clean_rails_memory_store_caching do
expect { merge_request }
.to change { user2.review_requested_open_merge_requests_count }
@@ -328,12 +294,6 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
end
end
- it 'increments the usage data counter of create event' do
- counter = Gitlab::UsageDataCounters::MergeRequestCounter
-
- expect { service.execute }.to change { counter.read(:create) }.by(1)
- end
-
context 'after_save callback to store_mentions' do
let(:labels) { create_pair(:label, project: project) }
let(:milestone) { create(:milestone, project: project) }
@@ -494,35 +454,6 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
end
end
- context 'while saving references to issues that the created merge request closes' do
- let(:first_issue) { create(:issue, project: project) }
- let(:second_issue) { create(:issue, project: project) }
-
- let(:opts) do
- {
- title: 'Awesome merge_request',
- source_branch: 'feature',
- target_branch: 'master',
- force_remove_source_branch: '1'
- }
- end
-
- before do
- project.add_maintainer(user)
- project.add_developer(user2)
- end
-
- it 'creates a `MergeRequestsClosingIssues` record for each issue' do
- issue_closing_opts = opts.merge(description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}")
- service = described_class.new(project, user, issue_closing_opts)
- allow(service).to receive(:execute_hooks)
- merge_request = service.execute
-
- issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id)
- expect(issue_ids).to match_array([first_issue.id, second_issue.id])
- end
- end
-
context 'when source and target projects are different' do
let(:target_project) { fork_project(project, nil, repository: true) }
@@ -571,14 +502,6 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
expect(merge_request).to be_persisted
end
- it 'calls MergeRequests::LinkLfsObjectsService#execute', :sidekiq_might_not_need_inline do
- expect_next_instance_of(MergeRequests::LinkLfsObjectsService) do |service|
- expect(service).to receive(:execute).with(instance_of(MergeRequest))
- end
-
- described_class.new(project, user, opts).execute
- end
-
it 'does not create the merge request when the target project is archived' do
target_project.update!(archived: true)
diff --git a/spec/services/merge_requests/export_csv_service_spec.rb b/spec/services/merge_requests/export_csv_service_spec.rb
index 4ce032c396e..97217e979a5 100644
--- a/spec/services/merge_requests/export_csv_service_spec.rb
+++ b/spec/services/merge_requests/export_csv_service_spec.rb
@@ -4,6 +4,7 @@ require 'spec_helper'
RSpec.describe MergeRequests::ExportCsvService do
let_it_be(:merge_request) { create(:merge_request) }
+
let(:csv) { CSV.parse(subject.csv_data, headers: true).first }
subject { described_class.new(MergeRequest.where(id: merge_request.id), merge_request.project) }
@@ -46,6 +47,7 @@ RSpec.describe MergeRequests::ExportCsvService do
describe 'approvers' do
context 'when approved' do
let_it_be(:merge_request) { create(:merge_request) }
+
let(:approvers) { create_list(:user, 2) }
before do
diff --git a/spec/services/merge_requests/handle_assignees_change_service_spec.rb b/spec/services/merge_requests/handle_assignees_change_service_spec.rb
new file mode 100644
index 00000000000..cc595aab04b
--- /dev/null
+++ b/spec/services/merge_requests/handle_assignees_change_service_spec.rb
@@ -0,0 +1,114 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+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(:old_assignees) { create_list(:user, 3) }
+
+ let(:options) { {} }
+ let(:service) { described_class.new(project, user) }
+
+ before_all do
+ project.add_maintainer(user)
+ project.add_developer(assignee)
+
+ old_assignees.each do |old_assignee|
+ project.add_developer(old_assignee)
+ end
+ end
+
+ describe '#async_execute' do
+ def async_execute
+ service.async_execute(merge_request, old_assignees, options)
+ end
+
+ it 'performs MergeRequests::HandleAssigneesChangeWorker asynchronously' do
+ expect(MergeRequests::HandleAssigneesChangeWorker)
+ .to receive(:perform_async)
+ .with(
+ merge_request.id,
+ user.id,
+ old_assignees.map(&:id),
+ options
+ )
+
+ async_execute
+ end
+
+ context 'when async_handle_merge_request_assignees_change feature is disabled' do
+ before do
+ stub_feature_flags(async_handle_merge_request_assignees_change: false)
+ end
+
+ it 'calls #execute' do
+ expect(service).to receive(:execute).with(merge_request, old_assignees, options)
+
+ async_execute
+ end
+ end
+ end
+
+ describe '#execute' do
+ def execute
+ service.execute(merge_request, old_assignees, options)
+ 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}"
+ end
+
+ it 'sends email notifications to old and new assignees', :mailer, :sidekiq_inline do
+ perform_enqueued_jobs do
+ execute
+ end
+
+ should_email(assignee)
+ old_assignees.each do |old_assignee|
+ should_email(old_assignee)
+ end
+ end
+
+ it 'creates pending todo for assignee' do
+ execute
+
+ todo = assignee.todos.last
+
+ expect(todo).to be_pending
+ end
+
+ it 'tracks users assigned event' do
+ expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+ .to receive(:track_users_assigned_to_mr).once.with(users: [assignee])
+
+ execute
+ end
+
+ it 'tracks assignees changed event' do
+ expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+ .to receive(:track_assignees_changed_action).once.with(user: user)
+
+ execute
+ end
+
+ context 'when execute_hooks option is set to true' do
+ let(:options) { { execute_hooks: true } }
+
+ it 'execute hooks and services' do
+ expect(merge_request.project).to receive(:execute_hooks).with(anything, :merge_request_hooks)
+ expect(merge_request.project).to receive(:execute_services).with(anything, :merge_request_hooks)
+ expect(service).to receive(:enqueue_jira_connect_messages_for).with(merge_request)
+
+ execute
+ end
+ end
+ end
+end
diff --git a/spec/services/merge_requests/merge_orchestration_service_spec.rb b/spec/services/merge_requests/merge_orchestration_service_spec.rb
index 67dbb5a1a01..da37cc97857 100644
--- a/spec/services/merge_requests/merge_orchestration_service_spec.rb
+++ b/spec/services/merge_requests/merge_orchestration_service_spec.rb
@@ -4,6 +4,7 @@ require 'spec_helper'
RSpec.describe MergeRequests::MergeOrchestrationService do
let_it_be(:maintainer) { create(:user) }
+
let(:merge_params) { { sha: merge_request.diff_head_sha } }
let(:user) { maintainer }
let(:service) { described_class.new(project, user, merge_params) }
diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb
index 87e5750ce6e..c73cbad9d2f 100644
--- a/spec/services/merge_requests/merge_service_spec.rb
+++ b/spec/services/merge_requests/merge_service_spec.rb
@@ -5,6 +5,7 @@ require 'spec_helper'
RSpec.describe MergeRequests::MergeService do
let_it_be(:user) { create(:user) }
let_it_be(:user2) { create(:user) }
+
let(:merge_request) { create(:merge_request, :simple, author: user2, assignees: [user2]) }
let(:project) { merge_request.project }
@@ -166,20 +167,6 @@ RSpec.describe MergeRequests::MergeService do
service.execute(merge_request)
end
- context 'when jira_issue_transition_id is not present' do
- before do
- allow_any_instance_of(JIRA::Resource::Issue).to receive(:resolution).and_return(nil)
- end
-
- it 'does not close issue' do
- jira_tracker.update!(jira_issue_transition_id: nil)
-
- expect_any_instance_of(JiraService).not_to receive(:transition_issue)
-
- service.execute(merge_request)
- end
- end
-
context 'wrong issue markdown' do
it 'does not close issues on Jira issue tracker' do
jira_issue = ExternalIssue.new('#JIRA-123', project)
diff --git a/spec/services/merge_requests/merge_to_ref_service_spec.rb b/spec/services/merge_requests/merge_to_ref_service_spec.rb
index 14ef5b0b772..938165a807c 100644
--- a/spec/services/merge_requests/merge_to_ref_service_spec.rb
+++ b/spec/services/merge_requests/merge_to_ref_service_spec.rb
@@ -68,6 +68,7 @@ RSpec.describe MergeRequests::MergeToRefService do
end
let_it_be(:user) { create(:user) }
+
let(:merge_request) { create(:merge_request, :simple) }
let(:project) { merge_request.project }
@@ -226,6 +227,7 @@ RSpec.describe MergeRequests::MergeToRefService do
describe 'cascading merge refs' do
let_it_be(:project) { create(:project, :repository) }
+
let(:params) { { commit_message: 'Cascading merge', first_parent_ref: first_parent_ref, target_ref: target_ref, sha: merge_request.diff_head_sha } }
context 'when first merge happens' do
@@ -257,8 +259,9 @@ RSpec.describe MergeRequests::MergeToRefService do
let(:params) { { allow_conflicts: true } }
it 'calls merge_to_ref with allow_conflicts param' do
- expect(project.repository).to receive(:merge_to_ref)
- .with(anything, anything, anything, anything, anything, anything, true)
+ expect(project.repository).to receive(:merge_to_ref) do |user, **kwargs|
+ expect(kwargs[:allow_conflicts]).to eq(true)
+ end.and_call_original
service.execute(merge_request)
end
diff --git a/spec/services/merge_requests/push_options_handler_service_spec.rb b/spec/services/merge_requests/push_options_handler_service_spec.rb
index c2769d4fa88..b5086ea3a82 100644
--- a/spec/services/merge_requests/push_options_handler_service_spec.rb
+++ b/spec/services/merge_requests/push_options_handler_service_spec.rb
@@ -6,10 +6,12 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
include ProjectForksHelper
let_it_be(:project) { create(:project, :public, :repository) }
- let_it_be(:user) { create(:user, developer_projects: [project]) }
- let_it_be(:forked_project) { fork_project(project, user, repository: true) }
+ let_it_be(:user1) { create(:user, developer_projects: [project]) }
+ let_it_be(:user2) { create(:user, developer_projects: [project]) }
+ let_it_be(:user3) { create(:user, developer_projects: [project]) }
+ let_it_be(:forked_project) { fork_project(project, user1, repository: true) }
- let(:service) { described_class.new(project, user, changes, push_options) }
+ let(:service) { described_class.new(project, user1, changes, push_options) }
let(:source_branch) { 'fix' }
let(:target_branch) { 'feature' }
let(:title) { 'my title' }
@@ -23,32 +25,8 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
let(:default_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{project.default_branch}" }
let(:error_mr_required) { "A merge_request.create push option is required to create a merge request for branch #{source_branch}" }
- shared_examples_for 'a service that can create a merge request' do
- subject(:last_mr) { MergeRequest.last }
-
- it 'creates a merge request with the correct target branch and assigned user' do
- branch = push_options[:target] || project.default_branch
-
- expect { service.execute }.to change { MergeRequest.count }.by(1)
- expect(last_mr.target_branch).to eq(branch)
- expect(last_mr.assignees).to contain_exactly(user)
- end
-
- context 'when project has been forked', :sidekiq_might_not_need_inline do
- let(:forked_project) { fork_project(project, user, repository: true) }
- let(:service) { described_class.new(forked_project, user, changes, push_options) }
-
- before do
- allow(forked_project).to receive(:empty_repo?).and_return(false)
- end
-
- it 'sets the correct source and target project' do
- service.execute
-
- expect(last_mr.source_project).to eq(forked_project)
- expect(last_mr.target_project).to eq(project)
- end
- end
+ before do
+ stub_licensed_features(multiple_merge_request_assignees: false)
end
shared_examples_for 'a service that can set the target of a merge request' do
@@ -91,7 +69,7 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
expect(last_mr.auto_merge_enabled).to eq(true)
expect(last_mr.auto_merge_strategy).to eq(AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS)
- expect(last_mr.merge_user).to eq(user)
+ expect(last_mr.merge_user).to eq(user1)
expect(last_mr.merge_params['sha']).to eq(change[:newrev])
end
end
@@ -116,12 +94,6 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
end
end
- shared_examples_for 'a service that does not create a merge request' do
- it do
- expect { service.execute }.not_to change { MergeRequest.count }
- end
- end
-
shared_examples_for 'a service that does not update a merge request' do
it do
expect { service.execute }.not_to change { MergeRequest.maximum(:updated_at) }
@@ -133,6 +105,18 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
include_examples 'a service that does not update a merge request'
end
+ shared_examples 'with a deleted branch' do
+ let(:changes) { deleted_branch_changes }
+
+ it_behaves_like 'a service that does nothing'
+ end
+
+ shared_examples 'with the project default branch' do
+ let(:changes) { default_branch_changes }
+
+ it_behaves_like 'a service that does nothing'
+ end
+
describe '`create` push option' do
let(:push_options) { { create: true } }
@@ -155,17 +139,8 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that does not create a merge request'
end
- context 'with a deleted branch' do
- let(:changes) { deleted_branch_changes }
-
- it_behaves_like 'a service that does nothing'
- end
-
- context 'with the project default branch' do
- let(:changes) { default_branch_changes }
-
- it_behaves_like 'a service that does nothing'
- end
+ it_behaves_like 'with a deleted branch'
+ it_behaves_like 'with the project default branch'
end
describe '`merge_when_pipeline_succeeds` push option' do
@@ -217,17 +192,8 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that can set the merge request to merge when pipeline succeeds'
end
- context 'with a deleted branch' do
- let(:changes) { deleted_branch_changes }
-
- it_behaves_like 'a service that does nothing'
- end
-
- context 'with the project default branch' do
- let(:changes) { default_branch_changes }
-
- it_behaves_like 'a service that does nothing'
- end
+ it_behaves_like 'with a deleted branch'
+ it_behaves_like 'with the project default branch'
end
describe '`remove_source_branch` push option' do
@@ -239,11 +205,9 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that does not create a merge request'
it 'adds an error to the service' do
- error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
-
service.execute
- expect(service.errors).to include(error)
+ expect(service.errors).to include(error_mr_required)
end
context 'when coupled with the `create` push option' do
@@ -281,17 +245,8 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that can remove the source branch when it is merged'
end
- context 'with a deleted branch' do
- let(:changes) { deleted_branch_changes }
-
- it_behaves_like 'a service that does nothing'
- end
-
- context 'with the project default branch' do
- let(:changes) { default_branch_changes }
-
- it_behaves_like 'a service that does nothing'
- end
+ it_behaves_like 'with a deleted branch'
+ it_behaves_like 'with the project default branch'
end
describe '`target` push option' do
@@ -343,17 +298,8 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that can set the target of a merge request'
end
- context 'with a deleted branch' do
- let(:changes) { deleted_branch_changes }
-
- it_behaves_like 'a service that does nothing'
- end
-
- context 'with the project default branch' do
- let(:changes) { default_branch_changes }
-
- it_behaves_like 'a service that does nothing'
- end
+ it_behaves_like 'with a deleted branch'
+ it_behaves_like 'with the project default branch'
end
describe '`title` push option' do
@@ -405,17 +351,8 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that can set the title of a merge request'
end
- context 'with a deleted branch' do
- let(:changes) { deleted_branch_changes }
-
- it_behaves_like 'a service that does nothing'
- end
-
- context 'with the project default branch' do
- let(:changes) { default_branch_changes }
-
- it_behaves_like 'a service that does nothing'
- end
+ it_behaves_like 'with a deleted branch'
+ it_behaves_like 'with the project default branch'
end
describe '`description` push option' do
@@ -467,17 +404,8 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that can set the description of a merge request'
end
- context 'with a deleted branch' do
- let(:changes) { deleted_branch_changes }
-
- it_behaves_like 'a service that does nothing'
- end
-
- context 'with the project default branch' do
- let(:changes) { default_branch_changes }
-
- it_behaves_like 'a service that does nothing'
- end
+ it_behaves_like 'with a deleted branch'
+ it_behaves_like 'with the project default branch'
end
describe '`label` push option' do
@@ -529,17 +457,8 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that can change labels of a merge request', 2
end
- context 'with a deleted branch' do
- let(:changes) { deleted_branch_changes }
-
- it_behaves_like 'a service that does nothing'
- end
-
- context 'with the project default branch' do
- let(:changes) { default_branch_changes }
-
- it_behaves_like 'a service that does nothing'
- end
+ it_behaves_like 'with a deleted branch'
+ it_behaves_like 'with the project default branch'
end
describe '`unlabel` push option' do
@@ -551,11 +470,9 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that does not create a merge request'
it 'adds an error to the service' do
- error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
-
service.execute
- expect(service.errors).to include(error)
+ expect(service.errors).to include(error_mr_required)
end
context 'when coupled with the `create` push option' do
@@ -572,11 +489,9 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that does not create a merge request'
it 'adds an error to the service' do
- error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
-
service.execute
- expect(service.errors).to include(error)
+ expect(service.errors).to include(error_mr_required)
end
context 'when coupled with the `create` push option' do
@@ -595,17 +510,42 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that can change labels of a merge request', 1
end
- context 'with a deleted branch' do
- let(:changes) { deleted_branch_changes }
+ it_behaves_like 'with a deleted branch'
+ it_behaves_like 'with the project default branch'
+ end
+
+ shared_examples 'with an existing branch that has a merge request open in foss' do
+ let(:changes) { existing_branch_changes }
+ let!(:merge_request) { create(:merge_request, source_project: project, source_branch: source_branch)}
- it_behaves_like 'a service that does nothing'
- end
+ it_behaves_like 'a service that does not create a merge request'
+ it_behaves_like 'a service that can change assignees of a merge request', 1
+ end
- context 'with the project default branch' do
- let(:changes) { default_branch_changes }
+ describe '`assign` push option' do
+ let(:assigned) { { user2.id => 1, user3.id => 1 } }
+ let(:unassigned) { nil }
+ let(:push_options) { { assign: assigned, unassign: unassigned } }
- it_behaves_like 'a service that does nothing'
- end
+ it_behaves_like 'with a new branch', 1
+ it_behaves_like 'with an existing branch but no open MR', 1
+ it_behaves_like 'with an existing branch that has a merge request open in foss'
+
+ it_behaves_like 'with a deleted branch'
+ it_behaves_like 'with the project default branch'
+ end
+
+ describe '`unassign` push option' do
+ let(:assigned) { { user2.id => 1, user3.id => 1 } }
+ let(:unassigned) { { user1.id => 1, user3.id => 1 } }
+ let(:push_options) { { assign: assigned, unassign: unassigned } }
+
+ it_behaves_like 'with a new branch', 1
+ it_behaves_like 'with an existing branch but no open MR', 1
+ it_behaves_like 'with an existing branch that has a merge request open in foss'
+
+ it_behaves_like 'with a deleted branch'
+ it_behaves_like 'with the project default branch'
end
describe 'multiple pushed branches' do
@@ -645,7 +585,9 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
end
describe 'no user' do
- let(:user) { nil }
+ let(:user1) { nil }
+ let(:user2) { nil }
+ let(:user3) { nil }
let(:push_options) { { create: true } }
let(:changes) { new_branch_changes }
@@ -661,7 +603,7 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
let(:changes) { new_branch_changes }
it 'records an error' do
- Members::DestroyService.new(user).execute(ProjectMember.find_by!(user_id: user.id))
+ Members::DestroyService.new(user1).execute(ProjectMember.find_by!(user_id: user1.id))
service.execute
@@ -707,7 +649,7 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
end
describe 'when MRs are not enabled' do
- let(:project) { create(:project, :public, :repository).tap { |pr| pr.add_developer(user) } }
+ let(:project) { create(:project, :public, :repository).tap { |pr| pr.add_developer(user1) } }
let(:push_options) { { create: true } }
let(:changes) { new_branch_changes }
diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb
index 2abe7a23bfe..f9b76db877b 100644
--- a/spec/services/merge_requests/refresh_service_spec.rb
+++ b/spec/services/merge_requests/refresh_service_spec.rb
@@ -198,7 +198,7 @@ RSpec.describe MergeRequests::RefreshService do
end
end
- describe 'Pipelines for merge requests' do
+ shared_examples 'Pipelines for merge requests' do
before do
stub_ci_pipeline_yaml_file(config)
end
@@ -256,7 +256,7 @@ RSpec.describe MergeRequests::RefreshService do
stub_feature_flags(ci_disallow_to_create_merge_request_pipelines_in_target_project: false)
end
- it 'creates detached merge request pipeline for fork merge request', :sidekiq_inline do
+ it 'creates detached merge request pipeline for fork merge request' do
expect { subject }
.to change { @fork_merge_request.pipelines_for_merge_request.count }.by(1)
@@ -364,6 +364,18 @@ RSpec.describe MergeRequests::RefreshService do
end
end
+ context 'when the code_review_async_pipeline_creation feature flag is on', :sidekiq_inline do
+ it_behaves_like 'Pipelines for merge requests'
+ end
+
+ context 'when the code_review_async_pipeline_creation feature flag is off', :sidekiq_inline do
+ before do
+ stub_feature_flags(code_review_async_pipeline_creation: false)
+ end
+
+ it_behaves_like 'Pipelines for merge requests'
+ end
+
context 'push to origin repo source branch' do
let(:refresh_service) { service.new(@project, @user) }
let(:notification_service) { spy('notification_service') }
diff --git a/spec/services/merge_requests/resolve_todos_service_spec.rb b/spec/services/merge_requests/resolve_todos_service_spec.rb
new file mode 100644
index 00000000000..3e6f2ea3f5d
--- /dev/null
+++ b/spec/services/merge_requests/resolve_todos_service_spec.rb
@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe MergeRequests::ResolveTodosService do
+ let_it_be(:merge_request) { create(:merge_request) }
+ let_it_be(:user) { create(:user) }
+
+ let(:service) { described_class.new(merge_request, user) }
+
+ describe '#async_execute' do
+ def async_execute
+ service.async_execute
+ end
+
+ it 'performs MergeRequests::ResolveTodosWorker asynchronously' do
+ expect(MergeRequests::ResolveTodosWorker)
+ .to receive(:perform_async)
+ .with(
+ merge_request.id,
+ user.id
+ )
+
+ async_execute
+ end
+
+ context 'when resolve_merge_request_todos_async feature is disabled' do
+ before do
+ stub_feature_flags(resolve_merge_request_todos_async: false)
+ end
+
+ it 'calls #execute' do
+ expect(service).to receive(:execute)
+
+ async_execute
+ end
+ end
+ end
+
+ describe '#execute' do
+ it 'marks pending todo as done' do
+ pending_todo = create(:todo, :pending, user: user, project: merge_request.project, target: merge_request)
+
+ service.execute
+
+ expect(pending_todo.reload).to be_done
+ end
+ end
+end
diff --git a/spec/services/merge_requests/update_assignees_service_spec.rb b/spec/services/merge_requests/update_assignees_service_spec.rb
new file mode 100644
index 00000000000..de03aab5418
--- /dev/null
+++ b/spec/services/merge_requests/update_assignees_service_spec.rb
@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe MergeRequests::UpdateAssigneesService do
+ include AfterNextHelpers
+
+ let_it_be(:group) { create(:group, :public) }
+ let_it_be(:project) { create(:project, :private, :repository, group: group) }
+ let_it_be(:user) { create(:user) }
+ let_it_be(:user2) { create(:user) }
+ let_it_be(:user3) { create(:user) }
+
+ let_it_be_with_reload(:merge_request) do
+ create(:merge_request, :simple, :unique_branches,
+ title: 'Old title',
+ description: "FYI #{user2.to_reference}",
+ assignee_ids: [user3.id],
+ source_project: project,
+ author: create(:user))
+ end
+
+ before do
+ project.add_maintainer(user)
+ project.add_developer(user2)
+ project.add_developer(user3)
+ end
+
+ let(:service) { described_class.new(project, user, opts) }
+ let(:opts) { { assignee_ids: [user2.id] } }
+
+ describe 'execute' do
+ def update_merge_request
+ service.execute(merge_request)
+ merge_request.reload
+ end
+
+ context 'when the parameters are valid' do
+ it 'updates the MR, and queues the more expensive work for later' do
+ expect_next(MergeRequests::HandleAssigneesChangeService, project, user) do |service|
+ expect(service)
+ .to receive(:async_execute)
+ .with(merge_request, [user3], execute_hooks: true)
+ end
+
+ expect { update_merge_request }
+ .to change(merge_request, :assignees).to([user2])
+ .and change(merge_request, :updated_at)
+ .and change(merge_request, :updated_by).to(user)
+ end
+
+ 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)
+ end
+
+ it 'is more efficient than using the full update-service' do
+ allow_next(MergeRequests::HandleAssigneesChangeService, project, user) do |service|
+ expect(service)
+ .to receive(:async_execute)
+ .with(merge_request, [user3], execute_hooks: true)
+ end
+
+ other_mr = create(:merge_request, :simple, :unique_branches,
+ title: merge_request.title,
+ description: merge_request.description,
+ assignee_ids: merge_request.assignee_ids,
+ source_project: merge_request.project,
+ author: merge_request.author)
+
+ update_service = ::MergeRequests::UpdateService.new(project, user, opts)
+
+ expect { service.execute(merge_request) }
+ .to issue_fewer_queries_than { update_service.execute(other_mr) }
+ end
+ end
+ end
+end
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index 7a7f684c6d0..8c010855eb2 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -205,30 +205,6 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
MergeRequests::UpdateService.new(project, user, opts).execute(merge_request)
end
- context 'assignees' do
- context 'when assignees changed' do
- it 'tracks assignees changed event' do
- expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
- .to receive(:track_assignees_changed_action).once.with(user: user)
-
- opts[:assignees] = [user2]
-
- MergeRequests::UpdateService.new(project, user, opts).execute(merge_request)
- end
- end
-
- context 'when assignees did not change' do
- it 'does not track assignees changed event' do
- expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
- .not_to receive(:track_assignees_changed_action)
-
- opts[:assignees] = merge_request.assignees
-
- MergeRequests::UpdateService.new(project, user, opts).execute(merge_request)
- end
- end
- end
-
context 'reviewers' do
context 'when reviewers changed' do
it 'tracks reviewers changed event' do
@@ -272,6 +248,41 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
it_behaves_like 'updates milestone'
end
+
+ context 'milestone counters cache reset' do
+ let(:milestone_old) { create(:milestone, project: project) }
+ let(:opts) { { milestone: milestone_old } }
+
+ it 'deletes milestone counters' do
+ expect_next_instance_of(Milestones::MergeRequestsCountService, milestone_old) do |service|
+ expect(service).to receive(:delete_cache).and_call_original
+ end
+
+ expect_next_instance_of(Milestones::MergeRequestsCountService, milestone) do |service|
+ expect(service).to receive(:delete_cache).and_call_original
+ end
+
+ update_merge_request(milestone: milestone)
+ end
+
+ it 'deletes milestone counters when the milestone is removed' do
+ expect_next_instance_of(Milestones::MergeRequestsCountService, milestone_old) do |service|
+ expect(service).to receive(:delete_cache).and_call_original
+ end
+
+ update_merge_request(milestone: nil)
+ end
+
+ it 'deletes milestone counters when the milestone was not set' do
+ update_merge_request(milestone: nil)
+
+ expect_next_instance_of(Milestones::MergeRequestsCountService, milestone) do |service|
+ expect(service).to receive(:delete_cache).and_call_original
+ end
+
+ update_merge_request(milestone: milestone)
+ end
+ end
end
it 'executes hooks with update action' do
@@ -291,21 +302,6 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
)
end
- it 'sends email to user2 about assign of new merge request and email to user3 about merge request unassignment', :sidekiq_might_not_need_inline do
- deliveries = ActionMailer::Base.deliveries
- email = deliveries.last
- recipients = deliveries.last(2).flat_map(&:to)
- expect(recipients).to include(user2.email, user3.email)
- expect(email.subject).to include(merge_request.title)
- end
-
- it 'creates system note about merge_request reassign' do
- note = find_note('assigned to')
-
- expect(note).not_to be_nil
- expect(note.note).to include "assigned to #{user.to_reference} and unassigned #{user3.to_reference}"
- end
-
context 'with reviewers' do
let(:opts) { { reviewer_ids: [user2.id] } }
@@ -594,62 +590,54 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
let!(:pending_todo) { create(:todo, :assigned, user: user, project: project, target: merge_request, author: user2) }
context 'when the title change' do
- before do
- update_merge_request({ title: 'New title' })
- end
+ it 'calls MergeRequest::ResolveTodosService#async_execute' do
+ expect_next_instance_of(MergeRequests::ResolveTodosService, merge_request, user) do |service|
+ expect(service).to receive(:async_execute)
+ end
- it 'marks pending todos as done' do
- expect(pending_todo.reload).to be_done
+ update_merge_request({ title: 'New title' })
end
it 'does not create any new todos' do
+ update_merge_request({ title: 'New title' })
+
expect(Todo.count).to eq(1)
end
end
context 'when the description change' do
- before do
- update_merge_request({ description: "Also please fix #{user2.to_reference} #{user3.to_reference}" })
- end
+ it 'calls MergeRequest::ResolveTodosService#async_execute' do
+ expect_next_instance_of(MergeRequests::ResolveTodosService, merge_request, user) do |service|
+ expect(service).to receive(:async_execute)
+ end
- it 'marks pending todos as done' do
- expect(pending_todo.reload).to be_done
+ update_merge_request({ description: "Also please fix #{user2.to_reference} #{user3.to_reference}" })
end
it 'creates only 1 new todo' do
+ update_merge_request({ description: "Also please fix #{user2.to_reference} #{user3.to_reference}" })
+
expect(Todo.count).to eq(2)
end
end
context 'when is reassigned' do
- before do
- update_merge_request({ assignee_ids: [user2.id] })
- end
-
- it 'marks previous assignee pending todos as done' do
- expect(pending_todo.reload).to be_done
- end
-
- it 'creates a pending todo for new assignee' do
- attributes = {
- project: project,
- author: user,
- user: user2,
- target_id: merge_request.id,
- target_type: merge_request.class.name,
- action: Todo::ASSIGNED,
- state: :pending
- }
+ it 'calls MergeRequest::ResolveTodosService#async_execute' do
+ expect_next_instance_of(MergeRequests::ResolveTodosService, merge_request, user) do |service|
+ expect(service).to receive(:async_execute)
+ end
- expect(Todo.where(attributes).count).to eq 1
+ update_merge_request({ assignee_ids: [user2.id] })
end
end
context 'when reviewers gets changed' do
- it 'marks pending todo as done' do
- update_merge_request({ reviewer_ids: [user2.id] })
+ it 'calls MergeRequest::ResolveTodosService#async_execute' do
+ expect_next_instance_of(MergeRequests::ResolveTodosService, merge_request, user) do |service|
+ expect(service).to receive(:async_execute)
+ end
- expect(pending_todo.reload).to be_done
+ update_merge_request({ reviewer_ids: [user2.id] })
end
it 'creates a pending todo for new review request' do
@@ -727,10 +715,12 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
end
end
- it 'marks pending todos as done' do
- update_merge_request({ milestone: create(:milestone, project: project) })
+ it 'calls MergeRequests::ResolveTodosService#async_execute' do
+ expect_next_instance_of(MergeRequests::ResolveTodosService, merge_request, user) do |service|
+ expect(service).to receive(:async_execute)
+ end
- expect(pending_todo.reload).to be_done
+ update_merge_request({ milestone: create(:milestone, project: project) })
end
it 'sends notifications for subscribers of changed milestone', :sidekiq_might_not_need_inline do
@@ -744,17 +734,19 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
end
context 'when the labels change' do
- before do
- travel_to(1.minute.from_now) do
- update_merge_request({ label_ids: [label.id] })
+ it 'calls MergeRequests::ResolveTodosService#async_execute' do
+ expect_next_instance_of(MergeRequests::ResolveTodosService, merge_request, user) do |service|
+ expect(service).to receive(:async_execute)
end
- end
- it 'marks pending todos as done' do
- expect(pending_todo.reload).to be_done
+ update_merge_request({ label_ids: [label.id] })
end
it 'updates updated_at' do
+ travel_to(1.minute.from_now) do
+ update_merge_request({ label_ids: [label.id] })
+ end
+
expect(merge_request.reload.updated_at).to be > Time.current
end
end
@@ -769,24 +761,26 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
end
context 'when the target branch change' do
- before do
- update_merge_request({ target_branch: 'target' })
- end
+ it 'calls MergeRequests::ResolveTodosService#async_execute' do
+ expect_next_instance_of(MergeRequests::ResolveTodosService, merge_request, user) do |service|
+ expect(service).to receive(:async_execute)
+ end
- it 'marks pending todos as done' do
- expect(pending_todo.reload).to be_done
+ update_merge_request({ target_branch: 'target' })
end
end
context 'when auto merge is enabled and target branch changed' do
before do
AutoMergeService.new(project, user, { sha: merge_request.diff_head_sha }).execute(merge_request, AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS)
-
- update_merge_request({ target_branch: 'target' })
end
- it 'marks pending todos as done' do
- expect(pending_todo.reload).to be_done
+ it 'calls MergeRequests::ResolveTodosService#async_execute' do
+ expect_next_instance_of(MergeRequests::ResolveTodosService, merge_request, user) do |service|
+ expect(service).to receive(:async_execute)
+ end
+
+ update_merge_request({ target_branch: 'target' })
end
end
end
@@ -948,18 +942,8 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
end
it 'removes `MergeRequestsClosingIssues` records when issues are not closed anymore' do
- opts = {
- title: 'Awesome merge_request',
- description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}",
- source_branch: 'feature',
- target_branch: 'master',
- force_remove_source_branch: '1'
- }
-
- merge_request = MergeRequests::CreateService.new(project, user, opts).execute
-
- issue_ids = MergeRequestsClosingIssues.where(merge_request: merge_request).pluck(:issue_id)
- expect(issue_ids).to match_array([first_issue.id, second_issue.id])
+ create(:merge_requests_closing_issues, issue: first_issue, merge_request: merge_request)
+ create(:merge_requests_closing_issues, issue: second_issue, merge_request: merge_request)
service = described_class.new(project, user, description: "not closing any issues")
allow(service).to receive(:execute_hooks)
@@ -971,9 +955,45 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
end
context 'updating asssignee_ids' do
+ context ':use_specialized_service' do
+ context 'when true' do
+ it 'passes the update action to ::MergeRequests::UpdateAssigneesService' do
+ expect(::MergeRequests::UpdateAssigneesService)
+ .to receive(:new).and_call_original
+
+ update_merge_request({
+ assignee_ids: [user2.id],
+ use_specialized_service: true
+ })
+ end
+ end
+
+ context 'when false or nil' do
+ before do
+ expect(::MergeRequests::UpdateAssigneesService).not_to receive(:new)
+ end
+
+ it 'does not pass the update action to ::MergeRequests::UpdateAssigneesService when false' do
+ update_merge_request({
+ assignee_ids: [user2.id],
+ use_specialized_service: false
+ })
+ end
+
+ it 'does not pass the update action to ::MergeRequests::UpdateAssigneesService when nil' do
+ update_merge_request({
+ assignee_ids: [user2.id],
+ use_specialized_service: nil
+ })
+ end
+ end
+ end
+
it 'does not update assignee when assignee_id is invalid' do
merge_request.update!(assignee_ids: [user.id])
+ expect(MergeRequests::HandleAssigneesChangeService).not_to receive(:new)
+
update_merge_request(assignee_ids: [-1])
expect(merge_request.reload.assignees).to eq([user])
@@ -982,29 +1002,35 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
it 'unassigns assignee when user id is 0' do
merge_request.update!(assignee_ids: [user.id])
+ expect_next_instance_of(MergeRequests::HandleAssigneesChangeService, project, user) do |service|
+ expect(service)
+ .to receive(:async_execute)
+ .with(merge_request, [user])
+ end
+
update_merge_request(assignee_ids: [0])
expect(merge_request.assignee_ids).to be_empty
end
it 'saves assignee when user id is valid' do
+ expect_next_instance_of(MergeRequests::HandleAssigneesChangeService, project, user) do |service|
+ expect(service)
+ .to receive(:async_execute)
+ .with(merge_request, [user3])
+ end
+
update_merge_request(assignee_ids: [user.id])
expect(merge_request.assignee_ids).to eq([user.id])
end
- it 'updates the tracking when user ids are valid' do
- expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
- .to receive(:track_users_assigned_to_mr)
- .with(users: [user])
-
- update_merge_request(assignee_ids: [user.id])
- end
-
it 'does not update assignee_id when user cannot read issue' do
non_member = create(:user)
original_assignees = merge_request.assignees
+ expect(MergeRequests::HandleAssigneesChangeService).not_to receive(:new)
+
update_merge_request(assignee_ids: [non_member.id])
expect(merge_request.reload.assignees).to eq(original_assignees)