diff options
Diffstat (limited to 'spec/services')
70 files changed, 1200 insertions, 226 deletions
diff --git a/spec/services/after_branch_delete_service_spec.rb b/spec/services/after_branch_delete_service_spec.rb index d29e0addb53..77ca17bc82c 100644 --- a/spec/services/after_branch_delete_service_spec.rb +++ b/spec/services/after_branch_delete_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe AfterBranchDeleteService, services: true do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:service) { described_class.new(project, user) } diff --git a/spec/services/boards/create_service_spec.rb b/spec/services/boards/create_service_spec.rb index 7b29b043296..a8555f5b4a0 100644 --- a/spec/services/boards/create_service_spec.rb +++ b/spec/services/boards/create_service_spec.rb @@ -15,7 +15,7 @@ describe Boards::CreateService, services: true do board = service.execute expect(board.lists.size).to eq 1 - expect(board.lists.first).to be_done + expect(board.lists.first).to be_closed end end diff --git a/spec/services/boards/issues/list_service_spec.rb b/spec/services/boards/issues/list_service_spec.rb index d841bdaa292..c982031c791 100644 --- a/spec/services/boards/issues/list_service_spec.rb +++ b/spec/services/boards/issues/list_service_spec.rb @@ -15,7 +15,7 @@ describe Boards::Issues::ListService, services: true do let!(:list1) { create(:list, board: board, label: development, position: 0) } let!(:list2) { create(:list, board: board, label: testing, position: 1) } - let!(:done) { create(:done_list, board: board) } + let!(:closed) { create(:closed_list, board: board) } let!(:opened_issue1) { create(:labeled_issue, project: project, labels: [bug]) } let!(:opened_issue2) { create(:labeled_issue, project: project, labels: [p2]) } @@ -53,8 +53,8 @@ describe Boards::Issues::ListService, services: true do expect(issues).to eq [opened_issue2, reopened_issue1, opened_issue1] end - it 'returns closed issues when listing issues from Done' do - params = { board_id: board.id, id: done.id } + it 'returns closed issues when listing issues from Closed' do + params = { board_id: board.id, id: closed.id } issues = described_class.new(project, user, params).execute diff --git a/spec/services/boards/issues/move_service_spec.rb b/spec/services/boards/issues/move_service_spec.rb index 727ea04ea5c..4ff7ac6bb2f 100644 --- a/spec/services/boards/issues/move_service_spec.rb +++ b/spec/services/boards/issues/move_service_spec.rb @@ -12,7 +12,7 @@ describe Boards::Issues::MoveService, services: true do let!(:list1) { create(:list, board: board1, label: development, position: 0) } let!(:list2) { create(:list, board: board1, label: testing, position: 1) } - let!(:done) { create(:done_list, board: board1) } + let!(:closed) { create(:closed_list, board: board1) } before do project.team << [user, :developer] @@ -35,13 +35,13 @@ describe Boards::Issues::MoveService, services: true do end end - context 'when moving to done' do + context 'when moving to closed' do let(:board2) { create(:board, project: project) } let(:regression) { create(:label, project: project, name: 'Regression') } let!(:list3) { create(:list, board: board2, label: regression, position: 1) } let(:issue) { create(:labeled_issue, project: project, labels: [bug, development, testing, regression]) } - let(:params) { { board_id: board1.id, from_list_id: list2.id, to_list_id: done.id } } + let(:params) { { board_id: board1.id, from_list_id: list2.id, to_list_id: closed.id } } it 'delegates the close proceedings to Issues::CloseService' do expect_any_instance_of(Issues::CloseService).to receive(:execute).with(issue).once @@ -58,9 +58,9 @@ describe Boards::Issues::MoveService, services: true do end end - context 'when moving from done' do + context 'when moving from closed' do let(:issue) { create(:labeled_issue, :closed, project: project, labels: [bug]) } - let(:params) { { board_id: board1.id, from_list_id: done.id, to_list_id: list2.id } } + let(:params) { { board_id: board1.id, from_list_id: closed.id, to_list_id: list2.id } } it 'delegates the re-open proceedings to Issues::ReopenService' do expect_any_instance_of(Issues::ReopenService).to receive(:execute).with(issue).once diff --git a/spec/services/boards/lists/destroy_service_spec.rb b/spec/services/boards/lists/destroy_service_spec.rb index a30860f828a..af2d7c784bb 100644 --- a/spec/services/boards/lists/destroy_service_spec.rb +++ b/spec/services/boards/lists/destroy_service_spec.rb @@ -18,18 +18,18 @@ describe Boards::Lists::DestroyService, services: true do development = create(:list, board: board, position: 0) review = create(:list, board: board, position: 1) staging = create(:list, board: board, position: 2) - done = board.done_list + closed = board.closed_list described_class.new(project, user).execute(development) expect(review.reload.position).to eq 0 expect(staging.reload.position).to eq 1 - expect(done.reload.position).to be_nil + expect(closed.reload.position).to be_nil end end - it 'does not remove list from board when list type is done' do - list = board.done_list + it 'does not remove list from board when list type is closed' do + list = board.closed_list service = described_class.new(project, user) expect { service.execute(list) }.not_to change(board.lists, :count) diff --git a/spec/services/boards/lists/list_service_spec.rb b/spec/services/boards/lists/list_service_spec.rb index 2dffc62b215..ab9fb1bc914 100644 --- a/spec/services/boards/lists/list_service_spec.rb +++ b/spec/services/boards/lists/list_service_spec.rb @@ -10,7 +10,7 @@ describe Boards::Lists::ListService, services: true do service = described_class.new(project, double) - expect(service.execute(board)).to eq [list, board.done_list] + expect(service.execute(board)).to eq [list, board.closed_list] end end end diff --git a/spec/services/boards/lists/move_service_spec.rb b/spec/services/boards/lists/move_service_spec.rb index 3786dc82bf0..4b3bdd133f2 100644 --- a/spec/services/boards/lists/move_service_spec.rb +++ b/spec/services/boards/lists/move_service_spec.rb @@ -10,7 +10,7 @@ describe Boards::Lists::MoveService, services: true do let!(:development) { create(:list, board: board, position: 1) } let!(:review) { create(:list, board: board, position: 2) } let!(:staging) { create(:list, board: board, position: 3) } - let!(:done) { create(:done_list, board: board) } + let!(:closed) { create(:closed_list, board: board) } context 'when list type is set to label' do it 'keeps position of lists when new position is nil' do @@ -86,10 +86,10 @@ describe Boards::Lists::MoveService, services: true do end end - it 'keeps position of lists when list type is done' do + it 'keeps position of lists when list type is closed' do service = described_class.new(project, user, position: 2) - service.execute(done) + service.execute(closed) expect(current_list_positions).to eq [0, 1, 2, 3] end diff --git a/spec/services/chat_names/find_user_service_spec.rb b/spec/services/chat_names/find_user_service_spec.rb index 51441e8f3be..0dc96521fa8 100644 --- a/spec/services/chat_names/find_user_service_spec.rb +++ b/spec/services/chat_names/find_user_service_spec.rb @@ -18,9 +18,16 @@ describe ChatNames::FindUserService, services: true do end it 'updates when last time chat name was used' do + expect(chat_name.last_used_at).to be_nil + subject - expect(chat_name.reload.last_used_at).to be_like_time(Time.now) + initial_last_used = chat_name.reload.last_used_at + expect(initial_last_used).to be_present + + Timecop.travel(2.days.from_now) { described_class.new(service, params).execute } + + expect(chat_name.reload.last_used_at).to be > initial_last_used end end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index a969829a63e..d2f0337c260 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Ci::CreatePipelineService, services: true do - let(:project) { FactoryGirl.create(:project) } + let(:project) { create(:project, :repository) } let(:user) { create(:admin) } before do diff --git a/spec/services/ci/create_trigger_request_service_spec.rb b/spec/services/ci/create_trigger_request_service_spec.rb index 5e68343784d..5a20102872a 100644 --- a/spec/services/ci/create_trigger_request_service_spec.rb +++ b/spec/services/ci/create_trigger_request_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Ci::CreateTriggerRequestService, services: true do let(:service) { described_class.new } - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:trigger) { create(:ci_trigger, project: project) } before do diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb index 5445b65f4e8..f1b2d3a4798 100644 --- a/spec/services/ci/retry_pipeline_service_spec.rb +++ b/spec/services/ci/retry_pipeline_service_spec.rb @@ -9,6 +9,19 @@ describe Ci::RetryPipelineService, '#execute', :services do context 'when user has ability to modify pipeline' do let(:user) { create(:admin) } + context 'when there are already retried jobs present' do + before do + create_build('rspec', :canceled, 0) + create_build('rspec', :failed, 0) + end + + it 'does not retry jobs that has already been retried' do + expect(statuses.first).to be_retried + expect { service.execute(pipeline) } + .to change { CommitStatus.count }.by(1) + end + end + context 'when there are failed builds in the last stage' do before do create_build('rspec 1', :success, 0) diff --git a/spec/services/ci/stop_environments_service_spec.rb b/spec/services/ci/stop_environments_service_spec.rb index 560f83d94f7..32c72a9cf5e 100644 --- a/spec/services/ci/stop_environments_service_spec.rb +++ b/spec/services/ci/stop_environments_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Ci::StopEnvironmentsService, services: true do - let(:project) { create(:project, :private) } + let(:project) { create(:project, :private, :repository) } let(:user) { create(:user) } let(:service) { described_class.new(project, user) } diff --git a/spec/services/ci/update_build_queue_service_spec.rb b/spec/services/ci/update_build_queue_service_spec.rb index f01a388b895..c44e6b2a48b 100644 --- a/spec/services/ci/update_build_queue_service_spec.rb +++ b/spec/services/ci/update_build_queue_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Ci::UpdateBuildQueueService, :services do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:build) { create(:ci_build, pipeline: pipeline) } let(:pipeline) { create(:ci_pipeline, project: project) } diff --git a/spec/services/compare_service_spec.rb b/spec/services/compare_service_spec.rb index 0a7fc58523f..bea7c965233 100644 --- a/spec/services/compare_service_spec.rb +++ b/spec/services/compare_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe CompareService, services: true do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:service) { described_class.new(project, 'feature') } diff --git a/spec/services/create_release_service_spec.rb b/spec/services/create_release_service_spec.rb index 61e5ae72f51..271ccfe7968 100644 --- a/spec/services/create_release_service_spec.rb +++ b/spec/services/create_release_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe CreateReleaseService, services: true do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:tag_name) { project.repository.tag_names.first } let(:description) { 'Awesome release!' } diff --git a/spec/services/delete_branch_service_spec.rb b/spec/services/delete_branch_service_spec.rb index 336f5dafb5b..c4685c9aa31 100644 --- a/spec/services/delete_branch_service_spec.rb +++ b/spec/services/delete_branch_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe DeleteBranchService, services: true do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:repository) { project.repository } let(:user) { create(:user) } let(:service) { described_class.new(project, user) } diff --git a/spec/services/delete_merged_branches_service_spec.rb b/spec/services/delete_merged_branches_service_spec.rb index 181488e89c7..a41a421fa6e 100644 --- a/spec/services/delete_merged_branches_service_spec.rb +++ b/spec/services/delete_merged_branches_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe DeleteMergedBranchesService, services: true do subject(:service) { described_class.new(project, project.owner) } - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } context '#execute' do context 'unprotected branches' do diff --git a/spec/services/files/update_service_spec.rb b/spec/services/files/update_service_spec.rb index 35e6e139238..26aa5b432d4 100644 --- a/spec/services/files/update_service_spec.rb +++ b/spec/services/files/update_service_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" describe Files::UpdateService do subject { described_class.new(project, user, commit_params) } - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:file_path) { 'files/ruby/popen.rb' } let(:new_contents) { 'New Content' } diff --git a/spec/services/git_hooks_service_spec.rb b/spec/services/git_hooks_service_spec.rb index 3318dfb22b6..ac7ccfbaab0 100644 --- a/spec/services/git_hooks_service_spec.rb +++ b/spec/services/git_hooks_service_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' describe GitHooksService, services: true do include RepoHelpers - let(:user) { create :user } - let(:project) { create :project } + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } let(:service) { GitHooksService.new } before do diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index bd71618e6f4..0477cac6677 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' describe GitPushService, services: true do include RepoHelpers - let(:user) { create :user } - let(:project) { create :project } + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } before do project.team << [user, :master] diff --git a/spec/services/git_tag_push_service_spec.rb b/spec/services/git_tag_push_service_spec.rb index bd074b9bd71..b73beb3f6fc 100644 --- a/spec/services/git_tag_push_service_spec.rb +++ b/spec/services/git_tag_push_service_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' describe GitTagPushService, services: true do include RepoHelpers - let(:user) { create :user } - let(:project) { create :project } + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } let(:service) { GitTagPushService.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref) } let(:oldrev) { Gitlab::Git::BLANK_SHA } diff --git a/spec/services/groups/create_service_spec.rb b/spec/services/groups/create_service_spec.rb index ec89b540e6a..bcb62429275 100644 --- a/spec/services/groups/create_service_spec.rb +++ b/spec/services/groups/create_service_spec.rb @@ -44,7 +44,7 @@ describe Groups::CreateService, '#execute', services: true do let!(:service) { described_class.new(user, params) } before do - Settings.mattermost['enabled'] = true + stub_mattermost_setting(enabled: true) end it 'create the chat team with the group' do diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb index 98c560ffb26..2ee11fc8b4c 100644 --- a/spec/services/groups/destroy_service_spec.rb +++ b/spec/services/groups/destroy_service_spec.rb @@ -6,7 +6,7 @@ describe Groups::DestroyService, services: true do let!(:user) { create(:user) } let!(:group) { create(:group) } let!(:nested_group) { create(:group, parent: group) } - let!(:project) { create(:project, namespace: group) } + let!(:project) { create(:empty_project, namespace: group) } let!(:gitlab_shell) { Gitlab::Shell.new } let!(:remove_path) { group.path + "+#{group.id}+deleted" } diff --git a/spec/services/groups/update_service_spec.rb b/spec/services/groups/update_service_spec.rb index 7c0fccb9d41..f6ad5cebd2c 100644 --- a/spec/services/groups/update_service_spec.rb +++ b/spec/services/groups/update_service_spec.rb @@ -13,7 +13,7 @@ describe Groups::UpdateService, services: true do before do public_group.add_user(user, Gitlab::Access::MASTER) - create(:project, :public, group: public_group) + create(:empty_project, :public, group: public_group) end it "does not change permission level" do @@ -27,7 +27,7 @@ describe Groups::UpdateService, services: true do before do internal_group.add_user(user, Gitlab::Access::MASTER) - create(:project, :internal, group: internal_group) + create(:empty_project, :internal, group: internal_group) end it "does not change permission level" do @@ -36,6 +36,20 @@ describe Groups::UpdateService, services: true do end end end + + context "with parent_id user doesn't have permissions for" do + let(:service) { described_class.new(public_group, user, parent_id: private_group.id) } + + before do + service.execute + end + + it 'does not update parent_id' do + updated_group = public_group.reload + + expect(updated_group.parent_id).to be_nil + end + end end context "unauthorized visibility_level validation" do @@ -55,7 +69,7 @@ describe Groups::UpdateService, services: true do before do internal_group.add_user(user, Gitlab::Access::MASTER) - create(:project, :internal, group: internal_group) + create(:empty_project, :internal, group: internal_group) end it 'returns true' do diff --git a/spec/services/issues/build_service_spec.rb b/spec/services/issues/build_service_spec.rb index 1dd53236fbd..17990f41b3b 100644 --- a/spec/services/issues/build_service_spec.rb +++ b/spec/services/issues/build_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper.rb' describe Issues::BuildService, services: true do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:user) { create(:user) } before do diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index db196ed5751..9f8346d52bb 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -5,8 +5,8 @@ describe Issues::MoveService, services: true do let(:author) { create(:user) } let(:title) { 'Some issue' } let(:description) { 'Some issue description' } - let(:old_project) { create(:project) } - let(:new_project) { create(:project) } + let(:old_project) { create(:empty_project) } + let(:new_project) { create(:empty_project) } let(:milestone1) { create(:milestone, project_id: old_project.id, title: 'v9.0') } let(:old_issue) do diff --git a/spec/services/issues/resolve_discussions_spec.rb b/spec/services/issues/resolve_discussions_spec.rb index 6cc738aec08..3a72f92383c 100644 --- a/spec/services/issues/resolve_discussions_spec.rb +++ b/spec/services/issues/resolve_discussions_spec.rb @@ -10,7 +10,7 @@ class DummyService < Issues::BaseService end describe DummyService, services: true do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:user) { create(:user) } before do diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index fa472f3e2c3..5b324f3c706 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -13,6 +13,7 @@ describe Issues::UpdateService, services: true do let(:issue) do create(:issue, title: 'Old title', + description: "for #{user2.to_reference}", assignee_id: user3.id, project: project) end @@ -182,16 +183,24 @@ describe Issues::UpdateService, services: true do it 'marks pending todos as done' do expect(todo.reload.done?).to eq true end + + it 'does not create any new todos' do + expect(Todo.count).to eq(1) + end end context 'when the description change' do before do - update_issue(description: 'Also please fix') + update_issue(description: "Also please fix #{user2.to_reference} #{user3.to_reference}") end it 'marks todos as done' do expect(todo.reload.done?).to eq true end + + it 'creates only 1 new todo' do + expect(Todo.count).to eq(2) + end end context 'when is reassigned' do diff --git a/spec/services/labels/create_service_spec.rb b/spec/services/labels/create_service_spec.rb new file mode 100644 index 00000000000..0ecab0c3475 --- /dev/null +++ b/spec/services/labels/create_service_spec.rb @@ -0,0 +1,186 @@ +require 'spec_helper' + +describe Labels::CreateService, services: true do + describe '#execute' do + let(:project) { create(:project) } + let(:group) { create(:group) } + + let(:hex_color) { '#FF0000' } + let(:named_color) { 'red' } + let(:upcase_color) { 'RED' } + let(:spaced_color) { ' red ' } + let(:unknown_color) { 'unknown' } + let(:no_color) { '' } + + let(:expected_saved_color) { hex_color } + + context 'in a project' do + context 'with color in hex-code' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(hex_color)).execute(project: project) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with color in allowed name' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(named_color)).execute(project: project) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with color in up-case allowed name' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(upcase_color)).execute(project: project) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with color surrounded by spaces' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(spaced_color)).execute(project: project) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with unknown color' do + it 'doesn\'t create a label' do + label = Labels::CreateService.new(params_with(unknown_color)).execute(project: project) + + expect(label).not_to be_persisted + end + end + + context 'with no color' do + it 'doesn\'t create a label' do + label = Labels::CreateService.new(params_with(no_color)).execute(project: project) + + expect(label).not_to be_persisted + end + end + end + + context 'in a group' do + context 'with color in hex-code' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(hex_color)).execute(group: group) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with color in allowed name' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(named_color)).execute(group: group) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with color in up-case allowed name' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(upcase_color)).execute(group: group) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with color surrounded by spaces' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(spaced_color)).execute(group: group) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with unknown color' do + it 'doesn\'t create a label' do + label = Labels::CreateService.new(params_with(unknown_color)).execute(group: group) + + expect(label).not_to be_persisted + end + end + + context 'with no color' do + it 'doesn\'t create a label' do + label = Labels::CreateService.new(params_with(no_color)).execute(group: group) + + expect(label).not_to be_persisted + end + end + end + + context 'in admin area' do + context 'with color in hex-code' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(hex_color)).execute(template: true) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with color in allowed name' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(named_color)).execute(template: true) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with color in up-case allowed name' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(upcase_color)).execute(template: true) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with color surrounded by spaces' do + it 'creates a label' do + label = Labels::CreateService.new(params_with(spaced_color)).execute(template: true) + + expect(label).to be_persisted + expect(label.color).to eq expected_saved_color + end + end + + context 'with unknown color' do + it 'doesn\'t create a label' do + label = Labels::CreateService.new(params_with(unknown_color)).execute(template: true) + + expect(label).not_to be_persisted + end + end + + context 'with no color' do + it 'doesn\'t create a label' do + label = Labels::CreateService.new(params_with(no_color)).execute(template: true) + + expect(label).not_to be_persisted + end + end + end + end + + def params_with(color) + { + title: 'A Label', + color: color + } + end +end diff --git a/spec/services/labels/find_or_create_service_spec.rb b/spec/services/labels/find_or_create_service_spec.rb index 7a9b34f9f96..de8f1827cce 100644 --- a/spec/services/labels/find_or_create_service_spec.rb +++ b/spec/services/labels/find_or_create_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Labels::FindOrCreateService, services: true do describe '#execute' do let(:group) { create(:group) } - let(:project) { create(:project, namespace: group) } + let(:project) { create(:empty_project, namespace: group) } let(:params) do { diff --git a/spec/services/labels/transfer_service_spec.rb b/spec/services/labels/transfer_service_spec.rb index 13654a0881c..11d5f1cad5e 100644 --- a/spec/services/labels/transfer_service_spec.rb +++ b/spec/services/labels/transfer_service_spec.rb @@ -6,8 +6,8 @@ describe Labels::TransferService, services: true do let(:group_1) { create(:group) } let(:group_2) { create(:group) } let(:group_3) { create(:group) } - let(:project_1) { create(:project, namespace: group_2) } - let(:project_2) { create(:project, namespace: group_3) } + let(:project_1) { create(:empty_project, namespace: group_2) } + let(:project_2) { create(:empty_project, namespace: group_3) } let(:group_label_1) { create(:group_label, group: group_1, name: 'Group Label 1') } let(:group_label_2) { create(:group_label, group: group_1, name: 'Group Label 2') } diff --git a/spec/services/labels/update_service_spec.rb b/spec/services/labels/update_service_spec.rb new file mode 100644 index 00000000000..f2498ea6990 --- /dev/null +++ b/spec/services/labels/update_service_spec.rb @@ -0,0 +1,80 @@ +require 'spec_helper' + +describe Labels::UpdateService, services: true do + describe '#execute' do + let(:project) { create(:project) } + + let(:hex_color) { '#FF0000' } + let(:named_color) { 'red' } + let(:upcase_color) { 'RED' } + let(:spaced_color) { ' red ' } + let(:unknown_color) { 'unknown' } + let(:no_color) { '' } + + let(:expected_saved_color) { hex_color } + + before(:each) do + @label = Labels::CreateService.new(title: 'Initial', color: '#000000').execute(project: project) + expect(@label).to be_persisted + end + + context 'with color in hex-code' do + it 'updates the label' do + label = Labels::UpdateService.new(params_with(hex_color)).execute(@label) + + expect(label).to be_valid + expect(label.reload.color).to eq expected_saved_color + end + end + + context 'with color in allowed name' do + it 'updates the label' do + label = Labels::UpdateService.new(params_with(named_color)).execute(@label) + + expect(label).to be_valid + expect(label.reload.color).to eq expected_saved_color + end + end + + context 'with color in up-case allowed name' do + it 'updates the label' do + label = Labels::UpdateService.new(params_with(upcase_color)).execute(@label) + + expect(label).to be_valid + expect(label.reload.color).to eq expected_saved_color + end + end + + context 'with color surrounded by spaces' do + it 'updates the label' do + label = Labels::UpdateService.new(params_with(spaced_color)).execute(@label) + + expect(label).to be_valid + expect(label.reload.color).to eq expected_saved_color + end + end + + context 'with unknown color' do + it 'doesn\'t update the label' do + label = Labels::UpdateService.new(params_with(unknown_color)).execute(@label) + + expect(label).not_to be_valid + end + end + + context 'with no color' do + it 'doesn\'t update the label' do + label = Labels::UpdateService.new(params_with(no_color)).execute(@label) + + expect(label).not_to be_valid + end + end + end + + def params_with(color) + { + title: 'A Label', + color: color + } + end +end diff --git a/spec/services/members/destroy_service_spec.rb b/spec/services/members/destroy_service_spec.rb index 574df6e0f42..41450c67d7e 100644 --- a/spec/services/members/destroy_service_spec.rb +++ b/spec/services/members/destroy_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Members::DestroyService, services: true do let(:user) { create(:user) } let(:member_user) { create(:user) } - let(:project) { create(:project, :public) } + let(:project) { create(:empty_project, :public) } let(:group) { create(:group, :public) } shared_examples 'a service raising ActiveRecord::RecordNotFound' do diff --git a/spec/services/members/request_access_service_spec.rb b/spec/services/members/request_access_service_spec.rb index 853c125dadb..814c17b9ad0 100644 --- a/spec/services/members/request_access_service_spec.rb +++ b/spec/services/members/request_access_service_spec.rb @@ -29,7 +29,7 @@ describe Members::RequestAccessService, services: true do end context 'when current user cannot request access to the project' do - %i[project group].each do |source_type| + %i[empty_project group].each do |source_type| it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do let(:source) { create(source_type, :private) } end @@ -37,7 +37,7 @@ describe Members::RequestAccessService, services: true do end context 'when access requests are disabled' do - %i[project group].each do |source_type| + %i[empty_project group].each do |source_type| it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do let(:source) { create(source_type, :public) } end @@ -45,7 +45,7 @@ describe Members::RequestAccessService, services: true do end context 'when current user can request access to the project' do - %i[project group].each do |source_type| + %i[empty_project group].each do |source_type| it_behaves_like 'a service creating a access request' do let(:source) { create(source_type, :public, :access_requestable) } end diff --git a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb index d80fb8a1af1..af0a214c00f 100644 --- a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb +++ b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe MergeRequests::AddTodoWhenBuildFailsService do let(:user) { create(:user) } let(:merge_request) { create(:merge_request) } - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:sha) { '1234567890abcdef1234567890abcdef12345678' } let(:ref) { merge_request.source_branch } diff --git a/spec/services/merge_requests/assign_issues_service_spec.rb b/spec/services/merge_requests/assign_issues_service_spec.rb index 5034b6ef33f..fe75757dd29 100644 --- a/spec/services/merge_requests/assign_issues_service_spec.rb +++ b/spec/services/merge_requests/assign_issues_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe MergeRequests::AssignIssuesService, services: true do let(:user) { create(:user) } - let(:project) { create(:project, :public) } + let(:project) { create(:project, :public, :repository) } let(:issue) { create(:issue, project: project) } let(:merge_request) { create(:merge_request, :simple, source_project: project, author: user, description: "fixes #{issue.to_reference}") } let(:service) { described_class.new(project, user, merge_request: merge_request) } diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb index adfa75a524f..c8bd4d4601a 100644 --- a/spec/services/merge_requests/build_service_spec.rb +++ b/spec/services/merge_requests/build_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe MergeRequests::BuildService, services: true do include RepoHelpers - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:issue_confidential) { false } let(:issue) { create(:issue, project: project, title: 'A bug', confidential: issue_confidential) } diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index 673c0bd6c9c..0e16c7cc94b 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe MergeRequests::CreateService, services: true do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:assignee) { create(:user) } diff --git a/spec/services/merge_requests/get_urls_service_spec.rb b/spec/services/merge_requests/get_urls_service_spec.rb index b7a05907208..290e00ea1ba 100644 --- a/spec/services/merge_requests/get_urls_service_spec.rb +++ b/spec/services/merge_requests/get_urls_service_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe MergeRequests::GetUrlsService do - let(:project) { create(:project, :public) } + let(:project) { create(:project, :public, :repository) } let(:service) { MergeRequests::GetUrlsService.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}" } 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 c2f205c389d..769b3193275 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 @@ -2,7 +2,7 @@ require 'spec_helper' describe MergeRequests::MergeWhenPipelineSucceedsService do let(:user) { create(:user) } - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:mr_merge_if_green_enabled) do create(:merge_request, merge_when_pipeline_succeeds: true, merge_user: user, diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 92729f68e5f..c22d145ca5d 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe MergeRequests::RefreshService, services: true do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:service) { MergeRequests::RefreshService } @@ -11,7 +11,7 @@ describe MergeRequests::RefreshService, services: true do group = create(:group) group.add_owner(@user) - @project = create(:project, namespace: group) + @project = create(:project, :repository, namespace: group) @fork_project = Projects::ForkService.new(@project, @user).execute @merge_request = create(:merge_request, source_project: @project, @@ -252,7 +252,7 @@ describe MergeRequests::RefreshService, services: true do context 'when the merge request is sourced from a different project' do it 'creates a `MergeRequestsClosingIssues` record for each issue closed by a commit' do - forked_project = create(:project) + forked_project = create(:project, :repository) create(:forked_project_link, forked_to_project: forked_project, forked_from_project: @project) merge_request = create(:merge_request, diff --git a/spec/services/merge_requests/resolve_service_spec.rb b/spec/services/merge_requests/resolve_service_spec.rb index d33535d22af..eaf7785e549 100644 --- a/spec/services/merge_requests/resolve_service_spec.rb +++ b/spec/services/merge_requests/resolve_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe MergeRequests::ResolveService do let(:user) { create(:user) } - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:fork_project) do create(:forked_project_with_submodules) do |fork_project| diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 7d73c0ea5d0..f2ca1e6fcbd 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe MergeRequests::UpdateService, services: true do include EmailHelpers - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:user2) { create(:user) } let(:user3) { create(:user) } @@ -12,6 +12,7 @@ describe MergeRequests::UpdateService, services: true do let(:merge_request) do create(:merge_request, :simple, title: 'Old title', + description: "FYI #{user2.to_reference}", assignee_id: user3.id, source_project: project) end @@ -225,16 +226,24 @@ describe MergeRequests::UpdateService, services: true do it 'marks pending todos as done' do expect(pending_todo.reload).to be_done end + + it 'does not create any new todos' do + expect(Todo.count).to eq(1) + end end context 'when the description change' do before do - update_merge_request({ description: 'Also please fix' }) + update_merge_request({ description: "Also please fix #{user2.to_reference} #{user3.to_reference}" }) end it 'marks pending todos as done' do expect(pending_todo.reload).to be_done end + + it 'creates only 1 new todo' do + expect(Todo.count).to eq(2) + end end context 'when is reassigned' do diff --git a/spec/services/milestones/close_service_spec.rb b/spec/services/milestones/close_service_spec.rb index fe6a19e97ea..d581b94ff43 100644 --- a/spec/services/milestones/close_service_spec.rb +++ b/spec/services/milestones/close_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Milestones::CloseService, services: true do let(:user) { create(:user) } - let(:project) { create(:project) } + let(:project) { create(:empty_project) } let(:milestone) { create(:milestone, title: "Milestone v1.2", project: project) } before do diff --git a/spec/services/note_summary_spec.rb b/spec/services/note_summary_spec.rb new file mode 100644 index 00000000000..39f06f8f8e7 --- /dev/null +++ b/spec/services/note_summary_spec.rb @@ -0,0 +1,44 @@ +require 'spec_helper' + +describe NoteSummary, services: true do + let(:project) { build(:empty_project) } + let(:noteable) { build(:issue) } + let(:user) { build(:user) } + + def create_note_summary + described_class.new(noteable, project, user, 'note', action: 'icon', commit_count: 5) + end + + describe '#metadata?' do + it 'returns true when metadata present' do + expect(create_note_summary.metadata?).to be_truthy + end + + it 'returns false when metadata not present' do + expect(described_class.new(noteable, project, user, 'note').metadata?).to be_falsey + end + end + + describe '#note' do + it 'returns note hash' do + expect(create_note_summary.note).to eq(noteable: noteable, project: project, author: user, note: 'note') + end + + context 'when noteable is a commit' do + let(:noteable) { build(:commit) } + + it 'returns note hash specific to commit' do + expect(create_note_summary.note).to eq( + noteable: nil, project: project, author: user, note: 'note', + noteable_type: 'Commit', commit_id: noteable.id + ) + end + end + end + + describe '#metadata' do + it 'returns metadata hash' do + expect(create_note_summary.metadata).to eq(action: 'icon', commit_count: 5) + end + end +end diff --git a/spec/services/notes/diff_position_update_service_spec.rb b/spec/services/notes/diff_position_update_service_spec.rb index 110efb54fa0..d73ae51fbc3 100644 --- a/spec/services/notes/diff_position_update_service_spec.rb +++ b/spec/services/notes/diff_position_update_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Notes::DiffPositionUpdateService, services: true do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:create_commit) { project.commit("913c66a37b4a45b9769037c55c2d238bd0942d2e") } let(:modify_commit) { project.commit("874797c3a73b60d2187ed6e2fcabd289ff75171e") } let(:edit_commit) { project.commit("570e7b2abdd848b95f2f578043fc23bd6f6fd24d") } diff --git a/spec/services/notes/update_service_spec.rb b/spec/services/notes/update_service_spec.rb index dde4bde7dc2..905e2f46bde 100644 --- a/spec/services/notes/update_service_spec.rb +++ b/spec/services/notes/update_service_spec.rb @@ -4,12 +4,14 @@ describe Notes::UpdateService, services: true do let(:project) { create(:empty_project) } let(:user) { create(:user) } let(:user2) { create(:user) } + let(:user3) { create(:user) } let(:issue) { create(:issue, project: project) } - let(:note) { create(:note, project: project, noteable: issue, author: user, note: 'Old note') } + 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] end describe '#execute' do @@ -23,22 +25,30 @@ describe Notes::UpdateService, services: true do context 'when the note change' do before do - update_note({ note: 'New note' }) + update_note({ note: "New note #{user2.to_reference} #{user3.to_reference}" }) end it 'marks todos as done' do expect(todo.reload).to be_done end + + it 'creates only 1 new todo' do + expect(Todo.count).to eq(2) + end end context 'when the note does not change' do before do - update_note({ note: 'Old note' }) + update_note({ note: "Old note #{user2.to_reference}" }) end it 'keep todos' do expect(todo.reload).to be_pending end + + it 'does not create any new todos' do + expect(Todo.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 f7240969588..5c841843b40 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -146,6 +146,16 @@ describe NotificationService, services: true do should_not_email(@u_lazy_participant) end + it "emails the note author if they've opted into notifications about their activity" do + add_users_with_subscription(note.project, issue) + note.author.notified_of_own_activity = true + reset_delivered_emails! + + notification.new_note(note) + + should_email(note.author) + end + it 'filters out "mentioned in" notes' do mentioned_note = SystemNoteService.cross_reference(mentioned_issue, issue, issue.author) @@ -362,7 +372,7 @@ describe NotificationService, services: true do end context 'commit note' do - let(:project) { create(:project, :public) } + let(:project) { create(:project, :public, :repository) } let(:note) { create(:note_on_commit, project: project) } before do @@ -411,7 +421,7 @@ describe NotificationService, services: true do end context "merge request diff note" do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:merge_request) { create(:merge_request, source_project: project, assignee: user) } let(:note) { create(:diff_note_on_merge_request, project: project, noteable: merge_request) } @@ -476,6 +486,20 @@ describe NotificationService, services: true do should_not_email(issue.assignee) end + it "emails the author if they've opted into notifications about their activity" do + issue.author.notified_of_own_activity = true + + notification.new_issue(issue, issue.author) + + should_email(issue.author) + end + + it "doesn't email the author if they haven't opted into notifications about their activity" do + notification.new_issue(issue, issue.author) + + should_not_email(issue.author) + end + it "emails subscribers of the issue's labels" do user_1 = create(:user) user_2 = create(:user) @@ -665,6 +689,19 @@ describe NotificationService, services: true do should_email(subscriber_to_label_2) end + it "emails the current user if they've opted into notifications about their activity" do + subscriber_to_label_2.notified_of_own_activity = true + notification.relabeled_issue(issue, [group_label_2, label_2], subscriber_to_label_2) + + should_email(subscriber_to_label_2) + end + + it "doesn't email the current user if they haven't opted into notifications about their activity" do + notification.relabeled_issue(issue, [group_label_2, label_2], subscriber_to_label_2) + + should_not_email(subscriber_to_label_2) + end + it "doesn't send email to anyone but subscribers of the given labels" do notification.relabeled_issue(issue, [group_label_2, label_2], @u_disabled) @@ -812,7 +849,7 @@ describe NotificationService, services: true do describe 'Merge Requests' do let(:group) { create(:group) } - let(:project) { create(:project, :public, namespace: group) } + let(:project) { create(:project, :public, :repository, namespace: group) } let(:another_project) { create(:empty_project, :public, namespace: group) } let(:merge_request) { create :merge_request, source_project: project, assignee: create(:user), description: 'cc @participant' } @@ -845,6 +882,20 @@ describe NotificationService, services: true do should_not_email(@u_lazy_participant) end + it "emails the author if they've opted into notifications about their activity" do + merge_request.author.notified_of_own_activity = true + + notification.new_merge_request(merge_request, merge_request.author) + + should_email(merge_request.author) + end + + it "doesn't email the author if they haven't opted into notifications about their activity" do + notification.new_merge_request(merge_request, merge_request.author) + + should_not_email(merge_request.author) + end + it "emails subscribers of the merge request's labels" do user_1 = create(:user) user_2 = create(:user) @@ -1040,6 +1091,14 @@ describe NotificationService, services: true do should_not_email(@u_watcher) end + it "notifies the merger when the pipeline succeeds is false but they've opted into notifications about their activity" do + merge_request.merge_when_pipeline_succeeds = false + @u_watcher.notified_of_own_activity = true + notification.merge_mr(merge_request, @u_watcher) + + should_email(@u_watcher) + end + it_behaves_like 'participating notifications' do let(:participant) { create(:user, username: 'user-participant') } let(:issuable) { merge_request } @@ -1102,7 +1161,7 @@ describe NotificationService, services: true do end describe 'Projects' do - let(:project) { create :project } + let(:project) { create(:empty_project) } before do build_team(project) @@ -1147,7 +1206,7 @@ describe NotificationService, services: true do describe 'ProjectMember' do describe '#decline_group_invite' do - let(:project) { create(:project) } + let(:project) { create(:empty_project) } let(:member) { create(:user) } before(:each) do @@ -1221,7 +1280,7 @@ describe NotificationService, services: true do describe 'Pipelines' do describe '#pipeline_finished' do - let(:project) { create(:project, :public) } + let(:project) { create(:project, :public, :repository) } let(:current_user) { create(:user) } let(:u_member) { create(:user) } let(:u_other) { create(:user) } diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index 193ccd17282..cf1f90becfd 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Projects::DestroyService, services: true do let!(:user) { create(:user) } - let!(:project) { create(:project, namespace: user.namespace) } + let!(:project) { create(:project, :repository, namespace: user.namespace) } let!(:path) { project.repository.path_to_repo } let!(:remove_path) { path.sub(/\.git\Z/, "+#{project.id}+deleted.git") } let!(:async) { false } # execute or async_execute diff --git a/spec/services/projects/download_service_spec.rb b/spec/services/projects/download_service_spec.rb index 122a7cea2a1..33b267c069c 100644 --- a/spec/services/projects/download_service_spec.rb +++ b/spec/services/projects/download_service_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' describe Projects::DownloadService, services: true do describe 'File service' do before do - @user = create :user - @project = create :project, creator_id: @user.id, namespace: @user.namespace + @user = create(:user) + @project = create(:empty_project, creator_id: @user.id, namespace: @user.namespace) end context 'for a URL that is not on whitelist' do diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index e3be1989c93..f8eb34f2ef4 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -7,6 +7,7 @@ describe Projects::ForkService, services: true do @from_user = create(:user, namespace: @from_namespace ) avatar = fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png") @from_project = create(:project, + :repository, creator_id: @from_user.id, namespace: @from_namespace, star_count: 107, @@ -54,7 +55,7 @@ describe Projects::ForkService, services: true do context 'project already exists' do it "fails due to validation, not transaction failure" do - @existing_project = create(:project, creator_id: @to_user.id, name: @from_project.name, namespace: @to_namespace) + @existing_project = create(:project, :repository, creator_id: @to_user.id, name: @from_project.name, namespace: @to_namespace) @to_project = fork_project(@from_project, @to_user) expect(@existing_project).to be_persisted @@ -104,9 +105,10 @@ describe Projects::ForkService, services: true do before do @group_owner = create(:user) @developer = create(:user) - @project = create(:project, creator_id: @group_owner.id, - star_count: 777, - description: 'Wow, such a cool project!') + @project = create(:project, :repository, + creator_id: @group_owner.id, + star_count: 777, + description: 'Wow, such a cool project!') @group = create(:group) @group.add_user(@group_owner, GroupMember::OWNER) @group.add_user(@developer, GroupMember::DEVELOPER) @@ -139,8 +141,9 @@ describe Projects::ForkService, services: true do context 'project already exists in group' do it 'fails due to validation, not transaction failure' do - existing_project = create(:project, name: @project.name, - namespace: @group) + existing_project = create(:project, :repository, + name: @project.name, + namespace: @group) to_project = fork_project(@project, @group_owner, @opts) expect(existing_project.persisted?).to be_truthy expect(to_project.errors[:name]).to eq(['has already been taken']) diff --git a/spec/services/projects/housekeeping_service_spec.rb b/spec/services/projects/housekeeping_service_spec.rb index 57a5aa5cedc..eaf63457b32 100644 --- a/spec/services/projects/housekeeping_service_spec.rb +++ b/spec/services/projects/housekeeping_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Projects::HousekeepingService do subject { Projects::HousekeepingService.new(project) } - let(:project) { create :project } + let(:project) { create(:project, :repository) } before do project.reset_pushes_since_gc diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index a3babaf1e0b..81e15f9dba6 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Projects::TransferService, services: true do let(:user) { create(:user) } let(:group) { create(:group) } - let(:project) { create(:project, namespace: user.namespace) } + let(:project) { create(:project, :repository, namespace: user.namespace) } context 'namespace -> namespace' do before do @@ -61,7 +61,7 @@ describe Projects::TransferService, services: true do before { internal_group.add_owner(user) } context 'when namespace visibility level < project visibility level' do - let(:public_project) { create(:project, :public, namespace: user.namespace) } + let(:public_project) { create(:project, :public, :repository, namespace: user.namespace) } before { transfer_project(public_project, user, internal_group) } @@ -69,7 +69,7 @@ describe Projects::TransferService, services: true do end context 'when namespace visibility level > project visibility level' do - let(:private_project) { create(:project, :private, namespace: user.namespace) } + let(:private_project) { create(:project, :private, :repository, namespace: user.namespace) } before { transfer_project(private_project, user, internal_group) } diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb index f75fdd9e03f..fc0a17296f3 100644 --- a/spec/services/projects/update_pages_service_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb @@ -1,9 +1,9 @@ require "spec_helper" describe Projects::UpdatePagesService do - let(:project) { create :project } - let(:pipeline) { create :ci_pipeline, project: project, sha: project.commit('HEAD').sha } - let(:build) { create :ci_build, pipeline: pipeline, ref: 'HEAD' } + let(:project) { create(:project, :repository) } + let(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit('HEAD').sha) } + let(:build) { create(:ci_build, pipeline: pipeline, ref: 'HEAD') } let(:invalid_file) { fixture_file_upload(Rails.root + 'spec/fixtures/dk.png') } subject { described_class.new(project, build) } diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index caa23962519..05b18fef061 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Projects::UpdateService, services: true do let(:user) { create(:user) } let(:admin) { create(:admin) } - let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } + let(:project) { create(:empty_project, creator_id: user.id, namespace: user.namespace) } describe 'update_by_user' do context 'when visibility_level is INTERNAL' do @@ -56,7 +56,7 @@ describe Projects::UpdateService, services: true do end describe 'visibility_level' do - let(:project) { create(:project, :internal) } + let(:project) { create(:empty_project, :internal) } let(:forked_project) { create(:forked_project_with_submodules, :internal) } before do diff --git a/spec/services/projects/upload_service_spec.rb b/spec/services/projects/upload_service_spec.rb index 150c8ccaef7..d2cefa46bfa 100644 --- a/spec/services/projects/upload_service_spec.rb +++ b/spec/services/projects/upload_service_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' describe Projects::UploadService, services: true do describe 'File service' do before do - @user = create :user - @project = create :project, creator_id: @user.id, namespace: @user.namespace + @user = create(:user) + @project = create(:empty_project, creator_id: @user.id, namespace: @user.namespace) end context 'for valid gif file' do diff --git a/spec/services/search/global_service_spec.rb b/spec/services/search/global_service_spec.rb new file mode 100644 index 00000000000..2531607acad --- /dev/null +++ b/spec/services/search/global_service_spec.rb @@ -0,0 +1,66 @@ +require 'spec_helper' + +describe Search::GlobalService, services: true do + let(:user) { create(:user) } + let(:internal_user) { create(:user) } + + let!(:found_project) { create(:empty_project, :private, name: 'searchable_project') } + let!(:unfound_project) { create(:empty_project, :private, name: 'unfound_project') } + let!(:internal_project) { create(:empty_project, :internal, name: 'searchable_internal_project') } + let!(:public_project) { create(:empty_project, :public, name: 'searchable_public_project') } + + before do + found_project.add_master(user) + end + + describe '#execute' do + context 'unauthenticated' do + it 'returns public projects only' do + results = Search::GlobalService.new(nil, search: "searchable").execute + + expect(results.objects('projects')).to match_array [public_project] + end + end + + context 'authenticated' do + it 'returns public, internal and private projects' do + results = Search::GlobalService.new(user, search: "searchable").execute + + expect(results.objects('projects')).to match_array [public_project, found_project, internal_project] + end + + it 'returns only public & internal projects' do + results = Search::GlobalService.new(internal_user, search: "searchable").execute + + expect(results.objects('projects')).to match_array [internal_project, public_project] + end + + it 'namespace name is searchable' do + results = Search::GlobalService.new(user, search: found_project.namespace.path).execute + + expect(results.objects('projects')).to match_array [found_project] + end + + context 'nested group' do + let!(:nested_group) { create(:group, :nested) } + let!(:project) { create(:empty_project, namespace: nested_group) } + + before do + project.add_master(user) + end + + it 'returns result from nested group' do + results = Search::GlobalService.new(user, search: project.path).execute + + expect(results.objects('projects')).to match_array [project] + end + + it 'returns result from descendants when search inside group' do + results = Search::GlobalService.new(user, search: project.path, group_id: nested_group.parent).execute + + expect(results.objects('projects')).to match_array [project] + end + end + end + end +end diff --git a/spec/services/search_service_spec.rb b/spec/services/search_service_spec.rb index bed1031e40a..2112f1cf9ea 100644 --- a/spec/services/search_service_spec.rb +++ b/spec/services/search_service_spec.rb @@ -1,65 +1,286 @@ require 'spec_helper' -describe 'Search::GlobalService', services: true do +describe SearchService, services: true do let(:user) { create(:user) } - let(:public_user) { create(:user) } - let(:internal_user) { create(:user) } - let!(:found_project) { create(:empty_project, :private, name: 'searchable_project') } - let!(:unfound_project) { create(:empty_project, :private, name: 'unfound_project') } - let!(:internal_project) { create(:empty_project, :internal, name: 'searchable_internal_project') } - let!(:public_project) { create(:empty_project, :public, name: 'searchable_public_project') } + let(:accessible_group) { create(:group, :private) } + let(:inaccessible_group) { create(:group, :private) } + let!(:group_member) { create(:group_member, group: accessible_group, user: user) } + + let!(:accessible_project) { create(:empty_project, :private, name: 'accessible_project') } + let!(:inaccessible_project) { create(:empty_project, :private, name: 'inaccessible_project') } + let(:note) { create(:note_on_issue, project: accessible_project) } + + let(:snippet) { create(:snippet, author: user) } + let(:group_project) { create(:empty_project, group: accessible_group, name: 'group_project') } + let(:public_project) { create(:empty_project, :public, name: 'public_project') } before do - found_project.team << [user, :master] + accessible_project.add_master(user) + end + + describe '#project' do + context 'when the project is accessible' do + it 'returns the project' do + project = SearchService.new(user, project_id: accessible_project.id).project + + expect(project).to eq accessible_project + end + end + + context 'when the project is not accessible' do + it 'returns nil' do + project = SearchService.new(user, project_id: inaccessible_project.id).project + + expect(project).to be_nil + end + end + + context 'when there is no project_id' do + it 'returns nil' do + project = SearchService.new(user).project + + expect(project).to be_nil + end + end end - describe '#execute' do - context 'unauthenticated' do - it 'returns public projects only' do - context = Search::GlobalService.new(nil, search: "searchable") - results = context.execute - expect(results.objects('projects')).to match_array [public_project] + describe '#group' do + context 'when the group is accessible' do + it 'returns the group' do + group = SearchService.new(user, group_id: accessible_group.id).group + + expect(group).to eq accessible_group end end - context 'authenticated' do - it 'returns public, internal and private projects' do - context = Search::GlobalService.new(user, search: "searchable") - results = context.execute - expect(results.objects('projects')).to match_array [public_project, found_project, internal_project] + context 'when the group is not accessible' do + it 'returns nil' do + group = SearchService.new(user, group_id: inaccessible_group.id).group + + expect(group).to be_nil end + end + + context 'when there is no group_id' do + it 'returns nil' do + group = SearchService.new(user).group - it 'returns only public & internal projects' do - context = Search::GlobalService.new(internal_user, search: "searchable") - results = context.execute - expect(results.objects('projects')).to match_array [internal_project, public_project] + expect(group).to be_nil end + end + end + + describe '#show_snippets?' do + context 'when :snippets is \'true\'' do + it 'returns true' do + show_snippets = SearchService.new(user, snippets: 'true').show_snippets? - it 'namespace name is searchable' do - context = Search::GlobalService.new(user, search: found_project.namespace.path) - results = context.execute - expect(results.objects('projects')).to match_array [found_project] + expect(show_snippets).to be_truthy end + end - context 'nested group' do - let!(:nested_group) { create(:group, :nested) } - let!(:project) { create(:project, namespace: nested_group) } + context 'when :snippets is not \'true\'' do + it 'returns false' do + show_snippets = SearchService.new(user, snippets: 'tru').show_snippets? + + expect(show_snippets).to be_falsey + end + end - before { project.add_master(user) } + context 'when :snippets is missing' do + it 'returns false' do + show_snippets = SearchService.new(user).show_snippets? - it 'returns result from nested group' do - context = Search::GlobalService.new(user, search: project.path) - results = context.execute - expect(results.objects('projects')).to match_array [project] + expect(show_snippets).to be_falsey + end + end + end + + describe '#scope' do + context 'with accessible project_id' do + context 'and allowed scope' do + it 'returns the specified scope' do + scope = SearchService.new(user, project_id: accessible_project.id, scope: 'notes').scope + + expect(scope).to eq 'notes' end + end + + context 'and disallowed scope' do + it 'returns the default scope' do + scope = SearchService.new(user, project_id: accessible_project.id, scope: 'projects').scope - it 'returns result from descendants when search inside group' do - context = Search::GlobalService.new(user, search: project.path, group_id: nested_group.parent) - results = context.execute - expect(results.objects('projects')).to match_array [project] + expect(scope).to eq 'blobs' end end + + context 'and no scope' do + it 'returns the default scope' do + scope = SearchService.new(user, project_id: accessible_project.id).scope + + expect(scope).to eq 'blobs' + end + end + end + + context 'with \'true\' snippets' do + context 'and allowed scope' do + it 'returns the specified scope' do + scope = SearchService.new(user, snippets: 'true', scope: 'snippet_titles').scope + + expect(scope).to eq 'snippet_titles' + end + end + + context 'and disallowed scope' do + it 'returns the default scope' do + scope = SearchService.new(user, snippets: 'true', scope: 'projects').scope + + expect(scope).to eq 'snippet_blobs' + end + end + + context 'and no scope' do + it 'returns the default scope' do + scope = SearchService.new(user, snippets: 'true').scope + + expect(scope).to eq 'snippet_blobs' + end + end + end + + context 'with no project_id, no snippets' do + context 'and allowed scope' do + it 'returns the specified scope' do + scope = SearchService.new(user, scope: 'issues').scope + + expect(scope).to eq 'issues' + end + end + + context 'and disallowed scope' do + it 'returns the default scope' do + scope = SearchService.new(user, scope: 'blobs').scope + + expect(scope).to eq 'projects' + end + end + + context 'and no scope' do + it 'returns the default scope' do + scope = SearchService.new(user).scope + + expect(scope).to eq 'projects' + end + end + end + end + + describe '#search_results' do + context 'with accessible project_id' do + it 'returns an instance of Gitlab::ProjectSearchResults' do + search_results = SearchService.new( + user, + project_id: accessible_project.id, + scope: 'notes', + search: note.note).search_results + + expect(search_results).to be_a Gitlab::ProjectSearchResults + end + end + + context 'with accessible project_id and \'true\' snippets' do + it 'returns an instance of Gitlab::ProjectSearchResults' do + search_results = SearchService.new( + user, + project_id: accessible_project.id, + snippets: 'true', + scope: 'notes', + search: note.note).search_results + + expect(search_results).to be_a Gitlab::ProjectSearchResults + end + end + + context 'with \'true\' snippets' do + it 'returns an instance of Gitlab::SnippetSearchResults' do + search_results = SearchService.new( + user, + snippets: 'true', + search: snippet.content).search_results + + expect(search_results).to be_a Gitlab::SnippetSearchResults + end + end + + context 'with no project_id and no snippets' do + it 'returns an instance of Gitlab::SearchResults' do + search_results = SearchService.new( + user, + search: public_project.name).search_results + + expect(search_results).to be_a Gitlab::SearchResults + end + end + end + + describe '#search_objects' do + context 'with accessible project_id' do + it 'returns objects in the project' do + search_objects = SearchService.new( + user, + project_id: accessible_project.id, + scope: 'notes', + search: note.note).search_objects + + expect(search_objects.first).to eq note + end + end + + context 'with accessible project_id and \'true\' snippets' do + it 'returns objects in the project' do + search_objects = SearchService.new( + user, + project_id: accessible_project.id, + snippets: 'true', + scope: 'notes', + search: note.note).search_objects + + expect(search_objects.first).to eq note + end + end + + context 'with \'true\' snippets' do + it 'returns objects in snippets' do + search_objects = SearchService.new( + user, + snippets: 'true', + search: snippet.content).search_objects + + expect(search_objects.first).to eq snippet + end + end + + context 'with accessible group_id' do + it 'returns objects in the group' do + search_objects = SearchService.new( + user, + group_id: accessible_group.id, + search: group_project.name).search_objects + + expect(search_objects.first).to eq group_project + end + end + + context 'with no project_id, group_id or snippets' do + it 'returns objects in global' do + search_objects = SearchService.new( + user, + search: public_project.name).search_objects + + expect(search_objects.first).to eq public_project + 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 52e8678cb9d..a63281f0eab 100644 --- a/spec/services/slash_commands/interpret_service_spec.rb +++ b/spec/services/slash_commands/interpret_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe SlashCommands::InterpretService, services: true do - let(:project) { create(:project, :public) } + let(:project) { create(:empty_project, :public) } let(:developer) { create(:user) } let(:issue) { create(:issue, project: project) } let(:milestone) { create(:milestone, project: project, title: '9.10') } @@ -260,6 +260,8 @@ describe SlashCommands::InterpretService, services: true do end shared_examples 'merge command' do + let(:project) { create(:project, :repository) } + it 'runs merge command if content contains /merge' do _, updates = service.execute(content, issuable) @@ -322,6 +324,7 @@ describe SlashCommands::InterpretService, services: true do end context 'when sha is missing' do + let(:project) { create(:project, :repository) } let(:service) { described_class.new(project, developer, {}) } it 'precheck passes and returns merge command' do @@ -694,7 +697,7 @@ describe SlashCommands::InterpretService, services: true do end context '/target_branch command' do - let(:non_empty_project) { create(:project) } + let(:non_empty_project) { create(:project, :repository) } let(:another_merge_request) { create(:merge_request, author: developer, source_project: non_empty_project) } let(:service) { described_class.new(non_empty_project, developer)} diff --git a/spec/services/spam_service_spec.rb b/spec/services/spam_service_spec.rb index e09c05ccf32..74cba8c014b 100644 --- a/spec/services/spam_service_spec.rb +++ b/spec/services/spam_service_spec.rb @@ -15,7 +15,7 @@ describe SpamService, services: true do end context 'when recaptcha was not verified' do - let(:project) { create(:project, :public) } + let(:project) { create(:empty_project, :public) } let(:issue) { create(:issue, project: project) } let(:request) { double(:request, env: {}) } diff --git a/spec/services/system_hooks_service_spec.rb b/spec/services/system_hooks_service_spec.rb index 11037a4917b..667059f230c 100644 --- a/spec/services/system_hooks_service_spec.rb +++ b/spec/services/system_hooks_service_spec.rb @@ -1,13 +1,13 @@ require 'spec_helper' describe SystemHooksService, services: true do - let(:user) { create :user } - let(:project) { create :project } - let(:project_member) { create :project_member } - let(:key) { create(:key, user: user) } - let(:deploy_key) { create(:key) } - let(:group) { create(:group) } - let(:group_member) { create(:group_member) } + let(:user) { create(:user) } + let(:project) { create(:empty_project) } + let(:project_member) { create(:project_member) } + let(:key) { create(:key, user: user) } + let(:deploy_key) { create(:key) } + let(:group) { create(:group) } + let(:group_member) { create(:group_member) } context 'event data' do it { expect(event_data(user, :create)).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id, :username) } diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 36a17a3bf2e..90cde705b85 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -3,17 +3,20 @@ require 'spec_helper' describe SystemNoteService, services: true do include Gitlab::Routing.url_helpers - let(:project) { create(:project) } + let(:project) { create(:empty_project) } let(:author) { create(:user) } let(:noteable) { create(:issue, project: project) } shared_examples_for 'a system note' do + let(:expected_noteable) { noteable } + let(:commit_count) { nil } + it 'is valid' do expect(subject).to be_valid end it 'sets the noteable model' do - expect(subject.noteable).to eq noteable + expect(subject.noteable).to eq expected_noteable end it 'sets the project' do @@ -27,17 +30,34 @@ describe SystemNoteService, services: true do it 'is a system note' do expect(subject).to be_system end + + context 'metadata' do + it 'creates a new system note metadata record' do + expect { subject }.to change{ SystemNoteMetadata.count }.from(0).to(1) + end + + it 'creates a record correctly' do + metadata = subject.system_note_metadata + + expect(metadata.commit_count).to eq(commit_count) + expect(metadata.action).to eq(action) + end + end end describe '.add_commits' do subject { described_class.add_commits(noteable, project, author, new_commits, old_commits, oldrev) } + let(:project) { create(:project, :repository) } let(:noteable) { create(:merge_request, source_project: project) } let(:new_commits) { noteable.commits } let(:old_commits) { [] } let(:oldrev) { nil } - it_behaves_like 'a system note' + it_behaves_like 'a system note' do + let(:commit_count) { new_commits.size } + let(:action) { 'commit' } + end describe 'note body' do let(:note_lines) { subject.note.split("\n").reject(&:blank?) } @@ -116,7 +136,9 @@ describe SystemNoteService, services: true do let(:assignee) { create(:user) } - it_behaves_like 'a system note' + it_behaves_like 'a system note' do + let(:action) { 'assignee' } + end context 'when assignee added' do it 'sets the note text' do @@ -140,7 +162,9 @@ describe SystemNoteService, services: true do let(:added) { [] } let(:removed) { [] } - it_behaves_like 'a system note' + it_behaves_like 'a system note' do + let(:action) { 'label' } + end context 'with added labels' do let(:added) { labels } @@ -175,7 +199,9 @@ describe SystemNoteService, services: true do let(:milestone) { create(:milestone, project: project) } - it_behaves_like 'a system note' + it_behaves_like 'a system note' do + let(:action) { 'milestone' } + end context 'when milestone added' do it 'sets the note text' do @@ -198,7 +224,9 @@ describe SystemNoteService, services: true do let(:status) { 'new_status' } let(:source) { nil } - it_behaves_like 'a system note' + it_behaves_like 'a system note' do + let(:action) { 'status' } + end context 'with a source' do let(:source) { double('commit', gfm_reference: 'commit 123456') } @@ -216,6 +244,7 @@ describe SystemNoteService, services: true do end describe '.merge_when_pipeline_succeeds' do + let(:project) { create(:project, :repository) } let(:pipeline) { build(:ci_pipeline_without_jobs )} let(:noteable) do create(:merge_request, source_project: project, target_project: project) @@ -223,21 +252,26 @@ describe SystemNoteService, services: true do subject { described_class.merge_when_pipeline_succeeds(noteable, project, author, noteable.diff_head_commit) } - it_behaves_like 'a system note' + it_behaves_like 'a system note' do + let(:action) { 'merge' } + end it "posts the 'merge when pipeline succeeds' system note" do - expect(subject.note).to match /enabled an automatic merge when the pipeline for (\w+\/\w+@)?\h{40} succeeds/ + expect(subject.note).to match(/enabled an automatic merge when the pipeline for (\w+\/\w+@)?\h{40} succeeds/) end end describe '.cancel_merge_when_pipeline_succeeds' do + let(:project) { create(:project, :repository) } let(:noteable) do create(:merge_request, source_project: project, target_project: project) end subject { described_class.cancel_merge_when_pipeline_succeeds(noteable, project, author) } - it_behaves_like 'a system note' + it_behaves_like 'a system note' do + let(:action) { 'merge' } + end it "posts the 'merge when pipeline succeeds' system note" do expect(subject.note).to eq "canceled the automatic merge" @@ -250,7 +284,9 @@ describe SystemNoteService, services: true do subject { described_class.change_title(noteable, project, author, 'Old title') } context 'when noteable responds to `title`' do - it_behaves_like 'a system note' + it_behaves_like 'a system note' do + let(:action) { 'title' } + end it 'sets the note text' do expect(subject.note). @@ -263,7 +299,9 @@ describe SystemNoteService, services: true do subject { described_class.change_issue_confidentiality(noteable, project, author) } context 'when noteable responds to `confidential`' do - it_behaves_like 'a system note' + it_behaves_like 'a system note' do + let(:action) { 'confidentiality' } + end it 'sets the note text' do expect(subject.note).to eq 'made the issue visible to everyone' @@ -273,10 +311,14 @@ describe SystemNoteService, services: true do describe '.change_branch' do subject { described_class.change_branch(noteable, project, author, 'target', old_branch, new_branch) } + + let(:project) { create(:project, :repository) } let(:old_branch) { 'old_branch'} let(:new_branch) { 'new_branch'} - it_behaves_like 'a system note' + it_behaves_like 'a system note' do + let(:action) { 'branch' } + end context 'when target branch name changed' do it 'sets the note text' do @@ -288,7 +330,11 @@ describe SystemNoteService, services: true do describe '.change_branch_presence' do subject { described_class.change_branch_presence(noteable, project, author, :source, 'feature', :delete) } - it_behaves_like 'a system note' + let(:project) { create(:project, :repository) } + + it_behaves_like 'a system note' do + let(:action) { 'branch' } + end context 'when source branch deleted' do it 'sets the note text' do @@ -300,11 +346,15 @@ describe SystemNoteService, services: true do describe '.new_issue_branch' do subject { described_class.new_issue_branch(noteable, project, author, "1-mepmep") } - it_behaves_like 'a system note' + let(:project) { create(:project, :repository) } + + it_behaves_like 'a system note' do + let(:action) { 'branch' } + end context 'when a branch is created from the new branch button' do it 'sets the note text' do - expect(subject.note).to match /\Acreated branch [`1-mepmep`]/ + expect(subject.note).to start_with("created branch [`1-mepmep`]") end end end @@ -314,7 +364,9 @@ describe SystemNoteService, services: true do let(:mentioner) { create(:issue, project: project) } - it_behaves_like 'a system note' + it_behaves_like 'a system note' do + let(:action) { 'cross_reference' } + end context 'when cross-reference disallowed' do before do @@ -324,6 +376,10 @@ describe SystemNoteService, services: true do it 'returns nil' do expect(subject).to be_nil end + + it 'does not create a system note metadata record' do + expect { subject }.not_to change{ SystemNoteMetadata.count } + end end context 'when cross-reference allowed' do @@ -331,9 +387,13 @@ describe SystemNoteService, services: true do expect(described_class).to receive(:cross_reference_disallowed?).and_return(false) end + it_behaves_like 'a system note' do + let(:action) { 'cross_reference' } + end + describe 'note_body' do context 'cross-project' do - let(:project2) { create(:project) } + let(:project2) { create(:project, :repository) } let(:mentioner) { create(:issue, project: project2) } context 'from Commit' do @@ -353,6 +413,7 @@ describe SystemNoteService, services: true do context 'within the same project' do context 'from Commit' do + let(:project) { create(:project, :repository) } let(:mentioner) { project.repository.commit } it 'references the mentioning commit' do @@ -394,6 +455,7 @@ describe SystemNoteService, services: true do end context 'when mentioner is a MergeRequest' do + let(:project) { create(:project, :repository) } let(:mentioner) { create(:merge_request, :simple, source_project: project) } let(:noteable) { project.commit } @@ -421,6 +483,7 @@ describe SystemNoteService, services: true do end describe '.cross_reference_exists?' do + let(:project) { create(:project, :repository) } let(:commit0) { project.commit } let(:commit1) { project.commit('HEAD~2') } @@ -513,7 +576,7 @@ describe SystemNoteService, services: true do end describe '.noteable_moved' do - let(:new_project) { create(:project) } + let(:new_project) { create(:empty_project) } let(:new_noteable) { create(:issue, project: new_project) } subject do @@ -540,9 +603,12 @@ describe SystemNoteService, services: true do let(:direction) { :to } it_behaves_like 'cross project mentionable' + it_behaves_like 'a system note' do + let(:action) { 'moved' } + end it 'notifies about noteable being moved to' do - expect(subject.note).to match /moved to/ + expect(subject.note).to match('moved to') end end @@ -550,9 +616,12 @@ describe SystemNoteService, services: true do let(:direction) { :from } it_behaves_like 'cross project mentionable' + it_behaves_like 'a system note' do + let(:action) { 'moved' } + end it 'notifies about noteable being moved from' do - expect(subject.note).to match /moved from/ + expect(subject.note).to match('moved from') end end @@ -574,13 +643,13 @@ describe SystemNoteService, services: true do end end - include JiraServiceHelper - describe 'JIRA integration' do + include JiraServiceHelper + let(:project) { create(:jira_project) } let(:author) { create(:user) } let(:issue) { create(:issue, project: project) } - let(:merge_request) { create(:merge_request, :simple, target_project: project, source_project: project) } + let(:merge_request) { create(:merge_request, :simple, target_project: project, source_project: project) } let(:jira_issue) { ExternalIssue.new("JIRA-1", project)} let(:jira_tracker) { project.jira_service } let(:commit) { project.commit } @@ -720,33 +789,34 @@ describe SystemNoteService, services: true do let(:merge_request) { discussion.noteable } let(:project) { merge_request.source_project } let(:issue) { create(:issue, project: project) } - let(:user) { create(:user) } def reloaded_merge_request MergeRequest.find(merge_request.id) end - before do - project.team << [user, :developer] + subject { described_class.discussion_continued_in_issue(discussion, project, author, issue) } + + it_behaves_like 'a system note' do + let(:expected_noteable) { discussion.first_note.noteable } + let(:action) { 'discussion' } 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 { SystemNoteService.discussion_continued_in_issue(discussion, project, user, issue) }. - to change { reloaded_merge_request.discussions.first.notes.size }.by(1) + expect { subject }.to change { reloaded_merge_request.discussions.first.notes.size }.by(1) end it 'mentions the created issue in the system note' do - note = SystemNoteService.discussion_continued_in_issue(discussion, project, user, issue) - - expect(note.note).to include(issue.to_reference) + expect(subject.note).to include(issue.to_reference) end end describe '.change_time_estimate' do subject { described_class.change_time_estimate(noteable, project, author) } - it_behaves_like 'a system note' + it_behaves_like 'a system note' do + let(:action) { 'time_tracking' } + end context 'with a time estimate' do it 'sets the note text' do @@ -776,7 +846,9 @@ describe SystemNoteService, services: true do described_class.change_time_spent(noteable, project, author) end - it_behaves_like 'a system note' + it_behaves_like 'a system note' do + let(:action) { 'time_tracking' } + end context 'when time was added' do it 'sets the note text' do @@ -808,7 +880,36 @@ describe SystemNoteService, services: true do end end + describe '.remove_merge_request_wip' do + let(:noteable) { create(:issue, project: project, title: 'WIP: Lorem ipsum') } + + subject { described_class.remove_merge_request_wip(noteable, project, author) } + + it_behaves_like 'a system note' do + let(:action) { 'title' } + end + + it 'sets the note text' do + expect(subject.note).to eq 'unmarked as a **Work In Progress**' + end + end + + describe '.add_merge_request_wip' do + let(:noteable) { create(:issue, project: project, title: 'Lorem ipsum') } + + subject { described_class.add_merge_request_wip(noteable, project, author) } + + it_behaves_like 'a system note' do + let(:action) { 'title' } + end + + it 'sets the note text' do + expect(subject.note).to eq 'marked as a **Work In Progress**' + end + end + describe '.add_merge_request_wip_from_commit' do + let(:project) { create(:project, :repository) } let(:noteable) do create(:merge_request, source_project: project, target_project: project) end @@ -822,7 +923,9 @@ describe SystemNoteService, services: true do ) end - it_behaves_like 'a system note' + it_behaves_like 'a system note' do + let(:action) { 'title' } + end it "posts the 'marked as a Work In Progress from commit' system note" do expect(subject.note).to match( @@ -830,4 +933,33 @@ describe SystemNoteService, services: true do ) end end + + describe '.change_task_status' do + let(:noteable) { create(:issue, project: project) } + let(:task) { double(:task, complete?: true, source: 'task') } + + subject { described_class.change_task_status(noteable, project, author, task) } + + it_behaves_like 'a system note' do + let(:action) { 'task' } + end + + it "posts the 'marked as a Work In Progress from commit' system note" do + expect(subject.note).to eq("marked the task **task** as completed") + end + end + + describe '.resolve_all_discussions' do + let(:noteable) { create(:merge_request, source_project: project, target_project: project) } + + subject { described_class.resolve_all_discussions(noteable, project, author) } + + it_behaves_like 'a system note' do + let(:action) { 'discussion' } + end + + it 'sets the note text' do + expect(subject.note).to eq 'resolved all discussions' + end + end end diff --git a/spec/services/tags/create_service_spec.rb b/spec/services/tags/create_service_spec.rb index 5478b8c9ec0..b9121b1de49 100644 --- a/spec/services/tags/create_service_spec.rb +++ b/spec/services/tags/create_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Tags::CreateService, services: true do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:repository) { project.repository } let(:user) { create(:user) } let(:service) { described_class.new(project, user) } diff --git a/spec/services/tags/destroy_service_spec.rb b/spec/services/tags/destroy_service_spec.rb index a388c93379a..28396fc3658 100644 --- a/spec/services/tags/destroy_service_spec.rb +++ b/spec/services/tags/destroy_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Tags::DestroyService, services: true do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:repository) { project.repository } let(:user) { create(:user) } let(:service) { described_class.new(project, user) } diff --git a/spec/services/test_hook_service_spec.rb b/spec/services/test_hook_service_spec.rb index 4f6dd8c6d3f..f99fd8434c2 100644 --- a/spec/services/test_hook_service_spec.rb +++ b/spec/services/test_hook_service_spec.rb @@ -1,9 +1,9 @@ require 'spec_helper' describe TestHookService, services: true do - let(:user) { create :user } - let(:project) { create :project } - let(:hook) { create :project_hook, project: project } + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } + let(:hook) { create(:project_hook, project: project) } describe '#execute' do it "executes successfully" do diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 3645b73b039..89b3b6aad10 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -8,10 +8,12 @@ describe TodoService, services: true do let(:guest) { create(:user) } let(:admin) { create(:admin) } let(:john_doe) { create(:user) } - let(:project) { create(:project) } - let(:mentions) { 'FYI: ' + [author, assignee, john_doe, member, guest, non_member, admin].map(&:to_reference).join(' ') } - let(:directly_addressed) { [author, assignee, john_doe, member, guest, non_member, admin].map(&:to_reference).join(' ') } - let(:directly_addressed_and_mentioned) { member.to_reference + ", what do you think? cc: " + [guest, admin].map(&:to_reference).join(' ') } + let(:skipped) { create(:user) } + let(:skip_users) { [skipped] } + let(:project) { create(:empty_project) } + let(:mentions) { 'FYI: ' + [author, assignee, john_doe, member, guest, non_member, admin, skipped].map(&:to_reference).join(' ') } + let(:directly_addressed) { [author, assignee, john_doe, member, guest, non_member, admin, skipped].map(&:to_reference).join(' ') } + let(:directly_addressed_and_mentioned) { member.to_reference + ", what do you think? cc: " + [guest, admin, skipped].map(&:to_reference).join(' ') } let(:service) { described_class.new } before do @@ -19,6 +21,7 @@ describe TodoService, services: true do project.team << [author, :developer] project.team << [member, :developer] project.team << [john_doe, :developer] + project.team << [skipped, :developer] end describe 'Issues' do @@ -99,9 +102,9 @@ describe TodoService, services: true do end context 'when a private group is mentioned' do - let(:group) { create :group, :private } - let(:project) { create :project, :private, group: group } - let(:issue) { create :issue, author: author, project: project, description: group.to_reference } + let(:group) { create(:group, :private) } + let(:project) { create(:empty_project, :private, group: group) } + let(:issue) { create(:issue, author: author, project: project, description: group.to_reference) } before do group.add_owner(author) @@ -119,46 +122,61 @@ describe TodoService, services: true do end describe '#update_issue' do - it 'creates a todo for each valid mentioned user' do - service.update_issue(issue, author) + it 'creates a todo for each valid mentioned user not included in skip_users' do + service.update_issue(issue, author, skip_users) should_create_todo(user: member, target: issue, action: Todo::MENTIONED) should_create_todo(user: guest, target: issue, action: Todo::MENTIONED) should_create_todo(user: john_doe, target: issue, action: Todo::MENTIONED) should_create_todo(user: author, target: issue, action: Todo::MENTIONED) should_not_create_todo(user: non_member, target: issue, action: Todo::MENTIONED) + should_not_create_todo(user: skipped, target: issue, action: Todo::MENTIONED) end - it 'creates a todo for each valid user based on the type of mention' do + it 'creates a todo for each valid user not included in skip_users based on the type of mention' do issue.update(description: directly_addressed_and_mentioned) - service.update_issue(issue, author) + service.update_issue(issue, author, skip_users) should_create_todo(user: member, target: issue, action: Todo::DIRECTLY_ADDRESSED) should_create_todo(user: guest, target: issue, action: Todo::MENTIONED) should_create_todo(user: admin, target: issue, action: Todo::MENTIONED) + should_not_create_todo(user: skipped, target: issue) end - it 'creates a directly addressed todo for each valid addressed user' do - service.update_issue(addressed_issue, author) + it 'creates a directly addressed todo for each valid addressed user not included in skip_users' do + service.update_issue(addressed_issue, author, skip_users) should_create_todo(user: member, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) should_create_todo(user: guest, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) should_create_todo(user: john_doe, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) should_create_todo(user: author, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) should_not_create_todo(user: non_member, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: skipped, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED) end - it 'does not create a todo if user was already mentioned' do + it 'does not create a todo if user was already mentioned and todo is pending' do create(:todo, :mentioned, user: member, project: project, target: issue, author: author) - expect { service.update_issue(issue, author) }.not_to change(member.todos, :count) + expect { service.update_issue(issue, author, skip_users) }.not_to change(member.todos, :count) + end + + it 'does not create a todo if user was already mentioned and todo is done' do + create(:todo, :mentioned, :done, user: skipped, project: project, target: issue, author: author) + + expect { service.update_issue(issue, author, skip_users) }.not_to change(skipped.todos, :count) end - it 'does not create a directly addressed todo if user was already mentioned or addressed' do + it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is pending' do create(:todo, :directly_addressed, user: member, project: project, target: addressed_issue, author: author) - expect { service.update_issue(addressed_issue, author) }.not_to change(member.todos, :count) + expect { service.update_issue(addressed_issue, author, skip_users) }.not_to change(member.todos, :count) + end + + it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is done' do + create(:todo, :directly_addressed, :done, user: skipped, project: project, target: addressed_issue, author: author) + + expect { service.update_issue(addressed_issue, author, skip_users) }.not_to change(skipped.todos, :count) end it 'does not create todo if user can not see the issue when issue is confidential' do @@ -422,22 +440,26 @@ describe TodoService, services: true do should_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_confidential_issue) end - it 'creates a todo for each valid mentioned user when leaving a note on commit' do - service.new_note(note_on_commit, john_doe) + context 'on commit' do + let(:project) { create(:project, :repository) } - should_create_todo(user: member, target_id: nil, target_type: 'Commit', commit_id: note_on_commit.commit_id, author: john_doe, action: Todo::MENTIONED, note: note_on_commit) - should_create_todo(user: author, target_id: nil, target_type: 'Commit', commit_id: note_on_commit.commit_id, author: john_doe, action: Todo::MENTIONED, note: note_on_commit) - should_create_todo(user: john_doe, target_id: nil, target_type: 'Commit', commit_id: note_on_commit.commit_id, author: john_doe, action: Todo::MENTIONED, note: note_on_commit) - should_not_create_todo(user: non_member, target_id: nil, target_type: 'Commit', commit_id: note_on_commit.commit_id, author: john_doe, action: Todo::MENTIONED, note: note_on_commit) - end + it 'creates a todo for each valid mentioned user when leaving a note on commit' do + service.new_note(note_on_commit, john_doe) + + should_create_todo(user: member, target_id: nil, target_type: 'Commit', commit_id: note_on_commit.commit_id, author: john_doe, action: Todo::MENTIONED, note: note_on_commit) + should_create_todo(user: author, target_id: nil, target_type: 'Commit', commit_id: note_on_commit.commit_id, author: john_doe, action: Todo::MENTIONED, note: note_on_commit) + should_create_todo(user: john_doe, target_id: nil, target_type: 'Commit', commit_id: note_on_commit.commit_id, author: john_doe, action: Todo::MENTIONED, note: note_on_commit) + should_not_create_todo(user: non_member, target_id: nil, target_type: 'Commit', commit_id: note_on_commit.commit_id, author: john_doe, action: Todo::MENTIONED, note: note_on_commit) + end - it 'creates a directly addressed todo for each valid mentioned user when leaving a note on commit' do - service.new_note(addressed_note_on_commit, john_doe) + it 'creates a directly addressed todo for each valid mentioned user when leaving a note on commit' do + service.new_note(addressed_note_on_commit, john_doe) - should_create_todo(user: member, target_id: nil, target_type: 'Commit', commit_id: addressed_note_on_commit.commit_id, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_commit) - should_create_todo(user: author, target_id: nil, target_type: 'Commit', commit_id: addressed_note_on_commit.commit_id, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_commit) - should_create_todo(user: john_doe, target_id: nil, target_type: 'Commit', commit_id: addressed_note_on_commit.commit_id, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_commit) - should_not_create_todo(user: non_member, target_id: nil, target_type: 'Commit', commit_id: addressed_note_on_commit.commit_id, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_commit) + should_create_todo(user: member, target_id: nil, target_type: 'Commit', commit_id: addressed_note_on_commit.commit_id, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_commit) + should_create_todo(user: author, target_id: nil, target_type: 'Commit', commit_id: addressed_note_on_commit.commit_id, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_commit) + should_create_todo(user: john_doe, target_id: nil, target_type: 'Commit', commit_id: addressed_note_on_commit.commit_id, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_commit) + should_not_create_todo(user: non_member, target_id: nil, target_type: 'Commit', commit_id: addressed_note_on_commit.commit_id, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_commit) + end end it 'does not create todo when leaving a note on snippet' do @@ -517,47 +539,62 @@ describe TodoService, services: true do end describe '#update_merge_request' do - it 'creates a todo for each valid mentioned user' do - service.update_merge_request(mr_assigned, author) + it 'creates a todo for each valid mentioned user not included in skip_users' do + service.update_merge_request(mr_assigned, author, skip_users) should_create_todo(user: member, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED) should_create_todo(user: john_doe, target: mr_assigned, action: Todo::MENTIONED) should_create_todo(user: author, target: mr_assigned, action: Todo::MENTIONED) should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED) + should_not_create_todo(user: skipped, target: mr_assigned, action: Todo::MENTIONED) end - it 'creates a todo for each valid user based on the type of mention' do + it 'creates a todo for each valid user not included in skip_users based on the type of mention' do mr_assigned.update(description: directly_addressed_and_mentioned) - service.update_merge_request(mr_assigned, author) + service.update_merge_request(mr_assigned, author, skip_users) should_create_todo(user: member, target: mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_create_todo(user: admin, target: mr_assigned, action: Todo::MENTIONED) + should_not_create_todo(user: skipped, target: mr_assigned) end - it 'creates a directly addressed todo for each valid addressed user' do - service.update_merge_request(addressed_mr_assigned, author) + it 'creates a directly addressed todo for each valid addressed user not included in skip_users' do + service.update_merge_request(addressed_mr_assigned, author, skip_users) should_create_todo(user: member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_not_create_todo(user: guest, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_create_todo(user: john_doe, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_create_todo(user: author, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) should_not_create_todo(user: non_member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: skipped, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED) end - it 'does not create a todo if user was already mentioned' do + it 'does not create a todo if user was already mentioned and todo is pending' do create(:todo, :mentioned, user: member, project: project, target: mr_assigned, author: author) expect { service.update_merge_request(mr_assigned, author) }.not_to change(member.todos, :count) end - it 'does not create a directly addressed todo if user was already mentioned or addressed' do + it 'does not create a todo if user was already mentioned and todo is done' do + create(:todo, :mentioned, :done, user: skipped, project: project, target: mr_assigned, author: author) + + expect { service.update_merge_request(mr_assigned, author, skip_users) }.not_to change(skipped.todos, :count) + end + + it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is pending' do create(:todo, :directly_addressed, user: member, project: project, target: addressed_mr_assigned, author: author) expect{ service.update_merge_request(addressed_mr_assigned, author) }.not_to change(member.todos, :count) end + it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is done' do + create(:todo, :directly_addressed, user: skipped, project: project, target: addressed_mr_assigned, author: author) + + expect{ service.update_merge_request(addressed_mr_assigned, author, skip_users) }.not_to change(skipped.todos, :count) + end + context 'with a task list' do it 'does not create todo when tasks are marked as completed' do mr_assigned.update(description: "- [x] Task 1\n- [X] Task 2 #{mentions}") @@ -720,6 +757,7 @@ describe TodoService, services: true do end describe '#new_note' do + let(:project) { create(:project, :repository) } let(:mention) { john_doe.to_reference } let(:diff_note_on_merge_request) { create(:diff_note_on_merge_request, project: project, noteable: mr_unassigned, author: author, note: "Hey #{mention}") } let(:addressed_diff_note_on_merge_request) { create(:diff_note_on_merge_request, project: project, noteable: mr_unassigned, author: author, note: "#{mention}, hey!") } @@ -752,6 +790,69 @@ describe TodoService, services: true do end end + describe '#update_note' do + let(:noteable) { create(:issue, project: project) } + let(:note) { create(:note, project: project, note: mentions, noteable: noteable) } + let(:addressed_note) { create(:note, project: project, note: "#{directly_addressed}", noteable: noteable) } + + it 'creates a todo for each valid mentioned user not included in skip_users' do + service.update_note(note, author, skip_users) + + should_create_todo(user: member, target: noteable, action: Todo::MENTIONED) + should_create_todo(user: guest, target: noteable, action: Todo::MENTIONED) + should_create_todo(user: john_doe, target: noteable, action: Todo::MENTIONED) + should_create_todo(user: author, target: noteable, action: Todo::MENTIONED) + should_not_create_todo(user: non_member, target: noteable, action: Todo::MENTIONED) + should_not_create_todo(user: skipped, target: noteable, action: Todo::MENTIONED) + end + + it 'creates a todo for each valid user not included in skip_users based on the type of mention' do + note.update(note: directly_addressed_and_mentioned) + + service.update_note(note, author, skip_users) + + should_create_todo(user: member, target: noteable, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: guest, target: noteable, action: Todo::MENTIONED) + should_create_todo(user: admin, target: noteable, action: Todo::MENTIONED) + should_not_create_todo(user: skipped, target: noteable) + end + + it 'creates a directly addressed todo for each valid addressed user not included in skip_users' do + service.update_note(addressed_note, author, skip_users) + + should_create_todo(user: member, target: noteable, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: guest, target: noteable, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: john_doe, target: noteable, action: Todo::DIRECTLY_ADDRESSED) + should_create_todo(user: author, target: noteable, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: non_member, target: noteable, action: Todo::DIRECTLY_ADDRESSED) + should_not_create_todo(user: skipped, target: noteable, action: Todo::DIRECTLY_ADDRESSED) + end + + it 'does not create a todo if user was already mentioned and todo is pending' do + create(:todo, :mentioned, user: member, project: project, target: noteable, author: author) + + expect { service.update_note(note, author, skip_users) }.not_to change(member.todos, :count) + end + + it 'does not create a todo if user was already mentioned and todo is done' do + create(:todo, :mentioned, :done, user: skipped, project: project, target: noteable, author: author) + + expect { service.update_note(note, author, skip_users) }.not_to change(skipped.todos, :count) + end + + it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is pending' do + create(:todo, :directly_addressed, user: member, project: project, target: noteable, author: author) + + expect { service.update_note(addressed_note, author, skip_users) }.not_to change(member.todos, :count) + end + + it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is done' do + create(:todo, :directly_addressed, :done, user: skipped, project: project, target: noteable, author: author) + + expect { service.update_note(addressed_note, author, skip_users) }.not_to change(skipped.todos, :count) + end + end + it 'updates cached counts when a todo is created' do issue = create(:issue, project: project, assignee: john_doe, author: author, description: mentions) diff --git a/spec/services/update_release_service_spec.rb b/spec/services/update_release_service_spec.rb index bba211089a8..69ed8de9c31 100644 --- a/spec/services/update_release_service_spec.rb +++ b/spec/services/update_release_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe UpdateReleaseService, services: true do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:tag_name) { project.repository.tag_names.first } let(:description) { 'Awesome release!' } diff --git a/spec/services/users/create_service_spec.rb b/spec/services/users/create_service_spec.rb index 5f79203701a..66f68650f81 100644 --- a/spec/services/users/create_service_spec.rb +++ b/spec/services/users/create_service_spec.rb @@ -61,6 +61,23 @@ describe Users::CreateService, services: true do ) end + context 'when the current_user is not persisted' do + let(:admin_user) { build(:admin) } + + it 'persists the given attributes and sets created_by_id to nil' do + user = service.execute + user.reload + + expect(user).to have_attributes( + name: params[:name], + username: params[:username], + email: params[:email], + password: params[:password], + created_by_id: nil + ) + end + end + it 'user is not confirmed if skip_confirmation param is not present' do expect(service.execute).not_to be_confirmed end diff --git a/spec/services/users/destroy_spec.rb b/spec/services/users/destroy_spec.rb index 922e82445d0..9a28c03d968 100644 --- a/spec/services/users/destroy_spec.rb +++ b/spec/services/users/destroy_spec.rb @@ -5,7 +5,7 @@ describe Users::DestroyService, services: true do let!(:user) { create(:user) } let!(:admin) { create(:admin) } let!(:namespace) { create(:namespace, owner: user) } - let!(:project) { create(:project, namespace: namespace) } + let!(:project) { create(:empty_project, namespace: namespace) } let(:service) { described_class.new(admin) } context 'no options are given' do @@ -25,7 +25,7 @@ describe Users::DestroyService, services: true do end context "a deleted user's issues" do - let(:project) { create :project } + let(:project) { create(:project) } before do project.add_developer(user) diff --git a/spec/services/users/refresh_authorized_projects_service_spec.rb b/spec/services/users/refresh_authorized_projects_service_spec.rb index 08733d6dcf1..b19374ef1a2 100644 --- a/spec/services/users/refresh_authorized_projects_service_spec.rb +++ b/spec/services/users/refresh_authorized_projects_service_spec.rb @@ -152,7 +152,7 @@ describe Users::RefreshAuthorizedProjectsService do context 'projects of groups the user is a member of' do let(:group) { create(:group) } - let!(:other_project) { create(:project, group: group) } + let!(:other_project) { create(:empty_project, group: group) } before do group.add_owner(user) @@ -166,7 +166,7 @@ describe Users::RefreshAuthorizedProjectsService do context 'projects of subgroups of groups the user is a member of' do let(:group) { create(:group) } let(:nested_group) { create(:group, parent: group) } - let!(:other_project) { create(:project, group: nested_group) } + let!(:other_project) { create(:empty_project, group: nested_group) } before do group.add_master(user) |