diff options
Diffstat (limited to 'spec/services')
57 files changed, 304 insertions, 141 deletions
diff --git a/spec/services/boards/issues/create_service_spec.rb b/spec/services/boards/issues/create_service_spec.rb index 1a56164dba4..f0179e35652 100644 --- a/spec/services/boards/issues/create_service_spec.rb +++ b/spec/services/boards/issues/create_service_spec.rb @@ -11,7 +11,7 @@ describe Boards::Issues::CreateService do subject(:service) { described_class.new(board.parent, project, user, board_id: board.id, list_id: list.id, title: 'New issue') } before do - project.team << [user, :developer] + project.add_developer(user) end it 'delegates the create proceedings to Issues::CreateService' do diff --git a/spec/services/boards/issues/list_service_spec.rb b/spec/services/boards/issues/list_service_spec.rb index 01ee3856c99..ff5733b7064 100644 --- a/spec/services/boards/issues/list_service_spec.rb +++ b/spec/services/boards/issues/list_service_spec.rb @@ -34,7 +34,7 @@ describe Boards::Issues::ListService do let!(:closed_issue5) { create(:labeled_issue, :closed, project: project, labels: [development]) } before do - project.team << [user, :developer] + project.add_developer(user) end it 'delegates search to IssuesFinder' do diff --git a/spec/services/boards/issues/move_service_spec.rb b/spec/services/boards/issues/move_service_spec.rb index 464ff9f94b3..280e411683e 100644 --- a/spec/services/boards/issues/move_service_spec.rb +++ b/spec/services/boards/issues/move_service_spec.rb @@ -15,7 +15,7 @@ describe Boards::Issues::MoveService do let!(:closed) { create(:closed_list, board: board1) } before do - project.team << [user, :developer] + project.add_developer(user) end context 'when moving an issue between lists' do diff --git a/spec/services/boards/lists/create_service_spec.rb b/spec/services/boards/lists/create_service_spec.rb index 7d0b396cd06..d5322e1bb21 100644 --- a/spec/services/boards/lists/create_service_spec.rb +++ b/spec/services/boards/lists/create_service_spec.rb @@ -10,7 +10,7 @@ describe Boards::Lists::CreateService do subject(:service) { described_class.new(project, user, label_id: label.id) } before do - project.team << [user, :developer] + project.add_developer(user) end context 'when board lists is empty' do diff --git a/spec/services/boards/lists/generate_service_spec.rb b/spec/services/boards/lists/generate_service_spec.rb index 592f25059ac..82dbd1ee744 100644 --- a/spec/services/boards/lists/generate_service_spec.rb +++ b/spec/services/boards/lists/generate_service_spec.rb @@ -9,7 +9,7 @@ describe Boards::Lists::GenerateService do subject(:service) { described_class.new(project, user) } before do - project.team << [user, :developer] + project.add_developer(user) end context 'when board lists is empty' do diff --git a/spec/services/ci/stop_environments_service_spec.rb b/spec/services/ci/stop_environments_service_spec.rb index e2a9ed27e87..3fc4e499b0c 100644 --- a/spec/services/ci/stop_environments_service_spec.rb +++ b/spec/services/ci/stop_environments_service_spec.rb @@ -15,7 +15,7 @@ describe Ci::StopEnvironmentsService do context 'when user has permission to stop environment' do before do - project.team << [user, :developer] + project.add_developer(user) end context 'when environment is associated with removed branch' do @@ -57,7 +57,7 @@ describe Ci::StopEnvironmentsService do 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] + project.add_guest(user) end it 'does not stop environment' do @@ -86,7 +86,7 @@ describe Ci::StopEnvironmentsService do context 'when user has permission to stop environments' do before do - project.team << [user, :master] + project.add_master(user) end it 'does not stop environment' do diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb index 08267d6e6a0..b9bfbb11511 100644 --- a/spec/services/create_deployment_service_spec.rb +++ b/spec/services/create_deployment_service_spec.rb @@ -266,7 +266,7 @@ describe CreateDeploymentService do context "while updating the 'first_deployed_to_production_at' time" do before do - merge_request.mark_as_merged + merge_request.metrics.update!(merged_at: Time.now) end context "for merge requests merged before the current deploy" do diff --git a/spec/services/delete_branch_service_spec.rb b/spec/services/delete_branch_service_spec.rb index 19855c9bee2..9c9fba030e7 100644 --- a/spec/services/delete_branch_service_spec.rb +++ b/spec/services/delete_branch_service_spec.rb @@ -9,7 +9,7 @@ describe DeleteBranchService do describe '#execute' do context 'when user has access to push to repository' do before do - project.team << [user, :developer] + project.add_developer(user) end it 'removes the branch' do diff --git a/spec/services/discussions/resolve_service_spec.rb b/spec/services/discussions/resolve_service_spec.rb index ab8df7b74cd..3895a0b3aea 100644 --- a/spec/services/discussions/resolve_service_spec.rb +++ b/spec/services/discussions/resolve_service_spec.rb @@ -9,7 +9,7 @@ describe Discussions::ResolveService do let(:service) { described_class.new(discussion.noteable.project, user, merge_request: merge_request) } before do - project.team << [user, :master] + project.add_master(user) end it "doesn't resolve discussions the user can't resolve" do diff --git a/spec/services/files/delete_service_spec.rb b/spec/services/files/delete_service_spec.rb index e9f8f0efe6b..ace5f293097 100644 --- a/spec/services/files/delete_service_spec.rb +++ b/spec/services/files/delete_service_spec.rb @@ -37,7 +37,7 @@ describe Files::DeleteService do end before do - project.team << [user, :master] + project.add_master(user) end describe "#execute" do diff --git a/spec/services/files/multi_service_spec.rb b/spec/services/files/multi_service_spec.rb index 085a28d267f..2b79609930c 100644 --- a/spec/services/files/multi_service_spec.rb +++ b/spec/services/files/multi_service_spec.rb @@ -36,7 +36,7 @@ describe Files::MultiService do end before do - project.team << [user, :master] + project.add_master(user) end describe '#execute' do diff --git a/spec/services/files/update_service_spec.rb b/spec/services/files/update_service_spec.rb index 2b4f8cd42ee..43b0c9a63a9 100644 --- a/spec/services/files/update_service_spec.rb +++ b/spec/services/files/update_service_spec.rb @@ -24,7 +24,7 @@ describe Files::UpdateService do end before do - project.team << [user, :master] + project.add_master(user) end describe "#execute" do diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index cc3d4e7da49..26fdf8d4b24 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -11,7 +11,7 @@ describe GitPushService, services: true do let(:ref) { 'refs/heads/master' } before do - project.team << [user, :master] + project.add_master(user) end describe 'Push branches' do @@ -266,8 +266,8 @@ describe GitPushService, services: true do let(:commit) { project.commit } before do - project.team << [commit_author, :developer] - project.team << [user, :developer] + project.add_developer(commit_author) + project.add_developer(user) allow(commit).to receive_messages( safe_message: "this commit \n mentions #{issue.to_reference}", @@ -323,8 +323,8 @@ describe GitPushService, services: true do let(:commit_time) { Time.now } before do - project.team << [commit_author, :developer] - project.team << [user, :developer] + project.add_developer(commit_author) + project.add_developer(user) allow(commit).to receive_messages( safe_message: "this commit \n mentions #{issue.to_reference}", @@ -376,7 +376,7 @@ describe GitPushService, services: true do allow_any_instance_of(ProcessCommitWorker).to receive(:build_commit) .and_return(closing_commit) - project.team << [commit_author, :master] + project.add_master(commit_author) end context "to default branches" do diff --git a/spec/services/git_tag_push_service_spec.rb b/spec/services/git_tag_push_service_spec.rb index 05695aa8188..33405d7a7ec 100644 --- a/spec/services/git_tag_push_service_spec.rb +++ b/spec/services/git_tag_push_service_spec.rb @@ -35,7 +35,7 @@ describe GitTagPushService do before do stub_ci_pipeline_to_return_yaml_file - project.team << [user, :developer] + project.add_developer(user) end it "creates a new pipeline" do diff --git a/spec/services/issuable/bulk_update_service_spec.rb b/spec/services/issuable/bulk_update_service_spec.rb index bdaab88d673..53c85f73cde 100644 --- a/spec/services/issuable/bulk_update_service_spec.rb +++ b/spec/services/issuable/bulk_update_service_spec.rb @@ -54,7 +54,7 @@ describe Issuable::BulkUpdateService do context 'when the new assignee ID is a valid user' do it 'succeeds' do new_assignee = create(:user) - project.team << [new_assignee, :developer] + project.add_developer(new_assignee) result = bulk_update(merge_request, assignee_id: new_assignee.id) @@ -64,7 +64,7 @@ describe Issuable::BulkUpdateService do it 'updates the assignee to the user ID passed' do assignee = create(:user) - project.team << [assignee, :developer] + project.add_developer(assignee) expect { bulk_update(merge_request, assignee_id: assignee.id) } .to change { merge_request.reload.assignee }.from(user).to(assignee) @@ -92,7 +92,7 @@ describe Issuable::BulkUpdateService do context 'when the new assignee ID is a valid user' do it 'succeeds' do new_assignee = create(:user) - project.team << [new_assignee, :developer] + project.add_developer(new_assignee) result = bulk_update(issue, assignee_ids: [new_assignee.id]) @@ -102,7 +102,7 @@ describe Issuable::BulkUpdateService do it 'updates the assignee to the user ID passed' do assignee = create(:user) - project.team << [assignee, :developer] + project.add_developer(assignee) expect { bulk_update(issue, assignee_ids: [assignee.id]) } .to change { issue.reload.assignees.first }.from(user).to(assignee) end diff --git a/spec/services/issues/build_service_spec.rb b/spec/services/issues/build_service_spec.rb index 03f76bd428d..248e7d5a389 100644 --- a/spec/services/issues/build_service_spec.rb +++ b/spec/services/issues/build_service_spec.rb @@ -5,7 +5,7 @@ describe Issues::BuildService do let(:user) { create(:user) } before do - project.team << [user, :developer] + project.add_developer(user) end context 'for a single discussion' do diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb index 5c27e8fd561..8897a64a138 100644 --- a/spec/services/issues/close_service_spec.rb +++ b/spec/services/issues/close_service_spec.rb @@ -9,9 +9,9 @@ describe Issues::CloseService do let!(:todo) { create(:todo, :assigned, user: user, project: project, target: issue, author: user2) } before do - project.team << [user, :master] - project.team << [user2, :developer] - project.team << [guest, :guest] + project.add_master(user) + project.add_developer(user2) + project.add_guest(guest) end describe '#execute' do diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index d86da244520..79bcdc41fb0 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -13,8 +13,8 @@ describe Issues::CreateService do let(:labels) { create_pair(:label, project: project) } before do - project.team << [user, :master] - project.team << [assignee, :master] + project.add_master(user) + project.add_master(assignee) end let(:opts) do @@ -43,7 +43,7 @@ describe Issues::CreateService do let(:guest) { create(:user) } before do - project.team << [guest, :guest] + project.add_guest(guest) end it 'filters out params that cannot be set without the :admin_issue permission' do @@ -130,7 +130,7 @@ describe Issues::CreateService do end it 'invalidates open issues counter for assignees when issue is assigned' do - project.team << [assignee, :master] + project.add_master(assignee) described_class.new(project, user, opts).execute @@ -160,7 +160,7 @@ describe Issues::CreateService do context 'issue create service' do context 'assignees' do before do - project.team << [user, :master] + project.add_master(user) end it 'removes assignee when user id is invalid' do @@ -180,7 +180,7 @@ describe Issues::CreateService do end it 'saves assignee when user id is valid' do - project.team << [assignee, :master] + project.add_master(assignee) opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] } issue = described_class.new(project, user, opts).execute @@ -224,8 +224,8 @@ describe Issues::CreateService do end before do - project.team << [user, :master] - project.team << [assignee, :master] + project.add_master(user) + project.add_master(assignee) end it 'assigns and sets milestone to issuable from command' do @@ -242,7 +242,7 @@ describe Issues::CreateService do let(:project) { merge_request.source_project } before do - project.team << [user, :master] + project.add_master(user) end describe 'for a single discussion' do diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index 2cad695d7c4..53ea88332fb 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -20,8 +20,8 @@ describe Issues::MoveService do shared_context 'user can move issue' do before do - old_project.team << [user, :reporter] - new_project.team << [user, :reporter] + old_project.add_reporter(user) + new_project.add_reporter(user) labels = Array.new(2) { |x| "label%d" % (x + 1) } @@ -313,7 +313,7 @@ describe Issues::MoveService do context 'user is reporter only in new project' do before do - new_project.team << [user, :reporter] + new_project.add_reporter(user) end it { expect { move }.to raise_error(StandardError, /permissions/) } @@ -321,7 +321,7 @@ describe Issues::MoveService do context 'user is reporter only in old project' do before do - old_project.team << [user, :reporter] + old_project.add_reporter(user) end it { expect { move }.to raise_error(StandardError, /permissions/) } @@ -329,8 +329,8 @@ describe Issues::MoveService do context 'user is reporter in one project and guest in another' do before do - new_project.team << [user, :guest] - old_project.team << [user, :reporter] + new_project.add_guest(user) + old_project.add_reporter(user) end it { expect { move }.to raise_error(StandardError, /permissions/) } @@ -358,8 +358,8 @@ describe Issues::MoveService do context 'movable issue with no assigned labels' do before do - old_project.team << [user, :reporter] - new_project.team << [user, :reporter] + old_project.add_reporter(user) + new_project.add_reporter(user) labels = Array.new(2) { |x| "label%d" % (x + 1) } diff --git a/spec/services/issues/reopen_service_spec.rb b/spec/services/issues/reopen_service_spec.rb index 48fc98b3b2f..42e5d544f4c 100644 --- a/spec/services/issues/reopen_service_spec.rb +++ b/spec/services/issues/reopen_service_spec.rb @@ -8,7 +8,7 @@ describe Issues::ReopenService do context 'when user is not authorized to reopen issue' do before do guest = create(:user) - project.team << [guest, :guest] + project.add_guest(guest) perform_enqueued_jobs do described_class.new(project, guest).execute(issue) @@ -24,7 +24,7 @@ describe Issues::ReopenService do let(:user) { create(:user) } before do - project.team << [user, :master] + project.add_master(user) end it 'invalidates counter cache for assignees' do diff --git a/spec/services/issues/resolve_discussions_spec.rb b/spec/services/issues/resolve_discussions_spec.rb index 67a86c50fc1..13accc6ae1b 100644 --- a/spec/services/issues/resolve_discussions_spec.rb +++ b/spec/services/issues/resolve_discussions_spec.rb @@ -14,7 +14,7 @@ describe Issues::ResolveDiscussions do let(:user) { create(:user) } before do - project.team << [user, :developer] + project.add_developer(user) end describe "for resolving discussions" do diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index f07b81e842a..1cb6f2e097f 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -17,9 +17,9 @@ describe Issues::UpdateService, :mailer do end before do - project.team << [user, :master] - project.team << [user2, :developer] - project.team << [user3, :developer] + project.add_master(user) + project.add_developer(user2) + project.add_developer(user3) end describe 'execute' do @@ -99,7 +99,7 @@ describe Issues::UpdateService, :mailer do context 'when current user cannot admin issues in the project' do let(:guest) { create(:user) } before do - project.team << [guest, :guest] + project.add_guest(guest) end it 'filters out params that cannot be set without the :admin_issue permission' do @@ -318,7 +318,7 @@ describe Issues::UpdateService, :mailer do let!(:subscriber) do create(:user).tap do |u| label.toggle_subscription(u, project) - project.team << [u, :developer] + project.add_developer(u) end end @@ -556,7 +556,7 @@ describe Issues::UpdateService, :mailer do context 'valid project' do before do - target_project.team << [user, :master] + target_project.add_master(user) end it 'calls the move service with the proper issue and project' do diff --git a/spec/services/labels/find_or_create_service_spec.rb b/spec/services/labels/find_or_create_service_spec.rb index a781fbc7f7d..78aa5d442e7 100644 --- a/spec/services/labels/find_or_create_service_spec.rb +++ b/spec/services/labels/find_or_create_service_spec.rb @@ -17,7 +17,7 @@ describe Labels::FindOrCreateService do let(:user) { create(:user) } subject(:service) { described_class.new(user, project, params) } before do - project.team << [user, :developer] + project.add_developer(user) end context 'when label does not exist at group level' do diff --git a/spec/services/members/approve_access_request_service_spec.rb b/spec/services/members/approve_access_request_service_spec.rb index 302c488d6c6..b3018169a1c 100644 --- a/spec/services/members/approve_access_request_service_spec.rb +++ b/spec/services/members/approve_access_request_service_spec.rb @@ -123,7 +123,7 @@ describe Members::ApproveAccessRequestService do context 'when current user can approve access request to the project' do before do - project.team << [user, :master] + project.add_master(user) group.add_owner(user) end diff --git a/spec/services/members/authorized_destroy_service_spec.rb b/spec/services/members/authorized_destroy_service_spec.rb index d4ef31c0c74..757c45708b9 100644 --- a/spec/services/members/authorized_destroy_service_spec.rb +++ b/spec/services/members/authorized_destroy_service_spec.rb @@ -13,7 +13,7 @@ describe Members::AuthorizedDestroyService do context 'Invited users' do # Regression spec for issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/32504 it 'destroys invited project member' do - project.team << [member_user, :developer] + project.add_developer(member_user) member = create :project_member, :invited, project: project @@ -52,7 +52,7 @@ describe Members::AuthorizedDestroyService do context 'Project member' do it "unassigns issues and merge requests" do - project.team << [member_user, :developer] + project.add_developer(member_user) create :issue, project: project, assignees: [member_user] create :merge_request, target_project: project, source_project: project, assignee: member_user diff --git a/spec/services/members/create_service_spec.rb b/spec/services/members/create_service_spec.rb index 2a793e0eb4d..6bd4718e780 100644 --- a/spec/services/members/create_service_spec.rb +++ b/spec/services/members/create_service_spec.rb @@ -6,7 +6,7 @@ describe Members::CreateService do let(:project_user) { create(:user) } before do - project.team << [user, :master] + project.add_master(user) end it 'adds user to members' do diff --git a/spec/services/members/destroy_service_spec.rb b/spec/services/members/destroy_service_spec.rb index 72f5e27180d..91152df3ad9 100644 --- a/spec/services/members/destroy_service_spec.rb +++ b/spec/services/members/destroy_service_spec.rb @@ -71,7 +71,7 @@ describe Members::DestroyService do context 'when a member is found' do before do - project.team << [member_user, :developer] + project.add_developer(member_user) group.add_developer(member_user) end let(:params) { { user_id: member_user.id } } @@ -88,7 +88,7 @@ describe Members::DestroyService do context 'when current user can destroy the given member' do before do - project.team << [user, :master] + project.add_master(user) group.add_owner(user) end diff --git a/spec/services/merge_requests/assign_issues_service_spec.rb b/spec/services/merge_requests/assign_issues_service_spec.rb index fcbe0e5985f..bda6383a346 100644 --- a/spec/services/merge_requests/assign_issues_service_spec.rb +++ b/spec/services/merge_requests/assign_issues_service_spec.rb @@ -8,7 +8,7 @@ describe MergeRequests::AssignIssuesService do let(:service) { described_class.new(project, user, merge_request: merge_request) } before do - project.team << [user, :developer] + project.add_developer(user) end it 'finds unassigned issues fixed in merge request' do diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb index b5c92e681fb..a9605c6e4c6 100644 --- a/spec/services/merge_requests/build_service_spec.rb +++ b/spec/services/merge_requests/build_service_spec.rb @@ -28,7 +28,7 @@ describe MergeRequests::BuildService do end before do - project.team << [user, :guest] + project.add_guest(user) end def stub_compare diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb index b3886987316..4d12de3ecce 100644 --- a/spec/services/merge_requests/close_service_spec.rb +++ b/spec/services/merge_requests/close_service_spec.rb @@ -9,9 +9,9 @@ describe MergeRequests::CloseService do let!(:todo) { create(:todo, :assigned, user: user, project: project, target: merge_request, author: user2) } before do - project.team << [user, :master] - project.team << [user2, :developer] - project.team << [guest, :guest] + project.add_master(user) + project.add_developer(user2) + project.add_guest(guest) end describe '#execute' do @@ -52,6 +52,19 @@ describe MergeRequests::CloseService do end end + it 'updates metrics' do + metrics = merge_request.metrics + metrics_service = double(MergeRequestMetricsService) + allow(MergeRequestMetricsService) + .to receive(:new) + .with(metrics) + .and_return(metrics_service) + + expect(metrics_service).to receive(:close) + + described_class.new(project, user, {}).execute(merge_request) + end + it 'refreshes the number of open merge requests for a valid MR', :use_clean_rails_memory_store_caching do service = described_class.new(project, user, {}) diff --git a/spec/services/merge_requests/conflicts/list_service_spec.rb b/spec/services/merge_requests/conflicts/list_service_spec.rb index 0b32c51a16f..6cadcd438c3 100644 --- a/spec/services/merge_requests/conflicts/list_service_spec.rb +++ b/spec/services/merge_requests/conflicts/list_service_spec.rb @@ -32,14 +32,6 @@ describe MergeRequests::Conflicts::ListService do 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_any_instance_of(Rugged::Repository).to receive(:merge_commits).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) @@ -76,5 +68,23 @@ describe MergeRequests::Conflicts::ListService do expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_truthy 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_any_instance_of(Gitlab::GitalyClient::ConflictsService).to receive(:list_conflict_files).and_raise(GRPC::Unknown) + + expect(service.can_be_resolved_in_ui?).to be_falsey + end + + context 'with gitaly disabled', :skip_gitaly_mock do + 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_any_instance_of(Rugged::Repository).to receive(:merge_commits).and_raise(Rugged::OdbError) + + expect(service.can_be_resolved_in_ui?).to be_falsey + end + end end end diff --git a/spec/services/merge_requests/conflicts/resolve_service_spec.rb b/spec/services/merge_requests/conflicts/resolve_service_spec.rb index e28d8d7ae5c..cff09237005 100644 --- a/spec/services/merge_requests/conflicts/resolve_service_spec.rb +++ b/spec/services/merge_requests/conflicts/resolve_service_spec.rb @@ -111,15 +111,6 @@ describe MergeRequests::Conflicts::ResolveService do described_class.new(merge_request_from_fork).execute(user, params) end - it 'gets conflicts from the source project' do - # REFACTOR NOTE: We used to test that `project.repository.rugged` wasn't - # used in this case, but since the refactor, for simplification, - # we always use that repository for read only operations. - expect(forked_project.repository.rugged).to receive(:merge_commits).and_call_original - - subject - end - it 'creates a commit with the message' do subject @@ -132,6 +123,17 @@ describe MergeRequests::Conflicts::ResolveService do expect(merge_request_from_fork.source_branch_head.parents.map(&:id)) .to eq(['404fa3fc7c2c9b5dacff102f353bdf55b1be2813', target_head]) end + + context 'when gitaly is disabled', :skip_gitaly_mock do + it 'gets conflicts from the source project' do + # REFACTOR NOTE: We used to test that `project.repository.rugged` wasn't + # used in this case, but since the refactor, for simplification, + # we always use that repository for read only operations. + expect(forked_project.repository.rugged).to receive(:merge_commits).and_call_original + + subject + end + end end end diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index a047f891ab2..dd8c803a2f7 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -21,8 +21,8 @@ describe MergeRequests::CreateService do let(:merge_request) { service.execute } before do - project.team << [user, :master] - project.team << [assignee, :developer] + project.add_master(user) + project.add_developer(assignee) allow(service).to receive(:execute_hooks) end @@ -148,8 +148,8 @@ describe MergeRequests::CreateService do end before do - project.team << [user, :master] - project.team << [assignee, :master] + project.add_master(user) + project.add_master(assignee) end it 'assigns and sets milestone to issuable from command' do @@ -165,7 +165,7 @@ describe MergeRequests::CreateService do let(:assignee) { create(:user) } before do - project.team << [user, :master] + project.add_master(user) end it 'removes assignee_id when user id is invalid' do @@ -185,7 +185,7 @@ describe MergeRequests::CreateService do end it 'saves assignee when user id is valid' do - project.team << [assignee, :master] + project.add_master(assignee) opts = { title: 'Title', description: 'Description', assignee_id: assignee.id } merge_request = described_class.new(project, user, opts).execute @@ -205,7 +205,7 @@ describe MergeRequests::CreateService do end it 'invalidates open merge request counter for assignees when merge request is assigned' do - project.team << [assignee, :master] + project.add_master(assignee) described_class.new(project, user, opts).execute @@ -249,8 +249,8 @@ describe MergeRequests::CreateService do end before do - project.team << [user, :master] - project.team << [assignee, :developer] + project.add_master(user) + project.add_developer(assignee) end it 'creates a `MergeRequestsClosingIssues` record for each issue' do diff --git a/spec/services/merge_requests/ff_merge_service_spec.rb b/spec/services/merge_requests/ff_merge_service_spec.rb index aaabf3ed2b0..aa90feeef89 100644 --- a/spec/services/merge_requests/ff_merge_service_spec.rb +++ b/spec/services/merge_requests/ff_merge_service_spec.rb @@ -12,8 +12,8 @@ describe MergeRequests::FfMergeService do let(:project) { merge_request.project } before do - project.team << [user, :master] - project.team << [user2, :developer] + project.add_master(user) + project.add_developer(user2) end describe '#execute' do diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb index d2bd05d921f..70957431942 100644 --- a/spec/services/merge_requests/post_merge_service_spec.rb +++ b/spec/services/merge_requests/post_merge_service_spec.rb @@ -6,7 +6,7 @@ describe MergeRequests::PostMergeService do let(:project) { merge_request.project } before do - project.team << [user, :master] + project.add_master(user) end describe '#execute' do @@ -22,5 +22,18 @@ describe MergeRequests::PostMergeService do expect { service.execute(merge_request) } .to change { project.open_merge_requests_count }.from(1).to(0) end + + it 'updates metrics' do + metrics = merge_request.metrics + metrics_service = double(MergeRequestMetricsService) + allow(MergeRequestMetricsService) + .to receive(:new) + .with(metrics) + .and_return(metrics_service) + + expect(metrics_service).to receive(:merge) + + described_class.new(project, user, {}).execute(merge_request) + end end end diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 61ec4709c59..7a01d3dd698 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -300,8 +300,8 @@ describe MergeRequests::RefreshService do let(:commit) { project.commit } before do - project.team << [commit_author, :developer] - project.team << [user, :developer] + project.add_developer(commit_author) + project.add_developer(user) allow(commit).to receive_messages( safe_message: "Closes #{issue.to_reference}", diff --git a/spec/services/merge_requests/reopen_service_spec.rb b/spec/services/merge_requests/reopen_service_spec.rb index fa652611c6b..a44d63e5f9f 100644 --- a/spec/services/merge_requests/reopen_service_spec.rb +++ b/spec/services/merge_requests/reopen_service_spec.rb @@ -8,9 +8,9 @@ describe MergeRequests::ReopenService do let(:project) { merge_request.project } before do - project.team << [user, :master] - project.team << [user2, :developer] - project.team << [guest, :guest] + project.add_master(user) + project.add_developer(user2) + project.add_guest(guest) end describe '#execute' do @@ -47,6 +47,19 @@ describe MergeRequests::ReopenService do end end + it 'updates metrics' do + metrics = merge_request.metrics + service = double(MergeRequestMetricsService) + allow(MergeRequestMetricsService) + .to receive(:new) + .with(metrics) + .and_return(service) + + expect(service).to receive(:reopen) + + described_class.new(project, user, {}).execute(merge_request) + end + it 'refreshes the number of open merge requests for a valid MR' do service = described_class.new(project, user, {}) diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 7a66b809550..2238da2d14d 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -16,9 +16,9 @@ describe MergeRequests::UpdateService, :mailer do end before do - project.team << [user, :master] - project.team << [user2, :developer] - project.team << [user3, :developer] + project.add_master(user) + project.add_developer(user2) + project.add_developer(user3) end describe 'execute' do @@ -356,8 +356,8 @@ describe MergeRequests::UpdateService, :mailer do let!(:subscriber) { create(:user) { |u| label.toggle_subscription(u, project) } } before do - project.team << [non_subscriber, :developer] - project.team << [subscriber, :developer] + project.add_developer(non_subscriber) + project.add_developer(subscriber) end it 'sends notifications for subscribers of newly added labels' do diff --git a/spec/services/milestones/close_service_spec.rb b/spec/services/milestones/close_service_spec.rb index 2bdf224804d..adad73f7e11 100644 --- a/spec/services/milestones/close_service_spec.rb +++ b/spec/services/milestones/close_service_spec.rb @@ -6,7 +6,7 @@ describe Milestones::CloseService do let(:milestone) { create(:milestone, title: "Milestone v1.2", project: project) } before do - project.team << [user, :master] + project.add_master(user) end describe '#execute' do diff --git a/spec/services/milestones/create_service_spec.rb b/spec/services/milestones/create_service_spec.rb index 8837b91051d..f2a18c7295a 100644 --- a/spec/services/milestones/create_service_spec.rb +++ b/spec/services/milestones/create_service_spec.rb @@ -7,7 +7,7 @@ describe Milestones::CreateService do describe '#execute' do context "valid params" do before do - project.team << [user, :master] + project.add_master(user) opts = { title: 'v2.1.9', diff --git a/spec/services/milestones/destroy_service_spec.rb b/spec/services/milestones/destroy_service_spec.rb index af35e17bfa7..9703780b0e9 100644 --- a/spec/services/milestones/destroy_service_spec.rb +++ b/spec/services/milestones/destroy_service_spec.rb @@ -8,7 +8,7 @@ describe Milestones::DestroyService do let!(:merge_request) { create(:merge_request, source_project: project, milestone: milestone) } before do - project.team << [user, :master] + project.add_master(user) end def service diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index 661d26946e7..0ae26e87154 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -10,7 +10,7 @@ describe Notes::CreateService do describe '#execute' do before do - project.team << [user, :master] + project.add_master(user) end context "valid params" do diff --git a/spec/services/notes/post_process_service_spec.rb b/spec/services/notes/post_process_service_spec.rb index a2b3638059f..6ef5e93cb20 100644 --- a/spec/services/notes/post_process_service_spec.rb +++ b/spec/services/notes/post_process_service_spec.rb @@ -7,7 +7,7 @@ describe Notes::PostProcessService do describe '#execute' do before do - project.team << [user, :master] + project.add_master(user) note_opts = { note: 'Awesome comment', noteable_type: 'Issue', diff --git a/spec/services/notes/quick_actions_service_spec.rb b/spec/services/notes/quick_actions_service_spec.rb index 0280a19098b..5eafe56c99d 100644 --- a/spec/services/notes/quick_actions_service_spec.rb +++ b/spec/services/notes/quick_actions_service_spec.rb @@ -3,11 +3,11 @@ require 'spec_helper' describe Notes::QuickActionsService do shared_context 'note on noteable' do let(:project) { create(:project) } - let(:master) { create(:user).tap { |u| project.team << [u, :master] } } + let(:master) { create(:user).tap { |u| project.add_master(u) } } let(:assignee) { create(:user) } before do - project.team << [assignee, :master] + project.add_master(assignee) end end @@ -226,7 +226,7 @@ describe Notes::QuickActionsService do context 'CE restriction for issue assignees' do describe '/assign' do let(:project) { create(:project) } - let(:master) { create(:user).tap { |u| project.team << [u, :master] } } + let(:master) { create(:user).tap { |u| project.add_master(u) } } let(:assignee) { create(:user) } let(:master) { create(:user) } let(:service) { described_class.new(project, master) } @@ -237,8 +237,8 @@ describe Notes::QuickActionsService do end before do - project.team << [master, :master] - project.team << [assignee, :master] + project.add_master(master) + project.add_master(assignee) end it 'adds only one assignee from the list' do diff --git a/spec/services/notes/update_service_spec.rb b/spec/services/notes/update_service_spec.rb index 3210539f3ee..65b1d613998 100644 --- a/spec/services/notes/update_service_spec.rb +++ b/spec/services/notes/update_service_spec.rb @@ -9,9 +9,9 @@ describe Notes::UpdateService do let(:note) { create(:note, project: project, noteable: issue, author: user, note: "Old note #{user2.to_reference}") } before do - project.team << [user, :master] - project.team << [user2, :developer] - project.team << [user3, :developer] + project.add_master(user) + project.add_developer(user2) + project.add_developer(user3) end describe '#execute' do diff --git a/spec/services/projects/autocomplete_service_spec.rb b/spec/services/projects/autocomplete_service_spec.rb index 426593be428..7a8c54673f7 100644 --- a/spec/services/projects/autocomplete_service_spec.rb +++ b/spec/services/projects/autocomplete_service_spec.rb @@ -34,7 +34,7 @@ describe Projects::AutocompleteService do end it 'does not list project confidential issues for project members with guest role' do - project.team << [member, :guest] + project.add_guest(member) autocomplete = described_class.new(project, non_member) issues = autocomplete.issues.map(&:iid) @@ -66,7 +66,7 @@ describe Projects::AutocompleteService do end it 'lists project confidential issues for project members' do - project.team << [member, :developer] + project.add_developer(member) autocomplete = described_class.new(project, member) issues = autocomplete.issues.map(&:iid) diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index dc89fdebce7..1833078f37c 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -252,6 +252,12 @@ describe Projects::CreateService, '#execute' do end end + it 'writes project full path to .git/config' do + project = create_project(user, opts) + + expect(project.repo.config['gitlab.fullpath']).to eq project.full_path + end + def create_project(user, opts) Projects::CreateService.new(user, opts).execute end diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index 4057caca2ac..409d5de8d43 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -139,10 +139,10 @@ describe Projects::ForkService do stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL]) end - it "creates fork with highest allowed level" do + it "creates fork with lowest level" do forked_project = fork_project(@from_project, @to_user) - expect(forked_project.visibility_level).to eq(Gitlab::VisibilityLevel::PUBLIC) + expect(forked_project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) end end @@ -209,6 +209,19 @@ describe Projects::ForkService do expect(to_project.errors[:path]).to eq(['has already been taken']) end end + + context 'when the namespace has a lower visibility level than the project' do + it 'creates the project with the lower visibility level' do + public_project = create(:project, :public) + private_group = create(:group, :private) + group_owner = create(:user) + private_group.add_owner(group_owner) + + forked_project = fork_project(public_project, group_owner, namespace: private_group) + + expect(forked_project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) + end + end end end diff --git a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb index 3a3e47fd9c0..ded864beb1d 100644 --- a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb +++ b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Projects::HashedStorage::MigrateRepositoryService do let(:gitlab_shell) { Gitlab::Shell.new } - let(:project) { create(:project, :empty_repo, :wiki_repo) } + let(:project) { create(:project, :repository, :wiki_repo) } let(:service) { described_class.new(project) } let(:legacy_storage) { Storage::LegacyProject.new(project) } let(:hashed_storage) { Storage::HashedProject.new(project) } @@ -33,6 +33,12 @@ describe Projects::HashedStorage::MigrateRepositoryService do service.execute end + + it 'writes project full path to .git/config' do + service.execute + + expect(project.repo.config['gitlab.fullpath']).to eq project.full_path + end end context 'when one move fails' do diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 2b1337bee7e..7377c748698 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -54,6 +54,12 @@ describe Projects::TransferService do expect(project.disk_path).not_to eq(old_path) expect(project.disk_path).to start_with(group.path) end + + it 'updates project full path in .git/config' do + transfer_project(project, user, group) + + expect(project.repo.config['gitlab.fullpath']).to eq "#{group.full_path}/#{project.path}" + end end context 'when transfer fails' do @@ -86,6 +92,12 @@ describe Projects::TransferService do expect(original_path).to eq current_path end + it 'rolls back project full path in .git/config' do + attempt_project_transfer + + expect(project.repo.config['gitlab.fullpath']).to eq project.full_path + end + it "doesn't send move notifications" do expect_any_instance_of(NotificationService).not_to receive(:project_was_moved) diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index d887f70efae..fc6aa713d6f 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -61,7 +61,7 @@ describe Projects::UpdateService do end end - context 'When project visibility is higher than parent group' do + context 'when project visibility is higher than parent group' do let(:group) { create(:group, visibility_level: Gitlab::VisibilityLevel::INTERNAL) } before do diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index eb46480fa54..ae160d104f1 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -12,7 +12,7 @@ describe QuickActions::InterpretService do let(:service) { described_class.new(project, developer) } before do - project.team << [developer, :developer] + project.add_developer(developer) end describe '#execute' do @@ -440,7 +440,7 @@ describe QuickActions::InterpretService do let(:content) { "/assign @#{developer.username} @#{developer2.username}" } before do - project.team << [developer2, :developer] + project.add_developer(developer2) end context 'Issue' do diff --git a/spec/services/search/snippet_service_spec.rb b/spec/services/search/snippet_service_spec.rb index eae9bd4f5cf..bc7885b03d9 100644 --- a/spec/services/search/snippet_service_spec.rb +++ b/spec/services/search/snippet_service_spec.rb @@ -33,7 +33,7 @@ describe Search::SnippetService do it 'returns public, internal snippets and project private snippets for project members' do member = create(:user) - project.team << [member, :developer] + project.add_developer(member) search = described_class.new(member, search: 'password') results = search.execute diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 9025589ae0b..4e640a82dfc 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe SystemNoteService do include Gitlab::Routing + include RepoHelpers set(:group) { create(:group) } set(:project) { create(:project, :repository, group: group) } @@ -1070,17 +1071,32 @@ describe SystemNoteService do let(:action) { 'outdated' } end - it 'creates a new note in the discussion' do - # we need to completely rebuild the merge request object, or the `@discussions` on the merge request are not reloaded. - expect { subject }.to change { reloaded_merge_request.discussions.first.notes.size }.by(1) + context 'when the change_position is valid for the discussion' do + it 'creates a new note in the discussion' do + # we need to completely rebuild the merge request object, or the `@discussions` on the merge request are not reloaded. + expect { subject }.to change { reloaded_merge_request.discussions.first.notes.size }.by(1) + end + + it 'links to the diff in the system note' do + expect(subject.note).to include('version 1') + + diff_id = merge_request.merge_request_diff.id + line_code = change_position.line_code(project.repository) + expect(subject.note).to include(diffs_project_merge_request_url(project, merge_request, diff_id: diff_id, anchor: line_code)) + end end - it 'links to the diff in the system note' do - expect(subject.note).to include('version 1') + context 'when the change_position is invalid for the discussion' do + let(:change_position) { project.commit(sample_commit.id) } - diff_id = merge_request.merge_request_diff.id - line_code = change_position.line_code(project.repository) - expect(subject.note).to include(diffs_project_merge_request_url(project, merge_request, diff_id: diff_id, anchor: line_code)) + it 'creates a new note in the discussion' do + # we need to completely rebuild the merge request object, or the `@discussions` on the merge request are not reloaded. + expect { subject }.to change { reloaded_merge_request.discussions.first.notes.size }.by(1) + end + + it 'does not create a link' do + expect(subject.note).to eq('changed this line in version 1 of the diff') + end end end diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 88013acae0a..5e6c24f5730 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -17,11 +17,11 @@ describe TodoService do let(:service) { described_class.new } before do - project.team << [guest, :guest] - project.team << [author, :developer] - project.team << [member, :developer] - project.team << [john_doe, :developer] - project.team << [skipped, :developer] + project.add_guest(guest) + project.add_developer(author) + project.add_developer(member) + project.add_developer(john_doe) + project.add_developer(skipped) end describe 'Issues' do diff --git a/spec/services/update_merge_request_metrics_service_spec.rb b/spec/services/update_merge_request_metrics_service_spec.rb new file mode 100644 index 00000000000..b5fb999381d --- /dev/null +++ b/spec/services/update_merge_request_metrics_service_spec.rb @@ -0,0 +1,42 @@ +require 'rails_helper' + +describe MergeRequestMetricsService do + let(:metrics) { create(:merge_request).metrics } + + describe '#merge' do + it 'updates metrics' do + user = create(:user) + service = described_class.new(metrics) + event = double(Event, author_id: user.id, created_at: Time.now) + + service.merge(event) + + expect(metrics.merged_by).to eq(user) + expect(metrics.merged_at).to eq(event.created_at) + end + end + + describe '#close' do + it 'updates metrics' do + user = create(:user) + service = described_class.new(metrics) + event = double(Event, author_id: user.id, created_at: Time.now) + + service.close(event) + + expect(metrics.latest_closed_by).to eq(user) + expect(metrics.latest_closed_at).to eq(event.created_at) + end + end + + describe '#reopen' do + it 'updates metrics' do + service = described_class.new(metrics) + + service.reopen + + expect(metrics.latest_closed_by).to be_nil + expect(metrics.latest_closed_at).to be_nil + end + end +end diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb index 58a5bede3de..aeba9cd60bc 100644 --- a/spec/services/users/destroy_service_spec.rb +++ b/spec/services/users/destroy_service_spec.rb @@ -188,5 +188,22 @@ describe Users::DestroyService do end end end + + describe "calls the before/after callbacks" do + it 'of project_members' do + expect_any_instance_of(ProjectMember).to receive(:run_callbacks).with(:destroy).once + + service.execute(user) + end + + it 'of group_members' do + group_member = create(:group_member) + group_member.group.group_members.create(user: user, access_level: 40) + + expect_any_instance_of(GroupMember).to receive(:run_callbacks).with(:destroy).once + + service.execute(user) + end + end end end |