diff options
| author | Grzegorz Bizon <grzesiek.bizon@gmail.com> | 2017-04-03 09:36:09 +0200 |
|---|---|---|
| committer | Grzegorz Bizon <grzesiek.bizon@gmail.com> | 2017-04-03 09:36:09 +0200 |
| commit | fc550b398be6cdc4584dad79411929305815ffaa (patch) | |
| tree | 0995b6990fa3599dddfed5bd2720589731bcdf31 /spec/services | |
| parent | 83d1fe9b5aeb947c1387666205ecaca81f2bf3a2 (diff) | |
| parent | e7e9307219d1c81427f95444b36471c519dc06c2 (diff) | |
| download | gitlab-ce-fc550b398be6cdc4584dad79411929305815ffaa.tar.gz | |
Merge branch 'master' into feature/multi-level-container-registry-images
* master: (230 commits)
Fix N+1 query in loading pipelines in merge requests
Fix Spinach and Capybara dependencies
Prevent users from disconnecting gitlab account from CAS
30276 Move issue, mr, todos next to profile dropdown in top nav
Refactor SearchController#show
Properly eagerly-load the Capybara server for JS feature specs only
Updating documentation to include a missing step in the update procedure
Eager-load the Capybara server to prevent timeouts
Increase Capybara's timeout
Add metrics button to Environment Overview page
Fix link to Jira service documentation
Handle parsing OpenBSD ps output properly to display sidekiq infos on ...
Eliminate unnecessary queries that add ~500 ms of load time for a large issue
20914 Limits line length for project home page
Allow users to import GitHub projects to subgroups
Update dpl CI example
Fix the docs:check:links job
Don't clean up the gitlab-test-fork_bare repo
Make GitLab use Gitaly for commit_is_ancestor
Remove unnecessary ORDER BY clause from `forked_to_project_id` subquery
...
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) |