diff options
Diffstat (limited to 'spec/services')
46 files changed, 1653 insertions, 235 deletions
diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index fa5014cee07..1ff1438ba06 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -34,6 +34,42 @@ describe Ci::CreatePipelineService, services: true do it { expect(pipeline).to have_attributes(status: 'pending') } it { expect(pipeline.builds.first).to be_kind_of(Ci::Build) } + context '#update_merge_requests_head_pipeline' do + it 'updates head pipeline of each merge request' do + merge_request_1 = create(:merge_request, source_branch: 'master', target_branch: "branch_1", source_project: project) + merge_request_2 = create(:merge_request, source_branch: 'master', target_branch: "branch_2", source_project: project) + + head_pipeline = pipeline + + expect(merge_request_1.reload.head_pipeline).to eq(head_pipeline) + expect(merge_request_2.reload.head_pipeline).to eq(head_pipeline) + end + + context 'when there is no pipeline for source branch' do + it "does not update merge request head pipeline" do + merge_request = create(:merge_request, source_branch: 'other_branch', target_branch: "branch_1", source_project: project) + + head_pipeline = pipeline + + expect(merge_request.reload.head_pipeline).not_to eq(head_pipeline) + end + end + + context 'when merge request target project is different from source project' do + let!(:target_project) { create(:empty_project) } + let!(:forked_project_link) { create(:forked_project_link, forked_to_project: project, forked_from_project: target_project) } + + it 'updates head pipeline for merge request' do + merge_request = + create(:merge_request, source_branch: 'master', target_branch: "branch_1", source_project: project, target_project: target_project) + + head_pipeline = pipeline + + expect(merge_request.reload.head_pipeline).to eq(head_pipeline) + end + end + end + context 'auto-cancel enabled' do before do project.update(auto_cancel_pending_pipelines: 'enabled') diff --git a/spec/services/ci/expire_pipeline_cache_service_spec.rb b/spec/services/ci/expire_pipeline_cache_service_spec.rb deleted file mode 100644 index 166c6dfc93e..00000000000 --- a/spec/services/ci/expire_pipeline_cache_service_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -require 'spec_helper' - -describe Ci::ExpirePipelineCacheService, services: true do - let(:user) { create(:user) } - let(:project) { create(:empty_project) } - let(:pipeline) { create(:ci_pipeline, project: project) } - subject { described_class.new(project, user) } - - describe '#execute' do - it 'invalidate Etag caching for project pipelines path' do - pipelines_path = "/#{project.full_path}/pipelines.json" - new_mr_pipelines_path = "/#{project.full_path}/merge_requests/new.json" - - expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(pipelines_path) - expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(new_mr_pipelines_path) - - subject.execute(pipeline) - end - - it 'updates the cached status for a project' do - expect(Gitlab::Cache::Ci::ProjectPipelineStatus).to receive(:update_for_pipeline). - with(pipeline) - - subject.execute(pipeline) - end - end -end diff --git a/spec/services/ci/play_build_service_spec.rb b/spec/services/ci/play_build_service_spec.rb new file mode 100644 index 00000000000..d6f9fa42045 --- /dev/null +++ b/spec/services/ci/play_build_service_spec.rb @@ -0,0 +1,105 @@ +require 'spec_helper' + +describe Ci::PlayBuildService, '#execute', :services do + let(:user) { create(:user) } + let(:project) { create(:empty_project) } + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:build) { create(:ci_build, :manual, pipeline: pipeline) } + + let(:service) do + described_class.new(project, user) + end + + context 'when project does not have repository yet' do + let(:project) { create(:empty_project) } + + it 'allows user with master role to play build' do + project.add_master(user) + + service.execute(build) + + expect(build.reload).to be_pending + end + + it 'does not allow user with developer role to play build' do + project.add_developer(user) + + expect { service.execute(build) } + .to raise_error Gitlab::Access::AccessDeniedError + end + end + + context 'when project has repository' do + let(:project) { create(:project) } + + it 'allows user with developer role to play a build' do + project.add_developer(user) + + service.execute(build) + + expect(build.reload).to be_pending + end + end + + context 'when build is a playable manual action' do + let(:build) { create(:ci_build, :manual, pipeline: pipeline) } + + before do + project.add_master(user) + end + + it 'enqueues the build' do + expect(service.execute(build)).to eq build + expect(build.reload).to be_pending + end + + it 'reassignes build user correctly' do + service.execute(build) + + expect(build.reload.user).to eq user + end + end + + context 'when build is not a playable manual action' do + let(:build) { create(:ci_build, when: :manual, pipeline: pipeline) } + + before do + project.add_master(user) + end + + it 'duplicates the build' do + duplicate = service.execute(build) + + expect(duplicate).not_to eq build + expect(duplicate).to be_pending + end + + it 'assigns users correctly' do + duplicate = service.execute(build) + + expect(build.user).not_to eq user + expect(duplicate.user).to eq user + end + end + + context 'when build is not action' do + let(:build) { create(:ci_build, :success, pipeline: pipeline) } + + it 'raises an error' do + expect { service.execute(build) } + .to raise_error Gitlab::Access::AccessDeniedError + end + end + + context 'when user does not have ability to trigger action' do + before do + create(:protected_branch, :no_one_can_push, + name: build.ref, project: project) + end + + it 'raises an error' do + expect { service.execute(build) } + .to raise_error Gitlab::Access::AccessDeniedError + end + end +end diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb index 245e19822f3..fc5de5d069a 100644 --- a/spec/services/ci/process_pipeline_service_spec.rb +++ b/spec/services/ci/process_pipeline_service_spec.rb @@ -268,6 +268,24 @@ describe Ci::ProcessPipelineService, '#execute', :services do end end + context 'when there are only manual actions in stages' do + before do + create_build('image', stage_idx: 0, when: 'manual', allow_failure: true) + create_build('build', stage_idx: 1, when: 'manual', allow_failure: true) + create_build('deploy', stage_idx: 2, when: 'manual') + create_build('check', stage_idx: 3) + + process_pipeline + end + + it 'processes all jobs until blocking actions encountered' do + expect(all_builds_statuses).to eq(%w[manual manual manual created]) + expect(all_builds_names).to eq(%w[image build deploy check]) + + expect(pipeline.reload).to be_blocked + end + end + context 'when blocking manual actions are defined' do before do create_build('code:test', stage_idx: 0) @@ -314,6 +332,13 @@ describe Ci::ProcessPipelineService, '#execute', :services do end context 'when pipeline is promoted sequentially up to the end' do + before do + # We are using create(:empty_project), and users has to be master in + # order to execute manual action when repository does not exist. + # + project.add_master(user) + end + it 'properly processes entire pipeline' do process_pipeline @@ -418,6 +443,21 @@ describe Ci::ProcessPipelineService, '#execute', :services do end end + context 'updates a list of retried builds' do + subject { described_class.retried.order(:id) } + + let!(:build_retried) { create_build('build') } + let!(:build) { create_build('build') } + let!(:test) { create_build('test') } + + it 'returns unique statuses' do + process_pipeline + + expect(all_builds.latest).to contain_exactly(build, test) + expect(all_builds.retried).to contain_exactly(build_retried) + end + end + def process_pipeline described_class.new(pipeline.project, user).execute(pipeline) end @@ -434,6 +474,10 @@ describe Ci::ProcessPipelineService, '#execute', :services do builds.pluck(:name) end + def all_builds_names + all_builds.pluck(:name) + end + def builds_statuses builds.pluck(:status) end diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index b2d37657770..7254e6b357a 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -22,7 +22,7 @@ describe Ci::RetryBuildService, :services do %i[type lock_version target_url base_tags commit_id deployments erased_by_id last_deployment project_id runner_id tag_taggings taggings tags trigger_request_id - user_id auto_canceled_by_id].freeze + user_id auto_canceled_by_id retried].freeze shared_examples 'build duplication' do let(:build) do @@ -115,7 +115,7 @@ describe Ci::RetryBuildService, :services do end describe '#reprocess' do - let(:new_build) { service.reprocess(build) } + let(:new_build) { service.reprocess!(build) } context 'when user has ability to execute build' do before do @@ -131,11 +131,16 @@ describe Ci::RetryBuildService, :services do it 'does not enqueue the new build' do expect(new_build).to be_created end + + it 'does mark old build as retried' do + expect(new_build).to be_latest + expect(build.reload).to be_retried + end end context 'when user does not have ability to execute build' do it 'raises an error' do - expect { service.reprocess(build) } + expect { service.reprocess!(build) } .to raise_error Gitlab::Access::AccessDeniedError end end diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb index f1b2d3a4798..d941d56c0d8 100644 --- a/spec/services/ci/retry_pipeline_service_spec.rb +++ b/spec/services/ci/retry_pipeline_service_spec.rb @@ -7,11 +7,13 @@ describe Ci::RetryPipelineService, '#execute', :services do let(:service) { described_class.new(project, user) } context 'when user has ability to modify pipeline' do - let(:user) { create(:admin) } + before do + project.add_master(user) + end context 'when there are already retried jobs present' do before do - create_build('rspec', :canceled, 0) + create_build('rspec', :canceled, 0, retried: true) create_build('rspec', :failed, 0) end @@ -227,6 +229,46 @@ describe Ci::RetryPipelineService, '#execute', :services do end end + context 'when user is not allowed to trigger manual action' do + before do + project.add_developer(user) + end + + context 'when there is a failed manual action present' do + before do + create_build('test', :failed, 0) + create_build('deploy', :failed, 0, when: :manual) + create_build('verify', :canceled, 1) + end + + it 'does not reprocess manual action' do + service.execute(pipeline) + + expect(build('test')).to be_pending + expect(build('deploy')).to be_failed + expect(build('verify')).to be_created + expect(pipeline.reload).to be_running + end + end + + context 'when there is a failed manual action in later stage' do + before do + create_build('test', :failed, 0) + create_build('deploy', :failed, 1, when: :manual) + create_build('verify', :canceled, 2) + end + + it 'does not reprocess manual action' do + service.execute(pipeline) + + expect(build('test')).to be_pending + expect(build('deploy')).to be_failed + expect(build('verify')).to be_created + expect(pipeline.reload).to be_running + end + end + end + def statuses pipeline.reload.statuses end diff --git a/spec/services/ci/stop_environments_service_spec.rb b/spec/services/ci/stop_environments_service_spec.rb index 32c72a9cf5e..98044ad232e 100644 --- a/spec/services/ci/stop_environments_service_spec.rb +++ b/spec/services/ci/stop_environments_service_spec.rb @@ -55,8 +55,22 @@ describe Ci::StopEnvironmentsService, services: true do end context 'when user does not have permission to stop environment' do + context 'when user has no access to manage deployments' do + before do + project.team << [user, :guest] + end + + it 'does not stop environment' do + expect_environment_not_stopped_on('master') + end + end + end + + context 'when branch for stop action is protected' do before do - project.team << [user, :guest] + project.add_developer(user) + create(:protected_branch, :no_one_can_push, + name: 'master', project: project) end it 'does not stop environment' do diff --git a/spec/services/cohorts_service_spec.rb b/spec/services/cohorts_service_spec.rb index 1e99442fdcb..77595d7ba2d 100644 --- a/spec/services/cohorts_service_spec.rb +++ b/spec/services/cohorts_service_spec.rb @@ -89,7 +89,7 @@ describe CohortsService do activity_months: [{ total: 2, percentage: 100 }], total: 2, inactive: 1 - }, + } ] expect(described_class.new.execute).to eq(months_included: 12, diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb index a883705bd45..f35d7a33548 100644 --- a/spec/services/create_deployment_service_spec.rb +++ b/spec/services/create_deployment_service_spec.rb @@ -255,7 +255,7 @@ describe CreateDeploymentService, services: true do environment: 'production', ref: 'master', tag: false, - sha: '97de212e80737a608d939f648d959671fb0a0142b', + sha: '97de212e80737a608d939f648d959671fb0a0142b' } end diff --git a/spec/services/delete_merged_branches_service_spec.rb b/spec/services/delete_merged_branches_service_spec.rb index 7b921f606f8..cae74df9c90 100644 --- a/spec/services/delete_merged_branches_service_spec.rb +++ b/spec/services/delete_merged_branches_service_spec.rb @@ -6,33 +6,22 @@ describe DeleteMergedBranchesService, services: true do let(:project) { create(:project, :repository) } context '#execute' do - context 'unprotected branches' do - before do - service.execute - end + it 'deletes a branch that was merged' do + service.execute - it 'deletes a branch that was merged' do - expect(project.repository.branch_names).not_to include('improve/awesome') - end + expect(project.repository.branch_names).not_to include('improve/awesome') + end - it 'keeps branch that is unmerged' do - expect(project.repository.branch_names).to include('feature') - end + it 'keeps branch that is unmerged' do + service.execute - it 'keeps "master"' do - expect(project.repository.branch_names).to include('master') - end + expect(project.repository.branch_names).to include('feature') end - context 'protected branches' do - before do - create(:protected_branch, name: 'improve/awesome', project: project) - service.execute - end + it 'keeps "master"' do + service.execute - it 'keeps protected branch' do - expect(project.repository.branch_names).to include('improve/awesome') - end + expect(project.repository.branch_names).to include('master') end context 'user without rights' do diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 0477cac6677..ab06f45dbb9 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -584,7 +584,7 @@ describe GitPushService, services: true do commit = double(:commit) diff = double(:diff, new_path: 'README.md') - expect(commit).to receive(:raw_diffs).with(deltas_only: true). + expect(commit).to receive(:raw_deltas). and_return([diff]) service.push_commits = [commit] @@ -622,12 +622,21 @@ describe GitPushService, services: true do it 'only schedules a limited number of commits' do allow(service).to receive(:push_commits). - and_return(Array.new(1000, double(:commit, to_hash: {}))) + and_return(Array.new(1000, double(:commit, to_hash: {}, matches_cross_reference_regex?: true))) expect(ProcessCommitWorker).to receive(:perform_async).exactly(100).times service.process_commit_messages end + + it "skips commits which don't include cross-references" do + allow(service).to receive(:push_commits). + and_return([double(:commit, to_hash: {}, matches_cross_reference_regex?: false)]) + + expect(ProcessCommitWorker).not_to receive(:perform_async) + + service.process_commit_messages + end end def execute_service(project, user, oldrev, newrev, ref) diff --git a/spec/services/issuable/bulk_update_service_spec.rb b/spec/services/issuable/bulk_update_service_spec.rb index 7a1ac027310..6437d00e451 100644 --- a/spec/services/issuable/bulk_update_service_spec.rb +++ b/spec/services/issuable/bulk_update_service_spec.rb @@ -4,11 +4,12 @@ describe Issuable::BulkUpdateService, services: true do let(:user) { create(:user) } let(:project) { create(:empty_project, namespace: user.namespace) } - def bulk_update(issues, extra_params = {}) + def bulk_update(issuables, extra_params = {}) bulk_update_params = extra_params - .reverse_merge(issuable_ids: Array(issues).map(&:id).join(',')) + .reverse_merge(issuable_ids: Array(issuables).map(&:id).join(',')) - Issuable::BulkUpdateService.new(project, user, bulk_update_params).execute('issue') + type = Array(issuables).first.model_name.param_key + Issuable::BulkUpdateService.new(project, user, bulk_update_params).execute(type) end describe 'close issues' do @@ -47,40 +48,77 @@ describe Issuable::BulkUpdateService, services: true do end end - describe 'updating assignee' do - let(:issue) { create(:issue, project: project, assignee: user) } + describe 'updating merge request assignee' do + let(:merge_request) { create(:merge_request, target_project: project, source_project: project, assignee: user) } context 'when the new assignee ID is a valid user' do it 'succeeds' do new_assignee = create(:user) project.team << [new_assignee, :developer] - result = bulk_update(issue, assignee_id: new_assignee.id) + result = bulk_update(merge_request, assignee_id: new_assignee.id) expect(result[:success]).to be_truthy expect(result[:count]).to eq(1) end - it 'updates the assignee to the use ID passed' do + it 'updates the assignee to the user ID passed' do assignee = create(:user) project.team << [assignee, :developer] - expect { bulk_update(issue, assignee_id: assignee.id) } - .to change { issue.reload.assignee }.from(user).to(assignee) + expect { bulk_update(merge_request, assignee_id: assignee.id) } + .to change { merge_request.reload.assignee }.from(user).to(assignee) end end context "when the new assignee ID is #{IssuableFinder::NONE}" do it "unassigns the issues" do - expect { bulk_update(issue, assignee_id: IssuableFinder::NONE) } - .to change { issue.reload.assignee }.to(nil) + expect { bulk_update(merge_request, assignee_id: IssuableFinder::NONE) } + .to change { merge_request.reload.assignee }.to(nil) end end context 'when the new assignee ID is not present' do it 'does not unassign' do - expect { bulk_update(issue, assignee_id: nil) } - .not_to change { issue.reload.assignee } + expect { bulk_update(merge_request, assignee_id: nil) } + .not_to change { merge_request.reload.assignee } + end + end + end + + describe 'updating issue assignee' do + let(:issue) { create(:issue, project: project, assignees: [user]) } + + context 'when the new assignee ID is a valid user' do + it 'succeeds' do + new_assignee = create(:user) + project.team << [new_assignee, :developer] + + result = bulk_update(issue, assignee_ids: [new_assignee.id]) + + expect(result[:success]).to be_truthy + expect(result[:count]).to eq(1) + end + + it 'updates the assignee to the user ID passed' do + assignee = create(:user) + project.team << [assignee, :developer] + expect { bulk_update(issue, assignee_ids: [assignee.id]) } + .to change { issue.reload.assignees.first }.from(user).to(assignee) + end + end + + context "when the new assignee ID is #{IssuableFinder::NONE}" do + it "unassigns the issues" do + expect { bulk_update(issue, assignee_ids: [IssuableFinder::NONE.to_s]) } + .to change { issue.reload.assignees.count }.from(1).to(0) + end + end + + context 'when the new assignee ID is not present' do + it 'does not unassign' do + expect { bulk_update(issue, assignee_ids: []) } + .not_to change{ issue.reload.assignees } end end end @@ -125,7 +163,7 @@ describe Issuable::BulkUpdateService, services: true do { label_ids: labels.map(&:id), add_label_ids: add_labels.map(&:id), - remove_label_ids: remove_labels.map(&:id), + remove_label_ids: remove_labels.map(&:id) } end diff --git a/spec/services/issues/build_service_spec.rb b/spec/services/issues/build_service_spec.rb index 55d635235b0..bed25fe7ccf 100644 --- a/spec/services/issues/build_service_spec.rb +++ b/spec/services/issues/build_service_spec.rb @@ -136,7 +136,7 @@ describe Issues::BuildService, services: true do user, title: 'Issue #1', description: 'Issue description', - milestone_id: milestone.id, + milestone_id: milestone.id ).execute expect(issue.title).to eq('Issue #1') diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb index 7a54373963e..51840531711 100644 --- a/spec/services/issues/close_service_spec.rb +++ b/spec/services/issues/close_service_spec.rb @@ -4,7 +4,7 @@ describe Issues::CloseService, services: true do let(:user) { create(:user) } let(:user2) { create(:user) } let(:guest) { create(:user) } - let(:issue) { create(:issue, assignee: user2) } + let(:issue) { create(:issue, assignees: [user2]) } let(:project) { issue.project } let!(:todo) { create(:todo, :assigned, user: user, project: project, target: issue, author: user2) } diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 80bfb731550..dab1a3469f7 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -6,10 +6,10 @@ describe Issues::CreateService, services: true do describe '#execute' do let(:issue) { described_class.new(project, user, opts).execute } + let(:assignee) { create(:user) } + let(:milestone) { create(:milestone, project: project) } context 'when params are valid' do - let(:assignee) { create(:user) } - let(:milestone) { create(:milestone, project: project) } let(:labels) { create_pair(:label, project: project) } before do @@ -20,7 +20,7 @@ describe Issues::CreateService, services: true do let(:opts) do { title: 'Awesome issue', description: 'please fix', - assignee_id: assignee.id, + assignee_ids: [assignee.id], label_ids: labels.map(&:id), milestone_id: milestone.id, due_date: Date.tomorrow } @@ -29,7 +29,7 @@ describe Issues::CreateService, services: true do it 'creates the issue with the given params' do expect(issue).to be_persisted expect(issue.title).to eq('Awesome issue') - expect(issue.assignee).to eq assignee + expect(issue.assignees).to eq [assignee] expect(issue.labels).to match_array labels expect(issue.milestone).to eq milestone expect(issue.due_date).to eq Date.tomorrow @@ -37,6 +37,7 @@ describe Issues::CreateService, services: true do context 'when current user cannot admin issues in the project' do let(:guest) { create(:user) } + before do project.team << [guest, :guest] end @@ -47,7 +48,7 @@ describe Issues::CreateService, services: true do expect(issue).to be_persisted expect(issue.title).to eq('Awesome issue') expect(issue.description).to eq('please fix') - expect(issue.assignee).to be_nil + expect(issue.assignees).to be_empty expect(issue.labels).to be_empty expect(issue.milestone).to be_nil expect(issue.due_date).to be_nil @@ -117,6 +118,22 @@ describe Issues::CreateService, services: true do end end + context 'when assignee is set' do + let(:opts) do + { title: 'Title', + description: 'Description', + assignees: [assignee] } + end + + it 'invalidates open issues counter for assignees when issue is assigned' do + project.team << [assignee, :master] + + described_class.new(project, user, opts).execute + + expect(assignee.assigned_open_issues_count).to eq 1 + end + end + it 'executes issue hooks when issue is not confidential' do opts = { title: 'Title', description: 'Description', confidential: false } @@ -136,10 +153,83 @@ describe Issues::CreateService, services: true do end end - it_behaves_like 'issuable create service' + context 'issue create service' do + context 'assignees' do + before { project.team << [user, :master] } + + it 'removes assignee when user id is invalid' do + opts = { title: 'Title', description: 'Description', assignee_ids: [-1] } + + issue = described_class.new(project, user, opts).execute + + expect(issue.assignees).to be_empty + end + + it 'removes assignee when user id is 0' do + opts = { title: 'Title', description: 'Description', assignee_ids: [0] } + + issue = described_class.new(project, user, opts).execute + + expect(issue.assignees).to be_empty + end + + it 'saves assignee when user id is valid' do + project.team << [assignee, :master] + opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] } + + issue = described_class.new(project, user, opts).execute + + expect(issue.assignees).to eq([assignee]) + end + + context "when issuable feature is private" do + before do + project.project_feature.update(issues_access_level: ProjectFeature::PRIVATE, + merge_requests_access_level: ProjectFeature::PRIVATE) + end + + levels = [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC] + + levels.each do |level| + it "removes not authorized assignee when project is #{Gitlab::VisibilityLevel.level_name(level)}" do + project.update(visibility_level: level) + opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] } + + issue = described_class.new(project, user, opts).execute + + expect(issue.assignees).to be_empty + end + end + end + end + end it_behaves_like 'new issuable record that supports slash commands' + context 'Slash commands' do + context 'with assignee and milestone in params and command' do + let(:opts) do + { + assignee_ids: [create(:user).id], + milestone_id: 1, + title: 'Title', + description: %(/assign @#{assignee.username}\n/milestone %"#{milestone.name}") + } + end + + before do + project.team << [user, :master] + project.team << [assignee, :master] + end + + it 'assigns and sets milestone to issuable from command' do + expect(issue).to be_persisted + expect(issue.assignees).to eq([assignee]) + expect(issue.milestone).to eq(milestone) + end + end + end + context 'resolving discussions' do let(:discussion) { create(:diff_note_on_merge_request).to_discussion } let(:merge_request) { discussion.noteable } diff --git a/spec/services/issues/resolve_discussions_spec.rb b/spec/services/issues/resolve_discussions_spec.rb index 4a4929daefc..86f218dec12 100644 --- a/spec/services/issues/resolve_discussions_spec.rb +++ b/spec/services/issues/resolve_discussions_spec.rb @@ -1,15 +1,15 @@ require 'spec_helper.rb' -class DummyService < Issues::BaseService - include ::Issues::ResolveDiscussions +describe Issues::ResolveDiscussions, services: true do + class DummyService < Issues::BaseService + include ::Issues::ResolveDiscussions - def initialize(*args) - super - filter_resolve_discussion_params + def initialize(*args) + super + filter_resolve_discussion_params + end end -end -describe DummyService, services: true do let(:project) { create(:project, :repository) } let(:user) { create(:user) } @@ -23,7 +23,7 @@ describe DummyService, services: true do 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) } + let(:service) { DummyService.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) @@ -43,7 +43,7 @@ describe DummyService, services: true do describe "#discussions_to_resolve" do it "contains a single discussion when matching merge request and discussion are passed" do - service = described_class.new( + service = DummyService.new( project, user, discussion_to_resolve: discussion.id, @@ -61,7 +61,7 @@ describe DummyService, services: true do noteable: merge_request, project: merge_request.target_project, line_number: 15)]) - service = described_class.new( + service = DummyService.new( project, user, merge_request_to_resolve_discussions_of: merge_request.iid @@ -77,9 +77,9 @@ describe DummyService, services: true do _second_discussion = Discussion.new([create(:diff_note_on_merge_request, :resolved, noteable: merge_request, project: merge_request.target_project, - line_number: 15, + line_number: 15 )]) - service = described_class.new( + service = DummyService.new( project, user, merge_request_to_resolve_discussions_of: merge_request.iid @@ -92,7 +92,7 @@ describe DummyService, services: true do end it "is empty when a discussion and another merge request are passed" do - service = described_class.new( + service = DummyService.new( project, user, discussion_to_resolve: discussion.id, diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 5b324f3c706..5184c1d5f19 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -14,7 +14,7 @@ describe Issues::UpdateService, services: true do let(:issue) do create(:issue, title: 'Old title', description: "for #{user2.to_reference}", - assignee_id: user3.id, + assignee_ids: [user3.id], project: project) end @@ -40,7 +40,7 @@ describe Issues::UpdateService, services: true do { title: 'New title', description: 'Also please fix', - assignee_id: user2.id, + assignee_ids: [user2.id], state_event: 'close', label_ids: [label.id], due_date: Date.tomorrow @@ -53,15 +53,22 @@ describe Issues::UpdateService, services: true do expect(issue).to be_valid expect(issue.title).to eq 'New title' expect(issue.description).to eq 'Also please fix' - expect(issue.assignee).to eq user2 + expect(issue.assignees).to match_array([user2]) expect(issue).to be_closed expect(issue.labels).to match_array [label] expect(issue.due_date).to eq Date.tomorrow end + it 'updates open issue counter for assignees when issue is reassigned' do + update_issue(assignee_ids: [user2.id]) + + expect(user3.assigned_open_issues_count).to eq 0 + expect(user2.assigned_open_issues_count).to eq 1 + end + it 'sorts issues as specified by parameters' do - issue1 = create(:issue, project: project, assignee_id: user3.id) - issue2 = create(:issue, project: project, assignee_id: user3.id) + issue1 = create(:issue, project: project, assignees: [user3]) + issue2 = create(:issue, project: project, assignees: [user3]) [issue, issue1, issue2].each do |issue| issue.move_to_end @@ -87,7 +94,7 @@ describe Issues::UpdateService, services: true do expect(issue).to be_valid expect(issue.title).to eq 'New title' expect(issue.description).to eq 'Also please fix' - expect(issue.assignee).to eq user3 + expect(issue.assignees).to match_array [user3] expect(issue.labels).to be_empty expect(issue.milestone).to be_nil expect(issue.due_date).to be_nil @@ -132,12 +139,23 @@ describe Issues::UpdateService, services: true do end end + context 'when description changed' do + it 'creates system note about description change' do + update_issue(description: 'Changed description') + + note = find_note('changed the description') + + expect(note).not_to be_nil + expect(note.note).to eq('changed the description') + end + end + context 'when issue turns confidential' do let(:opts) do { title: 'New title', description: 'Also please fix', - assignee_id: user2.id, + assignee_ids: [user2], state_event: 'close', label_ids: [label.id], confidential: true @@ -163,12 +181,12 @@ describe Issues::UpdateService, services: true do it 'does not update assignee_id with unauthorized users' do project.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC) update_issue(confidential: true) - non_member = create(:user) - original_assignee = issue.assignee + non_member = create(:user) + original_assignees = issue.assignees - update_issue(assignee_id: non_member.id) + update_issue(assignee_ids: [non_member.id]) - expect(issue.reload.assignee_id).to eq(original_assignee.id) + expect(issue.reload.assignees).to eq(original_assignees) end end @@ -205,7 +223,7 @@ describe Issues::UpdateService, services: true do context 'when is reassigned' do before do - update_issue(assignee: user2) + update_issue(assignees: [user2]) end it 'marks previous assignee todos as done' do @@ -408,6 +426,41 @@ describe Issues::UpdateService, services: true do end end + context 'updating asssignee_id' do + it 'does not update assignee when assignee_id is invalid' do + update_issue(assignee_ids: [-1]) + + expect(issue.reload.assignees).to eq([user3]) + end + + it 'unassigns assignee when user id is 0' do + update_issue(assignee_ids: [0]) + + expect(issue.reload.assignees).to be_empty + end + + it 'does not update assignee_id when user cannot read issue' do + update_issue(assignee_ids: [create(:user).id]) + + expect(issue.reload.assignees).to eq([user3]) + end + + context "when issuable feature is private" do + levels = [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC] + + levels.each do |level| + it "does not update with unauthorized assignee when project is #{Gitlab::VisibilityLevel.level_name(level)}" do + assignee = create(:user) + project.update(visibility_level: level) + feature_visibility_attr = :"#{issue.model_name.plural}_access_level" + project.project_feature.update_attribute(feature_visibility_attr, ProjectFeature::PRIVATE) + + expect{ update_issue(assignee_ids: [assignee.id]) }.not_to change{ issue.assignees } + end + end + end + end + context 'updating mentions' do let(:mentionable) { issue } include_examples 'updating mentions', Issues::UpdateService diff --git a/spec/services/members/authorized_destroy_service_spec.rb b/spec/services/members/authorized_destroy_service_spec.rb index 3b35a3b8e3a..ab440d18e9f 100644 --- a/spec/services/members/authorized_destroy_service_spec.rb +++ b/spec/services/members/authorized_destroy_service_spec.rb @@ -14,8 +14,8 @@ describe Members::AuthorizedDestroyService, services: true do it "unassigns issues and merge requests" do group.add_developer(member_user) - issue = create :issue, project: group_project, assignee: member_user - create :issue, assignee: member_user + issue = create :issue, project: group_project, assignees: [member_user] + create :issue, assignees: [member_user] merge_request = create :merge_request, target_project: group_project, source_project: group_project, assignee: member_user create :merge_request, target_project: project, source_project: project, assignee: member_user @@ -33,7 +33,7 @@ describe Members::AuthorizedDestroyService, services: true do it "unassigns issues and merge requests" do project.team << [member_user, :developer] - create :issue, project: project, assignee: member_user + create :issue, project: project, assignees: [member_user] create :merge_request, target_project: project, source_project: project, assignee: member_user member = project.members.find_by(user_id: member_user.id) diff --git a/spec/services/merge_requests/assign_issues_service_spec.rb b/spec/services/merge_requests/assign_issues_service_spec.rb index fe75757dd29..d3556020d4d 100644 --- a/spec/services/merge_requests/assign_issues_service_spec.rb +++ b/spec/services/merge_requests/assign_issues_service_spec.rb @@ -15,14 +15,14 @@ describe MergeRequests::AssignIssuesService, services: true do expect(service.assignable_issues.map(&:id)).to include(issue.id) end - it 'ignores issues already assigned to any user' do - issue.update!(assignee: create(:user)) + it 'ignores issues the user cannot update assignee on' do + project.team.truncate expect(service.assignable_issues).to be_empty end - it 'ignores issues the user cannot update assignee on' do - project.team.truncate + it 'ignores issues already assigned to any user' do + issue.assignees = [create(:user)] expect(service.assignable_issues).to be_empty end @@ -44,7 +44,7 @@ describe MergeRequests::AssignIssuesService, services: true do end it 'assigns these to the merge request owner' do - expect { service.execute }.to change { issue.reload.assignee }.to(user) + expect { service.execute }.to change { issue.assignees.first }.to(user) end it 'ignores external issues' do diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb index be9f9ea2dec..6f9d1208b1d 100644 --- a/spec/services/merge_requests/build_service_spec.rb +++ b/spec/services/merge_requests/build_service_spec.rb @@ -261,6 +261,16 @@ describe MergeRequests::BuildService, services: true do end end + context 'upstream project has disabled merge requests' do + let(:upstream_project) { create(:empty_project, :merge_requests_disabled) } + let(:project) { create(:empty_project, forked_from_project: upstream_project) } + let(:commits) { Commit.decorate([commit_1], project) } + + it 'sets target project correctly' do + expect(merge_request.target_project).to eq(project) + end + end + context 'target_project is set and accessible by current_user' do let(:target_project) { create(:project, :public, :repository)} let(:commits) { Commit.decorate([commit_1], project) } diff --git a/spec/services/merge_requests/conflicts/list_service_spec.rb b/spec/services/merge_requests/conflicts/list_service_spec.rb new file mode 100644 index 00000000000..e8a305d6130 --- /dev/null +++ b/spec/services/merge_requests/conflicts/list_service_spec.rb @@ -0,0 +1,73 @@ +require 'spec_helper' + +describe MergeRequests::Conflicts::ListService do + describe '#can_be_resolved_in_ui?' do + def create_merge_request(source_branch) + create(:merge_request, source_branch: source_branch, target_branch: 'conflict-start') do |mr| + mr.mark_as_unmergeable + end + end + + def conflicts_service(merge_request) + described_class.new(merge_request) + end + + it 'returns a falsey value when the MR can be merged without conflicts' do + merge_request = create_merge_request('master') + merge_request.mark_as_mergeable + + expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_falsey + end + + it 'returns a falsey value when the MR is marked as having conflicts, but has none' do + merge_request = create_merge_request('master') + + expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_falsey + end + + it 'returns a falsey value when the MR has a missing ref after a force push' do + merge_request = create_merge_request('conflict-resolvable') + service = conflicts_service(merge_request) + allow(service.conflicts).to receive(:merge_index).and_raise(Rugged::OdbError) + + expect(service.can_be_resolved_in_ui?).to be_falsey + end + + it 'returns a falsey value when the MR does not support new diff notes' do + merge_request = create_merge_request('conflict-resolvable') + merge_request.merge_request_diff.update_attributes(start_commit_sha: nil) + + expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_falsey + end + + it 'returns a falsey value when the conflicts contain a large file' do + merge_request = create_merge_request('conflict-too-large') + + expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_falsey + end + + it 'returns a falsey value when the conflicts contain a binary file' do + merge_request = create_merge_request('conflict-binary-file') + + expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_falsey + end + + it 'returns a falsey value when the conflicts contain a file edited in one branch and deleted in another' do + merge_request = create_merge_request('conflict-missing-side') + + expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_falsey + end + + it 'returns a truthy value when the conflicts are resolvable in the UI' do + merge_request = create_merge_request('conflict-resolvable') + + expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_truthy + end + + it 'returns a truthy value when the conflicts have to be resolved in an editor' do + merge_request = create_merge_request('conflict-contains-conflict-markers') + + expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_truthy + end + end +end diff --git a/spec/services/merge_requests/resolve_service_spec.rb b/spec/services/merge_requests/conflicts/resolve_service_spec.rb index eaf7785e549..19e8d5cc5f1 100644 --- a/spec/services/merge_requests/resolve_service_spec.rb +++ b/spec/services/merge_requests/conflicts/resolve_service_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe MergeRequests::ResolveService do +describe MergeRequests::Conflicts::ResolveService do let(:user) { create(:user) } let(:project) { create(:project, :repository) } @@ -24,6 +24,8 @@ describe MergeRequests::ResolveService do end describe '#execute' do + let(:service) { described_class.new(merge_request) } + context 'with section params' do let(:params) do { @@ -50,7 +52,7 @@ describe MergeRequests::ResolveService do context 'when the source and target project are the same' do before do - MergeRequests::ResolveService.new(project, user, params).execute(merge_request) + service.execute(user, params) end it 'creates a commit with the message' do @@ -74,15 +76,26 @@ describe MergeRequests::ResolveService do branch_name: 'conflict-start') end - before do - MergeRequests::ResolveService.new(fork_project, user, params).execute(merge_request_from_fork) + def resolve_conflicts + described_class.new(merge_request_from_fork).execute(user, params) + end + + it 'gets conflicts from the source project' do + expect(fork_project.repository.rugged).to receive(:merge_commits).and_call_original + expect(project.repository.rugged).not_to receive(:merge_commits) + + resolve_conflicts end it 'creates a commit with the message' do + resolve_conflicts + expect(merge_request_from_fork.source_branch_head.message).to eq(params[:commit_message]) end it 'creates a commit with the correct parents' do + resolve_conflicts + expect(merge_request_from_fork.source_branch_head.parents.map(&:id)). to eq(['404fa3fc7c2c9b5dacff102f353bdf55b1be2813', target_head]) @@ -115,7 +128,7 @@ describe MergeRequests::ResolveService do end before do - MergeRequests::ResolveService.new(project, user, params).execute(merge_request) + service.execute(user, params) end it 'creates a commit with the message' do @@ -154,15 +167,15 @@ describe MergeRequests::ResolveService do } end - let(:service) { MergeRequests::ResolveService.new(project, user, invalid_params) } - it 'raises a MissingResolution error' do - expect { service.execute(merge_request) }. + expect { service.execute(user, invalid_params) }. to raise_error(Gitlab::Conflict::File::MissingResolution) end end context 'when the content of a file is unchanged' do + let(:list_service) { MergeRequests::Conflicts::ListService.new(merge_request) } + let(:invalid_params) do { files: [ @@ -173,17 +186,15 @@ describe MergeRequests::ResolveService do }, { old_path: 'files/ruby/regex.rb', new_path: 'files/ruby/regex.rb', - content: merge_request.conflicts.file_for_path('files/ruby/regex.rb', 'files/ruby/regex.rb').content + content: list_service.conflicts.file_for_path('files/ruby/regex.rb', 'files/ruby/regex.rb').content } ], commit_message: 'This is a commit message!' } end - let(:service) { MergeRequests::ResolveService.new(project, user, invalid_params) } - it 'raises a MissingResolution error' do - expect { service.execute(merge_request) }. + expect { service.execute(user, invalid_params) }. to raise_error(Gitlab::Conflict::File::MissingResolution) end end @@ -202,11 +213,9 @@ describe MergeRequests::ResolveService do } end - let(:service) { MergeRequests::ResolveService.new(project, user, invalid_params) } - it 'raises a MissingFiles error' do - expect { service.execute(merge_request) }. - to raise_error(MergeRequests::ResolveService::MissingFiles) + expect { service.execute(user, invalid_params) }. + to raise_error(described_class::MissingFiles) end end end diff --git a/spec/services/merge_requests/create_from_issue_service_spec.rb b/spec/services/merge_requests/create_from_issue_service_spec.rb new file mode 100644 index 00000000000..1588d30c394 --- /dev/null +++ b/spec/services/merge_requests/create_from_issue_service_spec.rb @@ -0,0 +1,74 @@ +require 'spec_helper' + +describe MergeRequests::CreateFromIssueService, services: true do + let(:project) { create(:project, :repository) } + let(:user) { create(:user) } + let(:issue) { create(:issue, project: project) } + + subject(:service) { described_class.new(project, user, issue_iid: issue.iid) } + + before do + project.add_developer(user) + end + + describe '#execute' do + it 'returns an error with invalid issue iid' do + result = described_class.new(project, user, issue_iid: -1).execute + + expect(result[:status]).to eq :error + expect(result[:message]).to eq 'Invalid issue iid' + end + + it 'delegates issue search to IssuesFinder' do + expect_any_instance_of(IssuesFinder).to receive(:execute).once.and_call_original + + described_class.new(project, user, issue_iid: -1).execute + end + + it 'delegates the branch creation to CreateBranchService' do + expect_any_instance_of(CreateBranchService).to receive(:execute).once.and_call_original + + service.execute + end + + it 'creates a branch based on issue title' do + service.execute + + expect(project.repository.branch_exists?(issue.to_branch_name)).to be_truthy + end + + it 'creates a system note' do + expect(SystemNoteService).to receive(:new_issue_branch).with(issue, project, user, issue.to_branch_name) + + service.execute + end + + it 'creates a merge request' do + expect { service.execute }.to change(project.merge_requests, :count).by(1) + end + + it 'sets the merge request title to: "WIP: Resolves "$issue-title"' do + result = service.execute + + expect(result[:merge_request].title).to eq("WIP: Resolve \"#{issue.title}\"") + end + + it 'sets the merge request author to current user' do + result = service.execute + + expect(result[:merge_request].author).to eq user + end + + it 'sets the merge request source branch to the new issue branch' do + result = service.execute + + expect(result[:merge_request].source_branch).to eq issue.to_branch_name + end + + it 'sets the merge request target branch to the project default branch' do + result = service.execute + + expect(result[:merge_request].target_branch).to eq project.default_branch + end + end +end diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index 0e16c7cc94b..41752f1a01a 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -84,7 +84,107 @@ describe MergeRequests::CreateService, services: true do end end - it_behaves_like 'issuable create service' + context 'Slash commands' do + context 'with assignee and milestone in params and command' do + let(:merge_request) { described_class.new(project, user, opts).execute } + let(:milestone) { create(:milestone, project: project) } + + let(:opts) do + { + assignee_id: create(:user).id, + milestone_id: 1, + title: 'Title', + description: %(/assign @#{assignee.username}\n/milestone %"#{milestone.name}"), + source_branch: 'feature', + target_branch: 'master' + } + end + + before do + project.team << [user, :master] + project.team << [assignee, :master] + end + + it 'assigns and sets milestone to issuable from command' do + expect(merge_request).to be_persisted + expect(merge_request.assignee).to eq(assignee) + expect(merge_request.milestone).to eq(milestone) + end + end + end + + context 'merge request create service' do + context 'asssignee_id' do + let(:assignee) { create(:user) } + + before { project.team << [user, :master] } + + it 'removes assignee_id when user id is invalid' do + opts = { title: 'Title', description: 'Description', assignee_id: -1 } + + merge_request = described_class.new(project, user, opts).execute + + expect(merge_request.assignee_id).to be_nil + end + + it 'removes assignee_id when user id is 0' do + opts = { title: 'Title', description: 'Description', assignee_id: 0 } + + merge_request = described_class.new(project, user, opts).execute + + expect(merge_request.assignee_id).to be_nil + end + + it 'saves assignee when user id is valid' do + project.team << [assignee, :master] + opts = { title: 'Title', description: 'Description', assignee_id: assignee.id } + + merge_request = described_class.new(project, user, opts).execute + + expect(merge_request.assignee).to eq(assignee) + end + + context 'when assignee is set' do + let(:opts) do + { + title: 'Title', + description: 'Description', + assignee_id: assignee.id, + source_branch: 'feature', + target_branch: 'master' + } + end + + it 'invalidates open merge request counter for assignees when merge request is assigned' do + project.team << [assignee, :master] + + described_class.new(project, user, opts).execute + + expect(assignee.assigned_open_merge_requests_count).to eq 1 + end + end + + context "when issuable feature is private" do + before do + project.project_feature.update(issues_access_level: ProjectFeature::PRIVATE, + merge_requests_access_level: ProjectFeature::PRIVATE) + end + + levels = [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC] + + levels.each do |level| + it "removes not authorized assignee when project is #{Gitlab::VisibilityLevel.level_name(level)}" do + project.update(visibility_level: level) + opts = { title: 'Title', description: 'Description', assignee_id: assignee.id } + + merge_request = described_class.new(project, user, opts).execute + + expect(merge_request.assignee_id).to be_nil + end + end + end + end + end context 'while saving references to issues that the created merge request closes' do let(:first_issue) { create(:issue, project: project) } diff --git a/spec/services/merge_requests/get_urls_service_spec.rb b/spec/services/merge_requests/get_urls_service_spec.rb index 290e00ea1ba..4a7d8ab4c6c 100644 --- a/spec/services/merge_requests/get_urls_service_spec.rb +++ b/spec/services/merge_requests/get_urls_service_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" describe MergeRequests::GetUrlsService do let(:project) { create(:project, :public, :repository) } - let(:service) { MergeRequests::GetUrlsService.new(project) } + let(:service) { described_class.new(project) } let(:source_branch) { "my_branch" } let(:new_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=#{source_branch}" } let(:show_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/#{merge_request.iid}" } @@ -89,7 +89,7 @@ describe MergeRequests::GetUrlsService do let!(:merge_request) { create(:merge_request, source_project: forked_project, target_project: project, source_branch: source_branch) } let(:changes) { existing_branch_changes } # Source project is now the forked one - let(:service) { MergeRequests::GetUrlsService.new(forked_project) } + let(:service) { described_class.new(forked_project) } before do allow(forked_project).to receive(:empty_repo?).and_return(false) diff --git a/spec/services/merge_requests/merge_request_diff_cache_service_spec.rb b/spec/services/merge_requests/merge_request_diff_cache_service_spec.rb index 35804d41b46..935f4710851 100644 --- a/spec/services/merge_requests/merge_request_diff_cache_service_spec.rb +++ b/spec/services/merge_requests/merge_request_diff_cache_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe MergeRequests::MergeRequestDiffCacheService do - let(:subject) { MergeRequests::MergeRequestDiffCacheService.new } + let(:subject) { described_class.new } describe '#execute' do it 'retrieves the diff files to cache the highlighted result' do diff --git a/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb b/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb index 769b3193275..3ef5135e6a3 100644 --- a/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb +++ b/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb @@ -82,6 +82,10 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do sha: merge_request_head, status: 'success') end + before do + mr_merge_if_green_enabled.update(head_pipeline: triggering_pipeline) + end + it "merges all merge requests with merge when the pipeline succeeds enabled" do expect(MergeWorker).to receive(:perform_async) service.trigger(triggering_pipeline) @@ -124,6 +128,8 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do sha: mr_conflict.diff_head_sha, status: 'success') end + before { mr_conflict.update(head_pipeline: conflict_pipeline) } + it 'does not merge the merge request' do expect(MergeWorker).not_to receive(:perform_async) diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 03215a4624a..1f109eab268 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -348,7 +348,7 @@ describe MergeRequests::RefreshService, services: true do title: 'fixup! Fix issue', work_in_progress?: true, to_reference: 'ccccccc' - ), + ) ]) refresh_service.execute(@oldrev, @newrev, 'refs/heads/wip') reload_mrs diff --git a/spec/services/merge_requests/resolved_discussion_notification_service.rb b/spec/services/merge_requests/resolved_discussion_notification_service_spec.rb index 7ddd812e513..7ddd812e513 100644 --- a/spec/services/merge_requests/resolved_discussion_notification_service.rb +++ b/spec/services/merge_requests/resolved_discussion_notification_service_spec.rb diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index f2ca1e6fcbd..2c8fbb46e75 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -102,6 +102,13 @@ describe MergeRequests::UpdateService, services: true do expect(note.note).to eq 'changed title from **{-Old-} title** to **{+New+} title**' end + it 'creates system note about description change' do + note = find_note('changed the description') + + expect(note).not_to be_nil + expect(note.note).to eq('changed the description') + end + it 'creates system note about branch change' do note = find_note('changed target') @@ -167,11 +174,13 @@ describe MergeRequests::UpdateService, services: true do context 'with active pipeline' do before do service_mock = double - create(:ci_pipeline_with_one_job, + pipeline = create(:ci_pipeline_with_one_job, project: project, ref: merge_request.source_branch, sha: merge_request.diff_head_sha) + merge_request.update(head_pipeline: pipeline) + expect(MergeRequests::MergeWhenPipelineSucceedsService).to receive(:new).with(project, user). and_return(service_mock) expect(service_mock).to receive(:execute).with(merge_request) @@ -290,6 +299,15 @@ describe MergeRequests::UpdateService, services: true do end end + context 'when the assignee changes' do + it 'updates open merge request counter for assignees when merge request is reassigned' do + update_merge_request(assignee_id: user2.id) + + expect(user3.assigned_open_merge_requests_count).to eq 0 + expect(user2.assigned_open_merge_requests_count).to eq 1 + end + end + context 'when the target branch change' do before do update_merge_request({ target_branch: 'target' }) @@ -423,6 +441,54 @@ describe MergeRequests::UpdateService, services: true do end end + context 'updating asssignee_id' do + it 'does not update assignee when assignee_id is invalid' do + merge_request.update(assignee_id: user.id) + + update_merge_request(assignee_id: -1) + + expect(merge_request.reload.assignee).to eq(user) + end + + it 'unassigns assignee when user id is 0' do + merge_request.update(assignee_id: user.id) + + update_merge_request(assignee_id: 0) + + expect(merge_request.assignee_id).to be_nil + end + + it 'saves assignee when user id is valid' do + update_merge_request(assignee_id: user.id) + + expect(merge_request.assignee_id).to eq(user.id) + end + + it 'does not update assignee_id when user cannot read issue' do + non_member = create(:user) + original_assignee = merge_request.assignee + + update_merge_request(assignee_id: non_member.id) + + expect(merge_request.assignee_id).to eq(original_assignee.id) + end + + context "when issuable feature is private" do + levels = [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC] + + levels.each do |level| + it "does not update with unauthorized assignee when project is #{Gitlab::VisibilityLevel.level_name(level)}" do + assignee = create(:user) + project.update(visibility_level: level) + feature_visibility_attr = :"#{merge_request.model_name.plural}_access_level" + project.project_feature.update_attribute(feature_visibility_attr, ProjectFeature::PRIVATE) + + expect{ update_merge_request(assignee_id: assignee) }.not_to change{ merge_request.assignee } + end + end + end + end + include_examples 'issuable update service' do let(:open_issuable) { merge_request } let(:closed_issuable) { create(:closed_merge_request, source_project: project) } diff --git a/spec/services/notes/build_service_spec.rb b/spec/services/notes/build_service_spec.rb index f9dd5541b10..133175769ca 100644 --- a/spec/services/notes/build_service_spec.rb +++ b/spec/services/notes/build_service_spec.rb @@ -29,10 +29,82 @@ describe Notes::BuildService, services: true do expect(new_note.errors[:base]).to include('Discussion to reply to cannot be found') end end + + context 'personal snippet note' do + def reply(note, user = nil) + user ||= create(:user) + + described_class.new(nil, + user, + note: 'Test', + in_reply_to_discussion_id: note.discussion_id).execute + end + + let(:snippet_author) { create(:user) } + + context 'when a snippet is public' do + it 'creates a reply note' do + snippet = create(:personal_snippet, :public) + note = create(:discussion_note_on_personal_snippet, noteable: snippet) + + new_note = reply(note) + + expect(new_note).to be_valid + expect(new_note.in_reply_to?(note)).to be_truthy + end + end + + context 'when a snippet is private' do + let(:snippet) { create(:personal_snippet, :private, author: snippet_author) } + let(:note) { create(:discussion_note_on_personal_snippet, noteable: snippet) } + + it 'creates a reply note when the author replies' do + new_note = reply(note, snippet_author) + + expect(new_note).to be_valid + expect(new_note.in_reply_to?(note)).to be_truthy + end + + it 'sets an error when another user replies' do + new_note = reply(note) + + expect(new_note.errors[:base]).to include('Discussion to reply to cannot be found') + end + end + + context 'when a snippet is internal' do + let(:snippet) { create(:personal_snippet, :internal, author: snippet_author) } + let(:note) { create(:discussion_note_on_personal_snippet, noteable: snippet) } + + it 'creates a reply note when the author replies' do + new_note = reply(note, snippet_author) + + expect(new_note).to be_valid + expect(new_note.in_reply_to?(note)).to be_truthy + end + + it 'creates a reply note when a regular user replies' do + new_note = reply(note) + + expect(new_note).to be_valid + expect(new_note.in_reply_to?(note)).to be_truthy + end + + it 'sets an error when an external user replies' do + new_note = reply(note, create(:user, :external)) + + expect(new_note.errors[:base]).to include('Discussion to reply to cannot be found') + end + end + end end it 'builds a note without saving it' do - new_note = described_class.new(project, author, noteable_type: note.noteable_type, noteable_id: note.noteable_id, note: 'Test').execute + new_note = described_class.new(project, + author, + noteable_type: note.noteable_type, + noteable_id: note.noteable_id, + note: 'Test').execute expect(new_note).to be_valid expect(new_note).not_to be_persisted end diff --git a/spec/services/notes/slash_commands_service_spec.rb b/spec/services/notes/slash_commands_service_spec.rb index 1a64c8bbf00..c9954dc3603 100644 --- a/spec/services/notes/slash_commands_service_spec.rb +++ b/spec/services/notes/slash_commands_service_spec.rb @@ -66,7 +66,7 @@ describe Notes::SlashCommandsService, services: true do expect(content).to eq '' expect(note.noteable).to be_closed expect(note.noteable.labels).to match_array(labels) - expect(note.noteable.assignee).to eq(assignee) + expect(note.noteable.assignees).to eq([assignee]) expect(note.noteable.milestone).to eq(milestone) end end @@ -113,7 +113,7 @@ describe Notes::SlashCommandsService, services: true do expect(content).to eq "HELLO\nWORLD" expect(note.noteable).to be_closed expect(note.noteable.labels).to match_array(labels) - expect(note.noteable.assignee).to eq(assignee) + expect(note.noteable.assignees).to eq([assignee]) expect(note.noteable.milestone).to eq(milestone) end end @@ -220,4 +220,31 @@ describe Notes::SlashCommandsService, services: true do let(:note) { build(:note_on_commit, project: project) } end end + + context 'CE restriction for issue assignees' do + describe '/assign' do + let(:project) { create(:empty_project) } + let(:master) { create(:user).tap { |u| project.team << [u, :master] } } + let(:assignee) { create(:user) } + let(:master) { create(:user) } + let(:service) { described_class.new(project, master) } + let(:note) { create(:note_on_issue, note: note_text, project: project) } + + let(:note_text) do + %(/assign @#{assignee.username} @#{master.username}\n") + end + + before do + project.team << [master, :master] + project.team << [assignee, :master] + end + + it 'adds only one assignee from the list' do + _, command_params = service.extract_commands(note) + service.execute(command_params, note) + + expect(note.noteable.assignees.count).to eq(1) + end + end + end end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 989fd90cda9..de3bbc6b6a1 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -4,6 +4,7 @@ describe NotificationService, services: true do include EmailHelpers let(:notification) { NotificationService.new } + let(:assignee) { create(:user) } around(:each) do |example| perform_enqueued_jobs do @@ -52,7 +53,11 @@ describe NotificationService, services: true do shared_examples 'participating by assignee notification' do it 'emails the participant' do - issuable.update_attribute(:assignee, participant) + if issuable.is_a?(Issue) + issuable.assignees << participant + else + issuable.update_attribute(:assignee, participant) + end notification_trigger @@ -103,14 +108,14 @@ describe NotificationService, services: true do describe 'Notes' do context 'issue note' do let(:project) { create(:empty_project, :private) } - let(:issue) { create(:issue, project: project, assignee: create(:user)) } - let(:mentioned_issue) { create(:issue, assignee: issue.assignee) } + let(:issue) { create(:issue, project: project, assignees: [assignee]) } + let(:mentioned_issue) { create(:issue, assignees: issue.assignees) } let(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@mention referenced, @outsider also') } before do build_team(note.project) project.add_master(issue.author) - project.add_master(issue.assignee) + project.add_master(assignee) project.add_master(note.author) create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@subscribed_participant cc this guy') update_custom_notification(:new_note, @u_guest_custom, resource: project) @@ -130,7 +135,7 @@ describe NotificationService, services: true do should_email(@u_watcher) should_email(note.noteable.author) - should_email(note.noteable.assignee) + should_email(note.noteable.assignees.first) should_email(@u_custom_global) should_email(@u_mentioned) should_email(@subscriber) @@ -196,7 +201,7 @@ describe NotificationService, services: true do notification.new_note(note) should_email(note.noteable.author) - should_email(note.noteable.assignee) + should_email(note.noteable.assignees.first) should_email(@u_mentioned) should_email(@u_custom_global) should_not_email(@u_guest_custom) @@ -218,7 +223,7 @@ describe NotificationService, services: true do let(:member) { create(:user) } let(:guest) { create(:user) } let(:admin) { create(:admin) } - let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) } + let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee]) } let(:note) { create(:note_on_issue, noteable: confidential_issue, project: project, note: "#{author.to_reference} #{assignee.to_reference} #{non_member.to_reference} #{member.to_reference} #{admin.to_reference}") } let(:guest_watcher) { create_user_with_notification(:watch, "guest-watcher-confidential") } @@ -244,8 +249,8 @@ describe NotificationService, services: true do context 'issue note mention' do let(:project) { create(:empty_project, :public) } - let(:issue) { create(:issue, project: project, assignee: create(:user)) } - let(:mentioned_issue) { create(:issue, assignee: issue.assignee) } + let(:issue) { create(:issue, project: project, assignees: [assignee]) } + let(:mentioned_issue) { create(:issue, assignees: issue.assignees) } let(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@all mentioned') } before do @@ -269,7 +274,7 @@ describe NotificationService, services: true do should_email(@u_guest_watcher) should_email(note.noteable.author) - should_email(note.noteable.assignee) + should_email(note.noteable.assignees.first) should_not_email(note.author) should_email(@u_mentioned) should_not_email(@u_disabled) @@ -345,7 +350,7 @@ describe NotificationService, services: true do create(:note_on_personal_snippet, noteable: snippet, note: 'note', author: @u_participant), create(:note_on_personal_snippet, noteable: snippet, note: 'note', author: @u_mentioned), create(:note_on_personal_snippet, noteable: snippet, note: 'note', author: @u_disabled), - create(:note_on_personal_snippet, noteable: snippet, note: 'note', author: @u_note_author), + create(:note_on_personal_snippet, noteable: snippet, note: 'note', author: @u_note_author) ] end @@ -449,7 +454,7 @@ describe NotificationService, services: true do let(:group) { create(:group) } let(:project) { create(:empty_project, :public, namespace: group) } let(:another_project) { create(:empty_project, :public, namespace: group) } - let(:issue) { create :issue, project: project, assignee: create(:user), description: 'cc @participant' } + let(:issue) { create :issue, project: project, assignees: [assignee], description: 'cc @participant' } before do build_team(issue.project) @@ -465,7 +470,7 @@ describe NotificationService, services: true do it do notification.new_issue(issue, @u_disabled) - should_email(issue.assignee) + should_email(assignee) should_email(@u_watcher) should_email(@u_guest_watcher) should_email(@u_guest_custom) @@ -480,10 +485,10 @@ describe NotificationService, services: true do end it do - create_global_setting_for(issue.assignee, :mention) + create_global_setting_for(issue.assignees.first, :mention) notification.new_issue(issue, @u_disabled) - should_not_email(issue.assignee) + should_not_email(issue.assignees.first) end it "emails the author if they've opted into notifications about their activity" do @@ -528,7 +533,7 @@ describe NotificationService, services: true do let(:member) { create(:user) } let(:guest) { create(:user) } let(:admin) { create(:admin) } - let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignee: assignee) } + let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignees: [assignee]) } it "emails subscribers of the issue's labels that can read the issue" do project.add_developer(member) @@ -572,9 +577,9 @@ describe NotificationService, services: true do end it 'emails new assignee' do - notification.reassigned_issue(issue, @u_disabled) + notification.reassigned_issue(issue, @u_disabled, [assignee]) - should_email(issue.assignee) + should_email(issue.assignees.first) should_email(@u_watcher) should_email(@u_guest_watcher) should_email(@u_guest_custom) @@ -588,9 +593,8 @@ describe NotificationService, services: true do end it 'emails previous assignee even if he has the "on mention" notif level' do - issue.update_attribute(:assignee, @u_mentioned) - issue.update_attributes(assignee: @u_watcher) - notification.reassigned_issue(issue, @u_disabled) + issue.assignees = [@u_mentioned] + notification.reassigned_issue(issue, @u_disabled, [@u_watcher]) should_email(@u_mentioned) should_email(@u_watcher) @@ -606,11 +610,11 @@ describe NotificationService, services: true do end it 'emails new assignee even if he has the "on mention" notif level' do - issue.update_attributes(assignee: @u_mentioned) - notification.reassigned_issue(issue, @u_disabled) + issue.assignees = [@u_mentioned] + notification.reassigned_issue(issue, @u_disabled, [@u_mentioned]) - expect(issue.assignee).to be @u_mentioned - should_email(issue.assignee) + expect(issue.assignees.first).to be @u_mentioned + should_email(issue.assignees.first) should_email(@u_watcher) should_email(@u_guest_watcher) should_email(@u_guest_custom) @@ -624,11 +628,11 @@ describe NotificationService, services: true do end it 'emails new assignee' do - issue.update_attribute(:assignee, @u_mentioned) - notification.reassigned_issue(issue, @u_disabled) + issue.assignees = [@u_mentioned] + notification.reassigned_issue(issue, @u_disabled, [@u_mentioned]) - expect(issue.assignee).to be @u_mentioned - should_email(issue.assignee) + expect(issue.assignees.first).to be @u_mentioned + should_email(issue.assignees.first) should_email(@u_watcher) should_email(@u_guest_watcher) should_email(@u_guest_custom) @@ -642,17 +646,17 @@ describe NotificationService, services: true do end it 'does not email new assignee if they are the current user' do - issue.update_attribute(:assignee, @u_mentioned) - notification.reassigned_issue(issue, @u_mentioned) + issue.assignees = [@u_mentioned] + notification.reassigned_issue(issue, @u_mentioned, [@u_mentioned]) - expect(issue.assignee).to be @u_mentioned + expect(issue.assignees.first).to be @u_mentioned should_email(@u_watcher) should_email(@u_guest_watcher) should_email(@u_guest_custom) should_email(@u_participant_mentioned) should_email(@subscriber) should_email(@u_custom_global) - should_not_email(issue.assignee) + should_not_email(issue.assignees.first) should_not_email(@unsubscriber) should_not_email(@u_participating) should_not_email(@u_disabled) @@ -662,7 +666,7 @@ describe NotificationService, services: true do it_behaves_like 'participating notifications' do let(:participant) { create(:user, username: 'user-participant') } let(:issuable) { issue } - let(:notification_trigger) { notification.reassigned_issue(issue, @u_disabled) } + let(:notification_trigger) { notification.reassigned_issue(issue, @u_disabled, [assignee]) } end end @@ -705,7 +709,7 @@ describe NotificationService, services: true do 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) - should_not_email(issue.assignee) + should_not_email(issue.assignees.first) should_not_email(issue.author) should_not_email(@u_watcher) should_not_email(@u_guest_watcher) @@ -729,7 +733,7 @@ describe NotificationService, services: true do let(:member) { create(:user) } let(:guest) { create(:user) } let(:admin) { create(:admin) } - let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignee: assignee) } + let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignees: [assignee]) } let!(:label_1) { create(:label, project: project, issues: [confidential_issue]) } let!(:label_2) { create(:label, project: project) } @@ -767,7 +771,7 @@ describe NotificationService, services: true do it 'sends email to issue assignee and issue author' do notification.close_issue(issue, @u_disabled) - should_email(issue.assignee) + should_email(issue.assignees.first) should_email(issue.author) should_email(@u_watcher) should_email(@u_guest_watcher) @@ -798,7 +802,7 @@ describe NotificationService, services: true do it 'sends email to issue notification recipients' do notification.reopen_issue(issue, @u_disabled) - should_email(issue.assignee) + should_email(issue.assignees.first) should_email(issue.author) should_email(@u_watcher) should_email(@u_guest_watcher) @@ -826,7 +830,7 @@ describe NotificationService, services: true do it 'sends email to issue notification recipients' do notification.issue_moved(issue, new_issue, @u_disabled) - should_email(issue.assignee) + should_email(issue.assignees.first) should_email(issue.author) should_email(@u_watcher) should_email(@u_guest_watcher) diff --git a/spec/services/preview_markdown_service_spec.rb b/spec/services/preview_markdown_service_spec.rb new file mode 100644 index 00000000000..b2fb5c91313 --- /dev/null +++ b/spec/services/preview_markdown_service_spec.rb @@ -0,0 +1,67 @@ +require 'spec_helper' + +describe PreviewMarkdownService do + let(:user) { create(:user) } + let(:project) { create(:empty_project) } + + before do + project.add_developer(user) + end + + describe 'user references' do + let(:params) { { text: "Take a look #{user.to_reference}" } } + let(:service) { described_class.new(project, user, params) } + + it 'returns users referenced in text' do + result = service.execute + + expect(result[:users]).to eq [user.username] + end + end + + context 'new note with slash commands' do + let(:issue) { create(:issue, project: project) } + let(:params) do + { + text: "Please do it\n/assign #{user.to_reference}", + slash_commands_target_type: 'Issue', + slash_commands_target_id: issue.id + } + end + let(:service) { described_class.new(project, user, params) } + + it 'removes slash commands from text' do + result = service.execute + + expect(result[:text]).to eq 'Please do it' + end + + it 'explains slash commands effect' do + result = service.execute + + expect(result[:commands]).to eq "Assigns #{user.to_reference}." + end + end + + context 'merge request description' do + let(:params) do + { + text: "My work\n/estimate 2y", + slash_commands_target_type: 'MergeRequest' + } + end + let(:service) { described_class.new(project, user, params) } + + it 'removes slash commands from text' do + result = service.execute + + expect(result[:text]).to eq 'My work' + end + + it 'explains slash commands effect' do + result = service.execute + + expect(result[:commands]).to eq 'Sets time estimate to 2y.' + end + end +end diff --git a/spec/services/projects/autocomplete_service_spec.rb b/spec/services/projects/autocomplete_service_spec.rb index 7916c2d957c..c198c3eedfc 100644 --- a/spec/services/projects/autocomplete_service_spec.rb +++ b/spec/services/projects/autocomplete_service_spec.rb @@ -11,7 +11,7 @@ describe Projects::AutocompleteService, services: true do let(:project) { create(:empty_project, :public) } let!(:issue) { create(:issue, project: project, title: 'Issue 1') } let!(:security_issue_1) { create(:issue, :confidential, project: project, title: 'Security issue 1', author: author) } - let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project, assignee: assignee) } + let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project, assignees: [assignee]) } it 'does not list project confidential issues for guests' do autocomplete = described_class.new(project, nil) diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 7a07ea618c0..033e6ecd18c 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -27,6 +27,22 @@ describe Projects::CreateService, '#execute', services: true do end end + context "admin creates project with other user's namespace_id" do + it 'sets the correct permissions' do + admin = create(:admin) + opts = { + name: 'GitLab', + namespace_id: user.namespace.id + } + project = create_project(admin, opts) + + expect(project).to be_persisted + expect(project.owner).to eq(user) + expect(project.team.masters).to include(user, admin) + expect(project.namespace).to eq(user.namespace) + end + end + context 'group namespace' do let(:group) do create(:group).tap do |group| diff --git a/spec/services/projects/enable_deploy_key_service_spec.rb b/spec/services/projects/enable_deploy_key_service_spec.rb index a37510cf159..78626fbad4b 100644 --- a/spec/services/projects/enable_deploy_key_service_spec.rb +++ b/spec/services/projects/enable_deploy_key_service_spec.rb @@ -21,6 +21,16 @@ describe Projects::EnableDeployKeyService, services: true do end end + context 'add the same key twice' do + before do + project.deploy_keys << deploy_key + end + + it 'returns existing key' do + expect(service.execute).to eq(deploy_key) + end + end + def service Projects::EnableDeployKeyService.new(project, user, params) end diff --git a/spec/services/projects/housekeeping_service_spec.rb b/spec/services/projects/housekeeping_service_spec.rb index eaf63457b32..fff12beed71 100644 --- a/spec/services/projects/housekeeping_service_spec.rb +++ b/spec/services/projects/housekeeping_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Projects::HousekeepingService do - subject { Projects::HousekeepingService.new(project) } + subject { described_class.new(project) } let(:project) { create(:project, :repository) } before do diff --git a/spec/services/projects/participants_service_spec.rb b/spec/services/projects/participants_service_spec.rb index 063b3bd76eb..0657b7e93fe 100644 --- a/spec/services/projects/participants_service_spec.rb +++ b/spec/services/projects/participants_service_spec.rb @@ -6,7 +6,6 @@ describe Projects::ParticipantsService, services: true do let(:project) { create(:empty_project, :public) } let(:group) { create(:group, avatar: fixture_file_upload(Rails.root + 'spec/fixtures/dk.png')) } let(:user) { create(:user) } - let(:base_url) { Settings.send(:build_base_gitlab_url) } let!(:group_member) { create(:group_member, group: group, user: user) } it 'should return an url for the avatar' do @@ -14,7 +13,7 @@ describe Projects::ParticipantsService, services: true do groups = participants.groups expect(groups.size).to eq 1 - expect(groups.first[:avatar_url]).to eq "#{base_url}/uploads/group/avatar/#{group.id}/dk.png" + expect(groups.first[:avatar_url]).to eq("/uploads/group/avatar/#{group.id}/dk.png") end it 'should return an url for the avatar with relative url' do @@ -25,7 +24,7 @@ describe Projects::ParticipantsService, services: true do groups = participants.groups expect(groups.size).to eq 1 - expect(groups.first[:avatar_url]).to eq "#{base_url}/gitlab/uploads/group/avatar/#{group.id}/dk.png" + expect(groups.first[:avatar_url]).to eq("/gitlab/uploads/group/avatar/#{group.id}/dk.png") end end end diff --git a/spec/services/projects/propagate_service_template_spec.rb b/spec/services/projects/propagate_service_template_spec.rb new file mode 100644 index 00000000000..90eff3bbc1e --- /dev/null +++ b/spec/services/projects/propagate_service_template_spec.rb @@ -0,0 +1,103 @@ +require 'spec_helper' + +describe Projects::PropagateServiceTemplate, services: true do + describe '.propagate' do + let!(:service_template) do + PushoverService.create( + template: true, + active: true, + properties: { + device: 'MyDevice', + sound: 'mic', + priority: 4, + user_key: 'asdf', + api_key: '123456789' + }) + end + + let!(:project) { create(:empty_project) } + + it 'creates services for projects' do + expect(project.pushover_service).to be_nil + + described_class.propagate(service_template) + + expect(project.reload.pushover_service).to be_present + end + + it 'creates services for a project that has another service' do + BambooService.create( + template: true, + active: true, + project: project, + properties: { + bamboo_url: 'http://gitlab.com', + username: 'mic', + password: "password", + build_key: 'build' + } + ) + + expect(project.pushover_service).to be_nil + + described_class.propagate(service_template) + + expect(project.reload.pushover_service).to be_present + end + + it 'does not create the service if it exists already' do + other_service = BambooService.create( + template: true, + active: true, + properties: { + bamboo_url: 'http://gitlab.com', + username: 'mic', + password: "password", + build_key: 'build' + } + ) + + Service.build_from_template(project.id, service_template).save! + Service.build_from_template(project.id, other_service).save! + + expect { described_class.propagate(service_template) }. + not_to change { Service.count } + end + + it 'creates the service containing the template attributes' do + described_class.propagate(service_template) + + expect(project.pushover_service.properties).to eq(service_template.properties) + end + + describe 'bulk update' do + it 'creates services for all projects' do + project_total = 5 + stub_const 'Projects::PropagateServiceTemplate::BATCH_SIZE', 3 + + project_total.times { create(:empty_project) } + + expect { described_class.propagate(service_template) }. + to change { Service.count }.by(project_total + 1) + end + end + + describe 'external tracker' do + it 'updates the project external tracker' do + service_template.update!(category: 'issue_tracker', default: false) + + expect { described_class.propagate(service_template) }. + to change { project.reload.has_external_issue_tracker }.to(true) + end + end + + describe 'external wiki' do + it 'updates the project external tracker' do + service_template.update!(type: 'ExternalWikiService') + + expect { described_class.propagate(service_template) }. + to change { project.reload.has_external_wiki }.to(true) + end + end + end +end diff --git a/spec/services/slash_commands/interpret_service_spec.rb b/spec/services/slash_commands/interpret_service_spec.rb index a63281f0eab..e5e400ee281 100644 --- a/spec/services/slash_commands/interpret_service_spec.rb +++ b/spec/services/slash_commands/interpret_service_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' describe SlashCommands::InterpretService, services: true do let(:project) { create(:empty_project, :public) } let(:developer) { create(:user) } + let(:developer2) { create(:user) } let(:issue) { create(:issue, project: project) } let(:milestone) { create(:milestone, project: project, title: '9.10') } let(:inprogress) { create(:label, project: project, title: 'In Progress') } @@ -42,23 +43,6 @@ describe SlashCommands::InterpretService, services: true do end end - shared_examples 'assign command' do - it 'fetches assignee and populates assignee_id if content contains /assign' do - _, updates = service.execute(content, issuable) - - expect(updates).to eq(assignee_id: developer.id) - end - end - - shared_examples 'unassign command' do - it 'populates assignee_id: nil if content contains /unassign' do - issuable.update(assignee_id: developer.id) - _, updates = service.execute(content, issuable) - - expect(updates).to eq(assignee_id: nil) - end - end - shared_examples 'milestone command' do it 'fetches milestone and populates milestone_id if content contains /milestone' do milestone # populate the milestone @@ -70,7 +54,7 @@ describe SlashCommands::InterpretService, services: true do shared_examples 'remove_milestone command' do it 'populates milestone_id: nil if content contains /remove_milestone' do - issuable.update(milestone_id: milestone.id) + issuable.update!(milestone_id: milestone.id) _, updates = service.execute(content, issuable) expect(updates).to eq(milestone_id: nil) @@ -108,7 +92,7 @@ describe SlashCommands::InterpretService, services: true do shared_examples 'unlabel command' do it 'fetches label ids and populates remove_label_ids if content contains /unlabel' do - issuable.update(label_ids: [inprogress.id]) # populate the label + issuable.update!(label_ids: [inprogress.id]) # populate the label _, updates = service.execute(content, issuable) expect(updates).to eq(remove_label_ids: [inprogress.id]) @@ -117,7 +101,7 @@ describe SlashCommands::InterpretService, services: true do shared_examples 'multiple unlabel command' do it 'fetches label ids and populates remove_label_ids if content contains mutiple /unlabel' do - issuable.update(label_ids: [inprogress.id, bug.id]) # populate the label + issuable.update!(label_ids: [inprogress.id, bug.id]) # populate the label _, updates = service.execute(content, issuable) expect(updates).to eq(remove_label_ids: [inprogress.id, bug.id]) @@ -126,7 +110,7 @@ describe SlashCommands::InterpretService, services: true do shared_examples 'unlabel command with no argument' do it 'populates label_ids: [] if content contains /unlabel with no arguments' do - issuable.update(label_ids: [inprogress.id]) # populate the label + issuable.update!(label_ids: [inprogress.id]) # populate the label _, updates = service.execute(content, issuable) expect(updates).to eq(label_ids: []) @@ -135,7 +119,7 @@ describe SlashCommands::InterpretService, services: true do shared_examples 'relabel command' do it 'populates label_ids: [] if content contains /relabel' do - issuable.update(label_ids: [bug.id]) # populate the label + issuable.update!(label_ids: [bug.id]) # populate the label inprogress # populate the label _, updates = service.execute(content, issuable) @@ -187,7 +171,7 @@ describe SlashCommands::InterpretService, services: true do shared_examples 'remove_due_date command' do it 'populates due_date: nil if content contains /remove_due_date' do - issuable.update(due_date: Date.today) + issuable.update!(due_date: Date.today) _, updates = service.execute(content, issuable) expect(updates).to eq(due_date: nil) @@ -204,7 +188,7 @@ describe SlashCommands::InterpretService, services: true do shared_examples 'unwip command' do it 'returns wip_event: "unwip" if content contains /wip' do - issuable.update(title: issuable.wip_title) + issuable.update!(title: issuable.wip_title) _, updates = service.execute(content, issuable) expect(updates).to eq(wip_event: 'unwip') @@ -371,14 +355,46 @@ describe SlashCommands::InterpretService, services: true do let(:issuable) { issue } end - it_behaves_like 'assign command' do + context 'assign command' do let(:content) { "/assign @#{developer.username}" } - let(:issuable) { issue } + + context 'Issue' do + it 'fetches assignee and populates assignee_id if content contains /assign' do + _, updates = service.execute(content, issue) + + expect(updates).to eq(assignee_ids: [developer.id]) + end + end + + context 'Merge Request' do + it 'fetches assignee and populates assignee_id if content contains /assign' do + _, updates = service.execute(content, merge_request) + + expect(updates).to eq(assignee_id: developer.id) + end + end end - it_behaves_like 'assign command' do - let(:content) { "/assign @#{developer.username}" } - let(:issuable) { merge_request } + context 'assign command with multiple assignees' do + let(:content) { "/assign @#{developer.username} @#{developer2.username}" } + + before{ project.team << [developer2, :developer] } + + context 'Issue' do + it 'fetches assignee and populates assignee_id if content contains /assign' do + _, updates = service.execute(content, issue) + + expect(updates[:assignee_ids]).to match_array([developer.id, developer2.id]) + end + end + + context 'Merge Request' do + it 'fetches assignee and populates assignee_id if content contains /assign' do + _, updates = service.execute(content, merge_request) + + expect(updates).to eq(assignee_id: developer.id) + end + end end it_behaves_like 'empty command' do @@ -391,14 +407,26 @@ describe SlashCommands::InterpretService, services: true do let(:issuable) { issue } end - it_behaves_like 'unassign command' do + context 'unassign command' do let(:content) { '/unassign' } - let(:issuable) { issue } - end - it_behaves_like 'unassign command' do - let(:content) { '/unassign' } - let(:issuable) { merge_request } + context 'Issue' do + it 'populates assignee_ids: [] if content contains /unassign' do + issue.update(assignee_ids: [developer.id]) + _, updates = service.execute(content, issue) + + expect(updates).to eq(assignee_ids: []) + end + end + + context 'Merge Request' do + it 'populates assignee_id: nil if content contains /unassign' do + merge_request.update(assignee_id: developer.id) + _, updates = service.execute(content, merge_request) + + expect(updates).to eq(assignee_id: nil) + end + end end it_behaves_like 'milestone command' do @@ -727,5 +755,282 @@ describe SlashCommands::InterpretService, services: true do end end end + + context '/board_move command' do + let(:todo) { create(:label, project: project, title: 'To Do') } + let(:inreview) { create(:label, project: project, title: 'In Review') } + let(:content) { %{/board_move ~"#{inreview.title}"} } + + let!(:board) { create(:board, project: project) } + let!(:todo_list) { create(:list, board: board, label: todo) } + let!(:inreview_list) { create(:list, board: board, label: inreview) } + let!(:inprogress_list) { create(:list, board: board, label: inprogress) } + + it 'populates remove_label_ids for all current board columns' do + issue.update!(label_ids: [todo.id, inprogress.id]) + + _, updates = service.execute(content, issue) + + expect(updates[:remove_label_ids]).to match_array([todo.id, inprogress.id]) + end + + it 'populates add_label_ids with the id of the given label' do + _, updates = service.execute(content, issue) + + expect(updates[:add_label_ids]).to eq([inreview.id]) + end + + it 'does not include the given label id in remove_label_ids' do + issue.update!(label_ids: [todo.id, inreview.id]) + + _, updates = service.execute(content, issue) + + expect(updates[:remove_label_ids]).to match_array([todo.id]) + end + + it 'does not remove label ids that are not lists on the board' do + issue.update!(label_ids: [todo.id, bug.id]) + + _, updates = service.execute(content, issue) + + expect(updates[:remove_label_ids]).to match_array([todo.id]) + end + + context 'if the project has multiple boards' do + let(:issuable) { issue } + before { create(:board, project: project) } + it_behaves_like 'empty command' + end + + context 'if the given label does not exist' do + let(:issuable) { issue } + let(:content) { '/board_move ~"Fake Label"' } + it_behaves_like 'empty command' + end + + context 'if multiple labels are given' do + let(:issuable) { issue } + let(:content) { %{/board_move ~"#{inreview.title}" ~"#{todo.title}"} } + it_behaves_like 'empty command' + end + + context 'if the given label is not a list on the board' do + let(:issuable) { issue } + let(:content) { %{/board_move ~"#{bug.title}"} } + it_behaves_like 'empty command' + end + + context 'if issuable is not an Issue' do + let(:issuable) { merge_request } + it_behaves_like 'empty command' + end + end + end + + describe '#explain' do + let(:service) { described_class.new(project, developer) } + let(:merge_request) { create(:merge_request, source_project: project) } + + describe 'close command' do + let(:content) { '/close' } + + it 'includes issuable name' do + _, explanations = service.explain(content, issue) + + expect(explanations).to eq(['Closes this issue.']) + end + end + + describe 'reopen command' do + let(:content) { '/reopen' } + let(:merge_request) { create(:merge_request, :closed, source_project: project) } + + it 'includes issuable name' do + _, explanations = service.explain(content, merge_request) + + expect(explanations).to eq(['Reopens this merge request.']) + end + end + + describe 'title command' do + let(:content) { '/title This is new title' } + + it 'includes new title' do + _, explanations = service.explain(content, issue) + + expect(explanations).to eq(['Changes the title to "This is new title".']) + end + end + + describe 'assign command' do + let(:content) { "/assign @#{developer.username} do it!" } + + it 'includes only the user reference' do + _, explanations = service.explain(content, merge_request) + + expect(explanations).to eq(["Assigns @#{developer.username}."]) + end + end + + describe 'unassign command' do + let(:content) { '/unassign' } + let(:issue) { create(:issue, project: project, assignees: [developer]) } + + it 'includes current assignee reference' do + _, explanations = service.explain(content, issue) + + expect(explanations).to eq(["Removes assignee @#{developer.username}."]) + end + end + + describe 'milestone command' do + let(:content) { '/milestone %wrong-milestone' } + let!(:milestone) { create(:milestone, project: project, title: '9.10') } + + it 'is empty when milestone reference is wrong' do + _, explanations = service.explain(content, issue) + + expect(explanations).to eq([]) + end + end + + describe 'remove milestone command' do + let(:content) { '/remove_milestone' } + let(:merge_request) { create(:merge_request, source_project: project, milestone: milestone) } + + it 'includes current milestone name' do + _, explanations = service.explain(content, merge_request) + + expect(explanations).to eq(['Removes %"9.10" milestone.']) + end + end + + describe 'label command' do + let(:content) { '/label ~missing' } + let!(:label) { create(:label, project: project) } + + it 'is empty when there are no correct labels' do + _, explanations = service.explain(content, issue) + + expect(explanations).to eq([]) + end + end + + describe 'unlabel command' do + let(:content) { '/unlabel' } + + it 'says all labels if no parameter provided' do + merge_request.update!(label_ids: [bug.id]) + _, explanations = service.explain(content, merge_request) + + expect(explanations).to eq(['Removes all labels.']) + end + end + + describe 'relabel command' do + let(:content) { '/relabel Bug' } + let!(:bug) { create(:label, project: project, title: 'Bug') } + let(:feature) { create(:label, project: project, title: 'Feature') } + + it 'includes label name' do + issue.update!(label_ids: [feature.id]) + _, explanations = service.explain(content, issue) + + expect(explanations).to eq(["Replaces all labels with ~#{bug.id} label."]) + end + end + + describe 'subscribe command' do + let(:content) { '/subscribe' } + + it 'includes issuable name' do + _, explanations = service.explain(content, issue) + + expect(explanations).to eq(['Subscribes to this issue.']) + end + end + + describe 'unsubscribe command' do + let(:content) { '/unsubscribe' } + + it 'includes issuable name' do + merge_request.subscribe(developer, project) + _, explanations = service.explain(content, merge_request) + + expect(explanations).to eq(['Unsubscribes from this merge request.']) + end + end + + describe 'due command' do + let(:content) { '/due April 1st 2016' } + + it 'includes the date' do + _, explanations = service.explain(content, issue) + + expect(explanations).to eq(['Sets the due date to Apr 1, 2016.']) + end + end + + describe 'wip command' do + let(:content) { '/wip' } + + it 'includes the new status' do + _, explanations = service.explain(content, merge_request) + + expect(explanations).to eq(['Marks this merge request as Work In Progress.']) + end + end + + describe 'award command' do + let(:content) { '/award :confetti_ball: ' } + + it 'includes the emoji' do + _, explanations = service.explain(content, issue) + + expect(explanations).to eq(['Toggles :confetti_ball: emoji award.']) + end + end + + describe 'estimate command' do + let(:content) { '/estimate 79d' } + + it 'includes the formatted duration' do + _, explanations = service.explain(content, merge_request) + + expect(explanations).to eq(['Sets time estimate to 3mo 3w 4d.']) + end + end + + describe 'spend command' do + let(:content) { '/spend -120m' } + + it 'includes the formatted duration and proper verb' do + _, explanations = service.explain(content, issue) + + expect(explanations).to eq(['Substracts 2h spent time.']) + end + end + + describe 'target branch command' do + let(:content) { '/target_branch my-feature ' } + + it 'includes the branch name' do + _, explanations = service.explain(content, merge_request) + + expect(explanations).to eq(['Sets target branch to my-feature.']) + end + end + + describe 'board move command' do + let(:content) { '/board_move ~bug' } + let!(:bug) { create(:label, project: project, title: 'bug') } + let!(:board) { create(:board, project: project) } + + it 'includes the label name' do + _, explanations = service.explain(content, issue) + + expect(explanations).to eq(["Moves issue to ~#{bug.id} column in the board."]) + end + end end end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 42d63a9f9ba..7a9cd7553b1 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -6,6 +6,7 @@ describe SystemNoteService, services: true do let(:project) { create(:empty_project) } let(:author) { create(:user) } let(:noteable) { create(:issue, project: project) } + let(:issue) { noteable } shared_examples_for 'a system note' do let(:expected_noteable) { noteable } @@ -155,6 +156,52 @@ describe SystemNoteService, services: true do end end + describe '.change_issue_assignees' do + subject { described_class.change_issue_assignees(noteable, project, author, [assignee]) } + + let(:assignee) { create(:user) } + let(:assignee1) { create(:user) } + let(:assignee2) { create(:user) } + let(:assignee3) { create(:user) } + + it_behaves_like 'a system note' do + let(:action) { 'assignee' } + end + + def build_note(old_assignees, new_assignees) + issue.assignees = new_assignees + described_class.change_issue_assignees(issue, project, author, old_assignees).note + end + + it 'builds a correct phrase when an assignee is added to a non-assigned issue' do + expect(build_note([], [assignee1])).to eq "assigned to @#{assignee1.username}" + end + + it 'builds a correct phrase when assignee removed' do + expect(build_note([assignee1], [])).to eq 'removed assignee' + end + + it 'builds a correct phrase when assignees changed' do + expect(build_note([assignee1], [assignee2])).to eq \ + "assigned to @#{assignee2.username} and unassigned @#{assignee1.username}" + end + + it 'builds a correct phrase when three assignees removed and one added' do + expect(build_note([assignee, assignee1, assignee2], [assignee3])).to eq \ + "assigned to @#{assignee3.username} and unassigned @#{assignee.username}, @#{assignee1.username}, and @#{assignee2.username}" + end + + it 'builds a correct phrase when one assignee changed from a set' do + expect(build_note([assignee, assignee1], [assignee, assignee2])).to eq \ + "assigned to @#{assignee2.username} and unassigned @#{assignee1.username}" + end + + it 'builds a correct phrase when one assignee removed from a set' do + expect(build_note([assignee, assignee1, assignee2], [assignee, assignee1])).to eq \ + "unassigned @#{assignee2.username}" + end + end + describe '.change_label' do subject { described_class.change_label(noteable, project, author, added, removed) } @@ -292,6 +339,20 @@ describe SystemNoteService, services: true do end end + describe '.change_description' do + subject { described_class.change_description(noteable, project, author) } + + context 'when noteable responds to `description`' do + it_behaves_like 'a system note' do + let(:action) { 'description' } + end + + it 'sets the note text' do + expect(subject.note).to eq('changed the description') + end + end + end + describe '.change_issue_confidentiality' do subject { described_class.change_issue_confidentiality(noteable, project, author) } @@ -595,7 +656,7 @@ describe SystemNoteService, services: true do end shared_examples 'cross project mentionable' do - include GitlabMarkdownHelper + include MarkupHelper it 'contains cross reference to new noteable' do expect(subject.note).to include cross_project_reference(new_project, new_noteable) diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 89b3b6aad10..175a42a32d9 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -25,11 +25,11 @@ describe TodoService, services: true do end describe 'Issues' do - let(:issue) { create(:issue, project: project, assignee: john_doe, author: author, description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") } - let(:addressed_issue) { create(:issue, project: project, assignee: john_doe, author: author, description: "#{directly_addressed}\n- [ ] Task 1\n- [ ] Task 2") } - let(:unassigned_issue) { create(:issue, project: project, assignee: nil) } - let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee, description: mentions) } - let(:addressed_confident_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee, description: directly_addressed) } + let(:issue) { create(:issue, project: project, assignees: [john_doe], author: author, description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") } + let(:addressed_issue) { create(:issue, project: project, assignees: [john_doe], author: author, description: "#{directly_addressed}\n- [ ] Task 1\n- [ ] Task 2") } + let(:unassigned_issue) { create(:issue, project: project, assignees: []) } + let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee], description: mentions) } + let(:addressed_confident_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee], description: directly_addressed) } describe '#new_issue' do it 'creates a todo if assigned' do @@ -43,7 +43,7 @@ describe TodoService, services: true do end it 'creates a todo if assignee is the current user' do - unassigned_issue.update_attribute(:assignee, john_doe) + unassigned_issue.assignees = [john_doe] service.new_issue(unassigned_issue, john_doe) should_create_todo(user: john_doe, target: unassigned_issue, author: john_doe, action: Todo::ASSIGNED) @@ -258,20 +258,20 @@ describe TodoService, services: true do describe '#reassigned_issue' do it 'creates a pending todo for new assignee' do - unassigned_issue.update_attribute(:assignee, john_doe) + unassigned_issue.assignees << john_doe service.reassigned_issue(unassigned_issue, author) should_create_todo(user: john_doe, target: unassigned_issue, action: Todo::ASSIGNED) end it 'does not create a todo if unassigned' do - issue.update_attribute(:assignee, nil) + issue.assignees.destroy_all should_not_create_any_todo { service.reassigned_issue(issue, author) } end it 'creates a todo if new assignee is the current user' do - unassigned_issue.update_attribute(:assignee, john_doe) + unassigned_issue.assignees << john_doe service.reassigned_issue(unassigned_issue, john_doe) should_create_todo(user: john_doe, target: unassigned_issue, author: john_doe, action: Todo::ASSIGNED) @@ -361,7 +361,7 @@ describe TodoService, services: true do describe '#new_note' do let!(:first_todo) { create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author) } let!(:second_todo) { create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author) } - let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) } + let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee]) } let(:note) { create(:note, project: project, noteable: issue, author: john_doe, note: mentions) } let(:addressed_note) { create(:note, project: project, noteable: issue, author: john_doe, note: directly_addressed) } let(:note_on_commit) { create(:note_on_commit, project: project, author: john_doe, note: mentions) } @@ -854,7 +854,7 @@ describe TodoService, services: true do end it 'updates cached counts when a todo is created' do - issue = create(:issue, project: project, assignee: john_doe, author: author, description: mentions) + issue = create(:issue, project: project, assignees: [john_doe], author: author, description: mentions) expect(john_doe.todos_pending_count).to eq(0) expect(john_doe).to receive(:update_todos_count_cache).and_call_original @@ -866,8 +866,8 @@ describe TodoService, services: true do end describe '#mark_todos_as_done' do - let(:issue) { create(:issue, project: project, author: author, assignee: john_doe) } - let(:another_issue) { create(:issue, project: project, author: author, assignee: john_doe) } + let(:issue) { create(:issue, project: project, author: author, assignees: [john_doe]) } + let(:another_issue) { create(:issue, project: project, author: author, assignees: [john_doe]) } it 'marks a relation of todos as done' do create(:todo, :mentioned, user: john_doe, target: issue, project: project) diff --git a/spec/services/projects/upload_service_spec.rb b/spec/services/upload_service_spec.rb index d2cefa46bfa..95ba28dbecd 100644 --- a/spec/services/projects/upload_service_spec.rb +++ b/spec/services/upload_service_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Projects::UploadService, services: true do +describe UploadService, services: true do describe 'File service' do before do @user = create(:user) @@ -68,6 +68,6 @@ describe Projects::UploadService, services: true do end def upload_file(project, file) - Projects::UploadService.new(project, file).execute + described_class.new(project, file, FileUploader).execute end end diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb index 4bc30018ebd..de37a61e388 100644 --- a/spec/services/users/destroy_service_spec.rb +++ b/spec/services/users/destroy_service_spec.rb @@ -47,7 +47,7 @@ describe Users::DestroyService, services: true do end context "for an issue the user was assigned to" do - let!(:issue) { create(:issue, project: project, assignee: user) } + let!(:issue) { create(:issue, project: project, assignees: [user]) } before do service.execute(user) @@ -60,7 +60,7 @@ describe Users::DestroyService, services: true do it 'migrates the issue so that it is "Unassigned"' do migrated_issue = Issue.find_by_id(issue.id) - expect(migrated_issue.assignee).to be_nil + expect(migrated_issue.assignees).to be_empty end end end diff --git a/spec/services/users/migrate_to_ghost_user_service_spec.rb b/spec/services/users/migrate_to_ghost_user_service_spec.rb index 8c5b7e41c15..9e1edf1ac30 100644 --- a/spec/services/users/migrate_to_ghost_user_service_spec.rb +++ b/spec/services/users/migrate_to_ghost_user_service_spec.rb @@ -60,5 +60,23 @@ describe Users::MigrateToGhostUserService, services: true do end end end + + context "when record migration fails with a rollback exception" do + before do + expect_any_instance_of(MergeRequest::ActiveRecord_Associations_CollectionProxy) + .to receive(:update_all).and_raise(ActiveRecord::Rollback) + end + + context "for records that were already migrated" do + let!(:issue) { create(:issue, project: project, author: user) } + let!(:merge_request) { create(:merge_request, source_project: project, author: user, target_branch: "first") } + + it "reverses the migration" do + service.execute + + expect(issue.reload.author).to eq(user) + end + end + end end end |