summaryrefslogtreecommitdiff
path: root/spec/services
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2022-05-19 07:33:21 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2022-05-19 07:33:21 +0000
commit36a59d088eca61b834191dacea009677a96c052f (patch)
treee4f33972dab5d8ef79e3944a9f403035fceea43f /spec/services
parenta1761f15ec2cae7c7f7bbda39a75494add0dfd6f (diff)
downloadgitlab-ce-36a59d088eca61b834191dacea009677a96c052f.tar.gz
Add latest changes from gitlab-org/gitlab@15-0-stable-eev15.0.0-rc42
Diffstat (limited to 'spec/services')
-rw-r--r--spec/services/alert_management/alerts/update_service_spec.rb45
-rw-r--r--spec/services/audit_event_service_spec.rb12
-rw-r--r--spec/services/authorized_project_update/project_create_service_spec.rb185
-rw-r--r--spec/services/authorized_project_update/project_group_link_create_service_spec.rb222
-rw-r--r--spec/services/ci/create_downstream_pipeline_service_spec.rb118
-rw-r--r--spec/services/ci/create_pipeline_service_spec.rb2
-rw-r--r--spec/services/ci/generate_kubeconfig_service_spec.rb5
-rw-r--r--spec/services/ci/job_artifacts/create_service_spec.rb11
-rw-r--r--spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb2
-rw-r--r--spec/services/ci/pipeline_creation/start_pipeline_service_spec.rb6
-rw-r--r--spec/services/ci/pipeline_trigger_service_spec.rb8
-rw-r--r--spec/services/ci/retry_job_service_spec.rb442
-rw-r--r--spec/services/clusters/agents/delete_service_spec.rb2
-rw-r--r--spec/services/clusters/kubernetes/create_or_update_namespace_service_spec.rb2
-rw-r--r--spec/services/clusters/kubernetes/create_or_update_service_account_service_spec.rb43
-rw-r--r--spec/services/container_expiration_policies/cleanup_service_spec.rb6
-rw-r--r--spec/services/container_expiration_policies/update_service_spec.rb4
-rw-r--r--spec/services/container_expiration_policy_service_spec.rb32
-rw-r--r--spec/services/customer_relations/contacts/create_service_spec.rb2
-rw-r--r--spec/services/customer_relations/contacts/update_service_spec.rb27
-rw-r--r--spec/services/customer_relations/organizations/update_service_spec.rb25
-rw-r--r--spec/services/database/consistency_fix_service_spec.rb153
-rw-r--r--spec/services/dependency_proxy/group_settings/update_service_spec.rb2
-rw-r--r--spec/services/dependency_proxy/image_ttl_group_policies/update_service_spec.rb6
-rw-r--r--spec/services/environments/stop_service_spec.rb29
-rw-r--r--spec/services/error_tracking/base_service_spec.rb11
-rw-r--r--spec/services/error_tracking/collect_error_service_spec.rb15
-rw-r--r--spec/services/error_tracking/issue_details_service_spec.rb16
-rw-r--r--spec/services/error_tracking/issue_latest_event_service_spec.rb16
-rw-r--r--spec/services/error_tracking/issue_update_service_spec.rb21
-rw-r--r--spec/services/groups/group_links/create_service_spec.rb210
-rw-r--r--spec/services/groups/group_links/destroy_service_spec.rb85
-rw-r--r--spec/services/groups/open_issues_count_service_spec.rb64
-rw-r--r--spec/services/groups/transfer_service_spec.rb2
-rw-r--r--spec/services/import/bitbucket_server_service_spec.rb17
-rw-r--r--spec/services/import/github_service_spec.rb27
-rw-r--r--spec/services/incident_management/timeline_events/create_service_spec.rb117
-rw-r--r--spec/services/incident_management/timeline_events/destroy_service_spec.rb80
-rw-r--r--spec/services/incident_management/timeline_events/update_service_spec.rb148
-rw-r--r--spec/services/issues/close_service_spec.rb2
-rw-r--r--spec/services/issues/create_service_spec.rb8
-rw-r--r--spec/services/issues/set_crm_contacts_service_spec.rb16
-rw-r--r--spec/services/jira_connect/sync_service_spec.rb14
-rw-r--r--spec/services/keys/expiry_notification_service_spec.rb2
-rw-r--r--spec/services/loose_foreign_keys/batch_cleaner_service_spec.rb10
-rw-r--r--spec/services/members/create_service_spec.rb32
-rw-r--r--spec/services/members/groups/creator_service_spec.rb24
-rw-r--r--spec/services/members/invite_service_spec.rb70
-rw-r--r--spec/services/members/projects/creator_service_spec.rb24
-rw-r--r--spec/services/merge_requests/approval_service_spec.rb15
-rw-r--r--spec/services/merge_requests/close_service_spec.rb2
-rw-r--r--spec/services/merge_requests/handle_assignees_change_service_spec.rb8
-rw-r--r--spec/services/merge_requests/merge_service_spec.rb2
-rw-r--r--spec/services/merge_requests/post_merge_service_spec.rb78
-rw-r--r--spec/services/merge_requests/push_options_handler_service_spec.rb98
-rw-r--r--spec/services/merge_requests/rebase_service_spec.rb12
-rw-r--r--spec/services/merge_requests/remove_approval_service_spec.rb8
-rw-r--r--spec/services/merge_requests/remove_attention_requested_service_spec.rb135
-rw-r--r--spec/services/merge_requests/request_attention_service_spec.rb220
-rw-r--r--spec/services/merge_requests/squash_service_spec.rb12
-rw-r--r--spec/services/merge_requests/toggle_attention_requested_service_spec.rb4
-rw-r--r--spec/services/merge_requests/update_assignees_service_spec.rb43
-rw-r--r--spec/services/merge_requests/update_service_spec.rb90
-rw-r--r--spec/services/namespaces/package_settings/update_service_spec.rb4
-rw-r--r--spec/services/notes/create_service_spec.rb34
-rw-r--r--spec/services/notification_service_spec.rb75
-rw-r--r--spec/services/projects/after_import_service_spec.rb131
-rw-r--r--spec/services/projects/android_target_platform_detector_service_spec.rb30
-rw-r--r--spec/services/projects/batch_open_issues_count_service_spec.rb34
-rw-r--r--spec/services/projects/blame_service_spec.rb129
-rw-r--r--spec/services/projects/container_repository/cleanup_tags_service_spec.rb31
-rw-r--r--spec/services/projects/container_repository/delete_tags_service_spec.rb51
-rw-r--r--spec/services/projects/container_repository/gitlab/delete_tags_service_spec.rb77
-rw-r--r--spec/services/projects/create_service_spec.rb53
-rw-r--r--spec/services/projects/group_links/create_service_spec.rb121
-rw-r--r--spec/services/projects/in_product_marketing_campaign_emails_service_spec.rb130
-rw-r--r--spec/services/projects/open_issues_count_service_spec.rb109
-rw-r--r--spec/services/projects/prometheus/alerts/create_service_spec.rb52
-rw-r--r--spec/services/projects/prometheus/alerts/destroy_service_spec.rb21
-rw-r--r--spec/services/projects/prometheus/alerts/update_service_spec.rb53
-rw-r--r--spec/services/projects/prometheus/metrics/destroy_service_spec.rb13
-rw-r--r--spec/services/projects/prometheus/metrics/update_service_spec.rb44
-rw-r--r--spec/services/projects/record_target_platforms_service_spec.rb104
-rw-r--r--spec/services/projects/update_pages_service_spec.rb12
-rw-r--r--spec/services/prometheus/create_default_alerts_service_spec.rb92
-rw-r--r--spec/services/quick_actions/interpret_service_spec.rb15
-rw-r--r--spec/services/service_ping/build_payload_service_spec.rb47
-rw-r--r--spec/services/service_ping/permit_data_categories_service_spec.rb42
-rw-r--r--spec/services/service_ping/service_ping_settings_spec.rb46
-rw-r--r--spec/services/service_ping/submit_service_ping_service_spec.rb110
-rw-r--r--spec/services/system_note_service_spec.rb47
-rw-r--r--spec/services/system_notes/incidents_service_spec.rb88
-rw-r--r--spec/services/system_notes/time_tracking_service_spec.rb24
-rw-r--r--spec/services/timelogs/delete_service_spec.rb65
-rw-r--r--spec/services/users/destroy_service_spec.rb41
-rw-r--r--spec/services/users/in_product_marketing_email_records_spec.rb (renamed from spec/services/namespaces/in_product_marketing_email_records_spec.rb)25
-rw-r--r--spec/services/users/validate_manual_otp_service_spec.rb (renamed from spec/services/users/validate_otp_service_spec.rb)27
-rw-r--r--spec/services/users/validate_push_otp_service_spec.rb45
-rw-r--r--spec/services/work_items/delete_task_service_spec.rb88
-rw-r--r--spec/services/work_items/task_list_reference_removal_service_spec.rb151
100 files changed, 3372 insertions, 2038 deletions
diff --git a/spec/services/alert_management/alerts/update_service_spec.rb b/spec/services/alert_management/alerts/update_service_spec.rb
index 882543fd701..f02607b8174 100644
--- a/spec/services/alert_management/alerts/update_service_spec.rb
+++ b/spec/services/alert_management/alerts/update_service_spec.rb
@@ -88,7 +88,7 @@ RSpec.describe AlertManagement::Alerts::UpdateService do
it_behaves_like 'title update'
end
- context 'when alert is resolved and another existing open alert' do
+ context 'when alert is resolved and another existing unresolved alert' do
let!(:alert) { create(:alert_management_alert, :resolved, project: project) }
let!(:existing_alert) { create(:alert_management_alert, :triggered, project: project) }
@@ -193,27 +193,38 @@ RSpec.describe AlertManagement::Alerts::UpdateService do
end
end
- context 'with an opening status and existing open alert' do
- let_it_be(:alert) { create(:alert_management_alert, :resolved, :with_fingerprint, project: project) }
- let_it_be(:existing_alert) { create(:alert_management_alert, :triggered, fingerprint: alert.fingerprint, project: project) }
- let_it_be(:url) { Gitlab::Routing.url_helpers.details_project_alert_management_path(project, existing_alert) }
- let_it_be(:link) { ActionController::Base.helpers.link_to(_('alert'), url) }
+ context 'with existing unresolved alert' do
+ context 'with fingerprints' do
+ let_it_be(:existing_alert) { create(:alert_management_alert, :triggered, fingerprint: alert.fingerprint, project: project) }
- let(:message) do
- "An #{link} with the same fingerprint is already open. " \
- 'To change the status of this alert, resolve the linked alert.'
- end
+ it 'does not query for existing alerts' do
+ expect(::AlertManagement::Alert).not_to receive(:find_unresolved_alert)
- it_behaves_like 'does not add a todo'
- it_behaves_like 'does not add a system note'
+ response
+ end
- it 'has an informative message' do
- expect(response).to be_error
- expect(response.message).to eq(message)
+ context 'when status was resolved' do
+ let_it_be(:alert) { create(:alert_management_alert, :resolved, :with_fingerprint, project: project) }
+ let_it_be(:existing_alert) { create(:alert_management_alert, :triggered, fingerprint: alert.fingerprint, project: project) }
+
+ let(:url) { Gitlab::Routing.url_helpers.details_project_alert_management_path(project, existing_alert) }
+ let(:link) { ActionController::Base.helpers.link_to(_('alert'), url) }
+ let(:message) do
+ "An #{link} with the same fingerprint is already open. " \
+ 'To change the status of this alert, resolve the linked alert.'
+ end
+
+ it_behaves_like 'does not add a todo'
+ it_behaves_like 'does not add a system note'
+
+ it 'has an informative message' do
+ expect(response).to be_error
+ expect(response.message).to eq(message)
+ end
+ end
end
- context 'fingerprints are blank' do
- let_it_be(:alert) { create(:alert_management_alert, :resolved, project: project, fingerprint: nil) }
+ context 'without fingerprints' do
let_it_be(:existing_alert) { create(:alert_management_alert, :triggered, fingerprint: alert.fingerprint, project: project) }
it 'successfully changes the status' do
diff --git a/spec/services/audit_event_service_spec.rb b/spec/services/audit_event_service_spec.rb
index 6963515ba5c..063d250f22b 100644
--- a/spec/services/audit_event_service_spec.rb
+++ b/spec/services/audit_event_service_spec.rb
@@ -76,11 +76,13 @@ RSpec.describe AuditEventService do
it 'creates an authentication event' do
expect(AuthenticationEvent).to receive(:new).with(
- user: user,
- user_name: user.name,
- ip_address: user.current_sign_in_ip,
- result: AuthenticationEvent.results[:success],
- provider: 'standard'
+ {
+ user: user,
+ user_name: user.name,
+ ip_address: user.current_sign_in_ip,
+ result: AuthenticationEvent.results[:success],
+ provider: 'standard'
+ }
).and_call_original
audit_service.for_authentication.security_event
diff --git a/spec/services/authorized_project_update/project_create_service_spec.rb b/spec/services/authorized_project_update/project_create_service_spec.rb
deleted file mode 100644
index a9d0b82acfb..00000000000
--- a/spec/services/authorized_project_update/project_create_service_spec.rb
+++ /dev/null
@@ -1,185 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe AuthorizedProjectUpdate::ProjectCreateService do
- let_it_be(:group_parent) { create(:group, :private) }
- let_it_be(:group) { create(:group, :private, parent: group_parent) }
- let_it_be(:group_child) { create(:group, :private, parent: group) }
-
- let_it_be(:group_project) { create(:project, group: group) }
-
- let_it_be(:parent_group_user) { create(:user) }
- let_it_be(:group_user) { create(:user) }
- let_it_be(:child_group_user) { create(:user) }
-
- let(:access_level) { Gitlab::Access::MAINTAINER }
-
- subject(:service) { described_class.new(group_project) }
-
- describe '#perform' do
- context 'direct group members' do
- before do
- create(:group_member, access_level: access_level, group: group, user: group_user)
- ProjectAuthorization.delete_all
- end
-
- it 'creates project authorization' do
- expect { service.execute }.to(
- change { ProjectAuthorization.count }.from(0).to(1))
-
- project_authorization = ProjectAuthorization.where(
- project_id: group_project.id,
- user_id: group_user.id,
- access_level: access_level)
-
- expect(project_authorization).to exist
- end
- end
-
- context 'inherited group members' do
- before do
- create(:group_member, access_level: access_level, group: group_parent, user: parent_group_user)
- ProjectAuthorization.delete_all
- end
-
- it 'creates project authorization' do
- expect { service.execute }.to(
- change { ProjectAuthorization.count }.from(0).to(1))
-
- project_authorization = ProjectAuthorization.where(
- project_id: group_project.id,
- user_id: parent_group_user.id,
- access_level: access_level)
- expect(project_authorization).to exist
- end
- end
-
- context 'membership overrides' do
- context 'group hierarchy' do
- before do
- create(:group_member, access_level: Gitlab::Access::REPORTER, group: group_parent, user: group_user)
- create(:group_member, access_level: Gitlab::Access::DEVELOPER, group: group, user: group_user)
- ProjectAuthorization.delete_all
- end
-
- it 'creates project authorization' do
- expect { service.execute }.to(
- change { ProjectAuthorization.count }.from(0).to(1))
-
- project_authorization = ProjectAuthorization.where(
- project_id: group_project.id,
- user_id: group_user.id,
- access_level: Gitlab::Access::DEVELOPER)
- expect(project_authorization).to exist
- end
- end
-
- context 'group sharing' do
- let!(:shared_with_group) { create(:group) }
-
- before do
- create(:group_member, access_level: Gitlab::Access::REPORTER, group: group, user: group_user)
- create(:group_member, access_level: Gitlab::Access::MAINTAINER, group: shared_with_group, user: group_user)
- create(:group_member, :minimal_access, source: shared_with_group, user: create(:user))
-
- create(:group_group_link, shared_group: group, shared_with_group: shared_with_group, group_access: Gitlab::Access::DEVELOPER)
-
- ProjectAuthorization.delete_all
- end
-
- it 'creates project authorization' do
- expect { service.execute }.to(
- change { ProjectAuthorization.count }.from(0).to(1))
-
- project_authorization = ProjectAuthorization.where(
- project_id: group_project.id,
- user_id: group_user.id,
- access_level: Gitlab::Access::DEVELOPER)
- expect(project_authorization).to exist
- end
-
- it 'does not create project authorization for user with minimal access' do
- expect { service.execute }.to(
- change { ProjectAuthorization.count }.from(0).to(1))
- end
- end
- end
-
- context 'no group member' do
- it 'does not create project authorization' do
- expect { service.execute }.not_to(
- change { ProjectAuthorization.count }.from(0))
- end
- end
-
- context 'unapproved access requests' do
- before do
- create(:group_member, :guest, :access_request, user: group_user, group: group)
- end
-
- it 'does not create project authorization' do
- expect { service.execute }.not_to(
- change { ProjectAuthorization.count }.from(0))
- end
- end
-
- context 'member with minimal access' do
- before do
- create(:group_member, :minimal_access, user: group_user, source: group)
- end
-
- it 'does not create project authorization' do
- expect { service.execute }.not_to(
- change { ProjectAuthorization.count }.from(0))
- end
- end
-
- context 'project has more user than BATCH_SIZE' do
- let(:batch_size) { 2 }
- let(:users) { create_list(:user, batch_size + 1 ) }
-
- before do
- stub_const("#{described_class.name}::BATCH_SIZE", batch_size)
-
- users.each do |user|
- create(:group_member, access_level: access_level, group: group_parent, user: user)
- end
-
- ProjectAuthorization.delete_all
- end
-
- it 'bulk creates project authorizations in batches' do
- users.each_slice(batch_size) do |batch|
- attributes = batch.map do |user|
- { user_id: user.id, project_id: group_project.id, access_level: access_level }
- end
-
- expect(ProjectAuthorization).to(
- receive(:insert_all).with(array_including(attributes)).and_call_original)
- end
-
- expect { service.execute }.to(
- change { ProjectAuthorization.count }.from(0).to(batch_size + 1))
- end
- end
-
- context 'ignores existing project authorizations' do
- before do
- # ProjectAuthorizations is also created because of an after_commit
- # callback on Member model
- create(:group_member, access_level: access_level, group: group, user: group_user)
- end
-
- it 'does not create project authorization' do
- project_authorization = ProjectAuthorization.where(
- project_id: group_project.id,
- user_id: group_user.id,
- access_level: access_level)
-
- expect { service.execute }.not_to(
- change { project_authorization.reload.exists? }.from(true))
- end
- end
- end
-end
diff --git a/spec/services/authorized_project_update/project_group_link_create_service_spec.rb b/spec/services/authorized_project_update/project_group_link_create_service_spec.rb
deleted file mode 100644
index 1fd47f78c24..00000000000
--- a/spec/services/authorized_project_update/project_group_link_create_service_spec.rb
+++ /dev/null
@@ -1,222 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe AuthorizedProjectUpdate::ProjectGroupLinkCreateService do
- let_it_be(:group_parent) { create(:group, :private) }
- let_it_be(:group) { create(:group, :private, parent: group_parent) }
- let_it_be(:group_child) { create(:group, :private, parent: group) }
-
- let_it_be(:parent_group_user) { create(:user) }
- let_it_be(:group_user) { create(:user) }
-
- let_it_be(:project) { create(:project, :private, group: create(:group, :private)) }
-
- let(:access_level) { Gitlab::Access::MAINTAINER }
- let(:group_access) { nil }
-
- subject(:service) { described_class.new(project, group, group_access) }
-
- describe '#perform' do
- context 'direct group members' do
- before do
- create(:group_member, access_level: access_level, group: group, user: group_user)
- ProjectAuthorization.delete_all
- end
-
- it 'creates project authorization' do
- expect { service.execute }.to(
- change { ProjectAuthorization.count }.from(0).to(1))
-
- project_authorization = ProjectAuthorization.where(
- project_id: project.id,
- user_id: group_user.id,
- access_level: access_level)
-
- expect(project_authorization).to exist
- end
- end
-
- context 'inherited group members' do
- before do
- create(:group_member, access_level: access_level, group: group_parent, user: parent_group_user)
- ProjectAuthorization.delete_all
- end
-
- it 'creates project authorization' do
- expect { service.execute }.to(
- change { ProjectAuthorization.count }.from(0).to(1))
-
- project_authorization = ProjectAuthorization.where(
- project_id: project.id,
- user_id: parent_group_user.id,
- access_level: access_level)
- expect(project_authorization).to exist
- end
- end
-
- context 'with group_access' do
- let(:group_access) { Gitlab::Access::REPORTER }
-
- before do
- create(:group_member, access_level: access_level, group: group_parent, user: parent_group_user)
- ProjectAuthorization.delete_all
- end
-
- it 'creates project authorization' do
- expect { service.execute }.to(
- change { ProjectAuthorization.count }.from(0).to(1))
-
- project_authorization = ProjectAuthorization.where(
- project_id: project.id,
- user_id: parent_group_user.id,
- access_level: group_access)
- expect(project_authorization).to exist
- end
- end
-
- context 'membership overrides' do
- before do
- create(:group_member, access_level: Gitlab::Access::REPORTER, group: group_parent, user: group_user)
- create(:group_member, access_level: Gitlab::Access::DEVELOPER, group: group, user: group_user)
- ProjectAuthorization.delete_all
- end
-
- it 'creates project authorization' do
- expect { service.execute }.to(
- change { ProjectAuthorization.count }.from(0).to(1))
-
- project_authorization = ProjectAuthorization.where(
- project_id: project.id,
- user_id: group_user.id,
- access_level: Gitlab::Access::DEVELOPER)
- expect(project_authorization).to exist
- end
- end
-
- context 'no group member' do
- it 'does not create project authorization' do
- expect { service.execute }.not_to(
- change { ProjectAuthorization.count }.from(0))
- end
- end
-
- context 'unapproved access requests' do
- before do
- create(:group_member, :guest, :access_request, user: group_user, group: group)
- end
-
- it 'does not create project authorization' do
- expect { service.execute }.not_to(
- change { ProjectAuthorization.count }.from(0))
- end
- end
-
- context 'minimal access member' do
- before do
- create(:group_member, :minimal_access, user: group_user, source: group)
- end
-
- it 'does not create project authorization' do
- expect { service.execute }.not_to(
- change { ProjectAuthorization.count }.from(0))
- end
- end
-
- context 'project has more users than BATCH_SIZE' do
- let(:batch_size) { 2 }
- let(:users) { create_list(:user, batch_size + 1 ) }
-
- before do
- stub_const("#{described_class.name}::BATCH_SIZE", batch_size)
-
- users.each do |user|
- create(:group_member, access_level: access_level, group: group_parent, user: user)
- end
-
- ProjectAuthorization.delete_all
- end
-
- it 'bulk creates project authorizations in batches' do
- users.each_slice(batch_size) do |batch|
- attributes = batch.map do |user|
- { user_id: user.id, project_id: project.id, access_level: access_level }
- end
-
- expect(ProjectAuthorization).to(
- receive(:insert_all).with(array_including(attributes)).and_call_original)
- end
-
- expect { service.execute }.to(
- change { ProjectAuthorization.count }.from(0).to(batch_size + 1))
- end
- end
-
- context 'users have existing project authorizations' do
- before do
- create(:group_member, access_level: access_level, group: group, user: group_user)
- ProjectAuthorization.delete_all
-
- create(:project_authorization, user_id: group_user.id,
- project_id: project.id,
- access_level: existing_access_level)
- end
-
- context 'when access level is the same' do
- let(:existing_access_level) { access_level }
-
- it 'does not create project authorization' do
- project_authorization = ProjectAuthorization.where(
- project_id: project.id,
- user_id: group_user.id,
- access_level: existing_access_level)
-
- expect(ProjectAuthorization).not_to receive(:insert_all)
-
- expect { service.execute }.not_to(
- change { project_authorization.reload.exists? }.from(true))
- end
- end
-
- context 'when existing access level is lower' do
- let(:existing_access_level) { Gitlab::Access::DEVELOPER }
-
- it 'creates new project authorization' do
- project_authorization = ProjectAuthorization.where(
- project_id: project.id,
- user_id: group_user.id,
- access_level: access_level)
-
- expect { service.execute }.to(
- change { project_authorization.reload.exists? }.from(false).to(true))
- end
-
- it 'deletes previous project authorization' do
- project_authorization = ProjectAuthorization.where(
- project_id: project.id,
- user_id: group_user.id,
- access_level: existing_access_level)
-
- expect { service.execute }.to(
- change { project_authorization.reload.exists? }.from(true).to(false))
- end
- end
-
- context 'when existing access level is higher' do
- let(:existing_access_level) { Gitlab::Access::OWNER }
-
- it 'does not create project authorization' do
- project_authorization = ProjectAuthorization.where(
- project_id: project.id,
- user_id: group_user.id,
- access_level: existing_access_level)
-
- expect(ProjectAuthorization).not_to receive(:insert_all)
-
- expect { service.execute }.not_to(
- change { project_authorization.reload.exists? }.from(true))
- end
- end
- end
- end
-end
diff --git a/spec/services/ci/create_downstream_pipeline_service_spec.rb b/spec/services/ci/create_downstream_pipeline_service_spec.rb
index 6142704b00e..11fb564b843 100644
--- a/spec/services/ci/create_downstream_pipeline_service_spec.rb
+++ b/spec/services/ci/create_downstream_pipeline_service_spec.rb
@@ -35,18 +35,20 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
upstream_project.add_developer(user)
end
+ subject { service.execute(bridge) }
+
context 'when downstream project has not been found' do
let(:trigger) do
{ trigger: { project: 'unknown/project' } }
end
it 'does not create a pipeline' do
- expect { service.execute(bridge) }
+ expect { subject }
.not_to change { Ci::Pipeline.count }
end
it 'changes pipeline bridge job status to failed' do
- service.execute(bridge)
+ subject
expect(bridge.reload).to be_failed
expect(bridge.failure_reason)
@@ -56,12 +58,12 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
context 'when user can not access downstream project' do
it 'does not create a new pipeline' do
- expect { service.execute(bridge) }
+ expect { subject }
.not_to change { Ci::Pipeline.count }
end
it 'changes status of the bridge build' do
- service.execute(bridge)
+ subject
expect(bridge.reload).to be_failed
expect(bridge.failure_reason)
@@ -75,12 +77,12 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'does not create a new pipeline' do
- expect { service.execute(bridge) }
+ expect { subject }
.not_to change { Ci::Pipeline.count }
end
it 'changes status of the bridge build' do
- service.execute(bridge)
+ subject
expect(bridge.reload).to be_failed
expect(bridge.failure_reason).to eq 'insufficient_bridge_permissions'
@@ -96,12 +98,12 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'creates only one new pipeline' do
- expect { service.execute(bridge) }
+ expect { subject }
.to change { Ci::Pipeline.count }.by(1)
end
it 'creates a new pipeline in a downstream project' do
- pipeline = service.execute(bridge)
+ pipeline = subject
expect(pipeline.user).to eq bridge.user
expect(pipeline.project).to eq downstream_project
@@ -111,8 +113,14 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
expect(pipeline.source_bridge).to be_a ::Ci::Bridge
end
+ it_behaves_like 'logs downstream pipeline creation' do
+ let(:expected_root_pipeline) { upstream_pipeline }
+ let(:expected_hierarchy_size) { 2 }
+ let(:expected_downstream_relationship) { :multi_project }
+ end
+
it 'updates bridge status when downstream pipeline gets processed' do
- pipeline = service.execute(bridge)
+ pipeline = subject
expect(pipeline.reload).to be_created
expect(bridge.reload).to be_success
@@ -136,7 +144,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
bridge_id: bridge.id, project_id: bridge.project.id)
.and_call_original
expect(Ci::CreatePipelineService).not_to receive(:new)
- expect(service.execute(bridge)).to eq({ message: "Already has a downstream pipeline", status: :error })
+ expect(subject).to eq({ message: "Already has a downstream pipeline", status: :error })
end
end
@@ -146,7 +154,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'is using default branch name' do
- pipeline = service.execute(bridge)
+ pipeline = subject
expect(pipeline.ref).to eq 'master'
end
@@ -158,12 +166,12 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'creates only one new pipeline' do
- expect { service.execute(bridge) }
+ expect { subject }
.to change { Ci::Pipeline.count }.by(1)
end
it 'creates a new pipeline in a downstream project' do
- pipeline = service.execute(bridge)
+ pipeline = subject
expect(pipeline.user).to eq bridge.user
expect(pipeline.project).to eq downstream_project
@@ -174,7 +182,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'updates the bridge status when downstream pipeline gets processed' do
- pipeline = service.execute(bridge)
+ pipeline = subject
expect(pipeline.reload).to be_failed
expect(bridge.reload).to be_failed
@@ -188,12 +196,12 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
context 'detects a circular dependency' do
it 'does not create a new pipeline' do
- expect { service.execute(bridge) }
+ expect { subject }
.not_to change { Ci::Pipeline.count }
end
it 'changes status of the bridge build' do
- service.execute(bridge)
+ subject
expect(bridge.reload).to be_failed
expect(bridge.failure_reason).to eq 'invalid_bridge_trigger'
@@ -209,12 +217,12 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
shared_examples 'creates a child pipeline' do
it 'creates only one new pipeline' do
- expect { service.execute(bridge) }
+ expect { subject }
.to change { Ci::Pipeline.count }.by(1)
end
it 'creates a child pipeline in the same project' do
- pipeline = service.execute(bridge)
+ pipeline = subject
pipeline.reload
expect(pipeline.builds.map(&:name)).to match_array(%w[rspec echo])
@@ -227,14 +235,14 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'updates bridge status when downstream pipeline gets processed' do
- pipeline = service.execute(bridge)
+ pipeline = subject
expect(pipeline.reload).to be_created
expect(bridge.reload).to be_success
end
it 'propagates parent pipeline settings to the child pipeline' do
- pipeline = service.execute(bridge)
+ pipeline = subject
pipeline.reload
expect(pipeline.ref).to eq(upstream_pipeline.ref)
@@ -264,8 +272,14 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
it_behaves_like 'creates a child pipeline'
+ it_behaves_like 'logs downstream pipeline creation' do
+ let(:expected_root_pipeline) { upstream_pipeline }
+ let(:expected_hierarchy_size) { 2 }
+ let(:expected_downstream_relationship) { :parent_child }
+ end
+
it 'updates the bridge job to success' do
- expect { service.execute(bridge) }.to change { bridge.status }.to 'success'
+ expect { subject }.to change { bridge.status }.to 'success'
end
context 'when bridge uses "depend" strategy' do
@@ -276,7 +290,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'does not update the bridge job status' do
- expect { service.execute(bridge) }.not_to change { bridge.status }
+ expect { subject }.not_to change { bridge.status }
end
end
@@ -306,7 +320,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
it_behaves_like 'creates a child pipeline'
it 'propagates the merge request to the child pipeline' do
- pipeline = service.execute(bridge)
+ pipeline = subject
expect(pipeline.merge_request).to eq(merge_request)
expect(pipeline).to be_merge_request
@@ -322,11 +336,17 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'creates the pipeline' do
- expect { service.execute(bridge) }
+ expect { subject }
.to change { Ci::Pipeline.count }.by(1)
expect(bridge.reload).to be_success
end
+
+ it_behaves_like 'logs downstream pipeline creation' do
+ let(:expected_root_pipeline) { upstream_pipeline.parent_pipeline }
+ let(:expected_hierarchy_size) { 3 }
+ let(:expected_downstream_relationship) { :parent_child }
+ end
end
context 'when upstream pipeline has a parent pipeline, which has a parent pipeline' do
@@ -345,7 +365,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'does not create a second descendant pipeline' do
- expect { service.execute(bridge) }
+ expect { subject }
.not_to change { Ci::Pipeline.count }
expect(bridge.reload).to be_failed
@@ -370,7 +390,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'create the pipeline' do
- expect { service.execute(bridge) }.to change { Ci::Pipeline.count }.by(1)
+ expect { subject }.to change { Ci::Pipeline.count }.by(1)
end
end
@@ -382,11 +402,11 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'creates a new pipeline allowing variables to be passed downstream' do
- expect { service.execute(bridge) }.to change { Ci::Pipeline.count }.by(1)
+ expect { subject }.to change { Ci::Pipeline.count }.by(1)
end
it 'passes variables downstream from the bridge' do
- pipeline = service.execute(bridge)
+ pipeline = subject
pipeline.variables.map(&:key).tap do |variables|
expect(variables).to include 'BRIDGE'
@@ -444,12 +464,12 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
describe 'cyclical dependency detection' do
shared_examples 'detects cyclical pipelines' do
it 'does not create a new pipeline' do
- expect { service.execute(bridge) }
+ expect { subject }
.not_to change { Ci::Pipeline.count }
end
it 'changes status of the bridge build' do
- service.execute(bridge)
+ subject
expect(bridge.reload).to be_failed
expect(bridge.failure_reason).to eq 'pipeline_loop_detected'
@@ -458,12 +478,12 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
shared_examples 'passes cyclical pipeline precondition' do
it 'creates a new pipeline' do
- expect { service.execute(bridge) }
+ expect { subject }
.to change { Ci::Pipeline.count }
end
it 'expect bridge build not to be failed' do
- service.execute(bridge)
+ subject
expect(bridge.reload).not_to be_failed
end
@@ -537,19 +557,19 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'creates only one new pipeline' do
- expect { service.execute(bridge) }
+ expect { subject }
.to change { Ci::Pipeline.count }.by(1)
end
it 'creates a new pipeline in the downstream project' do
- pipeline = service.execute(bridge)
+ pipeline = subject
expect(pipeline.user).to eq bridge.user
expect(pipeline.project).to eq downstream_project
end
it 'drops the bridge' do
- pipeline = service.execute(bridge)
+ pipeline = subject
expect(pipeline.reload).to be_failed
expect(bridge.reload).to be_failed
@@ -573,7 +593,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
bridge_id: bridge.id,
downstream_pipeline_id: kind_of(Numeric))
- service.execute(bridge)
+ subject
end
end
@@ -583,7 +603,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'passes bridge variables to downstream pipeline' do
- pipeline = service.execute(bridge)
+ pipeline = subject
expect(pipeline.variables.first)
.to have_attributes(key: 'BRIDGE', value: 'var')
@@ -596,7 +616,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'does not pass pipeline variables directly downstream' do
- pipeline = service.execute(bridge)
+ pipeline = subject
pipeline.variables.map(&:key).tap do |variables|
expect(variables).not_to include 'PIPELINE_VARIABLE'
@@ -609,7 +629,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'makes it possible to pass pipeline variable downstream' do
- pipeline = service.execute(bridge)
+ pipeline = subject
pipeline.variables.find_by(key: 'BRIDGE').tap do |variable|
expect(variable.value).to eq 'my-value-var'
@@ -622,12 +642,12 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'does not create a new pipeline' do
- expect { service.execute(bridge) }
+ expect { subject }
.not_to change { Ci::Pipeline.count }
end
it 'ignores variables passed downstream from the bridge' do
- pipeline = service.execute(bridge)
+ pipeline = subject
pipeline.variables.map(&:key).tap do |variables|
expect(variables).not_to include 'BRIDGE'
@@ -635,7 +655,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'sets errors', :aggregate_failures do
- service.execute(bridge)
+ subject
expect(bridge.reload).to be_failed
expect(bridge.failure_reason).to eq('downstream_pipeline_creation_failed')
@@ -679,7 +699,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
context 'that include the bridge job' do
it 'creates the downstream pipeline' do
- expect { service.execute(bridge) }
+ expect { subject }
.to change(downstream_project.ci_pipelines, :count).by(1)
end
end
@@ -692,7 +712,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'changes status of the bridge build' do
- service.execute(bridge)
+ subject
expect(bridge.reload).to be_failed
expect(bridge.failure_reason).to eq 'insufficient_bridge_permissions'
@@ -710,7 +730,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'does not create a pipeline and drops the bridge' do
- expect { service.execute(bridge) }.not_to change(downstream_project.ci_pipelines, :count)
+ expect { subject }.not_to change(downstream_project.ci_pipelines, :count)
expect(bridge.reload).to be_failed
expect(bridge.failure_reason).to eq('downstream_pipeline_creation_failed')
@@ -733,7 +753,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'does not create a pipeline and drops the bridge' do
- expect { service.execute(bridge) }.not_to change(downstream_project.ci_pipelines, :count)
+ expect { subject }.not_to change(downstream_project.ci_pipelines, :count)
expect(bridge.reload).to be_failed
expect(bridge.failure_reason).to eq('downstream_pipeline_creation_failed')
@@ -755,7 +775,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'creates the pipeline but drops the bridge' do
- expect { service.execute(bridge) }.to change(downstream_project.ci_pipelines, :count).by(1)
+ expect { subject }.to change(downstream_project.ci_pipelines, :count).by(1)
expect(bridge.reload).to be_failed
expect(bridge.failure_reason).to eq('downstream_pipeline_creation_failed')
@@ -787,7 +807,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end
it 'creates the pipeline' do
- expect { service.execute(bridge) }.to change(downstream_project.ci_pipelines, :count).by(1)
+ expect { subject }.to change(downstream_project.ci_pipelines, :count).by(1)
expect(bridge.reload).to be_success
end
@@ -795,7 +815,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
context 'when not passing the required variable' do
it 'does not create the pipeline' do
- expect { service.execute(bridge) }.not_to change(downstream_project.ci_pipelines, :count)
+ expect { subject }.not_to change(downstream_project.ci_pipelines, :count)
end
end
end
diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb
index 943d70ba142..c39a76ad2fc 100644
--- a/spec/services/ci/create_pipeline_service_spec.rb
+++ b/spec/services/ci/create_pipeline_service_spec.rb
@@ -466,7 +466,7 @@ RSpec.describe Ci::CreatePipelineService do
it 'pull it from Auto-DevOps' do
pipeline = execute_service.payload
expect(pipeline).to be_auto_devops_source
- expect(pipeline.builds.map(&:name)).to match_array(%w[brakeman-sast build code_quality eslint-sast secret_detection semgrep-sast test])
+ expect(pipeline.builds.map(&:name)).to match_array(%w[brakeman-sast build code_quality container_scanning eslint-sast secret_detection semgrep-sast test])
end
end
diff --git a/spec/services/ci/generate_kubeconfig_service_spec.rb b/spec/services/ci/generate_kubeconfig_service_spec.rb
index b0673d16158..e3088ca6ea7 100644
--- a/spec/services/ci/generate_kubeconfig_service_spec.rb
+++ b/spec/services/ci/generate_kubeconfig_service_spec.rb
@@ -6,16 +6,17 @@ RSpec.describe Ci::GenerateKubeconfigService do
describe '#execute' do
let(:project) { create(:project) }
let(:build) { create(:ci_build, project: project) }
+ let(:pipeline) { build.pipeline }
let(:agent1) { create(:cluster_agent, project: project) }
let(:agent2) { create(:cluster_agent) }
let(:template) { instance_double(Gitlab::Kubernetes::Kubeconfig::Template) }
- subject { described_class.new(build).execute }
+ subject { described_class.new(pipeline, token: build.token).execute }
before do
expect(Gitlab::Kubernetes::Kubeconfig::Template).to receive(:new).and_return(template)
- expect(build.pipeline).to receive(:authorized_cluster_agents).and_return([agent1, agent2])
+ expect(pipeline).to receive(:authorized_cluster_agents).and_return([agent1, agent2])
end
it 'adds a cluster, and a user and context for each available agent' do
diff --git a/spec/services/ci/job_artifacts/create_service_spec.rb b/spec/services/ci/job_artifacts/create_service_spec.rb
index b8487e438a9..01f240805f5 100644
--- a/spec/services/ci/job_artifacts/create_service_spec.rb
+++ b/spec/services/ci/job_artifacts/create_service_spec.rb
@@ -42,6 +42,13 @@ RSpec.describe Ci::JobArtifacts::CreateService do
subject { service.execute(artifacts_file, params, metadata_file: metadata_file) }
context 'when artifacts file is uploaded' do
+ it 'returns artifact in the response' do
+ response = subject
+ new_artifact = job.job_artifacts.last
+
+ expect(response[:artifact]).to eq(new_artifact)
+ end
+
it 'saves artifact for the given type' do
expect { subject }.to change { Ci::JobArtifact.count }.by(1)
@@ -84,7 +91,7 @@ RSpec.describe Ci::JobArtifacts::CreateService do
it 'sets expiration date according to application settings' do
expected_expire_at = 1.day.from_now
- expect(subject).to match(a_hash_including(status: :success))
+ expect(subject).to match(a_hash_including(status: :success, artifact: anything))
archive_artifact, metadata_artifact = job.job_artifacts.last(2)
expect(job.artifacts_expire_at).to be_within(1.minute).of(expected_expire_at)
@@ -100,7 +107,7 @@ RSpec.describe Ci::JobArtifacts::CreateService do
it 'sets expiration date according to the parameter' do
expected_expire_at = 2.hours.from_now
- expect(subject).to match(a_hash_including(status: :success))
+ expect(subject).to match(a_hash_including(status: :success, artifact: anything))
archive_artifact, metadata_artifact = job.job_artifacts.last(2)
expect(job.artifacts_expire_at).to be_within(1.minute).of(expected_expire_at)
diff --git a/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb b/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb
index b48ea70aa4c..98b01e2b303 100644
--- a/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb
+++ b/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb
@@ -38,7 +38,7 @@ RSpec.describe ::Ci::PipelineArtifacts::CoverageReportService do
context 'when pipeline artifact has already been created' do
it 'do not raise an error and do not persist the same artifact twice' do
- expect { 2.times { described_class.new.execute(pipeline) } }.not_to raise_error(ActiveRecord::RecordNotUnique)
+ expect { 2.times { described_class.new.execute(pipeline) } }.not_to raise_error
expect(Ci::PipelineArtifact.count).to eq(1)
end
diff --git a/spec/services/ci/pipeline_creation/start_pipeline_service_spec.rb b/spec/services/ci/pipeline_creation/start_pipeline_service_spec.rb
index 2aa810e8ea1..ab4ba20e716 100644
--- a/spec/services/ci/pipeline_creation/start_pipeline_service_spec.rb
+++ b/spec/services/ci/pipeline_creation/start_pipeline_service_spec.rb
@@ -16,5 +16,11 @@ RSpec.describe Ci::PipelineCreation::StartPipelineService do
service.execute
end
+
+ it 'creates pipeline ref' do
+ expect(pipeline.persistent_ref).to receive(:create).once
+
+ service.execute
+ end
end
end
diff --git a/spec/services/ci/pipeline_trigger_service_spec.rb b/spec/services/ci/pipeline_trigger_service_spec.rb
index 29d12b0dd0e..a794dedc658 100644
--- a/spec/services/ci/pipeline_trigger_service_spec.rb
+++ b/spec/services/ci/pipeline_trigger_service_spec.rb
@@ -187,6 +187,14 @@ RSpec.describe Ci::PipelineTriggerService do
expect(result[:status]).to eq(:success)
end
+ it_behaves_like 'logs downstream pipeline creation' do
+ subject { result[:pipeline] }
+
+ let(:expected_root_pipeline) { pipeline }
+ let(:expected_hierarchy_size) { 2 }
+ let(:expected_downstream_relationship) { :multi_project }
+ end
+
context 'when commit message has [ci skip]' do
before do
allow_next_instance_of(Ci::Pipeline) do |instance|
diff --git a/spec/services/ci/retry_job_service_spec.rb b/spec/services/ci/retry_job_service_spec.rb
index 25aab73ab01..acc7a99637b 100644
--- a/spec/services/ci/retry_job_service_spec.rb
+++ b/spec/services/ci/retry_job_service_spec.rb
@@ -17,396 +17,276 @@ RSpec.describe Ci::RetryJobService do
name: 'test')
end
- let_it_be_with_refind(:build) { create(:ci_build, :success, pipeline: pipeline, stage_id: stage.id) }
-
let(:user) { developer }
- let(:service) do
- described_class.new(project, user)
- end
+ let(:service) { described_class.new(project, user) }
before_all do
project.add_developer(developer)
project.add_reporter(reporter)
end
- clone_accessors = ::Ci::Build.clone_accessors.without(::Ci::Build.extra_accessors)
-
- reject_accessors =
- %i[id status user token token_encrypted coverage trace runner
- artifacts_expire_at
- created_at updated_at started_at finished_at queued_at erased_by
- erased_at auto_canceled_by job_artifacts job_artifacts_archive
- job_artifacts_metadata job_artifacts_trace job_artifacts_junit
- job_artifacts_sast job_artifacts_secret_detection job_artifacts_dependency_scanning
- job_artifacts_container_scanning job_artifacts_cluster_image_scanning job_artifacts_dast
- job_artifacts_license_scanning
- job_artifacts_performance job_artifacts_browser_performance job_artifacts_load_performance
- job_artifacts_lsif job_artifacts_terraform job_artifacts_cluster_applications
- job_artifacts_codequality job_artifacts_metrics scheduled_at
- job_variables waiting_for_resource_at job_artifacts_metrics_referee
- job_artifacts_network_referee job_artifacts_dotenv
- job_artifacts_cobertura needs job_artifacts_accessibility
- job_artifacts_requirements job_artifacts_coverage_fuzzing
- job_artifacts_api_fuzzing terraform_state_versions].freeze
-
- ignore_accessors =
- %i[type lock_version target_url base_tags trace_sections
- commit_id deployment erased_by_id project_id
- runner_id tag_taggings taggings tags trigger_request_id
- user_id auto_canceled_by_id retried failure_reason
- sourced_pipelines artifacts_file_store artifacts_metadata_store
- metadata runner_session trace_chunks upstream_pipeline_id
- artifacts_file artifacts_metadata artifacts_size commands
- resource resource_group_id processed security_scans author
- pipeline_id report_results pending_state pages_deployments
- queuing_entry runtime_metadata trace_metadata
- dast_site_profile dast_scanner_profile].freeze
-
- shared_examples 'build duplication' do
- let_it_be(:another_pipeline) { create(:ci_empty_pipeline, project: project) }
+ shared_context 'retryable bridge' do
+ let_it_be(:downstream_project) { create(:project, :repository) }
- let_it_be(:build) do
- create(:ci_build, :failed, :picked, :expired, :erased, :queued, :coverage, :tags,
- :allowed_to_fail, :on_tag, :triggered, :teardown_environment, :resource_group,
- description: 'my-job', stage: 'test', stage_id: stage.id,
- pipeline: pipeline, auto_canceled_by: another_pipeline,
- scheduled_at: 10.seconds.since)
+ let_it_be_with_refind(:job) do
+ create(
+ :ci_bridge, :success, pipeline: pipeline, downstream: downstream_project,
+ description: 'a trigger job', stage_id: stage.id
+ )
end
- let_it_be(:internal_job_variable) { create(:ci_job_variable, job: build) }
+ let_it_be(:job_to_clone) { job }
- before_all do
- # Make sure that build has both `stage_id` and `stage` because FactoryBot
- # can reset one of the fields when assigning another. We plan to deprecate
- # and remove legacy `stage` column in the future.
- build.update!(stage: 'test', stage_id: stage.id)
-
- # Make sure we have one instance for every possible job_artifact_X
- # associations to check they are correctly rejected on build duplication.
- Ci::JobArtifact::TYPE_AND_FORMAT_PAIRS.each do |file_type, file_format|
- create(:ci_job_artifact, file_format,
- file_type: file_type, job: build, expire_at: build.artifacts_expire_at)
- end
+ before do
+ job.update!(retried: false)
+ end
+ end
+
+ shared_context 'retryable build' do
+ let_it_be_with_refind(:job) { create(:ci_build, :success, pipeline: pipeline, stage_id: stage.id) }
+ let_it_be(:another_pipeline) { create(:ci_empty_pipeline, project: project) }
- create(:ci_job_variable, :dotenv_source, job: build)
- create(:ci_build_need, build: build)
- create(:terraform_state_version, build: build)
+ let_it_be(:job_to_clone) do
+ create(:ci_build, :failed, :picked, :expired, :erased, :queued, :coverage, :tags,
+ :allowed_to_fail, :on_tag, :triggered, :teardown_environment, :resource_group,
+ description: 'my-job', stage: 'test', stage_id: stage.id,
+ pipeline: pipeline, auto_canceled_by: another_pipeline,
+ scheduled_at: 10.seconds.since)
end
before do
- build.update!(retried: false, status: :success)
+ job.update!(retried: false, status: :success)
+ job_to_clone.update!(retried: false, status: :success)
end
+ end
- describe 'clone accessors' do
- let(:forbidden_associations) do
- Ci::Build.reflect_on_all_associations.each_with_object(Set.new) do |assoc, memo|
- memo << assoc.name unless assoc.macro == :belongs_to
- end
- end
+ shared_examples_for 'clones the job' do
+ let(:job) { job_to_clone }
- clone_accessors.each do |attribute|
- it "clones #{attribute} build attribute", :aggregate_failures do
- expect(attribute).not_to be_in(forbidden_associations), "association #{attribute} must be `belongs_to`"
- expect(build.send(attribute)).not_to be_nil
- expect(new_build.send(attribute)).not_to be_nil
- expect(new_build.send(attribute)).to eq build.send(attribute)
- end
+ before_all do
+ # Make sure that job has both `stage_id` and `stage`
+ job_to_clone.update!(stage: 'test', stage_id: stage.id)
+
+ create(:ci_build_need, build: job_to_clone)
+ end
+
+ context 'when the user has ability to execute job' do
+ before do
+ stub_not_protect_default_branch
end
- context 'when job has nullified protected' do
- before do
- build.update_attribute(:protected, nil)
- end
+ context 'when there is a failed job ToDo for the MR' do
+ let!(:merge_request) { create(:merge_request, source_project: project, author: user, head_pipeline: pipeline) }
+ let!(:todo) { create(:todo, :build_failed, user: user, project: project, author: user, target: merge_request) }
- it "clones protected build attribute" do
- expect(new_build.protected).to be_nil
- expect(new_build.protected).to eq build.protected
+ it 'resolves the ToDo for the failed job' do
+ expect do
+ service.execute(job)
+ end.to change { todo.reload.state }.from('pending').to('done')
end
end
- it 'clones only the needs attributes' do
- expect(new_build.needs.exists?).to be_truthy
- expect(build.needs.exists?).to be_truthy
+ context 'when the job has needs' do
+ before do
+ create(:ci_build_need, build: job, name: 'build1')
+ create(:ci_build_need, build: job, name: 'build2')
+ end
- expect(new_build.needs_attributes).to match(build.needs_attributes)
- expect(new_build.needs).not_to match(build.needs)
- end
+ it 'bulk inserts all the needs' do
+ expect(Ci::BuildNeed).to receive(:bulk_insert!).and_call_original
- it 'clones only internal job variables' do
- expect(new_build.job_variables.count).to eq(1)
- expect(new_build.job_variables).to contain_exactly(having_attributes(key: internal_job_variable.key, value: internal_job_variable.value))
+ new_job
+ end
end
- end
- describe 'reject accessors' do
- reject_accessors.each do |attribute|
- it "does not clone #{attribute} build attribute" do
- expect(new_build.send(attribute)).not_to eq build.send(attribute)
- end
+ it 'marks the old job as retried' do
+ expect(new_job).to be_latest
+ expect(job).to be_retried
+ expect(job).to be_processed
end
end
- it 'has correct number of known attributes', :aggregate_failures do
- processed_accessors = clone_accessors + reject_accessors
- known_accessors = processed_accessors + ignore_accessors
-
- # :tag_list is a special case, this accessor does not exist
- # in reflected associations, comes from `act_as_taggable` and
- # we use it to copy tags, instead of reusing tags.
- #
- current_accessors =
- Ci::Build.attribute_names.map(&:to_sym) +
- Ci::Build.attribute_aliases.keys.map(&:to_sym) +
- Ci::Build.reflect_on_all_associations.map(&:name) +
- [:tag_list, :needs_attributes, :job_variables_attributes] -
- # ee-specific accessors should be tested in ee/spec/services/ci/retry_job_service_spec.rb instead
- Ci::Build.extra_accessors -
- [:dast_site_profiles_build, :dast_scanner_profiles_build] # join tables
-
- current_accessors.uniq!
-
- expect(current_accessors).to include(*processed_accessors)
- expect(known_accessors).to include(*current_accessors)
- end
- end
+ context 'when the user does not have permission to execute the job' do
+ let(:user) { reporter }
- describe '#execute' do
- let(:new_build) do
- travel_to(1.second.from_now) do
- service.execute(build)[:job]
+ it 'raises an error' do
+ expect { service.execute(job) }
+ .to raise_error Gitlab::Access::AccessDeniedError
end
end
+ end
- context 'when user has ability to execute build' do
- before do
- stub_not_protect_default_branch
- end
-
- it_behaves_like 'build duplication'
+ shared_examples_for 'retries the job' do
+ it_behaves_like 'clones the job'
- it 'creates a new build that represents the old one' do
- expect(new_build.name).to eq build.name
- end
+ it 'enqueues the new job' do
+ expect(new_job).to be_pending
+ end
- it 'enqueues the new build' do
- expect(new_build).to be_pending
+ context 'when there are subsequent processables that are skipped' do
+ let!(:subsequent_build) do
+ create(:ci_build, :skipped, stage_idx: 2,
+ pipeline: pipeline,
+ stage: 'deploy')
end
- context 'when there are subsequent processables that are skipped' do
- let!(:subsequent_build) do
- create(:ci_build, :skipped, stage_idx: 2,
+ let!(:subsequent_bridge) do
+ create(:ci_bridge, :skipped, stage_idx: 2,
pipeline: pipeline,
stage: 'deploy')
- end
-
- let!(:subsequent_bridge) do
- create(:ci_bridge, :skipped, stage_idx: 2,
- pipeline: pipeline,
- stage: 'deploy')
- end
-
- it 'resumes pipeline processing in the subsequent stage' do
- service.execute(build)
-
- expect(subsequent_build.reload).to be_created
- expect(subsequent_bridge.reload).to be_created
- end
-
- it 'updates ownership for subsequent builds' do
- expect { service.execute(build) }.to change { subsequent_build.reload.user }.to(user)
- end
-
- it 'updates ownership for subsequent bridges' do
- expect { service.execute(build) }.to change { subsequent_bridge.reload.user }.to(user)
- end
-
- it 'does not cause n+1 when updaing build ownership' do
- control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) { service.execute(build) }.count
+ end
- create_list(:ci_build, 2, :skipped, stage_idx: build.stage_idx + 1, pipeline: pipeline, stage: 'deploy')
+ it 'resumes pipeline processing in the subsequent stage' do
+ service.execute(job)
- expect { service.execute(build) }.not_to exceed_all_query_limit(control_count)
- end
+ expect(subsequent_build.reload).to be_created
+ expect(subsequent_bridge.reload).to be_created
end
- context 'when pipeline has other builds' do
- let!(:stage2) { create(:ci_stage_entity, project: project, pipeline: pipeline, name: 'deploy') }
- let!(:build2) { create(:ci_build, pipeline: pipeline, stage_id: stage.id ) }
- let!(:deploy) { create(:ci_build, pipeline: pipeline, stage_id: stage2.id) }
- let!(:deploy_needs_build2) { create(:ci_build_need, build: deploy, name: build2.name) }
-
- context 'when build has nil scheduling_type' do
- before do
- build.pipeline.processables.update_all(scheduling_type: nil)
- build.reload
- end
-
- it 'populates scheduling_type of processables' do
- expect(new_build.scheduling_type).to eq('stage')
- expect(build.reload.scheduling_type).to eq('stage')
- expect(build2.reload.scheduling_type).to eq('stage')
- expect(deploy.reload.scheduling_type).to eq('dag')
- end
- end
-
- context 'when build has scheduling_type' do
- it 'does not call populate_scheduling_type!' do
- expect_any_instance_of(Ci::Pipeline).not_to receive(:ensure_scheduling_type!) # rubocop: disable RSpec/AnyInstanceOf
+ it 'updates ownership for subsequent builds' do
+ expect { service.execute(job) }.to change { subsequent_build.reload.user }.to(user)
+ end
- expect(new_build.scheduling_type).to eq('stage')
- end
- end
+ it 'updates ownership for subsequent bridges' do
+ expect { service.execute(job) }.to change { subsequent_bridge.reload.user }.to(user)
end
+ end
- context 'when the pipeline is a child pipeline and the bridge is depended' do
- let!(:parent_pipeline) { create(:ci_pipeline, project: project) }
- let!(:bridge) { create(:ci_bridge, :strategy_depend, pipeline: parent_pipeline, status: 'success') }
- let!(:source_pipeline) { create(:ci_sources_pipeline, pipeline: pipeline, source_job: bridge) }
+ context 'when the pipeline has other jobs' do
+ let!(:stage2) { create(:ci_stage_entity, project: project, pipeline: pipeline, name: 'deploy') }
+ let!(:build2) { create(:ci_build, pipeline: pipeline, stage_id: stage.id ) }
+ let!(:deploy) { create(:ci_build, pipeline: pipeline, stage_id: stage2.id) }
+ let!(:deploy_needs_build2) { create(:ci_build_need, build: deploy, name: build2.name) }
- it 'marks source bridge as pending' do
- service.execute(build)
+ context 'when job has a nil scheduling_type' do
+ before do
+ job.pipeline.processables.update_all(scheduling_type: nil)
+ job.reload
+ end
- expect(bridge.reload).to be_pending
+ it 'populates scheduling_type of processables' do
+ expect(new_job.scheduling_type).to eq('stage')
+ expect(job.reload.scheduling_type).to eq('stage')
+ expect(build2.reload.scheduling_type).to eq('stage')
+ expect(deploy.reload.scheduling_type).to eq('dag')
end
end
- context 'when there is a failed job todo for the MR' do
- let!(:merge_request) { create(:merge_request, source_project: project, author: user, head_pipeline: pipeline) }
- let!(:todo) { create(:todo, :build_failed, user: user, project: project, author: user, target: merge_request) }
+ context 'when job has scheduling_type' do
+ it 'does not call populate_scheduling_type!' do
+ expect(job.pipeline).not_to receive(:ensure_scheduling_type!)
- it 'resolves the todo for the old failed build' do
- expect do
- service.execute(build)
- end.to change { todo.reload.state }.from('pending').to('done')
+ expect(new_job.scheduling_type).to eq('stage')
end
end
end
- context 'when user does not have ability to execute build' do
- let(:user) { reporter }
+ context 'when the pipeline is a child pipeline and the bridge uses strategy:depend' do
+ let!(:parent_pipeline) { create(:ci_pipeline, project: project) }
+ let!(:bridge) { create(:ci_bridge, :strategy_depend, pipeline: parent_pipeline, status: 'success') }
+ let!(:source_pipeline) { create(:ci_sources_pipeline, pipeline: pipeline, source_job: bridge) }
- it 'raises an error' do
- expect { service.execute(build) }
- .to raise_error Gitlab::Access::AccessDeniedError
- end
-
- context 'when the job is not retryable' do
- let(:build) { create(:ci_build, :created, pipeline: pipeline) }
+ it 'marks the source bridge as pending' do
+ service.execute(job)
- it 'returns a ServiceResponse error' do
- response = service.execute(build)
-
- expect(response).to be_a(ServiceResponse)
- expect(response).to be_error
- expect(response.message).to eq("Job cannot be retried")
- end
+ expect(bridge.reload).to be_pending
end
end
end
describe '#clone!' do
- let(:new_build) do
- travel_to(1.second.from_now) do
- service.clone!(build)
- end
- end
+ let(:new_job) { service.clone!(job) }
it 'raises an error when an unexpected class is passed' do
expect { service.clone!(create(:ci_build).present) }.to raise_error(TypeError)
end
- context 'when user has ability to execute build' do
- before do
- stub_not_protect_default_branch
- end
+ context 'when the job to be cloned is a bridge' do
+ include_context 'retryable bridge'
- it_behaves_like 'build duplication'
+ it_behaves_like 'clones the job'
+ end
- it 'creates a new build that represents the old one' do
- expect(new_build.name).to eq build.name
- end
+ context 'when the job to be cloned is a build' do
+ include_context 'retryable build'
- it 'does not enqueue the new build' do
- expect(new_build).to be_created
- expect(new_build).not_to be_processed
- end
+ let(:job) { job_to_clone }
- it 'does mark old build as retried' do
- expect(new_build).to be_latest
- expect(build).to be_retried
- expect(build).to be_processed
- end
+ it_behaves_like 'clones the job'
- shared_examples_for 'when build with deployment is retried' do
- let!(:build) do
+ context 'when a build with a deployment is retried' do
+ let!(:job) do
create(:ci_build, :with_deployment, :deploy_to_production,
- pipeline: pipeline, stage_id: stage.id, project: project)
+ pipeline: pipeline, stage_id: stage.id, project: project)
end
it 'creates a new deployment' do
- expect { new_build }.to change { Deployment.count }.by(1)
- end
-
- it 'persists expanded environment name' do
- expect(new_build.metadata.expanded_environment_name).to eq('production')
+ expect { new_job }.to change { Deployment.count }.by(1)
end
it 'does not create a new environment' do
- expect { new_build }.not_to change { Environment.count }
+ expect { new_job }.not_to change { Environment.count }
end
end
- shared_examples_for 'when build with dynamic environment is retried' do
+ context 'when a build with a dynamic environment is retried' do
let_it_be(:other_developer) { create(:user).tap { |u| project.add_developer(u) } }
let(:environment_name) { 'review/$CI_COMMIT_REF_SLUG-$GITLAB_USER_ID' }
- let!(:build) do
+ let!(:job) do
create(:ci_build, :with_deployment, environment: environment_name,
options: { environment: { name: environment_name } },
pipeline: pipeline, stage_id: stage.id, project: project,
user: other_developer)
end
- it 're-uses the previous persisted environment' do
- expect(build.persisted_environment.name).to eq("review/#{build.ref}-#{other_developer.id}")
-
- expect(new_build.persisted_environment.name).to eq("review/#{build.ref}-#{other_developer.id}")
- end
-
it 'creates a new deployment' do
- expect { new_build }.to change { Deployment.count }.by(1)
+ expect { new_job }.to change { Deployment.count }.by(1)
end
it 'does not create a new environment' do
- expect { new_build }.not_to change { Environment.count }
+ expect { new_job }.not_to change { Environment.count }
end
end
+ end
+ end
+
+ describe '#execute' do
+ let(:new_job) { service.execute(job)[:job] }
- it_behaves_like 'when build with deployment is retried'
- it_behaves_like 'when build with dynamic environment is retried'
+ context 'when the job to be retried is a bridge' do
+ include_context 'retryable bridge'
- context 'when build has needs' do
- before do
- create(:ci_build_need, build: build, name: 'build1')
- create(:ci_build_need, build: build, name: 'build2')
- end
+ it_behaves_like 'retries the job'
+ end
- it 'bulk inserts all needs' do
- expect(Ci::BuildNeed).to receive(:bulk_insert!).and_call_original
+ context 'when the job to be retried is a build' do
+ include_context 'retryable build'
+
+ it_behaves_like 'retries the job'
- new_build
+ context 'when there are subsequent jobs that are skipped' do
+ let!(:subsequent_build) do
+ create(:ci_build, :skipped, stage_idx: 2,
+ pipeline: pipeline,
+ stage: 'deploy')
end
- end
- end
- context 'when user does not have ability to execute build' do
- let(:user) { reporter }
+ let!(:subsequent_bridge) do
+ create(:ci_bridge, :skipped, stage_idx: 2,
+ pipeline: pipeline,
+ stage: 'deploy')
+ end
- it 'raises an error' do
- expect { service.clone!(build) }
- .to raise_error Gitlab::Access::AccessDeniedError
+ it 'does not cause an N+1 when updating the job ownership' do
+ control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) { service.execute(job) }.count
+
+ create_list(:ci_build, 2, :skipped, stage_idx: job.stage_idx + 1, pipeline: pipeline, stage: 'deploy')
+
+ expect { service.execute(job) }.not_to exceed_all_query_limit(control_count)
+ end
end
end
end
diff --git a/spec/services/clusters/agents/delete_service_spec.rb b/spec/services/clusters/agents/delete_service_spec.rb
index 1d6bc9618dd..abe1bdaab27 100644
--- a/spec/services/clusters/agents/delete_service_spec.rb
+++ b/spec/services/clusters/agents/delete_service_spec.rb
@@ -17,7 +17,7 @@ RSpec.describe Clusters::Agents::DeleteService do
expect(response.status).to eq(:error)
expect(response.message).to eq('You have insufficient permissions to delete this cluster agent')
- expect { cluster_agent.reload }.not_to raise_error(ActiveRecord::RecordNotFound)
+ expect { cluster_agent.reload }.not_to raise_error
end
end
diff --git a/spec/services/clusters/kubernetes/create_or_update_namespace_service_spec.rb b/spec/services/clusters/kubernetes/create_or_update_namespace_service_spec.rb
index 98963f57341..90956e7b4ea 100644
--- a/spec/services/clusters/kubernetes/create_or_update_namespace_service_spec.rb
+++ b/spec/services/clusters/kubernetes/create_or_update_namespace_service_spec.rb
@@ -39,8 +39,6 @@ RSpec.describe Clusters::Kubernetes::CreateOrUpdateNamespaceService, '#execute'
stub_kubeclient_put_role_binding(api_url, Clusters::Kubernetes::GITLAB_KNATIVE_SERVING_ROLE_BINDING_NAME, namespace: namespace)
stub_kubeclient_put_role(api_url, Clusters::Kubernetes::GITLAB_CROSSPLANE_DATABASE_ROLE_NAME, namespace: namespace)
stub_kubeclient_put_role_binding(api_url, Clusters::Kubernetes::GITLAB_CROSSPLANE_DATABASE_ROLE_BINDING_NAME, namespace: namespace)
- stub_kubeclient_put_role(api_url, Clusters::Kubernetes::GITLAB_CILIUM_ROLE_NAME, namespace: namespace)
- stub_kubeclient_put_role_binding(api_url, Clusters::Kubernetes::GITLAB_CILIUM_ROLE_BINDING_NAME, namespace: namespace)
stub_kubeclient_get_secret(
api_url,
diff --git a/spec/services/clusters/kubernetes/create_or_update_service_account_service_spec.rb b/spec/services/clusters/kubernetes/create_or_update_service_account_service_spec.rb
index 11045dfe950..a4f018aec0c 100644
--- a/spec/services/clusters/kubernetes/create_or_update_service_account_service_spec.rb
+++ b/spec/services/clusters/kubernetes/create_or_update_service_account_service_spec.rb
@@ -147,8 +147,6 @@ RSpec.describe Clusters::Kubernetes::CreateOrUpdateServiceAccountService do
stub_kubeclient_put_role_binding(api_url, Clusters::Kubernetes::GITLAB_KNATIVE_SERVING_ROLE_BINDING_NAME, namespace: namespace)
stub_kubeclient_put_role(api_url, Clusters::Kubernetes::GITLAB_CROSSPLANE_DATABASE_ROLE_NAME, namespace: namespace)
stub_kubeclient_put_role_binding(api_url, Clusters::Kubernetes::GITLAB_CROSSPLANE_DATABASE_ROLE_BINDING_NAME, namespace: namespace)
- stub_kubeclient_put_role(api_url, Clusters::Kubernetes::GITLAB_CILIUM_ROLE_NAME, namespace: namespace)
- stub_kubeclient_put_role_binding(api_url, Clusters::Kubernetes::GITLAB_CILIUM_ROLE_BINDING_NAME, namespace: namespace)
end
it 'creates a namespace object' do
@@ -245,47 +243,6 @@ RSpec.describe Clusters::Kubernetes::CreateOrUpdateServiceAccountService do
)
)
end
-
- it 'creates a role granting cilium permissions to the service account' do
- subject
-
- expect(WebMock).to have_requested(:put, api_url + "/apis/rbac.authorization.k8s.io/v1/namespaces/#{namespace}/roles/#{Clusters::Kubernetes::GITLAB_CILIUM_ROLE_NAME}").with(
- body: hash_including(
- metadata: {
- name: Clusters::Kubernetes::GITLAB_CILIUM_ROLE_NAME,
- namespace: namespace
- },
- rules: [{
- apiGroups: %w(cilium.io),
- resources: %w(ciliumnetworkpolicies),
- verbs: %w(get list create update patch)
- }]
- )
- )
- end
-
- it 'creates a role binding granting cilium permissions to the service account' do
- subject
-
- expect(WebMock).to have_requested(:put, api_url + "/apis/rbac.authorization.k8s.io/v1/namespaces/#{namespace}/rolebindings/#{Clusters::Kubernetes::GITLAB_CILIUM_ROLE_BINDING_NAME}").with(
- body: hash_including(
- metadata: {
- name: Clusters::Kubernetes::GITLAB_CILIUM_ROLE_BINDING_NAME,
- namespace: namespace
- },
- roleRef: {
- apiGroup: 'rbac.authorization.k8s.io',
- kind: 'Role',
- name: Clusters::Kubernetes::GITLAB_CILIUM_ROLE_NAME
- },
- subjects: [{
- kind: 'ServiceAccount',
- name: service_account_name,
- namespace: namespace
- }]
- )
- )
- end
end
end
end
diff --git a/spec/services/container_expiration_policies/cleanup_service_spec.rb b/spec/services/container_expiration_policies/cleanup_service_spec.rb
index a1f76e5e5dd..c265ce74d14 100644
--- a/spec/services/container_expiration_policies/cleanup_service_spec.rb
+++ b/spec/services/container_expiration_policies/cleanup_service_spec.rb
@@ -25,7 +25,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupService do
it 'completely clean up the repository' do
expect(Projects::ContainerRepository::CleanupTagsService)
.to receive(:new).with(repository, nil, cleanup_tags_service_params).and_return(cleanup_tags_service)
- expect(cleanup_tags_service).to receive(:execute).and_return(status: :success)
+ expect(cleanup_tags_service).to receive(:execute).and_return(status: :success, deleted_size: 1)
response = subject
@@ -36,6 +36,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupService do
expect(repository.reload.cleanup_unscheduled?).to be_truthy
expect(repository.expiration_policy_completed_at).not_to eq(nil)
expect(repository.expiration_policy_started_at).not_to eq(nil)
+ expect(repository.last_cleanup_deleted_tags_count).to eq(1)
end
end
end
@@ -58,6 +59,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupService do
expect(repository.reload.cleanup_unfinished?).to be_truthy
expect(repository.expiration_policy_started_at).not_to eq(nil)
expect(repository.expiration_policy_completed_at).to eq(nil)
+ expect(repository.last_cleanup_deleted_tags_count).to eq(nil)
end
end
@@ -94,6 +96,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupService do
expect(repository.reload.cleanup_unfinished?).to be_truthy
expect(repository.expiration_policy_started_at).not_to eq(nil)
expect(repository.expiration_policy_completed_at).to eq(nil)
+ expect(repository.last_cleanup_deleted_tags_count).to eq(nil)
end
end
end
@@ -138,6 +141,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupService do
expect(repository.reload.cleanup_unfinished?).to be_truthy
expect(repository.expiration_policy_started_at).not_to eq(nil)
expect(repository.expiration_policy_completed_at).to eq(nil)
+ expect(repository.last_cleanup_deleted_tags_count).to eq(nil)
end
end
diff --git a/spec/services/container_expiration_policies/update_service_spec.rb b/spec/services/container_expiration_policies/update_service_spec.rb
index d4b6715ae86..7d949b77de7 100644
--- a/spec/services/container_expiration_policies/update_service_spec.rb
+++ b/spec/services/container_expiration_policies/update_service_spec.rb
@@ -63,7 +63,7 @@ RSpec.describe ContainerExpirationPolicies::UpdateService do
context 'with existing container expiration policy' do
where(:user_role, :shared_examples_name) do
:maintainer | 'updating the container expiration policy'
- :developer | 'updating the container expiration policy'
+ :developer | 'denying access to container expiration policy'
:reporter | 'denying access to container expiration policy'
:guest | 'denying access to container expiration policy'
:anonymous | 'denying access to container expiration policy'
@@ -83,7 +83,7 @@ RSpec.describe ContainerExpirationPolicies::UpdateService do
where(:user_role, :shared_examples_name) do
:maintainer | 'creating the container expiration policy'
- :developer | 'creating the container expiration policy'
+ :developer | 'denying access to container expiration policy'
:reporter | 'denying access to container expiration policy'
:guest | 'denying access to container expiration policy'
:anonymous | 'denying access to container expiration policy'
diff --git a/spec/services/container_expiration_policy_service_spec.rb b/spec/services/container_expiration_policy_service_spec.rb
deleted file mode 100644
index 41dd890dd35..00000000000
--- a/spec/services/container_expiration_policy_service_spec.rb
+++ /dev/null
@@ -1,32 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe ContainerExpirationPolicyService do
- let_it_be(:user) { create(:user) }
- let_it_be(:container_expiration_policy) { create(:container_expiration_policy, :runnable) }
-
- let(:project) { container_expiration_policy.project }
- let(:container_repository) { create(:container_repository, project: project) }
-
- before do
- project.add_maintainer(user)
- end
-
- describe '#execute' do
- subject { described_class.new(project, user).execute(container_expiration_policy) }
-
- it 'kicks off a cleanup worker for the container repository' do
- expect(CleanupContainerRepositoryWorker).to receive(:perform_async)
- .with(nil, container_repository.id, hash_including(container_expiration_policy: true))
-
- subject
- end
-
- it 'sets next_run_at on the container_expiration_policy' do
- subject
-
- expect(container_expiration_policy.next_run_at).to be > Time.zone.now
- end
- end
-end
diff --git a/spec/services/customer_relations/contacts/create_service_spec.rb b/spec/services/customer_relations/contacts/create_service_spec.rb
index 567e1c91e78..db6cce799fe 100644
--- a/spec/services/customer_relations/contacts/create_service_spec.rb
+++ b/spec/services/customer_relations/contacts/create_service_spec.rb
@@ -20,7 +20,7 @@ RSpec.describe CustomerRelations::Contacts::CreateService do
it 'returns an error' do
expect(response).to be_error
- expect(response.message).to match_array(['You have insufficient permissions to create a contact for this group'])
+ expect(response.message).to match_array(['You have insufficient permissions to manage contacts for this group'])
end
end
diff --git a/spec/services/customer_relations/contacts/update_service_spec.rb b/spec/services/customer_relations/contacts/update_service_spec.rb
index 253bbc23226..729fdc2058b 100644
--- a/spec/services/customer_relations/contacts/update_service_spec.rb
+++ b/spec/services/customer_relations/contacts/update_service_spec.rb
@@ -5,7 +5,7 @@ require 'spec_helper'
RSpec.describe CustomerRelations::Contacts::UpdateService do
let_it_be(:user) { create(:user) }
- let(:contact) { create(:contact, first_name: 'Mark', group: group) }
+ let(:contact) { create(:contact, first_name: 'Mark', group: group, state: 'active') }
subject(:update) { described_class.new(group: group, current_user: user, params: params).execute(contact) }
@@ -19,7 +19,7 @@ RSpec.describe CustomerRelations::Contacts::UpdateService do
response = update
expect(response).to be_error
- expect(response.message).to match_array(['You have insufficient permissions to update a contact for this group'])
+ expect(response.message).to match_array(['You have insufficient permissions to manage contacts for this group'])
end
end
@@ -41,6 +41,29 @@ RSpec.describe CustomerRelations::Contacts::UpdateService do
end
end
+ context 'when activating' do
+ let(:contact) { create(:contact, state: 'inactive') }
+ let(:params) { { active: true } }
+
+ it 'updates the contact' do
+ response = update
+
+ expect(response).to be_success
+ expect(response.payload.active?).to be_truthy
+ end
+ end
+
+ context 'when deactivating' do
+ let(:params) { { active: false } }
+
+ it 'updates the contact' do
+ response = update
+
+ expect(response).to be_success
+ expect(response.payload.active?).to be_falsy
+ end
+ end
+
context 'when the contact is invalid' do
let(:params) { { first_name: nil } }
diff --git a/spec/services/customer_relations/organizations/update_service_spec.rb b/spec/services/customer_relations/organizations/update_service_spec.rb
index 8461c98ef0e..4764ba85551 100644
--- a/spec/services/customer_relations/organizations/update_service_spec.rb
+++ b/spec/services/customer_relations/organizations/update_service_spec.rb
@@ -5,7 +5,7 @@ require 'spec_helper'
RSpec.describe CustomerRelations::Organizations::UpdateService do
let_it_be(:user) { create(:user) }
- let(:organization) { create(:organization, name: 'Test', group: group) }
+ let(:organization) { create(:organization, name: 'Test', group: group, state: 'active') }
subject(:update) { described_class.new(group: group, current_user: user, params: params).execute(organization) }
@@ -41,6 +41,29 @@ RSpec.describe CustomerRelations::Organizations::UpdateService do
end
end
+ context 'when activating' do
+ let(:organization) { create(:organization, state: 'inactive') }
+ let(:params) { { active: true } }
+
+ it 'updates the contact' do
+ response = update
+
+ expect(response).to be_success
+ expect(response.payload.active?).to be_truthy
+ end
+ end
+
+ context 'when deactivating' do
+ let(:params) { { active: false } }
+
+ it 'updates the organization' do
+ response = update
+
+ expect(response).to be_success
+ expect(response.payload.active?).to be_falsy
+ end
+ end
+
context 'when the organization is invalid' do
let(:params) { { name: nil } }
diff --git a/spec/services/database/consistency_fix_service_spec.rb b/spec/services/database/consistency_fix_service_spec.rb
new file mode 100644
index 00000000000..9a0fac2191c
--- /dev/null
+++ b/spec/services/database/consistency_fix_service_spec.rb
@@ -0,0 +1,153 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Database::ConsistencyFixService do
+ describe '#execute' do
+ context 'fixing namespaces inconsistencies' do
+ subject(:consistency_fix_service) do
+ described_class.new(
+ source_model: Namespace,
+ target_model: Ci::NamespaceMirror,
+ sync_event_class: Namespaces::SyncEvent,
+ source_sort_key: :id,
+ target_sort_key: :namespace_id
+ )
+ end
+
+ let(:table) { 'public.namespaces' }
+ let!(:namespace) { create(:namespace) }
+ let!(:namespace_mirror) { Ci::NamespaceMirror.find_by(namespace_id: namespace.id) }
+
+ context 'when both objects exist' do
+ it 'creates a Namespaces::SyncEvent to modify the target object' do
+ expect do
+ consistency_fix_service.execute(ids: [namespace.id])
+ end.to change {
+ Namespaces::SyncEvent.where(namespace_id: namespace.id).count
+ }.by(1)
+ end
+
+ it 'enqueues the worker to process the Namespaces::SyncEvents' do
+ expect(::Namespaces::ProcessSyncEventsWorker).to receive(:perform_async)
+ consistency_fix_service.execute(ids: [namespace.id])
+ end
+ end
+
+ context 'when the source object has been deleted, but not the target' do
+ before do
+ namespace.delete
+ end
+
+ it 'deletes the target object' do
+ expect do
+ consistency_fix_service.execute(ids: [namespace.id])
+ end.to change { Ci::NamespaceMirror.where(namespace_id: namespace.id).count }.by(-1)
+ end
+ end
+ end
+
+ context 'fixing projects inconsistencies' do
+ subject(:consistency_fix_service) do
+ described_class.new(
+ source_model: Project,
+ target_model: Ci::ProjectMirror,
+ sync_event_class: Projects::SyncEvent,
+ source_sort_key: :id,
+ target_sort_key: :project_id
+ )
+ end
+
+ let(:table) { 'public.projects' }
+ let!(:project) { create(:project) }
+ let!(:project_mirror) { Ci::ProjectMirror.find_by(project_id: project.id) }
+
+ context 'when both objects exist' do
+ it 'creates a Projects::SyncEvent to modify the target object' do
+ expect do
+ consistency_fix_service.execute(ids: [project.id])
+ end.to change {
+ Projects::SyncEvent.where(project_id: project.id).count
+ }.by(1)
+ end
+
+ it 'enqueues the worker to process the Projects::SyncEvents' do
+ expect(::Projects::ProcessSyncEventsWorker).to receive(:perform_async)
+ consistency_fix_service.execute(ids: [project.id])
+ end
+ end
+
+ context 'when the source object has been deleted, but not the target' do
+ before do
+ project.delete
+ end
+
+ it 'deletes the target object' do
+ expect do
+ consistency_fix_service.execute(ids: [project.id])
+ end.to change { Ci::ProjectMirror.where(project_id: project.id).count }.by(-1)
+ end
+ end
+ end
+ end
+
+ describe '#create_sync_event_for' do
+ context 'when the source model is Namespace' do
+ let(:namespace) { create(:namespace) }
+
+ let(:service) do
+ described_class.new(
+ source_model: Namespace,
+ target_model: Ci::NamespaceMirror,
+ sync_event_class: Namespaces::SyncEvent,
+ source_sort_key: :id,
+ target_sort_key: :namespace_id
+ )
+ end
+
+ it 'creates a Namespaces::SyncEvent object' do
+ expect do
+ service.send(:create_sync_event_for, namespace.id)
+ end.to change { Namespaces::SyncEvent.where(namespace_id: namespace.id).count }.by(1)
+ end
+ end
+
+ context 'when the source model is Project' do
+ let(:project) { create(:project) }
+
+ let(:service) do
+ described_class.new(
+ source_model: Project,
+ target_model: Ci::ProjectMirror,
+ sync_event_class: Projects::SyncEvent,
+ source_sort_key: :id,
+ target_sort_key: :project_id
+ )
+ end
+
+ it 'creates a Projects::SyncEvent object' do
+ expect do
+ service.send(:create_sync_event_for, project.id)
+ end.to change { Projects::SyncEvent.where(project_id: project.id).count }.by(1)
+ end
+ end
+ end
+
+ context 'when the source model is User' do
+ let(:service) do
+ described_class.new(
+ source_model: User,
+ target_model: Ci::ProjectMirror,
+ sync_event_class: Projects::SyncEvent,
+ source_sort_key: :id,
+ target_sort_key: :project_id
+ )
+ end
+
+ it 'raises an error' do
+ expect do
+ service.send(:create_sync_event_for, 1)
+ end.to raise_error("Unknown Source Model User")
+ end
+ end
+end
diff --git a/spec/services/dependency_proxy/group_settings/update_service_spec.rb b/spec/services/dependency_proxy/group_settings/update_service_spec.rb
index 6f8c55daa8d..4954d9ec267 100644
--- a/spec/services/dependency_proxy/group_settings/update_service_spec.rb
+++ b/spec/services/dependency_proxy/group_settings/update_service_spec.rb
@@ -42,7 +42,7 @@ RSpec.describe ::DependencyProxy::GroupSettings::UpdateService do
where(:user_role, :shared_examples_name) do
:maintainer | 'updating the dependency proxy group settings'
- :developer | 'updating the dependency proxy group settings'
+ :developer | 'denying access to dependency proxy group settings'
:reporter | 'denying access to dependency proxy group settings'
:guest | 'denying access to dependency proxy group settings'
:anonymous | 'denying access to dependency proxy group settings'
diff --git a/spec/services/dependency_proxy/image_ttl_group_policies/update_service_spec.rb b/spec/services/dependency_proxy/image_ttl_group_policies/update_service_spec.rb
index ceac8985c8e..3a6ba2cca71 100644
--- a/spec/services/dependency_proxy/image_ttl_group_policies/update_service_spec.rb
+++ b/spec/services/dependency_proxy/image_ttl_group_policies/update_service_spec.rb
@@ -72,7 +72,7 @@ RSpec.describe ::DependencyProxy::ImageTtlGroupPolicies::UpdateService do
where(:user_role, :shared_examples_name) do
:maintainer | 'updating the dependency proxy image ttl policy'
- :developer | 'updating the dependency proxy image ttl policy'
+ :developer | 'denying access to dependency proxy image ttl policy'
:reporter | 'denying access to dependency proxy image ttl policy'
:guest | 'denying access to dependency proxy image ttl policy'
:anonymous | 'denying access to dependency proxy image ttl policy'
@@ -92,7 +92,7 @@ RSpec.describe ::DependencyProxy::ImageTtlGroupPolicies::UpdateService do
where(:user_role, :shared_examples_name) do
:maintainer | 'creating the dependency proxy image ttl policy'
- :developer | 'creating the dependency proxy image ttl policy'
+ :developer | 'denying access to dependency proxy image ttl policy'
:reporter | 'denying access to dependency proxy image ttl policy'
:guest | 'denying access to dependency proxy image ttl policy'
:anonymous | 'denying access to dependency proxy image ttl policy'
@@ -108,7 +108,7 @@ RSpec.describe ::DependencyProxy::ImageTtlGroupPolicies::UpdateService do
context 'when the policy is not found' do
before do
- group.add_developer(user)
+ group.add_maintainer(user)
expect(group).to receive(:dependency_proxy_image_ttl_policy).and_return nil
end
diff --git a/spec/services/environments/stop_service_spec.rb b/spec/services/environments/stop_service_spec.rb
index 9e9ef127c67..afbc0ba70f9 100644
--- a/spec/services/environments/stop_service_spec.rb
+++ b/spec/services/environments/stop_service_spec.rb
@@ -161,8 +161,8 @@ RSpec.describe Environments::StopService do
end
end
- describe '#execute_for_merge_request' do
- subject { service.execute_for_merge_request(merge_request) }
+ describe '#execute_for_merge_request_pipeline' do
+ subject { service.execute_for_merge_request_pipeline(merge_request) }
let(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master') }
let(:project) { merge_request.project }
@@ -199,6 +199,19 @@ RSpec.describe Environments::StopService do
expect(pipeline.environments_in_self_and_descendants.first).to be_stopped
end
+ context 'when pipeline is a branch pipeline for merge request' do
+ let(:pipeline) do
+ create(:ci_pipeline, source: :push, project: project, sha: merge_request.diff_head_sha,
+ merge_requests_as_head_pipeline: [merge_request])
+ end
+
+ it 'does not stop the active environment' do
+ subject
+
+ expect(pipeline.environments_in_self_and_descendants.first).to be_available
+ end
+ end
+
context 'with environment related jobs ' do
let!(:environment) { create(:environment, :available, name: 'staging', project: project) }
let!(:prepare_staging_job) { create(:ci_build, :prepare_staging, pipeline: pipeline, project: project) }
@@ -210,18 +223,6 @@ RSpec.describe Environments::StopService do
expect(prepare_staging_job.persisted_environment.state).to eq('available')
end
-
- context 'when fix_related_environments_for_merge_requests feature flag is disabled' do
- before do
- stub_feature_flags(fix_related_environments_for_merge_requests: false)
- end
-
- it 'stops unrelated environments too' do
- subject
-
- expect(prepare_staging_job.persisted_environment.state).to eq('stopped')
- end
- end
end
end
diff --git a/spec/services/error_tracking/base_service_spec.rb b/spec/services/error_tracking/base_service_spec.rb
index 2f2052f0189..de3523cb847 100644
--- a/spec/services/error_tracking/base_service_spec.rb
+++ b/spec/services/error_tracking/base_service_spec.rb
@@ -4,8 +4,8 @@ require 'spec_helper'
RSpec.describe ErrorTracking::BaseService do
describe '#compose_response' do
- let(:project) { double('project') }
- let(:user) { double('user', id: non_existing_record_id) }
+ let(:project) { build_stubbed(:project) }
+ let(:user) { build_stubbed(:user, id: non_existing_record_id) }
let(:service) { described_class.new(project, user) }
it 'returns bad_request error when response has an error key' do
@@ -19,7 +19,10 @@ RSpec.describe ErrorTracking::BaseService do
end
it 'returns server error when response has missing key error_type' do
- data = { error: 'Unexpected Error', error_type: ErrorTracking::ProjectErrorTrackingSetting::SENTRY_API_ERROR_TYPE_MISSING_KEYS }
+ data = {
+ error: 'Unexpected Error',
+ error_type: ErrorTracking::ProjectErrorTrackingSetting::SENTRY_API_ERROR_TYPE_MISSING_KEYS
+ }
result = service.send(:compose_response, data)
@@ -48,7 +51,7 @@ RSpec.describe ErrorTracking::BaseService do
context 'when parse_response is implemented' do
before do
- expect(service).to receive(:parse_response) do |response|
+ allow(service).to receive(:parse_response) do |response|
{ animal: response[:thing] }
end
end
diff --git a/spec/services/error_tracking/collect_error_service_spec.rb b/spec/services/error_tracking/collect_error_service_spec.rb
index faca3c12a48..159c070c683 100644
--- a/spec/services/error_tracking/collect_error_service_spec.rb
+++ b/spec/services/error_tracking/collect_error_service_spec.rb
@@ -52,12 +52,13 @@ RSpec.describe ErrorTracking::CollectErrorService do
end
context 'with unusual payload' do
- let(:modified_event) { parsed_event }
- let(:event) { described_class.new(project, nil, event: modified_event).execute }
+ let(:event) { ErrorTracking::ErrorEvent.last! }
context 'when transaction is missing' do
it 'builds actor from stacktrace' do
- modified_event.delete('transaction')
+ parsed_event.delete('transaction')
+
+ subject.execute
expect(event.error.actor).to eq 'find()'
end
@@ -65,7 +66,9 @@ RSpec.describe ErrorTracking::CollectErrorService do
context 'when transaction is an empty string' do \
it 'builds actor from stacktrace' do
- modified_event['transaction'] = ''
+ parsed_event['transaction'] = ''
+
+ subject.execute
expect(event.error.actor).to eq 'find()'
end
@@ -73,7 +76,9 @@ RSpec.describe ErrorTracking::CollectErrorService do
context 'when timestamp is numeric' do
it 'parses timestamp' do
- modified_event['timestamp'] = '1631015580.50'
+ parsed_event['timestamp'] = '1631015580.50'
+
+ subject.execute
expect(event.occurred_at).to eq '2021-09-07T11:53:00.5'
end
diff --git a/spec/services/error_tracking/issue_details_service_spec.rb b/spec/services/error_tracking/issue_details_service_spec.rb
index 8cc2688d198..29f8154a27c 100644
--- a/spec/services/error_tracking/issue_details_service_spec.rb
+++ b/spec/services/error_tracking/issue_details_service_spec.rb
@@ -14,7 +14,7 @@ RSpec.describe ErrorTracking::IssueDetailsService do
let(:params) { { issue_id: detailed_error.id } }
before do
- expect(error_tracking_setting)
+ allow(error_tracking_setting)
.to receive(:issue_details).and_return(issue: detailed_error)
end
@@ -40,7 +40,7 @@ RSpec.describe ErrorTracking::IssueDetailsService do
include_examples 'error tracking service sentry error handling', :issue_details
include_examples 'error tracking service http status handling', :issue_details
- context 'integrated error tracking' do
+ context 'with integrated error tracking' do
let_it_be(:error) { create(:error_tracking_error, project: project) }
let(:params) { { issue_id: error.id } }
@@ -53,6 +53,18 @@ RSpec.describe ErrorTracking::IssueDetailsService do
expect(result[:status]).to eq(:success)
expect(result[:issue].to_json).to eq(error.to_sentry_detailed_error.to_json)
end
+
+ context 'when error does not exist' do
+ let(:params) { { issue_id: non_existing_record_id } }
+
+ it 'returns the error in detailed format' do
+ expect(result).to match(
+ status: :error,
+ message: /Couldn't find ErrorTracking::Error/,
+ http_status: :bad_request
+ )
+ end
+ end
end
end
diff --git a/spec/services/error_tracking/issue_latest_event_service_spec.rb b/spec/services/error_tracking/issue_latest_event_service_spec.rb
index e914cb1241e..aa2430ddffb 100644
--- a/spec/services/error_tracking/issue_latest_event_service_spec.rb
+++ b/spec/services/error_tracking/issue_latest_event_service_spec.rb
@@ -15,7 +15,7 @@ RSpec.describe ErrorTracking::IssueLatestEventService do
let(:error_event) { build(:error_tracking_sentry_error_event) }
before do
- expect(error_tracking_setting)
+ allow(error_tracking_setting)
.to receive(:issue_latest_event).and_return(latest_event: error_event)
end
@@ -28,7 +28,7 @@ RSpec.describe ErrorTracking::IssueLatestEventService do
include_examples 'error tracking service sentry error handling', :issue_latest_event
include_examples 'error tracking service http status handling', :issue_latest_event
- context 'integrated error tracking' do
+ context 'with integrated error tracking' do
let_it_be(:error) { create(:error_tracking_error, project: project) }
let_it_be(:event) { create(:error_tracking_error_event, error: error) }
@@ -42,6 +42,18 @@ RSpec.describe ErrorTracking::IssueLatestEventService do
expect(result[:status]).to eq(:success)
expect(result[:latest_event].to_json).to eq(event.to_sentry_error_event.to_json)
end
+
+ context 'when error does not exist' do
+ let(:params) { { issue_id: non_existing_record_id } }
+
+ it 'returns the error in detailed format' do
+ expect(result).to match(
+ status: :error,
+ message: /Couldn't find ErrorTracking::Error/,
+ http_status: :bad_request
+ )
+ end
+ end
end
end
diff --git a/spec/services/error_tracking/issue_update_service_spec.rb b/spec/services/error_tracking/issue_update_service_spec.rb
index 31a66654100..a06c3588264 100644
--- a/spec/services/error_tracking/issue_update_service_spec.rb
+++ b/spec/services/error_tracking/issue_update_service_spec.rb
@@ -13,8 +13,7 @@ RSpec.describe ErrorTracking::IssueUpdateService do
it 'does not call the close issue service' do
update_service.execute
- expect(issue_close_service)
- .not_to have_received(:execute)
+ expect(issue_close_service).not_to have_received(:execute)
end
it 'does not create system note' do
@@ -29,8 +28,7 @@ RSpec.describe ErrorTracking::IssueUpdateService do
let(:update_issue_response) { { updated: true } }
before do
- expect(error_tracking_setting)
- .to receive(:update_issue).and_return(update_issue_response)
+ allow(error_tracking_setting).to receive(:update_issue).and_return(update_issue_response)
end
it 'returns the response' do
@@ -49,12 +47,11 @@ RSpec.describe ErrorTracking::IssueUpdateService do
result
end
- context 'related issue and resolving' do
+ context 'with related issue and resolving' do
let(:issue) { create(:issue, project: project) }
let(:sentry_issue) { create(:sentry_issue, issue: issue) }
let(:arguments) { { issue_id: sentry_issue.sentry_issue_identifier, status: 'resolved' } }
-
- let(:issue_close_service) { spy(:issue_close_service) }
+ let(:issue_close_service) { instance_double('Issues::CloseService') }
before do
allow_next_instance_of(SentryIssueFinder) do |finder|
@@ -78,11 +75,11 @@ RSpec.describe ErrorTracking::IssueUpdateService do
.with(issue, system_note: false)
end
- context 'issues gets closed' do
+ context 'when issue gets closed' do
let(:closed_issue) { create(:issue, :closed, project: project) }
before do
- expect(issue_close_service)
+ allow(issue_close_service)
.to receive(:execute)
.with(issue, system_note: false)
.and_return(closed_issue)
@@ -99,13 +96,13 @@ RSpec.describe ErrorTracking::IssueUpdateService do
end
end
- context 'issue is already closed' do
+ context 'when issue is already closed' do
let(:issue) { create(:issue, :closed, project: project) }
include_examples 'does not perform close issue flow'
end
- context 'status is not resolving' do
+ context 'when status is not resolving' do
let(:arguments) { { issue_id: sentry_issue.sentry_issue_identifier, status: 'ignored' } }
include_examples 'does not perform close issue flow'
@@ -115,7 +112,7 @@ RSpec.describe ErrorTracking::IssueUpdateService do
include_examples 'error tracking service sentry error handling', :update_issue
- context 'integrated error tracking' do
+ context 'with integrated error tracking' do
let(:error) { create(:error_tracking_error, project: project) }
let(:arguments) { { issue_id: error.id, status: 'resolved' } }
let(:update_issue_response) { { updated: true, status: :success, closed_issue_iid: nil } }
diff --git a/spec/services/groups/group_links/create_service_spec.rb b/spec/services/groups/group_links/create_service_spec.rb
index 03dac14be54..bfbaedbd06f 100644
--- a/spec/services/groups/group_links/create_service_spec.rb
+++ b/spec/services/groups/group_links/create_service_spec.rb
@@ -3,23 +3,13 @@
require 'spec_helper'
RSpec.describe Groups::GroupLinks::CreateService, '#execute' do
- let(:parent_group_user) { create(:user) }
- let(:group_user) { create(:user) }
- let(:child_group_user) { create(:user) }
- let(:prevent_sharing) { false }
+ let_it_be(:shared_with_group_parent) { create(:group, :private) }
+ let_it_be(:shared_with_group) { create(:group, :private, parent: shared_with_group_parent) }
+ let_it_be(:shared_with_group_child) { create(:group, :private, parent: shared_with_group) }
let_it_be(:group_parent) { create(:group, :private) }
- let_it_be(:group) { create(:group, :private, parent: group_parent) }
- let_it_be(:group_child) { create(:group, :private, parent: group) }
- let(:ns_for_parent) { create(:namespace_settings, prevent_sharing_groups_outside_hierarchy: prevent_sharing) }
- let(:shared_group_parent) { create(:group, :private, namespace_settings: ns_for_parent) }
- let(:shared_group) { create(:group, :private, parent: shared_group_parent) }
- let(:shared_group_child) { create(:group, :private, parent: shared_group) }
-
- let(:project_parent) { create(:project, group: shared_group_parent) }
- let(:project) { create(:project, group: shared_group) }
- let(:project_child) { create(:project, group: shared_group_child) }
+ let(:group) { create(:group, :private, parent: group_parent) }
let(:opts) do
{
@@ -28,127 +18,161 @@ RSpec.describe Groups::GroupLinks::CreateService, '#execute' do
}
end
- let(:user) { group_user }
+ subject { described_class.new(group, shared_with_group, user, opts) }
- subject { described_class.new(shared_group, group, user, opts) }
+ shared_examples_for 'not shareable' do
+ it 'does not share and returns an error' do
+ expect do
+ result = subject.execute
- before do
- group.add_guest(group_user)
- shared_group.add_owner(group_user)
+ expect(result[:status]).to eq(:error)
+ expect(result[:http_status]).to eq(404)
+ end.not_to change { group.shared_with_group_links.count }
+ end
end
- it 'adds group to another group' do
- expect { subject.execute }.to change { group.shared_group_links.count }.from(0).to(1)
- end
+ shared_examples_for 'shareable' do
+ it 'adds group to another group' do
+ expect do
+ result = subject.execute
- it 'returns false if shared group is blank' do
- expect { described_class.new(nil, group, user, opts) }.not_to change { group.shared_group_links.count }
+ expect(result[:status]).to eq(:success)
+ end.to change { group.shared_with_group_links.count }.from(0).to(1)
+ end
end
- context 'user does not have access to group' do
- let(:user) { create(:user) }
-
- before do
- shared_group.add_owner(user)
- end
+ context 'when user has proper membership to share a group' do
+ let_it_be(:group_user) { create(:user) }
- it 'returns error' do
- result = subject.execute
+ let(:user) { group_user }
- expect(result[:status]).to eq(:error)
- expect(result[:http_status]).to eq(404)
+ before do
+ shared_with_group.add_guest(group_user)
+ group.add_owner(group_user)
end
- end
- context 'user does not have admin access to shared group' do
- let(:user) { create(:user) }
+ it_behaves_like 'shareable'
- before do
- group.add_guest(user)
- shared_group.add_developer(user)
- end
+ context 'when sharing outside the hierarchy is disabled' do
+ let_it_be(:group_parent) do
+ create(:group,
+ namespace_settings: create(:namespace_settings, prevent_sharing_groups_outside_hierarchy: true))
+ end
- it 'returns error' do
- result = subject.execute
+ it_behaves_like 'not shareable'
- expect(result[:status]).to eq(:error)
- expect(result[:http_status]).to eq(404)
- end
- end
+ context 'when group is inside hierarchy' do
+ let(:shared_with_group) { create(:group, :private, parent: group_parent) }
- context 'project authorizations based on group hierarchies' do
- before do
- group_parent.add_owner(parent_group_user)
- group.add_owner(group_user)
- group_child.add_owner(child_group_user)
+ it_behaves_like 'shareable'
+ end
end
- context 'project authorizations refresh' do
- it 'is executed only for the direct members of the group' do
- expect(UserProjectAccessChangedService).to receive(:new).with(contain_exactly(group_user.id)).and_call_original
+ context 'project authorizations based on group hierarchies' do
+ let_it_be(:child_group_user) { create(:user) }
+ let_it_be(:parent_group_user) { create(:user) }
- subject.execute
+ before do
+ shared_with_group_parent.add_owner(parent_group_user)
+ shared_with_group.add_owner(group_user)
+ shared_with_group_child.add_owner(child_group_user)
end
- end
- context 'project authorizations' do
- context 'group user' do
- let(:user) { group_user }
+ context 'project authorizations refresh' do
+ it 'is executed only for the direct members of the group' do
+ expect(UserProjectAccessChangedService).to receive(:new).with(contain_exactly(group_user.id))
+ .and_call_original
- it 'create proper authorizations' do
subject.execute
-
- expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey
- expect(Ability.allowed?(user, :read_project, project)).to be_truthy
- expect(Ability.allowed?(user, :read_project, project_child)).to be_truthy
end
end
- context 'parent group user' do
- let(:user) { parent_group_user }
+ context 'project authorizations' do
+ let(:group_child) { create(:group, :private, parent: group) }
+ let(:project_parent) { create(:project, group: group_parent) }
+ let(:project) { create(:project, group: group) }
+ let(:project_child) { create(:project, group: group_child) }
- it 'create proper authorizations' do
- subject.execute
+ context 'group user' do
+ let(:user) { group_user }
+
+ it 'create proper authorizations' do
+ subject.execute
- expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey
- expect(Ability.allowed?(user, :read_project, project)).to be_falsey
- expect(Ability.allowed?(user, :read_project, project_child)).to be_falsey
+ expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey
+ expect(Ability.allowed?(user, :read_project, project)).to be_truthy
+ expect(Ability.allowed?(user, :read_project, project_child)).to be_truthy
+ end
end
- end
- context 'child group user' do
- let(:user) { child_group_user }
+ context 'parent group user' do
+ let(:user) { parent_group_user }
- it 'create proper authorizations' do
- subject.execute
+ it 'create proper authorizations' do
+ subject.execute
+
+ expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey
+ expect(Ability.allowed?(user, :read_project, project)).to be_falsey
+ expect(Ability.allowed?(user, :read_project, project_child)).to be_falsey
+ end
+ end
- expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey
- expect(Ability.allowed?(user, :read_project, project)).to be_falsey
- expect(Ability.allowed?(user, :read_project, project_child)).to be_falsey
+ context 'child group user' do
+ let(:user) { child_group_user }
+
+ it 'create proper authorizations' do
+ subject.execute
+
+ expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey
+ expect(Ability.allowed?(user, :read_project, project)).to be_falsey
+ expect(Ability.allowed?(user, :read_project, project_child)).to be_falsey
+ end
end
end
end
end
- context 'sharing outside the hierarchy is disabled' do
- let(:prevent_sharing) { true }
+ context 'user does not have access to group' do
+ let(:user) { create(:user) }
- it 'prevents sharing with a group outside the hierarchy' do
- result = subject.execute
+ before do
+ group.add_owner(user)
+ end
- expect(group.reload.shared_group_links.count).to eq(0)
- expect(result[:status]).to eq(:error)
- expect(result[:http_status]).to eq(404)
+ it_behaves_like 'not shareable'
+ end
+
+ context 'user does not have admin access to shared group' do
+ let(:user) { create(:user) }
+
+ before do
+ shared_with_group.add_guest(user)
+ group.add_developer(user)
end
- it 'allows sharing with a group within the hierarchy' do
- sibling_group = create(:group, :private, parent: shared_group_parent)
- sibling_group.add_guest(group_user)
+ it_behaves_like 'not shareable'
+ end
+
+ context 'when group is blank' do
+ let(:group_user) { create(:user) }
+ let(:user) { group_user }
+ let(:group) { nil }
- result = described_class.new(shared_group, sibling_group, user, opts).execute
+ it 'does not share and returns an error' do
+ expect do
+ result = subject.execute
- expect(sibling_group.reload.shared_group_links.count).to eq(1)
- expect(result[:status]).to eq(:success)
+ expect(result[:status]).to eq(:error)
+ expect(result[:http_status]).to eq(404)
+ end.not_to change { shared_with_group.shared_group_links.count }
end
end
+
+ context 'when shared_with_group is blank' do
+ let(:group_user) { create(:user) }
+ let(:user) { group_user }
+ let(:shared_with_group) { nil }
+
+ it_behaves_like 'not shareable'
+ end
end
diff --git a/spec/services/groups/group_links/destroy_service_spec.rb b/spec/services/groups/group_links/destroy_service_spec.rb
index e63adc07313..6aaf5f45069 100644
--- a/spec/services/groups/group_links/destroy_service_spec.rb
+++ b/spec/services/groups/group_links/destroy_service_spec.rb
@@ -3,54 +3,77 @@
require 'spec_helper'
RSpec.describe Groups::GroupLinks::DestroyService, '#execute' do
- let(:user) { create(:user) }
-
+ let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group, :private) }
let_it_be(:shared_group) { create(:group, :private) }
let_it_be(:project) { create(:project, group: shared_group) }
let_it_be(:owner) { create(:user) }
- before do
- group.add_developer(owner)
- shared_group.add_owner(owner)
- end
-
subject { described_class.new(shared_group, owner) }
- context 'single link' do
- let!(:link) { create(:group_group_link, shared_group: shared_group, shared_with_group: group) }
+ context 'when authorizing by user' do
+ before do
+ group.add_developer(owner)
+ shared_group.add_owner(owner)
+ end
+
+ context 'single link' do
+ let!(:link) { create(:group_group_link, shared_group: shared_group, shared_with_group: group) }
- it 'destroys link' do
- expect { subject.execute(link) }.to change { shared_group.shared_with_group_links.count }.from(1).to(0)
+ it 'destroys the link' do
+ expect { subject.execute(link) }.to change { shared_group.shared_with_group_links.count }.from(1).to(0)
+ end
+
+ it 'revokes project authorization', :sidekiq_inline do
+ group.add_developer(user)
+
+ expect { subject.execute(link) }.to(
+ change { Ability.allowed?(user, :read_project, project) }.from(true).to(false))
+ end
end
- it 'revokes project authorization', :sidekiq_inline do
- group.add_developer(user)
+ context 'multiple links' do
+ let_it_be(:another_group) { create(:group, :private) }
+ let_it_be(:another_shared_group) { create(:group, :private) }
+
+ let!(:links) do
+ [
+ create(:group_group_link, shared_group: shared_group, shared_with_group: group),
+ create(:group_group_link, shared_group: shared_group, shared_with_group: another_group),
+ create(:group_group_link, shared_group: another_shared_group, shared_with_group: group),
+ create(:group_group_link, shared_group: another_shared_group, shared_with_group: another_group)
+ ]
+ end
- expect { subject.execute(link) }.to(
- change { Ability.allowed?(user, :read_project, project) }.from(true).to(false))
+ it 'updates project authorization once per group' do
+ expect(GroupGroupLink).to receive(:delete).and_call_original
+ expect(group).to receive(:refresh_members_authorized_projects).with(direct_members_only: true, blocking: false).once
+ expect(another_group).to receive(:refresh_members_authorized_projects).with(direct_members_only: true, blocking: false).once
+
+ subject.execute(links)
+ end
end
end
- context 'multiple links' do
- let_it_be(:another_group) { create(:group, :private) }
- let_it_be(:another_shared_group) { create(:group, :private) }
-
- let!(:links) do
- [
- create(:group_group_link, shared_group: shared_group, shared_with_group: group),
- create(:group_group_link, shared_group: shared_group, shared_with_group: another_group),
- create(:group_group_link, shared_group: another_shared_group, shared_with_group: group),
- create(:group_group_link, shared_group: another_shared_group, shared_with_group: another_group)
- ]
+ context 'when skipping authorization' do
+ let!(:link) { create(:group_group_link, shared_group: shared_group, shared_with_group: group) }
+
+ context 'with provided group and owner' do
+ it 'destroys the link' do
+ expect do
+ subject.execute(link, skip_authorization: true)
+ end.to change { shared_group.shared_with_group_links.count }.from(1).to(0)
+ end
end
- it 'updates project authorization once per group' do
- expect(GroupGroupLink).to receive(:delete).and_call_original
- expect(group).to receive(:refresh_members_authorized_projects).with(direct_members_only: true, blocking: false).once
- expect(another_group).to receive(:refresh_members_authorized_projects).with(direct_members_only: true, blocking: false).once
+ context 'without providing group or owner' do
+ subject { described_class.new(nil, nil) }
- subject.execute(links)
+ it 'destroys the link' do
+ expect do
+ subject.execute(link, skip_authorization: true)
+ end.to change { shared_group.shared_with_group_links.count }.from(1).to(0)
+ end
end
end
end
diff --git a/spec/services/groups/open_issues_count_service_spec.rb b/spec/services/groups/open_issues_count_service_spec.rb
index 7dd8c2a59a0..fca09bfdebe 100644
--- a/spec/services/groups/open_issues_count_service_spec.rb
+++ b/spec/services/groups/open_issues_count_service_spec.rb
@@ -3,18 +3,12 @@
require 'spec_helper'
RSpec.describe Groups::OpenIssuesCountService, :use_clean_rails_memory_store_caching do
- let_it_be(:group) { create(:group, :public) }
+ let_it_be(:group) { create(:group, :public)}
let_it_be(:project) { create(:project, :public, namespace: group) }
- let_it_be(:admin) { create(:user, :admin) }
let_it_be(:user) { create(:user) }
- let_it_be(:banned_user) { create(:user, :banned) }
-
- before do
- create(:issue, :opened, project: project)
- create(:issue, :opened, confidential: true, project: project)
- create(:issue, :opened, author: banned_user, project: project)
- create(:issue, :closed, project: project)
- end
+ let_it_be(:issue) { create(:issue, :opened, project: project) }
+ let_it_be(:confidential) { create(:issue, :opened, confidential: true, project: project) }
+ let_it_be(:closed) { create(:issue, :closed, project: project) }
subject { described_class.new(group, user) }
@@ -26,27 +20,17 @@ RSpec.describe Groups::OpenIssuesCountService, :use_clean_rails_memory_store_cac
it 'uses the IssuesFinder to scope issues' do
expect(IssuesFinder)
.to receive(:new)
- .with(user, group_id: group.id, state: 'opened', non_archived: true, include_subgroups: true, public_only: true, include_hidden: false)
+ .with(user, group_id: group.id, state: 'opened', non_archived: true, include_subgroups: true, public_only: true)
subject.count
end
end
describe '#count' do
- shared_examples 'counts public issues, does not count hidden or confidential' do
- it 'counts only public issues' do
- expect(subject.count).to eq(1)
- end
-
- it 'uses PUBLIC_COUNT_WITHOUT_HIDDEN_KEY cache key' do
- expect(subject.cache_key).to include('group_open_public_issues_without_hidden_count')
- end
- end
-
context 'when user is nil' do
- let(:user) { nil }
-
- it_behaves_like 'counts public issues, does not count hidden or confidential'
+ it 'does not include confidential issues in the issue count' do
+ expect(described_class.new(group).count).to eq(1)
+ end
end
context 'when user is provided' do
@@ -55,13 +39,9 @@ RSpec.describe Groups::OpenIssuesCountService, :use_clean_rails_memory_store_cac
group.add_reporter(user)
end
- it 'includes confidential issues and does not include hidden issues in count' do
+ it 'returns the right count with confidential issues' do
expect(subject.count).to eq(2)
end
-
- it 'uses TOTAL_COUNT_WITHOUT_HIDDEN_KEY cache key' do
- expect(subject.cache_key).to include('group_open_issues_without_hidden_count')
- end
end
context 'when user cannot read confidential issues' do
@@ -69,24 +49,8 @@ RSpec.describe Groups::OpenIssuesCountService, :use_clean_rails_memory_store_cac
group.add_guest(user)
end
- it_behaves_like 'counts public issues, does not count hidden or confidential'
- end
-
- context 'when user is an admin' do
- let(:user) { admin }
-
- context 'when admin mode is enabled', :enable_admin_mode do
- it 'includes confidential and hidden issues in count' do
- expect(subject.count).to eq(3)
- end
-
- it 'uses TOTAL_COUNT_KEY cache key' do
- expect(subject.cache_key).to include('group_open_issues_including_hidden_count')
- end
- end
-
- context 'when admin mode is disabled' do
- it_behaves_like 'counts public issues, does not count hidden or confidential'
+ it 'does not include confidential issues' do
+ expect(subject.count).to eq(1)
end
end
@@ -97,13 +61,11 @@ RSpec.describe Groups::OpenIssuesCountService, :use_clean_rails_memory_store_cac
describe '#clear_all_cache_keys' do
it 'calls `Rails.cache.delete` with the correct keys' do
expect(Rails.cache).to receive(:delete)
- .with(['groups', 'open_issues_count_service', 1, group.id, described_class::PUBLIC_COUNT_WITHOUT_HIDDEN_KEY])
+ .with(['groups', 'open_issues_count_service', 1, group.id, described_class::PUBLIC_COUNT_KEY])
expect(Rails.cache).to receive(:delete)
.with(['groups', 'open_issues_count_service', 1, group.id, described_class::TOTAL_COUNT_KEY])
- expect(Rails.cache).to receive(:delete)
- .with(['groups', 'open_issues_count_service', 1, group.id, described_class::TOTAL_COUNT_WITHOUT_HIDDEN_KEY])
- described_class.new(group).clear_all_cache_keys
+ subject.clear_all_cache_keys
end
end
end
diff --git a/spec/services/groups/transfer_service_spec.rb b/spec/services/groups/transfer_service_spec.rb
index 1c4b7aac87e..20ea8b2bf1b 100644
--- a/spec/services/groups/transfer_service_spec.rb
+++ b/spec/services/groups/transfer_service_spec.rb
@@ -574,7 +574,7 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do
context 'resets project authorizations' do
let_it_be(:old_parent_group) { create(:group) }
- let_it_be_with_reload(:group) { create(:group, :private, parent: old_parent_group) }
+ let_it_be_with_refind(:group) { create(:group, :private, parent: old_parent_group) }
let_it_be(:new_group_member) { create(:user) }
let_it_be(:old_group_member) { create(:user) }
let_it_be(:unique_subgroup_member) { create(:user) }
diff --git a/spec/services/import/bitbucket_server_service_spec.rb b/spec/services/import/bitbucket_server_service_spec.rb
index 56d93625b91..0b9fe10e95a 100644
--- a/spec/services/import/bitbucket_server_service_spec.rb
+++ b/spec/services/import/bitbucket_server_service_spec.rb
@@ -48,6 +48,23 @@ RSpec.describe Import::BitbucketServerService do
end
end
+ context 'when import source is disabled' do
+ before do
+ stub_application_setting(import_sources: nil)
+ allow(subject).to receive(:authorized?).and_return(true)
+ allow(client).to receive(:repo).with(project_key, repo_slug).and_return(double(repo))
+ end
+
+ it 'returns forbidden' do
+ result = subject.execute(credentials)
+
+ expect(result).to include(
+ status: :error,
+ http_status: :forbidden
+ )
+ end
+ end
+
context 'when user is unauthorized' do
before do
allow(subject).to receive(:authorized?).and_return(false)
diff --git a/spec/services/import/github_service_spec.rb b/spec/services/import/github_service_spec.rb
index 58afae1e647..1c26677cfa5 100644
--- a/spec/services/import/github_service_spec.rb
+++ b/spec/services/import/github_service_spec.rb
@@ -111,6 +111,33 @@ RSpec.describe Import::GithubService do
end
end
+ context 'when import source is disabled' do
+ let(:repository_double) do
+ double({
+ name: 'vim',
+ description: 'test',
+ full_name: 'test/vim',
+ clone_url: 'http://repo.com/repo/repo.git',
+ private: false,
+ has_wiki?: false
+ })
+ end
+
+ before do
+ stub_application_setting(import_sources: nil)
+ allow(client).to receive(:repository).and_return(repository_double)
+ end
+
+ it 'returns forbidden' do
+ result = subject.execute(access_params, :github)
+
+ expect(result).to include(
+ status: :error,
+ http_status: :forbidden
+ )
+ end
+ end
+
context 'when a blocked/local URL is used as github_hostname' do
let(:message) { 'Error while attempting to import from GitHub' }
let(:error) { "Invalid URL: #{url}" }
diff --git a/spec/services/incident_management/timeline_events/create_service_spec.rb b/spec/services/incident_management/timeline_events/create_service_spec.rb
new file mode 100644
index 00000000000..38ce15e74f1
--- /dev/null
+++ b/spec/services/incident_management/timeline_events/create_service_spec.rb
@@ -0,0 +1,117 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe IncidentManagement::TimelineEvents::CreateService do
+ let_it_be(:user_with_permissions) { create(:user) }
+ let_it_be(:user_without_permissions) { create(:user) }
+ let_it_be(:project) { create(:project) }
+ let_it_be_with_refind(:incident) { create(:incident, project: project) }
+ let_it_be(:comment) { create(:note, project: project, noteable: incident) }
+
+ let(:args) do
+ {
+ note: 'note',
+ occurred_at: Time.current,
+ action: 'new comment',
+ promoted_from_note: comment
+ }
+ end
+
+ let(:current_user) { user_with_permissions }
+ let(:service) { described_class.new(incident, current_user, args) }
+
+ before_all do
+ project.add_developer(user_with_permissions)
+ project.add_reporter(user_without_permissions)
+ end
+
+ describe '#execute' do
+ shared_examples 'error response' do |message|
+ it 'has an informative message' do
+ expect(execute).to be_error
+ expect(execute.message).to eq(message)
+ end
+ end
+
+ shared_examples 'success response' do
+ it 'has timeline event', :aggregate_failures do
+ expect(execute).to be_success
+
+ result = execute.payload[:timeline_event]
+ expect(result).to be_a(::IncidentManagement::TimelineEvent)
+ expect(result.author).to eq(current_user)
+ expect(result.incident).to eq(incident)
+ expect(result.project).to eq(project)
+ expect(result.note).to eq(args[:note])
+ expect(result.promoted_from_note).to eq(comment)
+ end
+ end
+
+ subject(:execute) { service.execute }
+
+ context 'when current user is blank' do
+ let(:current_user) { nil }
+
+ it_behaves_like 'error response', 'You have insufficient permissions to manage timeline events for this incident'
+ end
+
+ context 'when user does not have permissions to create timeline events' do
+ let(:current_user) { user_without_permissions }
+
+ it_behaves_like 'error response', 'You have insufficient permissions to manage timeline events for this incident'
+ end
+
+ context 'when error occurs during creation' do
+ let(:args) { {} }
+
+ it_behaves_like 'error response', "Occurred at can't be blank, Note can't be blank, and Note html can't be blank"
+ end
+
+ context 'with default action' do
+ let(:args) { { note: 'note', occurred_at: Time.current, promoted_from_note: comment } }
+
+ it_behaves_like 'success response'
+
+ it 'matches the default action', :aggregate_failures do
+ result = execute.payload[:timeline_event]
+
+ expect(result.action).to eq(IncidentManagement::TimelineEvents::DEFAULT_ACTION)
+ end
+ end
+
+ context 'with non_default action' do
+ it_behaves_like 'success response'
+
+ it 'matches the action from arguments', :aggregate_failures do
+ result = execute.payload[:timeline_event]
+
+ expect(result.action).to eq(args[:action])
+ end
+ end
+
+ it 'successfully creates a database record', :aggregate_failures do
+ expect { execute }.to change { ::IncidentManagement::TimelineEvent.count }.by(1)
+ end
+
+ context 'when incident_timeline feature flag is enabled' do
+ before do
+ stub_feature_flags(incident_timeline: project)
+ end
+
+ it 'creates a system note' do
+ expect { execute }.to change { incident.notes.reload.count }.by(1)
+ end
+ end
+
+ context 'when incident_timeline feature flag is disabled' do
+ before do
+ stub_feature_flags(incident_timeline: false)
+ end
+
+ it 'does not create a system note' do
+ expect { execute }.not_to change { incident.notes.reload.count }
+ end
+ end
+ end
+end
diff --git a/spec/services/incident_management/timeline_events/destroy_service_spec.rb b/spec/services/incident_management/timeline_events/destroy_service_spec.rb
new file mode 100644
index 00000000000..01daee2b749
--- /dev/null
+++ b/spec/services/incident_management/timeline_events/destroy_service_spec.rb
@@ -0,0 +1,80 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe IncidentManagement::TimelineEvents::DestroyService do
+ let_it_be(:user_with_permissions) { create(:user) }
+ let_it_be(:user_without_permissions) { create(:user) }
+ let_it_be(:project) { create(:project) }
+ let_it_be_with_refind(:incident) { create(:incident, project: project) }
+
+ let!(:timeline_event) { create(:incident_management_timeline_event, incident: incident, project: project) }
+ let(:current_user) { user_with_permissions }
+ let(:params) { {} }
+ let(:service) { described_class.new(timeline_event, current_user) }
+
+ before_all do
+ project.add_developer(user_with_permissions)
+ project.add_reporter(user_without_permissions)
+ end
+
+ describe '#execute' do
+ shared_examples 'error response' do |message|
+ it 'has an informative message' do
+ expect(execute).to be_error
+ expect(execute.message).to eq(message)
+ end
+ end
+
+ subject(:execute) { service.execute }
+
+ context 'when current user is anonymous' do
+ let(:current_user) { nil }
+
+ it_behaves_like 'error response', 'You have insufficient permissions to manage timeline events for this incident'
+ end
+
+ context 'when user does not have permissions to remove timeline events' do
+ let(:current_user) { user_without_permissions }
+
+ it_behaves_like 'error response', 'You have insufficient permissions to manage timeline events for this incident'
+ end
+
+ context 'when an error occurs during removal' do
+ before do
+ allow(timeline_event).to receive(:destroy).and_return(false)
+ timeline_event.errors.add(:note, 'cannot be removed')
+ end
+
+ it_behaves_like 'error response', 'Note cannot be removed'
+ end
+
+ it 'successfully returns the timeline event', :aggregate_failures do
+ expect(execute).to be_success
+
+ result = execute.payload[:timeline_event]
+ expect(result).to be_a(::IncidentManagement::TimelineEvent)
+ expect(result.id).to eq(timeline_event.id)
+ end
+
+ context 'when incident_timeline feature flag is enabled' do
+ before do
+ stub_feature_flags(incident_timeline: project)
+ end
+
+ it 'creates a system note' do
+ expect { execute }.to change { incident.notes.reload.count }.by(1)
+ end
+ end
+
+ context 'when incident_timeline feature flag is disabled' do
+ before do
+ stub_feature_flags(incident_timeline: false)
+ end
+
+ it 'does not create a system note' do
+ expect { execute }.not_to change { incident.notes.reload.count }
+ end
+ end
+ end
+end
diff --git a/spec/services/incident_management/timeline_events/update_service_spec.rb b/spec/services/incident_management/timeline_events/update_service_spec.rb
new file mode 100644
index 00000000000..8bc0e5ce0ed
--- /dev/null
+++ b/spec/services/incident_management/timeline_events/update_service_spec.rb
@@ -0,0 +1,148 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe IncidentManagement::TimelineEvents::UpdateService do
+ let_it_be(:user) { create(:user) }
+ let_it_be(:project) { create(:project) }
+ let_it_be(:incident) { create(:incident, project: project) }
+
+ let!(:timeline_event) { create(:incident_management_timeline_event, project: project, incident: incident) }
+ let(:occurred_at) { 1.minute.ago }
+ let(:params) { { note: 'Updated note', occurred_at: occurred_at } }
+
+ before do
+ stub_feature_flags(incident_timeline: project)
+ end
+
+ describe '#execute' do
+ shared_examples 'successful response' do
+ it 'responds with success', :aggregate_failures do
+ expect(execute).to be_success
+ expect(execute.payload).to eq(timeline_event: timeline_event.reload)
+ end
+ end
+
+ shared_examples 'error response' do |message|
+ it 'has an informative message' do
+ expect(execute).to be_error
+ expect(execute.message).to eq(message)
+ end
+ end
+
+ shared_examples 'passing the correct was_changed value' do |was_changed|
+ it 'passes the correct was_changed value into SysteNoteService.edit_timeline_event' do
+ expect(SystemNoteService)
+ .to receive(:edit_timeline_event)
+ .with(timeline_event, user, was_changed: was_changed)
+ .and_call_original
+
+ execute
+ end
+ end
+
+ subject(:execute) { described_class.new(timeline_event, user, params).execute }
+
+ context 'when user has permissions' do
+ before do
+ project.add_developer(user)
+ end
+
+ it_behaves_like 'successful response'
+
+ it 'updates attributes' do
+ expect { execute }.to change { timeline_event.note }.to(params[:note])
+ .and change { timeline_event.occurred_at }.to(params[:occurred_at])
+ end
+
+ it 'creates a system note' do
+ expect { execute }.to change { incident.notes.reload.count }.by(1)
+ end
+
+ it_behaves_like 'passing the correct was_changed value', :occurred_at_and_note
+
+ context 'when incident_timeline feature flag is disabled' do
+ before do
+ stub_feature_flags(incident_timeline: false)
+ end
+
+ it 'does not add a system note' do
+ expect { execute }.not_to change { incident.notes }
+ end
+ end
+
+ context 'when note is nil' do
+ let(:params) { { occurred_at: occurred_at } }
+
+ it_behaves_like 'successful response'
+ it_behaves_like 'passing the correct was_changed value', :occurred_at
+
+ it 'does not update the note' do
+ expect { execute }.not_to change { timeline_event.reload.note }
+ end
+
+ it 'updates occurred_at' do
+ expect { execute }.to change { timeline_event.occurred_at }.to(params[:occurred_at])
+ end
+ end
+
+ context 'when note is blank' do
+ let(:params) { { note: '', occurred_at: occurred_at } }
+
+ it_behaves_like 'successful response'
+ it_behaves_like 'passing the correct was_changed value', :occurred_at
+
+ it 'does not update the note' do
+ expect { execute }.not_to change { timeline_event.reload.note }
+ end
+
+ it 'updates occurred_at' do
+ expect { execute }.to change { timeline_event.occurred_at }.to(params[:occurred_at])
+ end
+ end
+
+ context 'when occurred_at is nil' do
+ let(:params) { { note: 'Updated note' } }
+
+ it_behaves_like 'successful response'
+ it_behaves_like 'passing the correct was_changed value', :note
+
+ it 'updates the note' do
+ expect { execute }.to change { timeline_event.note }.to(params[:note])
+ end
+
+ it 'does not update occurred_at' do
+ expect { execute }.not_to change { timeline_event.reload.occurred_at }
+ end
+ end
+
+ context 'when both occurred_at and note is nil' do
+ let(:params) { {} }
+
+ it_behaves_like 'successful response'
+
+ it 'does not update the note' do
+ expect { execute }.not_to change { timeline_event.note }
+ end
+
+ it 'does not update occurred_at' do
+ expect { execute }.not_to change { timeline_event.reload.occurred_at }
+ end
+
+ it 'does not call SysteNoteService.edit_timeline_event' do
+ expect(SystemNoteService).not_to receive(:edit_timeline_event)
+
+ execute
+ end
+ end
+ end
+
+ context 'when user does not have permissions' do
+ before do
+ project.add_reporter(user)
+ end
+
+ it_behaves_like 'error response', 'You have insufficient permissions to manage timeline events for this incident'
+ end
+ end
+end
diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb
index 1f6118e9fcc..344da5a6582 100644
--- a/spec/services/issues/close_service_spec.rb
+++ b/spec/services/issues/close_service_spec.rb
@@ -279,7 +279,7 @@ RSpec.describe Issues::CloseService do
it 'verifies the number of queries' do
recorded = ActiveRecord::QueryRecorder.new { close_issue }
- expected_queries = 32
+ expected_queries = 30
expect(recorded.count).to be <= expected_queries
expect(recorded.cached_count).to eq(0)
diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb
index 6b7b72d83fc..3934ca04a00 100644
--- a/spec/services/issues/create_service_spec.rb
+++ b/spec/services/issues/create_service_spec.rb
@@ -47,6 +47,14 @@ RSpec.describe Issues::CreateService do
due_date: Date.tomorrow }
end
+ it 'works if base work item types were not created yet' do
+ WorkItems::Type.delete_all
+
+ expect do
+ issue
+ end.to change(Issue, :count).by(1)
+ end
+
it 'creates the issue with the given params' do
expect(Issuable::CommonSystemNotesService).to receive_message_chain(:new, :execute)
diff --git a/spec/services/issues/set_crm_contacts_service_spec.rb b/spec/services/issues/set_crm_contacts_service_spec.rb
index b0befb9f77c..5613cc49cc5 100644
--- a/spec/services/issues/set_crm_contacts_service_spec.rb
+++ b/spec/services/issues/set_crm_contacts_service_spec.rb
@@ -5,7 +5,7 @@ require 'spec_helper'
RSpec.describe Issues::SetCrmContactsService do
let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group, :crm_enabled) }
- let_it_be(:project) { create(:project, group: create(:group, parent: group)) }
+ let_it_be(:project) { create(:project, group: create(:group, :crm_enabled, parent: group)) }
let_it_be(:contacts) { create_list(:contact, 4, group: group) }
let_it_be(:issue, reload: true) { create(:issue, project: project) }
let_it_be(:issue_contact_1) do
@@ -58,6 +58,20 @@ RSpec.describe Issues::SetCrmContactsService do
group.add_reporter(user)
end
+ context 'but the crm setting is disabled' do
+ let(:params) { { replace_ids: [contacts[1].id, contacts[2].id] } }
+ let(:subgroup_with_crm_disabled) { create(:group, parent: group) }
+ let(:project_with_crm_disabled) { create(:project, group: subgroup_with_crm_disabled) }
+ let(:issue_with_crm_disabled) { create(:issue, project: project_with_crm_disabled) }
+
+ it 'returns expected error response' do
+ response = described_class.new(project: project_with_crm_disabled, current_user: user, params: params).execute(issue_with_crm_disabled)
+
+ expect(response).to be_error
+ expect(response.message).to eq('You have insufficient permissions to set customer relations contacts for this issue')
+ end
+ end
+
context 'when the contact does not exist' do
let(:params) { { replace_ids: [non_existing_record_id] } }
diff --git a/spec/services/jira_connect/sync_service_spec.rb b/spec/services/jira_connect/sync_service_spec.rb
index c20aecaaef0..7242b1f41f9 100644
--- a/spec/services/jira_connect/sync_service_spec.rb
+++ b/spec/services/jira_connect/sync_service_spec.rb
@@ -24,13 +24,15 @@ RSpec.describe JiraConnect::SyncService do
end
def expect_log(type, message)
- expect(Gitlab::ProjectServiceLogger)
+ expect(Gitlab::IntegrationsLogger)
.to receive(type).with(
- message: 'response from jira dev_info api',
- integration: 'JiraConnect',
- project_id: project.id,
- project_path: project.full_path,
- jira_response: message&.to_json
+ {
+ message: 'response from jira dev_info api',
+ integration: 'JiraConnect',
+ project_id: project.id,
+ project_path: project.full_path,
+ jira_response: message&.to_json
+ }
)
end
diff --git a/spec/services/keys/expiry_notification_service_spec.rb b/spec/services/keys/expiry_notification_service_spec.rb
index 1d1da179cf7..7cb6cbce311 100644
--- a/spec/services/keys/expiry_notification_service_spec.rb
+++ b/spec/services/keys/expiry_notification_service_spec.rb
@@ -44,7 +44,7 @@ RSpec.describe Keys::ExpiryNotificationService do
end
context 'with key expiring today', :mailer do
- let_it_be_with_reload(:key) { create(:key, expires_at: Time.current, user: user) }
+ let_it_be_with_reload(:key) { create(:key, expires_at: 10.minutes.from_now, user: user) }
let(:expiring_soon) { false }
diff --git a/spec/services/loose_foreign_keys/batch_cleaner_service_spec.rb b/spec/services/loose_foreign_keys/batch_cleaner_service_spec.rb
index 538d9638879..735f090d926 100644
--- a/spec/services/loose_foreign_keys/batch_cleaner_service_spec.rb
+++ b/spec/services/loose_foreign_keys/batch_cleaner_service_spec.rb
@@ -181,16 +181,6 @@ RSpec.describe LooseForeignKeys::BatchCleanerService do
end
end
end
-
- context 'when the lfk_fair_queueing FF is off' do
- before do
- stub_feature_flags(lfk_fair_queueing: false)
- end
-
- it 'does nothing' do
- expect { cleaner.execute }.not_to change { deleted_record.reload.cleanup_attempts }
- end
- end
end
end
end
diff --git a/spec/services/members/create_service_spec.rb b/spec/services/members/create_service_spec.rb
index 25437be1e78..730175af0bb 100644
--- a/spec/services/members/create_service_spec.rb
+++ b/spec/services/members/create_service_spec.rb
@@ -7,11 +7,11 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
let_it_be(:user) { create(:user) }
let_it_be(:member) { create(:user) }
let_it_be(:user_invited_by_id) { create(:user) }
- let_it_be(:user_ids) { member.id.to_s }
+ let_it_be(:user_id) { member.id.to_s }
let_it_be(:access_level) { Gitlab::Access::GUEST }
let(:additional_params) { { invite_source: '_invite_source_' } }
- let(:params) { { user_ids: user_ids, access_level: access_level }.merge(additional_params) }
+ let(:params) { { user_id: user_id, access_level: access_level }.merge(additional_params) }
let(:current_user) { user }
subject(:execute_service) { described_class.new(current_user, params.merge({ source: source })).execute }
@@ -51,7 +51,7 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
end
context 'when user_id is passed as an integer' do
- let(:user_ids) { member.id }
+ let(:user_id) { member.id }
it 'successfully creates member' do
expect(execute_service[:status]).to eq(:success)
@@ -60,8 +60,8 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
end
end
- context 'with user_ids as an array of integers' do
- let(:user_ids) { [member.id, user_invited_by_id.id] }
+ context 'with user_id as an array of integers' do
+ let(:user_id) { [member.id, user_invited_by_id.id] }
it 'successfully creates members' do
expect(execute_service[:status]).to eq(:success)
@@ -70,8 +70,8 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
end
end
- context 'with user_ids as an array of strings' do
- let(:user_ids) { [member.id.to_s, user_invited_by_id.id.to_s] }
+ context 'with user_id as an array of strings' do
+ let(:user_id) { [member.id.to_s, user_invited_by_id.id.to_s] }
it 'successfully creates members' do
expect(execute_service[:status]).to eq(:success)
@@ -101,7 +101,7 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
end
context 'when passing no user ids' do
- let(:user_ids) { '' }
+ let(:user_id) { '' }
it 'does not add a member' do
expect(execute_service[:status]).to eq(:error)
@@ -112,7 +112,7 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
end
context 'when passing many user ids' do
- let(:user_ids) { 1.upto(101).to_a.join(',') }
+ let(:user_id) { 1.upto(101).to_a.join(',') }
it 'limits the number of users to 100' do
expect(execute_service[:status]).to eq(:error)
@@ -134,7 +134,7 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
end
context 'when passing an existing invite user id' do
- let(:user_ids) { create(:project_member, :invited, project: source).invite_email }
+ let(:user_id) { create(:project_member, :invited, project: source).invite_email }
it 'does not add a member' do
expect(execute_service[:status]).to eq(:error)
@@ -146,7 +146,7 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
context 'when adding a project_bot' do
let_it_be(:project_bot) { create(:user, :project_bot) }
- let(:user_ids) { project_bot.id }
+ let(:user_id) { project_bot.id }
context 'when project_bot is already a member' do
before do
@@ -213,7 +213,7 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
end
context 'when it is a net_new_user' do
- let(:additional_params) { { invite_source: '_invite_source_', user_ids: 'email@example.org' } }
+ let(:additional_params) { { invite_source: '_invite_source_', user_id: 'email@example.org' } }
it 'tracks the invite source from params' do
execute_service
@@ -248,8 +248,8 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
)
end
- context 'when it is an invite by email passed to user_ids' do
- let(:user_ids) { 'email@example.org' }
+ context 'when it is an invite by email passed to user_id' do
+ let(:user_id) { 'email@example.org' }
it 'does not create task issues' do
expect(TasksToBeDone::CreateWorker).not_to receive(:perform_async)
@@ -263,7 +263,7 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
end
let(:another_user) { create(:user) }
- let(:user_ids) { [member.id, another_user.id].join(',') }
+ let(:user_id) { [member.id, another_user.id].join(',') }
it 'still creates 2 task issues', :aggregate_failures do
expect(TasksToBeDone::CreateWorker)
@@ -326,7 +326,7 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
end
context 'when a member was already invited' do
- let(:user_ids) { create(:project_member, :invited, project: source).invite_email }
+ let(:user_id) { create(:project_member, :invited, project: source).invite_email }
let(:additional_params) do
{ invite_source: '_invite_source_', tasks_project_id: source.id, tasks_to_be_done: %w(ci code) }
end
diff --git a/spec/services/members/groups/creator_service_spec.rb b/spec/services/members/groups/creator_service_spec.rb
index 4427c4e7d9f..c3ba7c0374d 100644
--- a/spec/services/members/groups/creator_service_spec.rb
+++ b/spec/services/members/groups/creator_service_spec.rb
@@ -3,14 +3,28 @@
require 'spec_helper'
RSpec.describe Members::Groups::CreatorService do
- it_behaves_like 'member creation' do
- let_it_be(:source, reload: true) { create(:group, :public) }
- let_it_be(:member_type) { GroupMember }
- end
-
describe '.access_levels' do
it 'returns Gitlab::Access.options_with_owner' do
expect(described_class.access_levels).to eq(Gitlab::Access.sym_options_with_owner)
end
end
+
+ describe '#execute' do
+ let_it_be(:source, reload: true) { create(:group, :public) }
+ let_it_be(:user) { create(:user) }
+
+ it_behaves_like 'member creation' do
+ let_it_be(:member_type) { GroupMember }
+ end
+
+ context 'authorized projects update' do
+ it 'schedules a single project authorization update job when called multiple times' do
+ expect(AuthorizedProjectsWorker).to receive(:bulk_perform_and_wait).once
+
+ 1.upto(3) do
+ described_class.new(source, user, :maintainer).execute
+ end
+ end
+ end
+ end
end
diff --git a/spec/services/members/invite_service_spec.rb b/spec/services/members/invite_service_spec.rb
index ab740138a8b..8213e8baae0 100644
--- a/spec/services/members/invite_service_spec.rb
+++ b/spec/services/members/invite_service_spec.rb
@@ -52,8 +52,8 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
end
end
- context 'with user_ids as integers' do
- let(:params) { { user_ids: [project_user.id, user_invited_by_id.id] } }
+ context 'with user_id as integers' do
+ let(:params) { { user_id: [project_user.id, user_invited_by_id.id] } }
it 'successfully creates members' do
expect_to_create_members(count: 2)
@@ -61,8 +61,8 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
end
end
- context 'with user_ids as strings' do
- let(:params) { { user_ids: [project_user.id.to_s, user_invited_by_id.id.to_s] } }
+ context 'with user_id as strings' do
+ let(:params) { { user_id: [project_user.id.to_s, user_invited_by_id.id.to_s] } }
it 'successfully creates members' do
expect_to_create_members(count: 2)
@@ -70,9 +70,9 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
end
end
- context 'with a mixture of emails and user_ids' do
+ context 'with a mixture of emails and user_id' do
let(:params) do
- { user_ids: [project_user.id, user_invited_by_id.id], email: %w[email@example.org email2@example.org] }
+ { user_id: [project_user.id, user_invited_by_id.id], email: %w[email@example.org email2@example.org] }
end
it 'successfully creates members' do
@@ -92,8 +92,8 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
end
end
- context 'with user_ids' do
- let(:params) { { user_ids: "#{project_user.id},#{user_invited_by_id.id}" } }
+ context 'with user_id' do
+ let(:params) { { user_id: "#{project_user.id},#{user_invited_by_id.id}" } }
it 'successfully creates members' do
expect_to_create_members(count: 2)
@@ -101,9 +101,9 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
end
end
- context 'with a mixture of emails and user_ids' do
+ context 'with a mixture of emails and user_id' do
let(:params) do
- { user_ids: "#{project_user.id},#{user_invited_by_id.id}", email: 'email@example.org,email2@example.org' }
+ { user_id: "#{project_user.id},#{user_invited_by_id.id}", email: 'email@example.org,email2@example.org' }
end
it 'successfully creates members' do
@@ -114,9 +114,9 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
end
context 'when invites formats are mixed' do
- context 'when user_ids is an array and emails is a string' do
+ context 'when user_id is an array and emails is a string' do
let(:params) do
- { user_ids: [project_user.id, user_invited_by_id.id], email: 'email@example.org,email2@example.org' }
+ { user_id: [project_user.id, user_invited_by_id.id], email: 'email@example.org,email2@example.org' }
end
it 'successfully creates members' do
@@ -125,9 +125,9 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
end
end
- context 'when user_ids is a string and emails is an array' do
+ context 'when user_id is a string and emails is an array' do
let(:params) do
- { user_ids: "#{project_user.id},#{user_invited_by_id.id}", email: %w[email@example.org email2@example.org] }
+ { user_id: "#{project_user.id},#{user_invited_by_id.id}", email: %w[email@example.org email2@example.org] }
end
it 'successfully creates members' do
@@ -147,8 +147,8 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
end
end
- context 'when user_ids are passed as an empty string' do
- let(:params) { { user_ids: '' } }
+ context 'when user_id are passed as an empty string' do
+ let(:params) { { user_id: '' } }
it 'returns an error' do
expect_not_to_create_members
@@ -156,8 +156,8 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
end
end
- context 'when user_ids and emails are both passed as empty strings' do
- let(:params) { { user_ids: '', email: '' } }
+ context 'when user_id and emails are both passed as empty strings' do
+ let(:params) { { user_id: '', email: '' } }
it 'returns an error' do
expect_not_to_create_members
@@ -166,7 +166,7 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
end
context 'when user_id is passed as an integer' do
- let(:params) { { user_ids: project_user.id } }
+ let(:params) { { user_id: project_user.id } }
it 'successfully creates member' do
expect_to_create_members(count: 1)
@@ -196,7 +196,7 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
end
context 'with user_id and singular invalid email' do
- let(:params) { { user_ids: project_user.id, email: '_bogus_' } }
+ let(:params) { { user_id: project_user.id, email: '_bogus_' } }
it 'has partial success' do
expect_to_create_members(count: 1)
@@ -219,7 +219,7 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
end
context 'with duplicate user ids' do
- let(:params) { { user_ids: "#{project_user.id},#{project_user.id}" } }
+ let(:params) { { user_id: "#{project_user.id},#{project_user.id}" } }
it 'only creates one member per unique invite' do
expect_to_create_members(count: 1)
@@ -228,7 +228,7 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
end
context 'with duplicate member by adding as user id and email' do
- let(:params) { { user_ids: project_user.id, email: project_user.email } }
+ let(:params) { { user_id: project_user.id, email: project_user.email } }
it 'only creates one member per unique invite' do
expect_to_create_members(count: 1)
@@ -269,9 +269,9 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
end
end
- context 'with user_ids' do
- let(:user_ids) { 1.upto(101).to_a.join(',') }
- let(:params) { { user_ids: user_ids } }
+ context 'with user_id' do
+ let(:user_id) { 1.upto(101).to_a.join(',') }
+ let(:params) { { user_id: user_id } }
it 'limits the number of users to 100' do
expect_not_to_create_members
@@ -292,7 +292,7 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
end
context 'with user_id' do
- let(:params) { { user_ids: project_user.id } }
+ let(:params) { { user_id: project_user.id } }
it_behaves_like 'records an onboarding progress action', :user_added
@@ -304,7 +304,7 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
context 'when assigning tasks to be done' do
let(:params) do
- { user_ids: project_user.id, tasks_to_be_done: %w(ci code), tasks_project_id: project.id }
+ { user_id: project_user.id, tasks_to_be_done: %w(ci code), tasks_project_id: project.id }
end
it 'creates 2 task issues', :aggregate_failures do
@@ -332,7 +332,7 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
end
context 'with user_id' do
- let(:params) { { user_ids: user_invited_by_id.id, access_level: -1 } }
+ let(:params) { { user_id: user_invited_by_id.id, access_level: -1 } }
it 'returns an error' do
expect_not_to_create_members
@@ -341,7 +341,7 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
end
context 'with a mix of user_id and email' do
- let(:params) { { user_ids: user_invited_by_id.id, email: project_user.email, access_level: -1 } }
+ let(:params) { { user_id: user_invited_by_id.id, email: project_user.email, access_level: -1 } }
it 'returns errors' do
expect_not_to_create_members
@@ -387,7 +387,7 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
context 'with user_id that already exists' do
let!(:existing_member) { create(:project_member, project: project, user: project_user) }
- let(:params) { { user_ids: existing_member.user_id } }
+ let(:params) { { user_id: existing_member.user_id } }
it 'does not add the member again and is successful' do
expect_to_create_members(count: 0)
@@ -397,7 +397,7 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
context 'with user_id that already exists with a lower access_level' do
let!(:existing_member) { create(:project_member, :developer, project: project, user: project_user) }
- let(:params) { { user_ids: existing_member.user_id, access_level: ProjectMember::MAINTAINER } }
+ let(:params) { { user_id: existing_member.user_id, access_level: ProjectMember::MAINTAINER } }
it 'does not add the member again and updates the access_level' do
expect_to_create_members(count: 0)
@@ -408,7 +408,7 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
context 'with user_id that already exists with a higher access_level' do
let!(:existing_member) { create(:project_member, :developer, project: project, user: project_user) }
- let(:params) { { user_ids: existing_member.user_id, access_level: ProjectMember::GUEST } }
+ let(:params) { { user_id: existing_member.user_id, access_level: ProjectMember::GUEST } }
it 'does not add the member again and updates the access_level' do
expect_to_create_members(count: 0)
@@ -428,7 +428,7 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
end
context 'when access_level is lower than inheriting member' do
- let(:params) { { user_ids: group_member.user_id, access_level: ProjectMember::GUEST }}
+ let(:params) { { user_id: group_member.user_id, access_level: ProjectMember::GUEST }}
it 'does not add the member and returns an error' do
msg = "Access level should be greater than or equal " \
@@ -440,7 +440,7 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
end
context 'when access_level is the same as the inheriting member' do
- let(:params) { { user_ids: group_member.user_id, access_level: ProjectMember::DEVELOPER }}
+ let(:params) { { user_id: group_member.user_id, access_level: ProjectMember::DEVELOPER }}
it 'adds the member with correct access_level' do
expect_to_create_members(count: 1)
@@ -450,7 +450,7 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
end
context 'when access_level is greater than the inheriting member' do
- let(:params) { { user_ids: group_member.user_id, access_level: ProjectMember::MAINTAINER }}
+ let(:params) { { user_id: group_member.user_id, access_level: ProjectMember::MAINTAINER }}
it 'adds the member with correct access_level' do
expect_to_create_members(count: 1)
diff --git a/spec/services/members/projects/creator_service_spec.rb b/spec/services/members/projects/creator_service_spec.rb
index 7ba183759bc..7605238c3c5 100644
--- a/spec/services/members/projects/creator_service_spec.rb
+++ b/spec/services/members/projects/creator_service_spec.rb
@@ -3,14 +3,28 @@
require 'spec_helper'
RSpec.describe Members::Projects::CreatorService do
- it_behaves_like 'member creation' do
- let_it_be(:source, reload: true) { create(:project, :public) }
- let_it_be(:member_type) { ProjectMember }
- end
-
describe '.access_levels' do
it 'returns Gitlab::Access.sym_options_with_owner' do
expect(described_class.access_levels).to eq(Gitlab::Access.sym_options_with_owner)
end
end
+
+ describe '#execute' do
+ let_it_be(:source, reload: true) { create(:project, :public) }
+ let_it_be(:user) { create(:user) }
+
+ it_behaves_like 'member creation' do
+ let_it_be(:member_type) { ProjectMember }
+ end
+
+ context 'authorized projects update' do
+ it 'schedules a single project authorization update job when called multiple times' do
+ expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to receive(:bulk_perform_in).once
+
+ 1.upto(3) do
+ described_class.new(source, user, :maintainer).execute
+ end
+ end
+ end
+ end
end
diff --git a/spec/services/merge_requests/approval_service_spec.rb b/spec/services/merge_requests/approval_service_spec.rb
index 9b064da44b8..e500102a00c 100644
--- a/spec/services/merge_requests/approval_service_spec.rb
+++ b/spec/services/merge_requests/approval_service_spec.rb
@@ -41,6 +41,12 @@ RSpec.describe MergeRequests::ApprovalService do
end
context 'with valid approval' do
+ let(:notification_service) { NotificationService.new }
+
+ before do
+ allow(service).to receive(:notification_service).and_return(notification_service)
+ end
+
it 'creates an approval note and marks pending todos as done' do
expect(SystemNoteService).to receive(:approve_mr).with(merge_request, user)
expect(merge_request.approvals).to receive(:reset)
@@ -59,9 +65,16 @@ RSpec.describe MergeRequests::ApprovalService do
service.execute(merge_request)
end
+ it 'sends a notification when approving' do
+ expect(notification_service).to receive_message_chain(:async, :approve_mr)
+ .with(merge_request, user)
+
+ service.execute(merge_request)
+ end
+
it 'removes attention requested state' do
expect(MergeRequests::RemoveAttentionRequestedService).to receive(:new)
- .with(project: project, current_user: user, merge_request: merge_request)
+ .with(project: project, current_user: user, merge_request: merge_request, user: user)
.and_call_original
service.execute(merge_request)
diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb
index d36a2f75cfe..cd1c362a19f 100644
--- a/spec/services/merge_requests/close_service_spec.rb
+++ b/spec/services/merge_requests/close_service_spec.rb
@@ -97,7 +97,7 @@ RSpec.describe MergeRequests::CloseService do
it 'clean up environments for the merge request' do
expect_next_instance_of(::Environments::StopService) do |service|
- expect(service).to receive(:execute_for_merge_request).with(merge_request)
+ expect(service).to receive(:execute_for_merge_request_pipeline).with(merge_request)
end
described_class.new(project: project, current_user: user).execute(merge_request)
diff --git a/spec/services/merge_requests/handle_assignees_change_service_spec.rb b/spec/services/merge_requests/handle_assignees_change_service_spec.rb
index 26f53f55b0f..fa3b1614e21 100644
--- a/spec/services/merge_requests/handle_assignees_change_service_spec.rb
+++ b/spec/services/merge_requests/handle_assignees_change_service_spec.rb
@@ -89,18 +89,12 @@ RSpec.describe MergeRequests::HandleAssigneesChangeService do
it 'removes attention requested state' do
expect(MergeRequests::RemoveAttentionRequestedService).to receive(:new)
- .with(project: project, current_user: user, merge_request: merge_request)
+ .with(project: project, current_user: user, merge_request: merge_request, user: user)
.and_call_original
execute
end
- it 'updates attention requested by of assignee' do
- execute
-
- expect(merge_request.find_assignee(assignee).updated_state_by).to eq(user)
- end
-
it 'tracks users assigned event' do
expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
.to receive(:track_users_assigned_to_mr).once.with(users: [assignee])
diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb
index ecb856bd1a4..78deab64b1c 100644
--- a/spec/services/merge_requests/merge_service_spec.rb
+++ b/spec/services/merge_requests/merge_service_spec.rb
@@ -149,7 +149,7 @@ RSpec.describe MergeRequests::MergeService do
allow(project).to receive(:default_branch).and_return(merge_request.target_branch)
end
- it 'closes GitLab issue tracker issues' do
+ it 'closes GitLab issue tracker issues', :sidekiq_inline do
issue = create :issue, project: project
commit = double('commit', safe_message: "Fixes #{issue.to_reference}", date: Time.current, authored_date: Time.current)
allow(merge_request).to receive(:commits).and_return([commit])
diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb
index 8d9a32c3e9e..f0885365f96 100644
--- a/spec/services/merge_requests/post_merge_service_spec.rb
+++ b/spec/services/merge_requests/post_merge_service_spec.rb
@@ -59,24 +59,9 @@ RSpec.describe MergeRequests::PostMergeService do
expect(diff_removal_service).to have_received(:execute)
end
- it 'marks MR as merged regardless of errors when closing issues' do
- merge_request.update!(target_branch: 'foo')
- allow(project).to receive(:default_branch).and_return('foo')
-
- issue = create(:issue, project: project)
- allow(merge_request).to receive(:visible_closing_issues_for).and_return([issue])
- expect_next_instance_of(Issues::CloseService) do |close_service|
- allow(close_service).to receive(:execute).with(issue, commit: merge_request).and_raise(RuntimeError)
- end
-
- expect { subject }.to raise_error(RuntimeError)
-
- expect(merge_request.reload).to be_merged
- end
-
it 'clean up environments for the merge request' do
expect_next_instance_of(::Environments::StopService) do |stop_environment_service|
- expect(stop_environment_service).to receive(:execute_for_merge_request).with(merge_request)
+ expect(stop_environment_service).to receive(:execute_for_merge_request_pipeline).with(merge_request)
end
subject
@@ -88,6 +73,67 @@ RSpec.describe MergeRequests::PostMergeService do
subject
end
+ context 'when there are issues to be closed' do
+ let_it_be(:issue) { create(:issue, project: project) }
+
+ before do
+ merge_request.update!(target_branch: 'foo')
+
+ allow(project).to receive(:default_branch).and_return('foo')
+ allow(merge_request).to receive(:visible_closing_issues_for).and_return([issue])
+ end
+
+ it 'performs MergeRequests::CloseIssueWorker asynchronously' do
+ expect(MergeRequests::CloseIssueWorker)
+ .to receive(:perform_async)
+ .with(project.id, user.id, issue.id, merge_request.id)
+
+ subject
+
+ expect(merge_request.reload).to be_merged
+ end
+
+ context 'when issue is an external issue' do
+ let_it_be(:issue) { ExternalIssue.new('JIRA-123', project) }
+
+ it 'executes Issues::CloseService' do
+ expect_next_instance_of(Issues::CloseService) do |close_service|
+ expect(close_service).to receive(:execute).with(issue, commit: merge_request)
+ end
+
+ subject
+
+ expect(merge_request.reload).to be_merged
+ end
+ end
+
+ context 'when async_mr_close_issue feature flag is disabled' do
+ before do
+ stub_feature_flags(async_mr_close_issue: false)
+ end
+
+ it 'executes Issues::CloseService' do
+ expect_next_instance_of(Issues::CloseService) do |close_service|
+ expect(close_service).to receive(:execute).with(issue, commit: merge_request)
+ end
+
+ subject
+
+ expect(merge_request.reload).to be_merged
+ end
+
+ it 'marks MR as merged regardless of errors when closing issues' do
+ expect_next_instance_of(Issues::CloseService) do |close_service|
+ allow(close_service).to receive(:execute).with(issue, commit: merge_request).and_raise(RuntimeError)
+ end
+
+ expect { subject }.to raise_error(RuntimeError)
+
+ expect(merge_request.reload).to be_merged
+ end
+ end
+ end
+
context 'when the merge request has review apps' do
it 'cancels all review app deployments' do
pipeline = create(:ci_pipeline,
diff --git a/spec/services/merge_requests/push_options_handler_service_spec.rb b/spec/services/merge_requests/push_options_handler_service_spec.rb
index 348ea9ad7d4..338057f23d5 100644
--- a/spec/services/merge_requests/push_options_handler_service_spec.rb
+++ b/spec/services/merge_requests/push_options_handler_service_spec.rb
@@ -20,7 +20,17 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
let(:source_branch) { 'fix' }
let(:target_branch) { 'feature' }
let(:title) { 'my title' }
+ let(:draft_title) { 'Draft: my title' }
+ let(:draft) { true }
let(:description) { 'my description' }
+ let(:multiline_description) do
+ <<~MD.chomp
+ Line 1
+ Line 2
+ Line 3
+ MD
+ end
+
let(:label1) { 'mylabel1' }
let(:label2) { 'mylabel2' }
let(:label3) { 'mylabel3' }
@@ -64,6 +74,26 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
end
end
+ shared_examples_for 'a service that can set the multiline description of a merge request' do
+ subject(:last_mr) { MergeRequest.last }
+
+ it 'sets the multiline description' do
+ service.execute
+
+ expect(last_mr.description).to eq(multiline_description)
+ end
+ end
+
+ shared_examples_for 'a service that can set the draft of a merge request' do
+ subject(:last_mr) { MergeRequest.last }
+
+ it 'sets the draft' do
+ service.execute
+
+ expect(last_mr.draft).to eq(draft)
+ end
+ end
+
shared_examples_for 'a service that can set the milestone of a merge request' do
subject(:last_mr) { MergeRequest.last }
@@ -417,6 +447,74 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that does not create a merge request'
it_behaves_like 'a service that can set the description of a merge request'
+
+ context 'with a multiline description' do
+ let(:push_options) { { description: "Line 1\\nLine 2\\nLine 3" } }
+
+ it_behaves_like 'a service that does not create a merge request'
+ it_behaves_like 'a service that can set the multiline description of a merge request'
+ end
+ end
+
+ it_behaves_like 'with a deleted branch'
+ it_behaves_like 'with the project default branch'
+ end
+
+ describe '`draft` push option' do
+ let(:push_options) { { draft: draft } }
+
+ context 'with a new branch' do
+ let(:changes) { new_branch_changes }
+
+ it_behaves_like 'a service that does not create a merge request'
+
+ it 'adds an error to the service' do
+ service.execute
+
+ expect(service.errors).to include(error_mr_required)
+ end
+
+ context 'when coupled with the `create` push option' do
+ let(:push_options) { { create: true, draft: draft } }
+
+ it_behaves_like 'a service that can create a merge request'
+ it_behaves_like 'a service that can set the draft of a merge request'
+ end
+ end
+
+ context 'with an existing branch but no open MR' do
+ let(:changes) { existing_branch_changes }
+
+ it_behaves_like 'a service that does not create a merge request'
+
+ it 'adds an error to the service' do
+ service.execute
+
+ expect(service.errors).to include(error_mr_required)
+ end
+
+ context 'when coupled with the `create` push option' do
+ let(:push_options) { { create: true, draft: draft } }
+
+ it_behaves_like 'a service that can create a merge request'
+ it_behaves_like 'a service that can set the draft of a merge request'
+ end
+ end
+
+ context 'with an existing branch that has a merge request open' do
+ let(:changes) { existing_branch_changes }
+ let!(:merge_request) { create(:merge_request, source_project: project, source_branch: source_branch)}
+
+ it_behaves_like 'a service that does not create a merge request'
+ it_behaves_like 'a service that can set the draft of a merge request'
+ end
+
+ context 'draft title provided while `draft` push option is set to false' do
+ let(:push_options) { { create: true, draft: false, title: draft_title } }
+ let(:changes) { new_branch_changes }
+
+ it_behaves_like 'a service that can create a merge request'
+ it_behaves_like 'a service that can set the draft of a merge request'
end
it_behaves_like 'with a deleted branch'
diff --git a/spec/services/merge_requests/rebase_service_spec.rb b/spec/services/merge_requests/rebase_service_spec.rb
index a47e626666b..e7aa6e74246 100644
--- a/spec/services/merge_requests/rebase_service_spec.rb
+++ b/spec/services/merge_requests/rebase_service_spec.rb
@@ -70,11 +70,13 @@ RSpec.describe MergeRequests::RebaseService do
it 'logs the error' do
expect(service).to receive(:log_error).with(exception: exception, message: described_class::REBASE_ERROR, save_message_on_model: true).and_call_original
expect(Gitlab::ErrorTracking).to receive(:track_exception).with(exception,
- class: described_class.to_s,
- merge_request: merge_request_ref,
- merge_request_id: merge_request.id,
- message: described_class::REBASE_ERROR,
- save_message_on_model: true).and_call_original
+ {
+ class: described_class.to_s,
+ merge_request: merge_request_ref,
+ merge_request_id: merge_request.id,
+ message: described_class::REBASE_ERROR,
+ save_message_on_model: true
+ }).and_call_original
service.execute(merge_request)
end
diff --git a/spec/services/merge_requests/remove_approval_service_spec.rb b/spec/services/merge_requests/remove_approval_service_spec.rb
index ef6a0ec69bd..5a319e90a68 100644
--- a/spec/services/merge_requests/remove_approval_service_spec.rb
+++ b/spec/services/merge_requests/remove_approval_service_spec.rb
@@ -21,14 +21,20 @@ RSpec.describe MergeRequests::RemoveApprovalService do
context 'with a user who has approved' do
let!(:approval) { create(:approval, user: user, merge_request: merge_request) }
+ let(:notification_service) { NotificationService.new }
+
+ before do
+ allow(service).to receive(:notification_service).and_return(notification_service)
+ end
it 'removes the approval' do
expect { execute! }.to change { merge_request.approvals.size }.from(2).to(1)
end
- it 'creates an unapproval note and triggers web hook' do
+ it 'creates an unapproval note, triggers a web hook, and sends a notification' do
expect(service).to receive(:execute_hooks).with(merge_request, 'unapproved')
expect(SystemNoteService).to receive(:unapprove_mr)
+ expect(notification_service).to receive_message_chain(:async, :unapprove_mr).with(merge_request, user)
execute!
end
diff --git a/spec/services/merge_requests/remove_attention_requested_service_spec.rb b/spec/services/merge_requests/remove_attention_requested_service_spec.rb
index 450204ebfdd..576049b9f1b 100644
--- a/spec/services/merge_requests/remove_attention_requested_service_spec.rb
+++ b/spec/services/merge_requests/remove_attention_requested_service_spec.rb
@@ -3,64 +3,112 @@
require 'spec_helper'
RSpec.describe MergeRequests::RemoveAttentionRequestedService do
- let(:current_user) { create(:user) }
- let(:merge_request) { create(:merge_request, reviewers: [current_user], assignees: [current_user]) }
- let(:reviewer) { merge_request.find_reviewer(current_user) }
- let(:assignee) { merge_request.find_assignee(current_user) }
+ let_it_be(:current_user) { create(:user) }
+ let_it_be(:user) { create(:user) }
+ let_it_be(:assignee_user) { create(:user) }
+ let_it_be(:merge_request) { create(:merge_request, reviewers: [user], assignees: [assignee_user]) }
+
+ let(:reviewer) { merge_request.find_reviewer(user) }
+ let(:assignee) { merge_request.find_assignee(assignee_user) }
let(:project) { merge_request.project }
- let(:service) { described_class.new(project: project, current_user: current_user, merge_request: merge_request) }
+
+ let(:service) do
+ described_class.new(
+ project: project,
+ current_user: current_user,
+ merge_request: merge_request,
+ user: user
+ )
+ end
+
let(:result) { service.execute }
before do
+ allow(SystemNoteService).to receive(:remove_attention_request)
+
project.add_developer(current_user)
+ project.add_developer(user)
end
describe '#execute' do
- context 'invalid permissions' do
- let(:service) { described_class.new(project: project, current_user: create(:user), merge_request: merge_request) }
+ context 'when current user cannot update merge request' do
+ let(:service) do
+ described_class.new(
+ project: project,
+ current_user: create(:user),
+ merge_request: merge_request,
+ user: user
+ )
+ end
it 'returns an error' do
expect(result[:status]).to eq :error
end
end
- context 'reviewer does not exist' do
- let(:service) { described_class.new(project: project, current_user: create(:user), merge_request: merge_request) }
+ context 'when user is not a reviewer nor assignee' do
+ let(:service) do
+ described_class.new(
+ project: project,
+ current_user: current_user,
+ merge_request: merge_request,
+ user: create(:user)
+ )
+ end
it 'returns an error' do
expect(result[:status]).to eq :error
end
end
- context 'reviewer exists' do
+ context 'when user is a reviewer' do
+ before do
+ reviewer.update!(state: :attention_requested)
+ end
+
it 'returns success' do
expect(result[:status]).to eq :success
end
- it 'updates reviewers state' do
+ it 'updates reviewer state' do
service.execute
reviewer.reload
expect(reviewer.state).to eq 'reviewed'
end
+ it 'creates a remove attention request system note' do
+ expect(SystemNoteService)
+ .to receive(:remove_attention_request)
+ .with(merge_request, merge_request.project, current_user, user)
+
+ service.execute
+ end
+
it_behaves_like 'invalidates attention request cache' do
- let(:users) { [current_user] }
+ let(:users) { [user] }
end
end
- context 'assignee exists' do
- let(:service) { described_class.new(project: project, current_user: current_user, merge_request: merge_request) }
+ context 'when user is an assignee' do
+ let(:service) do
+ described_class.new(
+ project: project,
+ current_user: current_user,
+ merge_request: merge_request,
+ user: assignee_user
+ )
+ end
before do
- assignee.update!(state: :reviewed)
+ assignee.update!(state: :attention_requested)
end
it 'returns success' do
expect(result[:status]).to eq :success
end
- it 'updates assignees state' do
+ it 'updates assignee state' do
service.execute
assignee.reload
@@ -68,14 +116,40 @@ RSpec.describe MergeRequests::RemoveAttentionRequestedService do
end
it_behaves_like 'invalidates attention request cache' do
- let(:users) { [current_user] }
+ let(:users) { [assignee_user] }
+ end
+
+ it 'creates a remove attention request system note' do
+ expect(SystemNoteService)
+ .to receive(:remove_attention_request)
+ .with(merge_request, merge_request.project, current_user, assignee_user)
+
+ service.execute
end
end
- context 'assignee is the same as reviewer' do
- let(:merge_request) { create(:merge_request, reviewers: [current_user], assignees: [current_user]) }
- let(:service) { described_class.new(project: project, current_user: current_user, merge_request: merge_request) }
- let(:assignee) { merge_request.find_assignee(current_user) }
+ context 'when user is an assignee and reviewer at the same time' do
+ let_it_be(:merge_request) { create(:merge_request, reviewers: [user], assignees: [user]) }
+
+ let(:assignee) { merge_request.find_assignee(user) }
+
+ let(:service) do
+ described_class.new(
+ project: project,
+ current_user: current_user,
+ merge_request: merge_request,
+ user: user
+ )
+ end
+
+ before do
+ reviewer.update!(state: :attention_requested)
+ assignee.update!(state: :attention_requested)
+ end
+
+ it 'returns success' do
+ expect(result[:status]).to eq :success
+ end
it 'updates reviewers and assignees state' do
service.execute
@@ -86,5 +160,24 @@ RSpec.describe MergeRequests::RemoveAttentionRequestedService do
expect(assignee.state).to eq 'reviewed'
end
end
+
+ context 'when state is already not attention_requested' do
+ before do
+ reviewer.update!(state: :reviewed)
+ end
+
+ it 'does not change state' do
+ service.execute
+ reviewer.reload
+
+ expect(reviewer.state).to eq 'reviewed'
+ end
+
+ it 'does not create a remove attention request system note' do
+ expect(SystemNoteService).not_to receive(:remove_attention_request)
+
+ service.execute
+ end
+ end
end
end
diff --git a/spec/services/merge_requests/request_attention_service_spec.rb b/spec/services/merge_requests/request_attention_service_spec.rb
new file mode 100644
index 00000000000..813a8150625
--- /dev/null
+++ b/spec/services/merge_requests/request_attention_service_spec.rb
@@ -0,0 +1,220 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe MergeRequests::RequestAttentionService do
+ let_it_be(:current_user) { create(:user) }
+ let_it_be(:user) { create(:user) }
+ let_it_be(:assignee_user) { create(:user) }
+ let_it_be(:merge_request) { create(:merge_request, reviewers: [user], assignees: [assignee_user]) }
+
+ let(:reviewer) { merge_request.find_reviewer(user) }
+ let(:assignee) { merge_request.find_assignee(assignee_user) }
+ let(:project) { merge_request.project }
+
+ let(:service) do
+ described_class.new(
+ project: project,
+ current_user: current_user,
+ merge_request: merge_request,
+ user: user
+ )
+ end
+
+ let(:result) { service.execute }
+ let(:todo_svc) { instance_double('TodoService') }
+ let(:notification_svc) { instance_double('NotificationService') }
+
+ before do
+ allow(service).to receive(:todo_service).and_return(todo_svc)
+ allow(service).to receive(:notification_service).and_return(notification_svc)
+ allow(todo_svc).to receive(:create_attention_requested_todo)
+ allow(notification_svc).to receive_message_chain(:async, :attention_requested_of_merge_request)
+ allow(SystemNoteService).to receive(:request_attention)
+
+ project.add_developer(current_user)
+ project.add_developer(user)
+ end
+
+ describe '#execute' do
+ context 'when current user cannot update merge request' do
+ let(:service) do
+ described_class.new(
+ project: project,
+ current_user: create(:user),
+ merge_request: merge_request,
+ user: user
+ )
+ end
+
+ it 'returns an error' do
+ expect(result[:status]).to eq :error
+ end
+ end
+
+ context 'when user is not a reviewer nor assignee' do
+ let(:service) do
+ described_class.new(
+ project: project,
+ current_user: current_user,
+ merge_request: merge_request,
+ user: create(:user)
+ )
+ end
+
+ it 'returns an error' do
+ expect(result[:status]).to eq :error
+ end
+ end
+
+ context 'when user is a reviewer' do
+ before do
+ reviewer.update!(state: :reviewed)
+ end
+
+ it 'returns success' do
+ expect(result[:status]).to eq :success
+ end
+
+ it 'updates reviewers state' do
+ service.execute
+ reviewer.reload
+
+ expect(reviewer.state).to eq 'attention_requested'
+ end
+
+ it 'adds who toggled attention' do
+ service.execute
+ reviewer.reload
+
+ expect(reviewer.updated_state_by).to eq current_user
+ end
+
+ it 'creates a new todo for the reviewer' do
+ expect(todo_svc).to receive(:create_attention_requested_todo).with(merge_request, current_user, user)
+
+ service.execute
+ end
+
+ it 'sends email to reviewer' do
+ expect(notification_svc)
+ .to receive_message_chain(:async, :attention_requested_of_merge_request)
+ .with(merge_request, current_user, user)
+
+ service.execute
+ end
+
+ it 'removes attention requested state' do
+ expect(MergeRequests::RemoveAttentionRequestedService).to receive(:new)
+ .with(project: project, current_user: current_user, merge_request: merge_request, user: current_user)
+ .and_call_original
+
+ service.execute
+ end
+
+ it_behaves_like 'invalidates attention request cache' do
+ let(:users) { [user] }
+ end
+ end
+
+ context 'when user is an assignee' do
+ let(:service) do
+ described_class.new(
+ project: project,
+ current_user: current_user,
+ merge_request: merge_request,
+ user: assignee_user
+ )
+ end
+
+ before do
+ assignee.update!(state: :reviewed)
+ end
+
+ it 'returns success' do
+ expect(result[:status]).to eq :success
+ end
+
+ it 'updates assignees state' do
+ service.execute
+ assignee.reload
+
+ expect(assignee.state).to eq 'attention_requested'
+ end
+
+ it 'creates a new todo for the reviewer' do
+ expect(todo_svc).to receive(:create_attention_requested_todo).with(merge_request, current_user, assignee_user)
+
+ service.execute
+ end
+
+ it 'creates a request attention system note' do
+ expect(SystemNoteService)
+ .to receive(:request_attention)
+ .with(merge_request, merge_request.project, current_user, assignee_user)
+
+ service.execute
+ end
+
+ it 'removes attention requested state' do
+ expect(MergeRequests::RemoveAttentionRequestedService).to receive(:new)
+ .with(project: project, current_user: current_user, merge_request: merge_request, user: current_user)
+ .and_call_original
+
+ service.execute
+ end
+
+ it_behaves_like 'invalidates attention request cache' do
+ let(:users) { [assignee_user] }
+ end
+ end
+
+ context 'when user is an assignee and reviewer at the same time' do
+ let_it_be(:merge_request) { create(:merge_request, reviewers: [user], assignees: [user]) }
+
+ let(:assignee) { merge_request.find_assignee(user) }
+
+ let(:service) do
+ described_class.new(
+ project: project,
+ current_user: current_user,
+ merge_request: merge_request,
+ user: user
+ )
+ end
+
+ before do
+ reviewer.update!(state: :reviewed)
+ assignee.update!(state: :reviewed)
+ end
+
+ it 'updates reviewers and assignees state' do
+ service.execute
+ reviewer.reload
+ assignee.reload
+
+ expect(reviewer.state).to eq 'attention_requested'
+ expect(assignee.state).to eq 'attention_requested'
+ end
+ end
+
+ context 'when state is attention_requested' do
+ before do
+ reviewer.update!(state: :attention_requested)
+ end
+
+ it 'does not change state' do
+ service.execute
+ reviewer.reload
+
+ expect(reviewer.state).to eq 'attention_requested'
+ end
+
+ it 'does not create a new todo for the reviewer' do
+ expect(todo_svc).not_to receive(:create_attention_requested_todo).with(merge_request, current_user, user)
+
+ service.execute
+ end
+ end
+ end
+end
diff --git a/spec/services/merge_requests/squash_service_spec.rb b/spec/services/merge_requests/squash_service_spec.rb
index 387be8471b5..9210242a11e 100644
--- a/spec/services/merge_requests/squash_service_spec.rb
+++ b/spec/services/merge_requests/squash_service_spec.rb
@@ -222,11 +222,13 @@ RSpec.describe MergeRequests::SquashService do
it 'logs the error' do
expect(service).to receive(:log_error).with(exception: exception, message: 'Failed to squash merge request').and_call_original
expect(Gitlab::ErrorTracking).to receive(:track_exception).with(exception,
- class: described_class.to_s,
- merge_request: merge_request_ref,
- merge_request_id: merge_request.id,
- message: 'Failed to squash merge request',
- save_message_on_model: false).and_call_original
+ {
+ class: described_class.to_s,
+ merge_request: merge_request_ref,
+ merge_request_id: merge_request.id,
+ message: 'Failed to squash merge request',
+ save_message_on_model: false
+ }).and_call_original
service.execute
end
diff --git a/spec/services/merge_requests/toggle_attention_requested_service_spec.rb b/spec/services/merge_requests/toggle_attention_requested_service_spec.rb
index dcaac5d2699..20bc536b21e 100644
--- a/spec/services/merge_requests/toggle_attention_requested_service_spec.rb
+++ b/spec/services/merge_requests/toggle_attention_requested_service_spec.rb
@@ -80,7 +80,7 @@ RSpec.describe MergeRequests::ToggleAttentionRequestedService do
it 'removes attention requested state' do
expect(MergeRequests::RemoveAttentionRequestedService).to receive(:new)
- .with(project: project, current_user: current_user, merge_request: merge_request)
+ .with(project: project, current_user: current_user, merge_request: merge_request, user: current_user)
.and_call_original
service.execute
@@ -129,7 +129,7 @@ RSpec.describe MergeRequests::ToggleAttentionRequestedService do
it 'removes attention requested state' do
expect(MergeRequests::RemoveAttentionRequestedService).to receive(:new)
- .with(project: project, current_user: current_user, merge_request: merge_request)
+ .with(project: project, current_user: current_user, merge_request: merge_request, user: current_user)
.and_call_original
service.execute
diff --git a/spec/services/merge_requests/update_assignees_service_spec.rb b/spec/services/merge_requests/update_assignees_service_spec.rb
index 3a0b17c2768..f5f6f0ca301 100644
--- a/spec/services/merge_requests/update_assignees_service_spec.rb
+++ b/spec/services/merge_requests/update_assignees_service_spec.rb
@@ -113,6 +113,49 @@ RSpec.describe MergeRequests::UpdateAssigneesService do
expect { service.execute(merge_request) }
.to issue_fewer_queries_than { update_service.execute(other_mr) }
end
+
+ context 'setting state of assignees' do
+ before do
+ stub_feature_flags(mr_attention_requests: false)
+ end
+
+ it 'does not set state as attention_requested if feature flag is disabled' do
+ update_merge_request
+
+ expect(merge_request.merge_request_assignees[0].state).not_to eq('attention_requested')
+ end
+
+ context 'feature flag is enabled for current_user' do
+ before do
+ stub_feature_flags(mr_attention_requests: user)
+ end
+
+ it 'sets state as attention_requested' do
+ update_merge_request
+
+ expect(merge_request.merge_request_assignees[0].state).to eq('attention_requested')
+ expect(merge_request.merge_request_assignees[0].updated_state_by).to eq(user)
+ end
+
+ it 'uses reviewers state if it is same user as new assignee' do
+ merge_request.reviewers << user2
+
+ update_merge_request
+
+ expect(merge_request.merge_request_assignees[0].state).to eq('unreviewed')
+ end
+
+ context 'when assignee_ids matches existing assignee' do
+ let(:opts) { { assignee_ids: [user3.id] } }
+
+ it 'keeps original assignees state' do
+ update_merge_request
+
+ expect(merge_request.find_assignee(user3).state).to eq('unreviewed')
+ end
+ end
+ end
+ end
end
end
end
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index 30095ebeb50..7164ba8fac0 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -328,6 +328,49 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
update_merge_request(reviewer_ids: [user.id])
end
+
+ context 'setting state of reviewers' do
+ before do
+ stub_feature_flags(mr_attention_requests: false)
+ end
+
+ it 'does not set state as attention_requested if feature flag is disabled' do
+ update_merge_request(reviewer_ids: [user.id])
+
+ expect(merge_request.merge_request_reviewers[0].state).not_to eq('attention_requested')
+ end
+
+ context 'feature flag is enabled for current_user' do
+ before do
+ stub_feature_flags(mr_attention_requests: user)
+ end
+
+ it 'sets state as attention_requested' do
+ update_merge_request(reviewer_ids: [user2.id])
+
+ expect(merge_request.merge_request_reviewers[0].state).to eq('attention_requested')
+ expect(merge_request.merge_request_reviewers[0].updated_state_by).to eq(user)
+ end
+
+ it 'keeps original reviewers state' do
+ merge_request.find_reviewer(user2).update!(state: :unreviewed)
+
+ update_merge_request({
+ reviewer_ids: [user2.id]
+ })
+
+ expect(merge_request.find_reviewer(user2).state).to eq('unreviewed')
+ end
+
+ it 'uses reviewers state if it is same user as new assignee' do
+ merge_request.assignees << user
+
+ update_merge_request(reviewer_ids: [user.id])
+
+ expect(merge_request.merge_request_reviewers[0].state).to eq('unreviewed')
+ end
+ end
+ end
end
it 'creates a resource label event' do
@@ -1066,6 +1109,53 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
end
end
end
+
+ context 'setting state of assignees' do
+ before do
+ stub_feature_flags(mr_attention_requests: false)
+ end
+
+ it 'does not set state as attention_requested if feature flag is disabled' do
+ update_merge_request({
+ assignee_ids: [user2.id]
+ })
+
+ expect(merge_request.merge_request_assignees[0].state).not_to eq('attention_requested')
+ end
+
+ context 'feature flag is enabled for current_user' do
+ before do
+ stub_feature_flags(mr_attention_requests: user)
+ end
+
+ it 'sets state as attention_requested' do
+ update_merge_request({
+ assignee_ids: [user2.id]
+ })
+
+ expect(merge_request.merge_request_assignees[0].state).to eq('attention_requested')
+ expect(merge_request.merge_request_assignees[0].updated_state_by).to eq(user)
+ end
+
+ it 'keeps original assignees state' do
+ update_merge_request({
+ assignee_ids: [user3.id]
+ })
+
+ expect(merge_request.find_assignee(user3).state).to eq('unreviewed')
+ end
+
+ it 'uses reviewers state if it is same user as new assignee' do
+ merge_request.reviewers << user2
+
+ update_merge_request({
+ assignee_ids: [user2.id]
+ })
+
+ expect(merge_request.merge_request_assignees[0].state).to eq('unreviewed')
+ end
+ end
+ end
end
context 'when adding time spent' do
diff --git a/spec/services/namespaces/package_settings/update_service_spec.rb b/spec/services/namespaces/package_settings/update_service_spec.rb
index 030bc03038e..ed385f1cd7f 100644
--- a/spec/services/namespaces/package_settings/update_service_spec.rb
+++ b/spec/services/namespaces/package_settings/update_service_spec.rb
@@ -71,7 +71,7 @@ RSpec.describe ::Namespaces::PackageSettings::UpdateService do
where(:user_role, :shared_examples_name) do
:maintainer | 'updating the namespace package setting'
- :developer | 'updating the namespace package setting'
+ :developer | 'denying access to namespace package setting'
:reporter | 'denying access to namespace package setting'
:guest | 'denying access to namespace package setting'
:anonymous | 'denying access to namespace package setting'
@@ -91,7 +91,7 @@ RSpec.describe ::Namespaces::PackageSettings::UpdateService do
where(:user_role, :shared_examples_name) do
:maintainer | 'creating the namespace package setting'
- :developer | 'creating the namespace package setting'
+ :developer | 'denying access to namespace package setting'
:reporter | 'denying access to namespace package setting'
:guest | 'denying access to namespace package setting'
:anonymous | 'denying access to namespace package setting'
diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb
index b0410123630..c72a9465f20 100644
--- a/spec/services/notes/create_service_spec.rb
+++ b/spec/services/notes/create_service_spec.rb
@@ -168,7 +168,6 @@ RSpec.describe Notes::CreateService do
before do
project_with_repo.add_maintainer(user)
end
-
context 'when eligible to have a note diff file' do
let(:new_opts) do
opts.merge(in_reply_to_discussion_id: nil,
@@ -196,6 +195,39 @@ RSpec.describe Notes::CreateService do
described_class.new(project_with_repo, user, new_opts).execute(skip_capture_diff_note_position: true)
end
end
+
+ it 'does not track ipynb note usage data' do
+ expect(::Gitlab::UsageDataCounters::IpynbDiffActivityCounter).not_to receive(:note_created)
+
+ described_class.new(project_with_repo, user, new_opts).execute
+ end
+
+ context 'is ipynb file' do
+ before do
+ allow_any_instance_of(::Gitlab::Diff::File).to receive(:ipynb?).and_return(true)
+ stub_feature_flags(ipynbdiff_notes_tracker: false)
+ end
+
+ context ':ipynbdiff_notes_tracker is off' do
+ it 'does not track ipynb note usage data' do
+ expect(::Gitlab::UsageDataCounters::IpynbDiffActivityCounter).not_to receive(:note_created)
+
+ described_class.new(project_with_repo, user, new_opts).execute
+ end
+ end
+
+ context ':ipynbdiff_notes_tracker is on' do
+ before do
+ stub_feature_flags(ipynbdiff_notes_tracker: true)
+ end
+
+ it 'tracks ipynb diff note creation' do
+ expect(::Gitlab::UsageDataCounters::IpynbDiffActivityCounter).to receive(:note_created)
+
+ described_class.new(project_with_repo, user, new_opts).execute
+ end
+ end
+ end
end
context 'when DiffNote is a reply' do
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index d2d55c5ab79..743a04eabe6 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -1869,6 +1869,61 @@ RSpec.describe NotificationService, :mailer do
let(:notification_trigger) { notification.new_merge_request(merge_request, @u_disabled) }
end
+ describe 'Approvals' do
+ let(:notification_target) { merge_request }
+ let(:maintainer) { create(:user) }
+
+ describe '#approve_mr' do
+ it 'will notify the author, subscribers, and assigned users' do
+ notification.approve_mr(merge_request, maintainer)
+
+ merge_request.assignees.each { |assignee| should_email(assignee) }
+ should_email(merge_request.author)
+ should_email(@u_watcher)
+ should_email(@u_participant_mentioned)
+ should_email(@subscribed_participant)
+ should_email(@subscriber)
+ should_email(@watcher_and_subscriber)
+ should_email(@u_guest_watcher)
+
+ should_not_email(@unsubscriber)
+ should_not_email(@u_participating)
+ should_not_email(@u_disabled)
+ should_not_email(@u_lazy_participant)
+
+ expect(email_recipients.size).to eq(8)
+ # assignee, author, @u_watcher,
+ # @u_participant_mentioned, @subscribed_participant,
+ # @subscriber, @watcher_and_subscriber, @u_guest_watcher
+ end
+ end
+
+ describe '#unapprove_mr' do
+ it 'will notify the author, subscribers, and assigned users' do
+ notification.unapprove_mr(merge_request, maintainer)
+
+ merge_request.assignees.each { |assignee| should_email(assignee) }
+ should_email(merge_request.author)
+ should_email(@u_watcher)
+ should_email(@u_participant_mentioned)
+ should_email(@subscribed_participant)
+ should_email(@subscriber)
+ should_email(@watcher_and_subscriber)
+ should_email(@u_guest_watcher)
+
+ should_not_email(@unsubscriber)
+ should_not_email(@u_participating)
+ should_not_email(@u_disabled)
+ should_not_email(@u_lazy_participant)
+
+ expect(email_recipients.size).to eq(8)
+ # assignee, author, @u_watcher,
+ # @u_participant_mentioned, @subscribed_participant,
+ # @subscriber, @watcher_and_subscriber, @u_guest_watcher
+ end
+ end
+ end
+
context 'participating' do
it_behaves_like 'participating by assignee notification' do
let(:participant) { create(:user, username: 'user-participant')}
@@ -3653,6 +3708,26 @@ RSpec.describe NotificationService, :mailer do
end
end
+ describe '#inactive_project_deletion_warning' do
+ let_it_be(:deletion_date) { Date.current }
+ let_it_be(:project) { create(:project) }
+ let_it_be(:maintainer) { create(:user) }
+ let_it_be(:developer) { create(:user) }
+
+ before do
+ project.add_maintainer(maintainer)
+ end
+
+ subject { notification.inactive_project_deletion_warning(project, deletion_date) }
+
+ it "sends email to project owners and maintainers" do
+ expect { subject }.to have_enqueued_email(project, maintainer, deletion_date,
+ mail: "inactive_project_deletion_warning_email")
+ expect { subject }.not_to have_enqueued_email(project, developer, deletion_date,
+ mail: "inactive_project_deletion_warning_email")
+ end
+ end
+
def build_team(project)
@u_watcher = create_global_setting_for(create(:user), :watch)
@u_participating = create_global_setting_for(create(:user), :participating)
diff --git a/spec/services/projects/after_import_service_spec.rb b/spec/services/projects/after_import_service_spec.rb
deleted file mode 100644
index a16aec891a9..00000000000
--- a/spec/services/projects/after_import_service_spec.rb
+++ /dev/null
@@ -1,131 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Projects::AfterImportService do
- include GitHelpers
-
- subject { described_class.new(project) }
-
- let(:project) { create(:project, :repository) }
- let(:repository) { project.repository }
- let(:sha) { project.commit.sha }
- let(:housekeeping_service) { double(:housekeeping_service) }
-
- describe '#execute' do
- before do
- allow(Repositories::HousekeepingService)
- .to receive(:new).with(project).and_return(housekeeping_service)
-
- allow(housekeeping_service)
- .to receive(:execute).and_yield
-
- allow(housekeeping_service).to receive(:increment!)
- end
-
- it 'performs housekeeping' do
- subject.execute
-
- expect(housekeeping_service).to have_received(:execute)
- end
-
- context 'with some refs in refs/pull/**/*' do
- before do
- repository.write_ref('refs/pull/1/head', sha)
- repository.write_ref('refs/pull/1/merge', sha)
-
- subject.execute
- end
-
- it 'removes refs/pull/**/*' do
- expect(rugged.references.map(&:name))
- .not_to include(%r{\Arefs/pull/})
- end
- end
-
- Repository::RESERVED_REFS_NAMES.each do |name|
- context "with a ref in refs/#{name}/tmp" do
- before do
- repository.write_ref("refs/#{name}/tmp", sha)
-
- subject.execute
- end
-
- it "does not remove refs/#{name}/tmp" do
- expect(rugged.references.map(&:name))
- .to include("refs/#{name}/tmp")
- end
- end
- end
-
- context 'when after import action throw non-retriable exception' do
- let(:exception) { StandardError.new('after import error') }
-
- before do
- allow(repository)
- .to receive(:delete_all_refs_except)
- .and_raise(exception)
- end
-
- it 'throws after import error' do
- expect { subject.execute }.to raise_exception('after import error')
- end
- end
-
- context 'when housekeeping service lease is taken' do
- let(:exception) { Repositories::HousekeepingService::LeaseTaken.new }
-
- it 'logs the error message' do
- allow_next_instance_of(Repositories::HousekeepingService) do |instance|
- expect(instance).to receive(:execute).and_raise(exception)
- end
-
- expect(Gitlab::Import::Logger).to receive(:info).with(
- {
- message: 'Project housekeeping failed',
- project_full_path: project.full_path,
- project_id: project.id,
- 'error.message' => exception.to_s
- }).and_call_original
-
- subject.execute
- end
- end
-
- context 'when after import action throw retriable exception one time' do
- let(:exception) { GRPC::DeadlineExceeded.new }
-
- before do
- expect(repository)
- .to receive(:delete_all_refs_except)
- .and_raise(exception)
- expect(repository)
- .to receive(:delete_all_refs_except)
- .and_call_original
-
- subject.execute
- end
-
- it 'removes refs/pull/**/*' do
- expect(rugged.references.map(&:name))
- .not_to include(%r{\Arefs/pull/})
- end
-
- it 'records the failures in the database', :aggregate_failures do
- import_failure = ImportFailure.last
-
- expect(import_failure.source).to eq('delete_all_refs')
- expect(import_failure.project_id).to eq(project.id)
- expect(import_failure.relation_key).to be_nil
- expect(import_failure.relation_index).to be_nil
- expect(import_failure.exception_class).to eq('GRPC::DeadlineExceeded')
- expect(import_failure.exception_message).to be_present
- expect(import_failure.correlation_id_value).not_to be_empty
- end
- end
-
- def rugged
- rugged_repo(repository)
- end
- end
-end
diff --git a/spec/services/projects/android_target_platform_detector_service_spec.rb b/spec/services/projects/android_target_platform_detector_service_spec.rb
new file mode 100644
index 00000000000..74fd320bb48
--- /dev/null
+++ b/spec/services/projects/android_target_platform_detector_service_spec.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Projects::AndroidTargetPlatformDetectorService do
+ let_it_be(:project) { build(:project) }
+
+ subject { described_class.new(project).execute }
+
+ before do
+ allow(Gitlab::FileFinder).to receive(:new) { finder }
+ end
+
+ context 'when project is not an Android project' do
+ let(:finder) { instance_double(Gitlab::FileFinder, find: []) }
+
+ it { is_expected.to be_nil }
+ end
+
+ context 'when project is an Android project' do
+ let(:finder) { instance_double(Gitlab::FileFinder) }
+
+ before do
+ query = described_class::MANIFEST_FILE_SEARCH_QUERY
+ allow(finder).to receive(:find).with(query) { [instance_double(Gitlab::Search::FoundBlob)] }
+ end
+
+ it { is_expected.to eq :android }
+ end
+end
diff --git a/spec/services/projects/batch_open_issues_count_service_spec.rb b/spec/services/projects/batch_open_issues_count_service_spec.rb
index 17bd5f7a37b..89a4abbf9c9 100644
--- a/spec/services/projects/batch_open_issues_count_service_spec.rb
+++ b/spec/services/projects/batch_open_issues_count_service_spec.rb
@@ -5,7 +5,6 @@ require 'spec_helper'
RSpec.describe Projects::BatchOpenIssuesCountService do
let!(:project_1) { create(:project) }
let!(:project_2) { create(:project) }
- let!(:banned_user) { create(:user, :banned) }
let(:subject) { described_class.new([project_1, project_2]) }
@@ -13,41 +12,32 @@ RSpec.describe Projects::BatchOpenIssuesCountService do
before do
create(:issue, project: project_1)
create(:issue, project: project_1, confidential: true)
- create(:issue, project: project_1, author: banned_user)
+
create(:issue, project: project_2)
create(:issue, project: project_2, confidential: true)
- create(:issue, project: project_2, author: banned_user)
end
- context 'when cache is clean', :aggregate_failures do
+ context 'when cache is clean' do
it 'refreshes cache keys correctly' do
- expect(get_cache_key(project_1)).to eq(nil)
- expect(get_cache_key(project_2)).to eq(nil)
-
- subject.count_service.new(project_1).refresh_cache
- subject.count_service.new(project_2).refresh_cache
-
- expect(get_cache_key(project_1)).to eq(1)
- expect(get_cache_key(project_2)).to eq(1)
+ subject.refresh_cache_and_retrieve_data
- expect(get_cache_key(project_1, true)).to eq(2)
- expect(get_cache_key(project_2, true)).to eq(2)
+ # It does not update total issues cache
+ expect(Rails.cache.read(get_cache_key(subject, project_1))).to eq(nil)
+ expect(Rails.cache.read(get_cache_key(subject, project_2))).to eq(nil)
- expect(get_cache_key(project_1, true, true)).to eq(3)
- expect(get_cache_key(project_2, true, true)).to eq(3)
+ expect(Rails.cache.read(get_cache_key(subject, project_1, true))).to eq(1)
+ expect(Rails.cache.read(get_cache_key(subject, project_1, true))).to eq(1)
end
end
end
- def get_cache_key(project, with_confidential = false, with_hidden = false)
+ def get_cache_key(subject, project, public_key = false)
service = subject.count_service.new(project)
- if with_confidential && with_hidden
- Rails.cache.read(service.cache_key(service.class::TOTAL_COUNT_KEY))
- elsif with_confidential
- Rails.cache.read(service.cache_key(service.class::TOTAL_COUNT_WITHOUT_HIDDEN_KEY))
+ if public_key
+ service.cache_key(service.class::PUBLIC_COUNT_KEY)
else
- Rails.cache.read(service.cache_key(service.class::PUBLIC_COUNT_WITHOUT_HIDDEN_KEY))
+ service.cache_key(service.class::TOTAL_COUNT_KEY)
end
end
end
diff --git a/spec/services/projects/blame_service_spec.rb b/spec/services/projects/blame_service_spec.rb
new file mode 100644
index 00000000000..40b2bc869dc
--- /dev/null
+++ b/spec/services/projects/blame_service_spec.rb
@@ -0,0 +1,129 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Projects::BlameService, :aggregate_failures do
+ subject(:service) { described_class.new(blob, commit, params) }
+
+ let_it_be(:project) { create(:project, :repository) }
+ let_it_be(:commit) { project.repository.commit }
+ let_it_be(:blob) { project.repository.blob_at('HEAD', 'README.md') }
+
+ let(:params) { { page: page } }
+ let(:page) { nil }
+
+ before do
+ stub_const("#{described_class.name}::PER_PAGE", 2)
+ end
+
+ describe '#blame' do
+ subject { service.blame }
+
+ it 'returns a correct Gitlab::Blame object' do
+ is_expected.to be_kind_of(Gitlab::Blame)
+
+ expect(subject.blob).to eq(blob)
+ expect(subject.commit).to eq(commit)
+ expect(subject.range).to eq(1..2)
+ end
+
+ describe 'Pagination range calculation' do
+ subject { service.blame.range }
+
+ context 'with page = 1' do
+ let(:page) { 1 }
+
+ it { is_expected.to eq(1..2) }
+ end
+
+ context 'with page = 2' do
+ let(:page) { 2 }
+
+ it { is_expected.to eq(3..4) }
+ end
+
+ context 'with page = 3 (overlimit)' do
+ let(:page) { 3 }
+
+ it { is_expected.to eq(1..2) }
+ end
+
+ context 'with page = 0 (incorrect)' do
+ let(:page) { 0 }
+
+ it { is_expected.to eq(1..2) }
+ end
+
+ context 'when feature flag disabled' do
+ before do
+ stub_feature_flags(blame_page_pagination: false)
+ end
+
+ it { is_expected.to be_nil }
+ end
+ end
+ end
+
+ describe '#pagination' do
+ subject { service.pagination }
+
+ it 'returns a pagination object' do
+ is_expected.to be_kind_of(Kaminari::PaginatableArray)
+
+ expect(subject.current_page).to eq(1)
+ expect(subject.total_pages).to eq(2)
+ expect(subject.total_count).to eq(4)
+ end
+
+ context 'when feature flag disabled' do
+ before do
+ stub_feature_flags(blame_page_pagination: false)
+ end
+
+ it { is_expected.to be_nil }
+ end
+
+ context 'when per_page is above the global max per page limit' do
+ before do
+ stub_const("#{described_class.name}::PER_PAGE", 1000)
+ allow(blob).to receive_message_chain(:data, :lines, :count) { 500 }
+ end
+
+ it 'returns a correct pagination object' do
+ is_expected.to be_kind_of(Kaminari::PaginatableArray)
+
+ expect(subject.current_page).to eq(1)
+ expect(subject.total_pages).to eq(1)
+ expect(subject.total_count).to eq(500)
+ end
+ end
+
+ describe 'Current page' do
+ subject { service.pagination.current_page }
+
+ context 'with page = 1' do
+ let(:page) { 1 }
+
+ it { is_expected.to eq(1) }
+ end
+
+ context 'with page = 2' do
+ let(:page) { 2 }
+
+ it { is_expected.to eq(2) }
+ end
+
+ context 'with page = 3 (overlimit)' do
+ let(:page) { 3 }
+
+ it { is_expected.to eq(1) }
+ end
+
+ context 'with page = 0 (incorrect)' do
+ let(:page) { 0 }
+
+ it { is_expected.to eq(1) }
+ end
+ end
+ end
+end
diff --git a/spec/services/projects/container_repository/cleanup_tags_service_spec.rb b/spec/services/projects/container_repository/cleanup_tags_service_spec.rb
index 86c0ba4222c..79904e2bf72 100644
--- a/spec/services/projects/container_repository/cleanup_tags_service_spec.rb
+++ b/spec/services/projects/container_repository/cleanup_tags_service_spec.rb
@@ -34,8 +34,6 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService, :clean_gitlab_
stub_digest_config('sha256:configB', 5.days.ago)
stub_digest_config('sha256:configC', 1.month.ago)
stub_digest_config('sha256:configD', nil)
-
- stub_feature_flags(container_registry_expiration_policies_throttling: false)
end
describe '#execute' do
@@ -334,24 +332,17 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService, :clean_gitlab_
end
end
- where(:feature_flag_enabled, :max_list_size, :delete_tags_service_status, :expected_status, :expected_truncated) do
- false | 10 | :success | :success | false
- false | 10 | :error | :error | false
- false | 3 | :success | :success | false
- false | 3 | :error | :error | false
- false | 0 | :success | :success | false
- false | 0 | :error | :error | false
- true | 10 | :success | :success | false
- true | 10 | :error | :error | false
- true | 3 | :success | :error | true
- true | 3 | :error | :error | true
- true | 0 | :success | :success | false
- true | 0 | :error | :error | false
+ where(:max_list_size, :delete_tags_service_status, :expected_status, :expected_truncated) do
+ 10 | :success | :success | false
+ 10 | :error | :error | false
+ 3 | :success | :error | true
+ 3 | :error | :error | true
+ 0 | :success | :success | false
+ 0 | :error | :error | false
end
with_them do
before do
- stub_feature_flags(container_registry_expiration_policies_throttling: feature_flag_enabled)
stub_application_setting(container_registry_cleanup_tags_service_max_list_size: max_list_size)
allow_next_instance_of(Projects::ContainerRepository::DeleteTagsService) do |service|
expect(service).to receive(:execute).and_return(status: delete_tags_service_status)
@@ -429,10 +420,10 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService, :clean_gitlab_
end
# We will ping the container registry for all tags *except* for C because it's cached
- expect(ContainerRegistry::Blob).to receive(:new).with(repository, "digest" => "sha256:configA").and_call_original
- expect(ContainerRegistry::Blob).to receive(:new).with(repository, "digest" => "sha256:configB").twice.and_call_original
- expect(ContainerRegistry::Blob).not_to receive(:new).with(repository, "digest" => "sha256:configC")
- expect(ContainerRegistry::Blob).to receive(:new).with(repository, "digest" => "sha256:configD").and_call_original
+ expect(ContainerRegistry::Blob).to receive(:new).with(repository, { "digest" => "sha256:configA" }).and_call_original
+ expect(ContainerRegistry::Blob).to receive(:new).with(repository, { "digest" => "sha256:configB" }).twice.and_call_original
+ expect(ContainerRegistry::Blob).not_to receive(:new).with(repository, { "digest" => "sha256:configC" })
+ expect(ContainerRegistry::Blob).to receive(:new).with(repository, { "digest" => "sha256:configD" }).and_call_original
expect(subject).to include(cached_tags_count: 1)
end
diff --git a/spec/services/projects/container_repository/delete_tags_service_spec.rb b/spec/services/projects/container_repository/delete_tags_service_spec.rb
index 246ca301cfa..9e6849aa514 100644
--- a/spec/services/projects/container_repository/delete_tags_service_spec.rb
+++ b/spec/services/projects/container_repository/delete_tags_service_spec.rb
@@ -3,6 +3,7 @@
require 'spec_helper'
RSpec.describe Projects::ContainerRepository::DeleteTagsService do
+ using RSpec::Parameterized::TableSyntax
include_context 'container repository delete tags service shared context'
let(:service) { described_class.new(project, user, params) }
@@ -17,11 +18,13 @@ RSpec.describe Projects::ContainerRepository::DeleteTagsService do
shared_examples 'logging a success response' do
it 'logs an info message' do
expect(service).to receive(:log_info).with(
- service_class: 'Projects::ContainerRepository::DeleteTagsService',
- message: 'deleted tags',
- container_repository_id: repository.id,
- project_id: repository.project_id,
- deleted_tags_count: tags.size
+ {
+ service_class: 'Projects::ContainerRepository::DeleteTagsService',
+ message: 'deleted tags',
+ container_repository_id: repository.id,
+ project_id: repository.project_id,
+ deleted_tags_count: tags.size
+ }
)
subject
@@ -131,10 +134,6 @@ RSpec.describe Projects::ContainerRepository::DeleteTagsService do
subject { service.execute(repository) }
- before do
- stub_feature_flags(container_registry_expiration_policies_throttling: false)
- end
-
context 'without permissions' do
it { is_expected.to include(status: :error) }
end
@@ -157,11 +156,39 @@ RSpec.describe Projects::ContainerRepository::DeleteTagsService do
end
context 'when the repository is importing' do
- before do
- repository.update_columns(migration_state: 'importing', migration_import_started_at: Time.zone.now)
+ where(:migration_state, :called_by_policy, :error_expected) do
+ 'default' | false | false
+ 'default' | true | false
+ 'pre_importing' | false | false
+ 'pre_importing' | true | true
+ 'pre_import_done' | false | false
+ 'pre_import_done' | true | true
+ 'importing' | false | true
+ 'importing' | true | true
+ 'import_done' | false | false
+ 'import_done' | true | false
+ 'import_aborted' | false | false
+ 'import_aborted' | true | false
+ 'import_skipped' | false | false
+ 'import_skipped' | true | false
end
- it { is_expected.to include(status: :error, message: 'repository importing') }
+ with_them do
+ let(:params) { { tags: tags, container_expiration_policy: called_by_policy ? true : nil } }
+
+ before do
+ repository.update_columns(migration_state: migration_state, migration_import_started_at: Time.zone.now, migration_pre_import_started_at: Time.zone.now, migration_pre_import_done_at: Time.zone.now)
+ end
+
+ it 'returns an error response if expected' do
+ if error_expected
+ expect(subject).to include(status: :error, message: 'repository importing')
+ else
+ expect(service).to receive(:delete_tags).and_return(status: :success)
+ expect(subject).not_to include(status: :error)
+ end
+ end
+ end
end
end
diff --git a/spec/services/projects/container_repository/gitlab/delete_tags_service_spec.rb b/spec/services/projects/container_repository/gitlab/delete_tags_service_spec.rb
index 74f782538c5..8d8907119f0 100644
--- a/spec/services/projects/container_repository/gitlab/delete_tags_service_spec.rb
+++ b/spec/services/projects/container_repository/gitlab/delete_tags_service_spec.rb
@@ -12,10 +12,6 @@ RSpec.describe Projects::ContainerRepository::Gitlab::DeleteTagsService do
subject { service.execute }
- before do
- stub_feature_flags(container_registry_expiration_policies_throttling: false)
- end
-
RSpec.shared_examples 'deleting tags' do
it 'deletes the tags by name' do
stub_delete_reference_requests(tags)
@@ -26,6 +22,8 @@ RSpec.describe Projects::ContainerRepository::Gitlab::DeleteTagsService do
end
context 'with tags to delete' do
+ let(:timeout) { 10 }
+
it_behaves_like 'deleting tags'
it 'succeeds when tag delete returns 404' do
@@ -50,59 +48,52 @@ RSpec.describe Projects::ContainerRepository::Gitlab::DeleteTagsService do
end
end
- context 'with throttling enabled' do
- let(:timeout) { 10 }
-
- before do
- stub_feature_flags(container_registry_expiration_policies_throttling: true)
- stub_application_setting(container_registry_delete_tags_service_timeout: timeout)
- end
-
- it_behaves_like 'deleting tags'
+ before do
+ stub_application_setting(container_registry_delete_tags_service_timeout: timeout)
+ end
- context 'with timeout' do
- context 'set to a valid value' do
- before do
- allow(Time.zone).to receive(:now).and_return(10, 15, 25) # third call to Time.zone.now will be triggering the timeout
- stub_delete_reference_requests('A' => 200)
- end
+ context 'with timeout' do
+ context 'set to a valid value' do
+ before do
+ allow(Time.zone).to receive(:now).and_return(10, 15, 25) # third call to Time.zone.now will be triggering the timeout
+ stub_delete_reference_requests('A' => 200)
+ end
- it { is_expected.to eq(status: :error, message: 'error while deleting tags', deleted: ['A'], exception_class_name: Projects::ContainerRepository::Gitlab::DeleteTagsService::TimeoutError.name) }
+ it { is_expected.to eq(status: :error, message: 'error while deleting tags', deleted: ['A'], exception_class_name: Projects::ContainerRepository::Gitlab::DeleteTagsService::TimeoutError.name) }
- it 'tracks the exception' do
- expect(::Gitlab::ErrorTracking)
- .to receive(:track_exception).with(::Projects::ContainerRepository::Gitlab::DeleteTagsService::TimeoutError, tags_count: tags.size, container_repository_id: repository.id)
+ it 'tracks the exception' do
+ expect(::Gitlab::ErrorTracking)
+ .to receive(:track_exception).with(::Projects::ContainerRepository::Gitlab::DeleteTagsService::TimeoutError, tags_count: tags.size, container_repository_id: repository.id)
- subject
- end
+ subject
end
+ end
- context 'set to 0' do
- let(:timeout) { 0 }
+ context 'set to 0' do
+ let(:timeout) { 0 }
- it_behaves_like 'deleting tags'
- end
+ it_behaves_like 'deleting tags'
+ end
- context 'set to nil' do
- let(:timeout) { nil }
+ context 'set to nil' do
+ let(:timeout) { nil }
- it_behaves_like 'deleting tags'
- end
+ it_behaves_like 'deleting tags'
end
+ end
- context 'with a network error' do
- before do
- expect(service).to receive(:delete_tags).and_raise(::Faraday::TimeoutError)
- end
+ context 'with a network error' do
+ before do
+ expect(service).to receive(:delete_tags).and_raise(::Faraday::TimeoutError)
+ end
- it { is_expected.to eq(status: :error, message: 'error while deleting tags', deleted: [], exception_class_name: ::Faraday::TimeoutError.name) }
+ it { is_expected.to eq(status: :error, message: 'error while deleting tags', deleted: [], exception_class_name: ::Faraday::TimeoutError.name) }
- it 'tracks the exception' do
- expect(::Gitlab::ErrorTracking)
- .to receive(:track_exception).with(::Faraday::TimeoutError, tags_count: tags.size, container_repository_id: repository.id)
+ it 'tracks the exception' do
+ expect(::Gitlab::ErrorTracking)
+ .to receive(:track_exception).with(::Faraday::TimeoutError, tags_count: tags.size, container_repository_id: repository.id)
- subject
- end
+ subject
end
end
end
diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb
index c5c5af3cb01..cd1e629e1d2 100644
--- a/spec/services/projects/create_service_spec.rb
+++ b/spec/services/projects/create_service_spec.rb
@@ -141,18 +141,6 @@ RSpec.describe Projects::CreateService, '#execute' do
expect(project.project_setting).to be_persisted
end
- context 'create_project_settings feature flag is disabled' do
- before do
- stub_feature_flags(create_project_settings: false)
- end
-
- it 'builds associated project settings' do
- project = create_project(user, opts)
-
- expect(project.project_setting).to be_new_record
- end
- end
-
it_behaves_like 'storing arguments in the application context' do
let(:expected_params) { { project: subject.full_path } }
@@ -780,6 +768,45 @@ RSpec.describe Projects::CreateService, '#execute' do
create_project(user, opts)
end
+ context 'when import source is enabled' do
+ before do
+ stub_application_setting(import_sources: ['github'])
+ end
+
+ it 'does not raise an error when import_source is string' do
+ opts[:import_type] = 'github'
+
+ project = create_project(user, opts)
+
+ expect(project).to be_persisted
+ expect(project.errors).to be_blank
+ end
+
+ it 'does not raise an error when import_source is symbol' do
+ opts[:import_type] = :github
+
+ project = create_project(user, opts)
+
+ expect(project).to be_persisted
+ expect(project.errors).to be_blank
+ end
+ end
+
+ context 'when import source is disabled' do
+ before do
+ stub_application_setting(import_sources: [])
+ opts[:import_type] = 'git'
+ end
+
+ it 'raises an error' do
+ project = create_project(user, opts)
+
+ expect(project).to respond_to(:errors)
+ expect(project.errors).to have_key(:import_source_disabled)
+ expect(project.saved?).to be_falsey
+ end
+ end
+
context 'with external authorization enabled' do
before do
enable_external_authorization_service_check
@@ -797,7 +824,7 @@ RSpec.describe Projects::CreateService, '#execute' do
it 'saves the project when the user has access to the label' do
expect(::Gitlab::ExternalAuthorization)
- .to receive(:access_allowed?).with(user, 'new-label', any_args) { true }
+ .to receive(:access_allowed?).with(user, 'new-label', any_args) { true }.at_least(1).time
project = create_project(user, opts.merge({ external_authorization_classification_label: 'new-label' }))
diff --git a/spec/services/projects/group_links/create_service_spec.rb b/spec/services/projects/group_links/create_service_spec.rb
index 4ea5f2b3a53..65d3085a850 100644
--- a/spec/services/projects/group_links/create_service_spec.rb
+++ b/spec/services/projects/group_links/create_service_spec.rb
@@ -5,65 +5,104 @@ require 'spec_helper'
RSpec.describe Projects::GroupLinks::CreateService, '#execute' do
let_it_be(:user) { create :user }
let_it_be(:group) { create :group }
- let_it_be(:project) { create :project }
+ let_it_be(:project) { create(:project, namespace: create(:namespace, :with_namespace_settings)) }
- let(:group_access) { Gitlab::Access::DEVELOPER }
let(:opts) do
{
- link_group_access: group_access,
+ link_group_access: Gitlab::Access::DEVELOPER,
expires_at: nil
}
end
- subject { described_class.new(project, user, opts) }
+ subject { described_class.new(project, group, user, opts) }
- before do
- group.add_developer(user)
- end
+ shared_examples_for 'not shareable' do
+ it 'does not share and returns an error' do
+ expect do
+ result = subject.execute
- it 'adds group to project' do
- expect { subject.execute(group) }.to change { project.project_group_links.count }.from(0).to(1)
+ expect(result[:status]).to eq(:error)
+ expect(result[:http_status]).to eq(404)
+ end.not_to change { project.project_group_links.count }
+ end
end
- it 'updates authorization', :sidekiq_inline do
- expect { subject.execute(group) }.to(
- change { Ability.allowed?(user, :read_project, project) }
- .from(false).to(true))
- end
+ shared_examples_for 'shareable' do
+ it 'adds group to project' do
+ expect do
+ result = subject.execute
- it 'returns false if group is blank' do
- expect { subject.execute(nil) }.not_to change { project.project_group_links.count }
+ expect(result[:status]).to eq(:success)
+ end.to change { project.project_group_links.count }.from(0).to(1)
+ end
end
- it 'returns error if user is not allowed to share with a group' do
- expect { subject.execute(create(:group)) }.not_to change { project.project_group_links.count }
- end
+ context 'when user has proper membership to share a group' do
+ before do
+ group.add_guest(user)
+ end
- context 'with specialized project_authorization workers' do
- let_it_be(:other_user) { create(:user) }
+ it_behaves_like 'shareable'
- before do
- group.add_developer(other_user)
+ it 'updates authorization', :sidekiq_inline do
+ expect { subject.execute }.to(
+ change { Ability.allowed?(user, :read_project, project) }
+ .from(false).to(true))
+ end
+
+ context 'with specialized project_authorization workers' do
+ let_it_be(:other_user) { create(:user) }
+
+ before do
+ group.add_developer(other_user)
+ end
+
+ it 'schedules authorization update for users with access to group' do
+ expect(AuthorizedProjectsWorker).not_to(
+ receive(:bulk_perform_async)
+ )
+ expect(AuthorizedProjectUpdate::ProjectRecalculateWorker).to(
+ receive(:perform_async)
+ .with(project.id)
+ .and_call_original
+ )
+ expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to(
+ receive(:bulk_perform_in)
+ .with(1.hour,
+ array_including([user.id], [other_user.id]),
+ batch_delay: 30.seconds, batch_size: 100)
+ .and_call_original
+ )
+
+ subject.execute
+ end
end
- it 'schedules authorization update for users with access to group' do
- expect(AuthorizedProjectsWorker).not_to(
- receive(:bulk_perform_async)
- )
- expect(AuthorizedProjectUpdate::ProjectRecalculateWorker).to(
- receive(:perform_async)
- .with(project.id)
- .and_call_original
- )
- expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to(
- receive(:bulk_perform_in)
- .with(1.hour,
- array_including([user.id], [other_user.id]),
- batch_delay: 30.seconds, batch_size: 100)
- .and_call_original
- )
-
- subject.execute(group)
+ context 'when sharing outside the hierarchy is disabled' do
+ let_it_be(:shared_group_parent) do
+ create(:group,
+ namespace_settings: create(:namespace_settings, prevent_sharing_groups_outside_hierarchy: true))
+ end
+
+ let_it_be(:project, reload: true) { create(:project, group: shared_group_parent) }
+
+ it_behaves_like 'not shareable'
+
+ context 'when group is inside hierarchy' do
+ let(:group) { create(:group, :private, parent: shared_group_parent) }
+
+ it_behaves_like 'shareable'
+ end
end
end
+
+ context 'when user does not have permissions for the group' do
+ it_behaves_like 'not shareable'
+ end
+
+ context 'when group is blank' do
+ let(:group) { nil }
+
+ it_behaves_like 'not shareable'
+ end
end
diff --git a/spec/services/projects/in_product_marketing_campaign_emails_service_spec.rb b/spec/services/projects/in_product_marketing_campaign_emails_service_spec.rb
new file mode 100644
index 00000000000..4c51c8a4ac8
--- /dev/null
+++ b/spec/services/projects/in_product_marketing_campaign_emails_service_spec.rb
@@ -0,0 +1,130 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Projects::InProductMarketingCampaignEmailsService do
+ describe '#execute' do
+ let(:user) { create(:user, email_opted_in: true) }
+ let(:project) { create(:project) }
+ let(:campaign) { Users::InProductMarketingEmail::BUILD_IOS_APP_GUIDE }
+
+ subject(:execute) do
+ described_class.new(project, campaign).execute
+ end
+
+ context 'users can receive marketing emails' do
+ let(:owner) { create(:user, email_opted_in: true) }
+ let(:maintainer) { create(:user, email_opted_in: true) }
+ let(:developer) { create(:user, email_opted_in: true) }
+
+ before do
+ project.add_owner(owner)
+ project.add_developer(developer)
+ project.add_maintainer(maintainer)
+ end
+
+ it 'sends the email to all project members with access_level >= Developer', :aggregate_failures do
+ double = instance_double(ActionMailer::MessageDelivery, deliver_later: true)
+
+ [owner, maintainer, developer].each do |user|
+ email = user.notification_email_or_default
+
+ expect(Notify).to receive(:build_ios_app_guide_email).with(email) { double }
+ expect(double).to receive(:deliver_later)
+ end
+
+ execute
+ end
+
+ it 'records sent emails', :aggregate_failures do
+ expect { execute }.to change { Users::InProductMarketingEmail.count }.from(0).to(3)
+
+ [owner, maintainer, developer].each do |user|
+ expect(
+ Users::InProductMarketingEmail.where(
+ user: user,
+ campaign: campaign
+ )
+ ).to exist
+ end
+ end
+
+ it 'tracks experiment :email_sent event', :experiment do
+ expect(experiment(:build_ios_app_guide_email)).to track(:email_sent)
+ .on_next_instance
+ .with_context(project: project)
+
+ execute
+ end
+ end
+
+ shared_examples 'does nothing' do
+ it 'does not send the email' do
+ email = user.notification_email_or_default
+ expect(Notify).not_to receive(:build_ios_app_guide_email).with(email)
+ execute
+ end
+
+ it 'does not create a record of the sent email' do
+ expect { execute }.not_to change { Users::InProductMarketingEmail.count }
+ end
+ end
+
+ context "when user can't receive marketing emails" do
+ before do
+ project.add_developer(user)
+ end
+
+ context 'when user.can?(:receive_notifications) is false' do
+ it 'does not send the email' do
+ allow_next_found_instance_of(User) do |user|
+ allow(user).to receive(:can?).with(:receive_notifications) { false }
+
+ email = user.notification_email_or_default
+ expect(Notify).not_to receive(:build_ios_app_guide_email).with(email)
+
+ expect(
+ Users::InProductMarketingEmail.where(
+ user: user,
+ campaign: campaign
+ )
+ ).not_to exist
+ end
+
+ execute
+ end
+ end
+
+ context 'when user is not opted in to receive marketing emails' do
+ let(:user) { create(:user, email_opted_in: false) }
+
+ it_behaves_like 'does nothing'
+ end
+ end
+
+ context 'when campaign email has already been sent to the user' do
+ before do
+ project.add_developer(user)
+ create(:in_product_marketing_email, :campaign, user: user, campaign: campaign)
+ end
+
+ it_behaves_like 'does nothing'
+ end
+
+ context "when user is a reporter" do
+ before do
+ project.add_reporter(user)
+ end
+
+ it_behaves_like 'does nothing'
+ end
+
+ context "when user is a guest" do
+ before do
+ project.add_guest(user)
+ end
+
+ it_behaves_like 'does nothing'
+ end
+ end
+end
diff --git a/spec/services/projects/open_issues_count_service_spec.rb b/spec/services/projects/open_issues_count_service_spec.rb
index 8710d0c0267..c739fea5ecf 100644
--- a/spec/services/projects/open_issues_count_service_spec.rb
+++ b/spec/services/projects/open_issues_count_service_spec.rb
@@ -4,102 +4,89 @@ require 'spec_helper'
RSpec.describe Projects::OpenIssuesCountService, :use_clean_rails_memory_store_caching do
let(:project) { create(:project) }
- let(:user) { create(:user) }
- let(:banned_user) { create(:user, :banned) }
- subject { described_class.new(project, user) }
+ subject { described_class.new(project) }
it_behaves_like 'a counter caching service'
- before do
- create(:issue, :opened, project: project)
- create(:issue, :opened, confidential: true, project: project)
- create(:issue, :opened, author: banned_user, project: project)
- create(:issue, :closed, project: project)
-
- described_class.new(project).refresh_cache
- end
-
describe '#count' do
- shared_examples 'counts public issues, does not count hidden or confidential' do
- it 'counts only public issues' do
- expect(subject.count).to eq(1)
- end
-
- it 'uses PUBLIC_COUNT_WITHOUT_HIDDEN_KEY cache key' do
- expect(subject.cache_key).to include('project_open_public_issues_without_hidden_count')
- end
- end
-
context 'when user is nil' do
- let(:user) { nil }
+ it 'does not include confidential issues in the issue count' do
+ create(:issue, :opened, project: project)
+ create(:issue, :opened, confidential: true, project: project)
- it_behaves_like 'counts public issues, does not count hidden or confidential'
+ expect(described_class.new(project).count).to eq(1)
+ end
end
context 'when user is provided' do
+ let(:user) { create(:user) }
+
context 'when user can read confidential issues' do
before do
project.add_reporter(user)
end
- it 'includes confidential issues and does not include hidden issues in count' do
- expect(subject.count).to eq(2)
+ it 'returns the right count with confidential issues' do
+ create(:issue, :opened, project: project)
+ create(:issue, :opened, confidential: true, project: project)
+
+ expect(described_class.new(project, user).count).to eq(2)
end
- it 'uses TOTAL_COUNT_WITHOUT_HIDDEN_KEY cache key' do
- expect(subject.cache_key).to include('project_open_issues_without_hidden_count')
+ it 'uses total_open_issues_count cache key' do
+ expect(described_class.new(project, user).cache_key_name).to eq('total_open_issues_count')
end
end
- context 'when user cannot read confidential or hidden issues' do
+ context 'when user cannot read confidential issues' do
before do
project.add_guest(user)
end
- it_behaves_like 'counts public issues, does not count hidden or confidential'
- end
-
- context 'when user is an admin' do
- let_it_be(:user) { create(:user, :admin) }
-
- context 'when admin mode is enabled', :enable_admin_mode do
- it 'includes confidential and hidden issues in count' do
- expect(subject.count).to eq(3)
- end
+ it 'does not include confidential issues' do
+ create(:issue, :opened, project: project)
+ create(:issue, :opened, confidential: true, project: project)
- it 'uses TOTAL_COUNT_KEY cache key' do
- expect(subject.cache_key).to include('project_open_issues_including_hidden_count')
- end
+ expect(described_class.new(project, user).count).to eq(1)
end
- context 'when admin mode is disabled' do
- it_behaves_like 'counts public issues, does not count hidden or confidential'
+ it 'uses public_open_issues_count cache key' do
+ expect(described_class.new(project, user).cache_key_name).to eq('public_open_issues_count')
end
end
end
- end
-
- describe '#refresh_cache', :aggregate_failures do
- context 'when cache is empty' do
- it 'refreshes cache keys correctly' do
- expect(Rails.cache.read(described_class.new(project).cache_key(described_class::PUBLIC_COUNT_WITHOUT_HIDDEN_KEY))).to eq(1)
- expect(Rails.cache.read(described_class.new(project).cache_key(described_class::TOTAL_COUNT_WITHOUT_HIDDEN_KEY))).to eq(2)
- expect(Rails.cache.read(described_class.new(project).cache_key(described_class::TOTAL_COUNT_KEY))).to eq(3)
- end
- end
- context 'when cache is outdated' do
- it 'refreshes cache keys correctly' do
+ describe '#refresh_cache' do
+ before do
+ create(:issue, :opened, project: project)
create(:issue, :opened, project: project)
create(:issue, :opened, confidential: true, project: project)
- create(:issue, :opened, author: banned_user, project: project)
+ end
- described_class.new(project).refresh_cache
+ context 'when cache is empty' do
+ it 'refreshes cache keys correctly' do
+ subject.refresh_cache
- expect(Rails.cache.read(described_class.new(project).cache_key(described_class::PUBLIC_COUNT_WITHOUT_HIDDEN_KEY))).to eq(2)
- expect(Rails.cache.read(described_class.new(project).cache_key(described_class::TOTAL_COUNT_WITHOUT_HIDDEN_KEY))).to eq(4)
- expect(Rails.cache.read(described_class.new(project).cache_key(described_class::TOTAL_COUNT_KEY))).to eq(6)
+ expect(Rails.cache.read(subject.cache_key(described_class::PUBLIC_COUNT_KEY))).to eq(2)
+ expect(Rails.cache.read(subject.cache_key(described_class::TOTAL_COUNT_KEY))).to eq(3)
+ end
+ end
+
+ context 'when cache is outdated' do
+ before do
+ subject.refresh_cache
+ end
+
+ it 'refreshes cache keys correctly' do
+ create(:issue, :opened, project: project)
+ create(:issue, :opened, confidential: true, project: project)
+
+ subject.refresh_cache
+
+ expect(Rails.cache.read(subject.cache_key(described_class::PUBLIC_COUNT_KEY))).to eq(3)
+ expect(Rails.cache.read(subject.cache_key(described_class::TOTAL_COUNT_KEY))).to eq(5)
+ end
end
end
end
diff --git a/spec/services/projects/prometheus/alerts/create_service_spec.rb b/spec/services/projects/prometheus/alerts/create_service_spec.rb
deleted file mode 100644
index 6b9d43e4e81..00000000000
--- a/spec/services/projects/prometheus/alerts/create_service_spec.rb
+++ /dev/null
@@ -1,52 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Projects::Prometheus::Alerts::CreateService do
- let_it_be(:project) { create(:project) }
- let_it_be(:user) { create(:user) }
-
- let(:service) { described_class.new(project: project, current_user: user, params: params) }
-
- subject { service.execute }
-
- describe '#execute' do
- context 'with params' do
- let_it_be(:environment) { create(:environment, project: project) }
-
- let_it_be(:metric) do
- create(:prometheus_metric, project: project)
- end
-
- let(:params) do
- {
- environment_id: environment.id,
- prometheus_metric_id: metric.id,
- operator: '<',
- threshold: 1.0
- }
- end
-
- it 'creates an alert' do
- expect(subject).to be_persisted
-
- expect(subject).to have_attributes(
- project: project,
- environment: environment,
- prometheus_metric: metric,
- operator: 'lt',
- threshold: 1.0
- )
- end
- end
-
- context 'without params' do
- let(:params) { {} }
-
- it 'fails to create' do
- expect(subject).to be_new_record
- expect(subject).to be_invalid
- end
- end
- end
-end
diff --git a/spec/services/projects/prometheus/alerts/destroy_service_spec.rb b/spec/services/projects/prometheus/alerts/destroy_service_spec.rb
deleted file mode 100644
index a3e9c3516c2..00000000000
--- a/spec/services/projects/prometheus/alerts/destroy_service_spec.rb
+++ /dev/null
@@ -1,21 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Projects::Prometheus::Alerts::DestroyService do
- let_it_be(:project) { create(:project) }
- let_it_be(:user) { create(:user) }
- let_it_be(:alert) { create(:prometheus_alert, project: project) }
-
- let(:service) { described_class.new(project: project, current_user: user, params: nil) }
-
- describe '#execute' do
- subject { service.execute(alert) }
-
- it 'deletes the alert' do
- expect(subject).to be_truthy
-
- expect(alert).to be_destroyed
- end
- end
-end
diff --git a/spec/services/projects/prometheus/alerts/update_service_spec.rb b/spec/services/projects/prometheus/alerts/update_service_spec.rb
deleted file mode 100644
index ec6766221f6..00000000000
--- a/spec/services/projects/prometheus/alerts/update_service_spec.rb
+++ /dev/null
@@ -1,53 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Projects::Prometheus::Alerts::UpdateService do
- let_it_be(:project) { create(:project) }
- let_it_be(:user) { create(:user) }
- let_it_be(:environment) { create(:environment, project: project) }
-
- let_it_be(:alert) do
- create(:prometheus_alert, project: project, environment: environment)
- end
-
- let(:service) { described_class.new(project: project, current_user: user, params: params) }
-
- let(:params) do
- {
- environment_id: alert.environment_id,
- prometheus_metric_id: alert.prometheus_metric_id,
- operator: '==',
- threshold: 2.0
- }
- end
-
- describe '#execute' do
- subject { service.execute(alert) }
-
- context 'with valid params' do
- it 'updates the alert' do
- expect(subject).to be_truthy
-
- expect(alert.reload).to have_attributes(
- operator: 'eq',
- threshold: 2.0
- )
- end
- end
-
- context 'with invalid params' do
- let(:other_environment) { create(:environment) }
-
- before do
- params[:environment_id] = other_environment.id
- end
-
- it 'fails to update' do
- expect(subject).to be_falsey
-
- expect(alert).to be_invalid
- end
- end
- end
-end
diff --git a/spec/services/projects/prometheus/metrics/destroy_service_spec.rb b/spec/services/projects/prometheus/metrics/destroy_service_spec.rb
index 17cc88b27b6..b4af81f2c87 100644
--- a/spec/services/projects/prometheus/metrics/destroy_service_spec.rb
+++ b/spec/services/projects/prometheus/metrics/destroy_service_spec.rb
@@ -12,17 +12,4 @@ RSpec.describe Projects::Prometheus::Metrics::DestroyService do
expect(PrometheusMetric.find_by(id: metric.id)).to be_nil
end
-
- context 'when metric has a prometheus alert associated' do
- it 'schedules a prometheus alert update' do
- create(:prometheus_alert, project: metric.project, prometheus_metric: metric)
-
- schedule_update_service = spy
- allow(::Clusters::Applications::ScheduleUpdateService).to receive(:new).and_return(schedule_update_service)
-
- subject.execute
-
- expect(schedule_update_service).to have_received(:execute)
- end
- end
end
diff --git a/spec/services/projects/prometheus/metrics/update_service_spec.rb b/spec/services/projects/prometheus/metrics/update_service_spec.rb
deleted file mode 100644
index bf87093150c..00000000000
--- a/spec/services/projects/prometheus/metrics/update_service_spec.rb
+++ /dev/null
@@ -1,44 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Projects::Prometheus::Metrics::UpdateService do
- let(:metric) { create(:prometheus_metric) }
-
- it 'updates the prometheus metric' do
- expect do
- described_class.new(metric, { title: "bar" }).execute
- end.to change { metric.reload.title }.to("bar")
- end
-
- context 'when metric has a prometheus alert associated' do
- let(:schedule_update_service) { spy }
-
- before do
- create(:prometheus_alert, project: metric.project, prometheus_metric: metric)
- allow(::Clusters::Applications::ScheduleUpdateService).to receive(:new).and_return(schedule_update_service)
- end
-
- context 'when updating title' do
- it 'schedules a prometheus alert update' do
- described_class.new(metric, { title: "bar" }).execute
-
- expect(schedule_update_service).to have_received(:execute)
- end
- end
-
- context 'when updating query' do
- it 'schedules a prometheus alert update' do
- described_class.new(metric, { query: "sum(bar)" }).execute
-
- expect(schedule_update_service).to have_received(:execute)
- end
- end
-
- it 'does not schedule a prometheus alert update without title nor query being changed' do
- described_class.new(metric, { y_label: "bar" }).execute
-
- expect(schedule_update_service).not_to have_received(:execute)
- end
- end
-end
diff --git a/spec/services/projects/record_target_platforms_service_spec.rb b/spec/services/projects/record_target_platforms_service_spec.rb
index 85311f36428..22ff325a62e 100644
--- a/spec/services/projects/record_target_platforms_service_spec.rb
+++ b/spec/services/projects/record_target_platforms_service_spec.rb
@@ -5,21 +5,38 @@ require 'spec_helper'
RSpec.describe Projects::RecordTargetPlatformsService, '#execute' do
let_it_be(:project) { create(:project) }
- subject(:execute) { described_class.new(project).execute }
+ let(:detector_service) { Projects::AppleTargetPlatformDetectorService }
+
+ subject(:execute) { described_class.new(project, detector_service).execute }
+
+ context 'when detector returns target platform values' do
+ let(:detector_result) { [:ios, :osx] }
+ let(:service_result) { detector_result.map(&:to_s) }
- context 'when project is an XCode project' do
before do
- double = instance_double(Projects::AppleTargetPlatformDetectorService, execute: [:ios, :osx])
- allow(Projects::AppleTargetPlatformDetectorService).to receive(:new) { double }
+ double = instance_double(detector_service, execute: detector_result)
+ allow(detector_service).to receive(:new) { double }
end
- it 'creates a new setting record for the project', :aggregate_failures do
- expect { execute }.to change { ProjectSetting.count }.from(0).to(1)
- expect(ProjectSetting.last.target_platforms).to match_array(%w(ios osx))
+ shared_examples 'saves and returns detected target platforms' do
+ it 'creates a new setting record for the project', :aggregate_failures do
+ expect { execute }.to change { ProjectSetting.count }.from(0).to(1)
+ expect(ProjectSetting.last.target_platforms).to match_array(service_result)
+ end
+
+ it 'returns the array of stored target platforms' do
+ expect(execute).to match_array service_result
+ end
end
- it 'returns array of detected target platforms' do
- expect(execute).to match_array %w(ios osx)
+ it_behaves_like 'saves and returns detected target platforms'
+
+ context 'when detector returns a non-array value' do
+ let(:detector_service) { Projects::AndroidTargetPlatformDetectorService }
+ let(:detector_result) { :android }
+ let(:service_result) { [detector_result.to_s] }
+
+ it_behaves_like 'saves and returns detected target platforms'
end
context 'when a project has an existing setting record' do
@@ -49,9 +66,76 @@ RSpec.describe Projects::RecordTargetPlatformsService, '#execute' do
end
end
end
+
+ describe 'Build iOS guide email experiment' do
+ shared_examples 'tracks experiment assignment event' do
+ it 'tracks the assignment event', :experiment do
+ expect(experiment(:build_ios_app_guide_email))
+ .to track(:assignment)
+ .with_context(project: project)
+ .on_next_instance
+
+ execute
+ end
+ end
+
+ context 'experiment candidate' do
+ before do
+ stub_experiments(build_ios_app_guide_email: :candidate)
+ end
+
+ it 'executes a Projects::InProductMarketingCampaignEmailsService' do
+ service_double = instance_double(Projects::InProductMarketingCampaignEmailsService, execute: true)
+
+ expect(Projects::InProductMarketingCampaignEmailsService)
+ .to receive(:new).with(project, Users::InProductMarketingEmail::BUILD_IOS_APP_GUIDE)
+ .and_return service_double
+ expect(service_double).to receive(:execute)
+
+ execute
+ end
+
+ it_behaves_like 'tracks experiment assignment event'
+ end
+
+ shared_examples 'does not send email' do
+ it 'does not execute a Projects::InProductMarketingCampaignEmailsService' do
+ expect(Projects::InProductMarketingCampaignEmailsService).not_to receive(:new)
+
+ execute
+ end
+ end
+
+ context 'experiment control' do
+ before do
+ stub_experiments(build_ios_app_guide_email: :control)
+ end
+
+ it_behaves_like 'does not send email'
+ it_behaves_like 'tracks experiment assignment event'
+ end
+
+ context 'when project is not an iOS project' do
+ let(:detector_service) { Projects::AppleTargetPlatformDetectorService }
+ let(:detector_result) { :android }
+
+ before do
+ stub_experiments(build_ios_app_guide_email: :candidate)
+ end
+
+ it_behaves_like 'does not send email'
+
+ it 'does not track experiment assignment event', :experiment do
+ expect(experiment(:build_ios_app_guide_email))
+ .not_to track(:assignment)
+
+ execute
+ end
+ end
+ end
end
- context 'when project is not an XCode project' do
+ context 'when detector does not return any target platform values' do
before do
double = instance_double(Projects::AppleTargetPlatformDetectorService, execute: [])
allow(Projects::AppleTargetPlatformDetectorService).to receive(:new).with(project) { double }
diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb
index 6407b8d3940..777162b6196 100644
--- a/spec/services/projects/update_pages_service_spec.rb
+++ b/spec/services/projects/update_pages_service_spec.rb
@@ -13,6 +13,7 @@ RSpec.describe Projects::UpdatePagesService do
let(:file) { fixture_file_upload("spec/fixtures/pages.zip") }
let(:empty_file) { fixture_file_upload("spec/fixtures/pages_empty.zip") }
+ let(:empty_metadata_filename) { "spec/fixtures/pages_empty.zip.meta" }
let(:metadata_filename) { "spec/fixtures/pages.zip.meta" }
let(:metadata) { fixture_file_upload(metadata_filename) if File.exist?(metadata_filename) }
@@ -91,6 +92,17 @@ RSpec.describe Projects::UpdatePagesService do
end
end
+ context 'when archive does not have pages directory' do
+ let(:file) { empty_file }
+ let(:metadata_filename) { empty_metadata_filename }
+
+ it 'returns an error' do
+ expect(execute).not_to eq(:success)
+
+ expect(GenericCommitStatus.last.description).to eq("Error: The `public/` folder is missing, or not declared in `.gitlab-ci.yml`.")
+ end
+ end
+
it 'limits pages size' do
stub_application_setting(max_pages_size: 1)
expect(execute).not_to eq(:success)
diff --git a/spec/services/prometheus/create_default_alerts_service_spec.rb b/spec/services/prometheus/create_default_alerts_service_spec.rb
deleted file mode 100644
index 0880799b589..00000000000
--- a/spec/services/prometheus/create_default_alerts_service_spec.rb
+++ /dev/null
@@ -1,92 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Prometheus::CreateDefaultAlertsService do
- let_it_be(:project) { create(:project, :repository) }
-
- let(:instance) { described_class.new(project: project) }
- let(:expected_alerts) { described_class::DEFAULT_ALERTS }
-
- describe '#execute' do
- subject(:execute) { instance.execute }
-
- shared_examples 'no alerts created' do
- it 'does not create alerts' do
- expect { execute }.not_to change { project.reload.prometheus_alerts.count }
- end
- end
-
- context 'no environment' do
- it_behaves_like 'no alerts created'
- end
-
- context 'environment exists' do
- let_it_be(:environment) { create(:environment, project: project) }
-
- context 'no found metric' do
- it_behaves_like 'no alerts created'
- end
-
- context 'metric exists' do
- before do
- create_expected_metrics!
- end
-
- context 'alert exists already' do
- before do
- create_pre_existing_alerts!(environment)
- end
-
- it_behaves_like 'no alerts created'
- end
-
- it 'creates alerts' do
- expect { execute }.to change { project.reload.prometheus_alerts.count }
- .by(expected_alerts.size)
- end
-
- it 'does not schedule an update to prometheus' do
- expect(::Clusters::Applications::ScheduleUpdateService).not_to receive(:new)
- execute
- end
-
- context 'cluster with prometheus exists' do
- let!(:cluster) { create(:cluster, :with_installed_prometheus, :provided_by_user, projects: [project]) }
-
- it 'schedules an update to prometheus' do
- expect_next_instance_of(::Clusters::Applications::ScheduleUpdateService) do |instance|
- expect(instance).to receive(:execute)
- end
-
- execute
- end
- end
-
- context 'multiple environments' do
- let!(:production) { create(:environment, project: project, name: 'production') }
-
- it 'uses the production environment' do
- expect { execute }.to change { production.reload.prometheus_alerts.count }
- .by(expected_alerts.size)
- end
- end
- end
- end
- end
-
- private
-
- def create_expected_metrics!
- expected_alerts.each do |alert_hash|
- create(:prometheus_metric, :common, identifier: alert_hash.fetch(:identifier))
- end
- end
-
- def create_pre_existing_alerts!(environment)
- expected_alerts.each do |alert_hash|
- metric = PrometheusMetric.for_identifier(alert_hash[:identifier]).first!
- create(:prometheus_alert, prometheus_metric: metric, project: project, environment: environment)
- end
- end
-end
diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb
index 85dbc39edcf..f7a22b1b92f 100644
--- a/spec/services/quick_actions/interpret_service_spec.rb
+++ b/spec/services/quick_actions/interpret_service_spec.rb
@@ -696,6 +696,21 @@ RSpec.describe QuickActions::InterpretService do
expect(message).to eq("Assigned #{developer.to_reference}.")
end
+ context 'when the reference does not match the exact case' do
+ let(:user) { create(:user) }
+ let(:content) { "/assign #{user.to_reference.upcase}" }
+
+ it 'assigns to the user' do
+ issuable.project.add_developer(user)
+
+ _, updates, message = service.execute(content, issuable)
+
+ expect(content).not_to include(user.to_reference)
+ expect(updates).to eq(assignee_ids: [user.id])
+ expect(message).to eq("Assigned #{user.to_reference}.")
+ end
+ end
+
context 'when the user has a private profile' do
let(:user) { create(:user, :private_profile) }
let(:content) { "/assign #{user.to_reference}" }
diff --git a/spec/services/service_ping/build_payload_service_spec.rb b/spec/services/service_ping/build_payload_service_spec.rb
deleted file mode 100644
index cd2685069c9..00000000000
--- a/spec/services/service_ping/build_payload_service_spec.rb
+++ /dev/null
@@ -1,47 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe ServicePing::BuildPayloadService do
- describe '#execute', :without_license do
- subject(:service_ping_payload) { described_class.new.execute }
-
- include_context 'stubbed service ping metrics definitions' do
- let(:subscription_metrics) do
- [
- metric_attributes('active_user_count', "Subscription")
- ]
- end
- end
-
- context 'when usage_ping_enabled setting is false' do
- before do
- # Gitlab::CurrentSettings.usage_ping_enabled? == false
- stub_config_setting(usage_ping_enabled: false)
- end
-
- it 'returns empty service ping payload' do
- expect(service_ping_payload).to eq({})
- end
- end
-
- context 'when usage_ping_enabled setting is true' do
- before do
- # Gitlab::CurrentSettings.usage_ping_enabled? == true
- stub_config_setting(usage_ping_enabled: true)
- end
-
- it_behaves_like 'complete service ping payload'
-
- context 'with require stats consent enabled' do
- before do
- allow(User).to receive(:single_user).and_return(double(:user, requires_usage_stats_consent?: true))
- end
-
- it 'returns empty service ping payload' do
- expect(service_ping_payload).to eq({})
- end
- end
- end
- end
-end
diff --git a/spec/services/service_ping/permit_data_categories_service_spec.rb b/spec/services/service_ping/permit_data_categories_service_spec.rb
deleted file mode 100644
index 550c0ea5e13..00000000000
--- a/spec/services/service_ping/permit_data_categories_service_spec.rb
+++ /dev/null
@@ -1,42 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe ServicePing::PermitDataCategoriesService do
- describe '#execute', :without_license do
- subject(:permitted_categories) { described_class.new.execute }
-
- context 'when usage ping setting is set to true' do
- before do
- allow(User).to receive(:single_user).and_return(double(:user, requires_usage_stats_consent?: false))
- stub_config_setting(usage_ping_enabled: true)
- end
-
- it 'returns all categories' do
- expect(permitted_categories).to match_array(%w[standard subscription operational optional])
- end
- end
-
- context 'when usage ping setting is set to false' do
- before do
- allow(User).to receive(:single_user).and_return(double(:user, requires_usage_stats_consent?: false))
- stub_config_setting(usage_ping_enabled: false)
- end
-
- it 'returns no categories' do
- expect(permitted_categories).to match_array([])
- end
- end
-
- context 'when User.single_user&.requires_usage_stats_consent? is required' do
- before do
- allow(User).to receive(:single_user).and_return(double(:user, requires_usage_stats_consent?: true))
- stub_config_setting(usage_ping_enabled: true)
- end
-
- it 'returns no categories' do
- expect(permitted_categories).to match_array([])
- end
- end
- end
-end
diff --git a/spec/services/service_ping/service_ping_settings_spec.rb b/spec/services/service_ping/service_ping_settings_spec.rb
deleted file mode 100644
index 90a5c6b30eb..00000000000
--- a/spec/services/service_ping/service_ping_settings_spec.rb
+++ /dev/null
@@ -1,46 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe ServicePing::ServicePingSettings do
- using RSpec::Parameterized::TableSyntax
-
- describe '#product_intelligence_enabled?' do
- where(:usage_ping_enabled, :requires_usage_stats_consent, :expected_product_intelligence_enabled) do
- # Usage ping enabled
- true | false | true
- true | true | false
-
- # Usage ping disabled
- false | false | false
- false | true | false
- end
-
- with_them do
- before do
- allow(User).to receive(:single_user).and_return(double(:user, requires_usage_stats_consent?: requires_usage_stats_consent))
- stub_config_setting(usage_ping_enabled: usage_ping_enabled)
- end
-
- it 'has the correct product_intelligence_enabled?' do
- expect(described_class.product_intelligence_enabled?).to eq(expected_product_intelligence_enabled)
- end
- end
- end
-
- describe '#enabled?' do
- describe 'has the correct enabled' do
- it 'when false' do
- stub_config_setting(usage_ping_enabled: false)
-
- expect(described_class.enabled?).to eq(false)
- end
-
- it 'when true' do
- stub_config_setting(usage_ping_enabled: true)
-
- expect(described_class.enabled?).to eq(true)
- end
- end
- end
-end
diff --git a/spec/services/service_ping/submit_service_ping_service_spec.rb b/spec/services/service_ping/submit_service_ping_service_spec.rb
index 73be8f000a9..7a8bd1910fe 100644
--- a/spec/services/service_ping/submit_service_ping_service_spec.rb
+++ b/spec/services/service_ping/submit_service_ping_service_spec.rb
@@ -51,6 +51,9 @@ RSpec.describe ServicePing::SubmitService do
let(:with_dev_ops_score_params) { { dev_ops_score: score_params[:score] } }
let(:with_conv_index_params) { { conv_index: score_params[:score] } }
let(:with_usage_data_id_params) { { conv_index: { usage_data_id: usage_data_id } } }
+ let(:service_ping_payload_url) { File.join(described_class::STAGING_BASE_URL, described_class::USAGE_DATA_PATH) }
+ let(:service_ping_errors_url) { File.join(described_class::STAGING_BASE_URL, described_class::ERROR_PATH) }
+ let(:service_ping_metadata_url) { File.join(described_class::STAGING_BASE_URL, described_class::METADATA_PATH) }
shared_examples 'does not run' do
it do
@@ -63,7 +66,7 @@ RSpec.describe ServicePing::SubmitService do
shared_examples 'does not send a blank usage ping payload' do
it do
- expect(Gitlab::HTTP).not_to receive(:post).with(subject.url, any_args)
+ expect(Gitlab::HTTP).not_to receive(:post).with(service_ping_payload_url, any_args)
expect { subject.execute }.to raise_error(described_class::SubmissionError) do |error|
expect(error.message).to include('Usage data is blank')
@@ -117,6 +120,7 @@ RSpec.describe ServicePing::SubmitService do
it 'generates service ping' do
stub_response(body: with_dev_ops_score_params)
+ stub_response(body: nil, url: service_ping_metadata_url, status: 201)
expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values).and_call_original
@@ -129,18 +133,21 @@ RSpec.describe ServicePing::SubmitService do
stub_usage_data_connections
stub_database_flavor_check
stub_application_setting(usage_ping_enabled: true)
- stub_response(body: nil, url: subject.error_url, status: 201)
+ stub_response(body: nil, url: service_ping_errors_url, status: 201)
+ stub_response(body: nil, url: service_ping_metadata_url, status: 201)
end
context 'and user requires usage stats consent' do
before do
- allow(User).to receive(:single_user).and_return(double(:user, requires_usage_stats_consent?: true))
+ allow(User).to receive(:single_user)
+ .and_return(instance_double(User, :user, requires_usage_stats_consent?: true))
end
it_behaves_like 'does not run'
end
it 'sends a POST request' do
+ stub_response(body: nil, url: service_ping_metadata_url, status: 201)
response = stub_response(body: with_dev_ops_score_params)
subject.execute
@@ -167,7 +174,8 @@ RSpec.describe ServicePing::SubmitService do
recorded_at = Time.current
usage_data = { uuid: 'uuid', recorded_at: recorded_at }
- expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values).and_return(usage_data)
+ expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values)
+ .and_return(usage_data)
subject.execute
@@ -190,7 +198,8 @@ RSpec.describe ServicePing::SubmitService do
recorded_at = Time.current
usage_data = { uuid: 'uuid', recorded_at: recorded_at }
- expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values).and_return(usage_data)
+ expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values)
+ .and_return(usage_data)
subject.execute
@@ -235,7 +244,8 @@ RSpec.describe ServicePing::SubmitService do
recorded_at = Time.current
usage_data = { uuid: 'uuid', recorded_at: recorded_at }
- expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values).and_return(usage_data)
+ expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values)
+ .and_return(usage_data)
subject.execute
@@ -268,7 +278,7 @@ RSpec.describe ServicePing::SubmitService do
context 'and usage data is nil' do
before do
- allow(ServicePing::BuildPayloadService).to receive(:execute).and_return(nil)
+ allow(ServicePing::BuildPayload).to receive(:execute).and_return(nil)
allow(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values).and_return(nil)
end
@@ -278,29 +288,33 @@ RSpec.describe ServicePing::SubmitService do
context 'if payload service fails' do
before do
stub_response(body: with_dev_ops_score_params)
- allow(ServicePing::BuildPayloadService).to receive_message_chain(:new, :execute)
+
+ allow(ServicePing::BuildPayload).to receive_message_chain(:new, :execute)
.and_raise(described_class::SubmissionError, 'SubmissionError')
end
it 'calls Gitlab::Usage::ServicePingReport .for method' do
usage_data = build_usage_data
- expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values).and_return(usage_data)
+ expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values)
+ .and_return(usage_data)
subject.execute
end
it 'submits error' do
- expect(Gitlab::HTTP).to receive(:post).with(subject.url, any_args)
+ expect(Gitlab::HTTP).to receive(:post).with(URI.join(service_ping_payload_url), any_args)
+ .and_call_original
+ expect(Gitlab::HTTP).to receive(:post).with(URI.join(service_ping_errors_url), any_args)
.and_call_original
- expect(Gitlab::HTTP).to receive(:post).with(subject.error_url, any_args)
+ expect(Gitlab::HTTP).to receive(:post).with(URI.join(service_ping_metadata_url), any_args)
.and_call_original
subject.execute
end
end
- context 'calls BuildPayloadService first' do
+ context 'calls BuildPayload first' do
before do
stub_response(body: with_dev_ops_score_params)
end
@@ -308,7 +322,7 @@ RSpec.describe ServicePing::SubmitService do
it 'returns usage data' do
usage_data = build_usage_data
- expect_next_instance_of(ServicePing::BuildPayloadService) do |service|
+ expect_next_instance_of(ServicePing::BuildPayload) do |service|
expect(service).to receive(:execute).and_return(usage_data)
end
@@ -321,7 +335,7 @@ RSpec.describe ServicePing::SubmitService do
stub_response(body: with_dev_ops_score_params, status: 404)
usage_data = build_usage_data
- allow_next_instance_of(ServicePing::BuildPayloadService) do |service|
+ allow_next_instance_of(ServicePing::BuildPayload) do |service|
allow(service).to receive(:execute).and_return(usage_data)
end
end
@@ -329,7 +343,8 @@ RSpec.describe ServicePing::SubmitService do
it 'calls Gitlab::Usage::ServicePingReport .for method' do
usage_data = build_usage_data
- expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values).and_return(usage_data)
+ expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values)
+ .and_return(usage_data)
# SubmissionError is raised as a result of 404 in response from HTTP Request
expect { subject.execute }.to raise_error(described_class::SubmissionError)
@@ -349,38 +364,79 @@ RSpec.describe ServicePing::SubmitService do
end
it 'does not call DevOpsReport service' do
- expect(ServicePing::DevopsReportService).not_to receive(:new)
+ expect(ServicePing::DevopsReport).not_to receive(:new)
subject.execute
end
end
end
- describe '#url' do
- let(:url) { subject.url.to_s }
+ context 'metadata reporting' do
+ before do
+ stub_usage_data_connections
+ stub_database_flavor_check
+ stub_application_setting(usage_ping_enabled: true)
+ stub_response(body: with_conv_index_params)
+ end
+
+ context 'with feature flag measure_service_ping_metric_collection turned on' do
+ let(:metric_double) { instance_double(Gitlab::Usage::ServicePing::LegacyMetricTimingDecorator, duration: 123) }
+ let(:payload) do
+ {
+ metric_a: metric_double,
+ metric_group: {
+ metric_b: metric_double
+ },
+ metric_without_timing: "value",
+ recorded_at: Time.current
+ }
+ end
+
+ let(:metadata_payload) do
+ {
+ metadata: {
+ metrics: [
+ { name: 'metric_a', time_elapsed: 123 },
+ { name: 'metric_group.metric_b', time_elapsed: 123 }
+ ]
+ }
+ }
+ end
- context 'when Rails.env is production' do
before do
- stub_rails_env('production')
+ stub_feature_flags(measure_service_ping_metric_collection: true)
+
+ allow_next_instance_of(ServicePing::BuildPayload) do |service|
+ allow(service).to receive(:execute).and_return(payload)
+ end
end
- it 'points to the production Version app' do
- expect(url).to eq("#{described_class::PRODUCTION_BASE_URL}/#{described_class::USAGE_DATA_PATH}")
+ it 'submits metadata' do
+ response = stub_full_request(service_ping_metadata_url, method: :post)
+ .with(body: metadata_payload)
+
+ subject.execute
+
+ expect(response).to have_been_requested
end
end
- context 'when Rails.env is not production' do
+ context 'with feature flag measure_service_ping_metric_collection turned off' do
before do
- stub_rails_env('development')
+ stub_feature_flags(measure_service_ping_metric_collection: false)
end
- it 'points to the staging Version app' do
- expect(url).to eq("#{described_class::STAGING_BASE_URL}/#{described_class::USAGE_DATA_PATH}")
+ it 'does NOT submit metadata' do
+ response = stub_full_request(service_ping_metadata_url, method: :post)
+
+ subject.execute
+
+ expect(response).not_to have_been_requested
end
end
end
- def stub_response(url: subject.url, body:, status: 201)
+ def stub_response(url: service_ping_payload_url, body:, status: 201)
stub_full_request(url, method: :post)
.to_return(
headers: { 'Content-Type' => 'application/json' },
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index c322ec35e86..741d136b9a0 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -431,6 +431,19 @@ RSpec.describe SystemNoteService do
end
end
+ describe '.remove_timelog' do
+ let(:issue) { create(:issue, project: project) }
+ let(:timelog) { create(:timelog, user: author, issue: issue, time_spent: 1800)}
+
+ it 'calls TimeTrackingService' do
+ expect_next_instance_of(::SystemNotes::TimeTrackingService) do |service|
+ expect(service).to receive(:remove_timelog)
+ end
+
+ described_class.remove_timelog(noteable, project, author, timelog)
+ end
+ end
+
describe '.handle_merge_request_draft' do
it 'calls MergeRequestsService' do
expect_next_instance_of(::SystemNotes::MergeRequestsService) do |service|
@@ -695,4 +708,38 @@ RSpec.describe SystemNoteService do
described_class.change_issue_type(incident, author)
end
end
+
+ describe '.add_timeline_event' do
+ let(:timeline_event) { instance_double('IncidentManagement::TimelineEvent', incident: noteable, project: project) }
+
+ it 'calls IncidentsService' do
+ expect_next_instance_of(::SystemNotes::IncidentsService) do |service|
+ expect(service).to receive(:add_timeline_event).with(timeline_event)
+ end
+
+ described_class.add_timeline_event(timeline_event)
+ end
+ end
+
+ describe '.edit_timeline_event' do
+ let(:timeline_event) { instance_double('IncidentManagement::TimelineEvent', incident: noteable, project: project) }
+
+ it 'calls IncidentsService' do
+ expect_next_instance_of(::SystemNotes::IncidentsService) do |service|
+ expect(service).to receive(:edit_timeline_event).with(timeline_event, author, was_changed: :occurred_at)
+ end
+
+ described_class.edit_timeline_event(timeline_event, author, was_changed: :occurred_at)
+ end
+ end
+
+ describe '.delete_timeline_event' do
+ it 'calls IncidentsService' do
+ expect_next_instance_of(::SystemNotes::IncidentsService) do |service|
+ expect(service).to receive(:delete_timeline_event).with(author)
+ end
+
+ described_class.delete_timeline_event(noteable, author)
+ end
+ end
end
diff --git a/spec/services/system_notes/incidents_service_spec.rb b/spec/services/system_notes/incidents_service_spec.rb
new file mode 100644
index 00000000000..d1b831e9c4c
--- /dev/null
+++ b/spec/services/system_notes/incidents_service_spec.rb
@@ -0,0 +1,88 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe SystemNotes::IncidentsService do
+ include Gitlab::Routing
+
+ let_it_be(:project) { create(:project) }
+ let_it_be(:user) { create(:user) }
+ let_it_be(:author) { create(:user) }
+ let_it_be(:incident) { create(:incident, project: project, author: user) }
+ let_it_be(:timeline_event) do
+ create(:incident_management_timeline_event, project: project, incident: incident, author: author)
+ end
+
+ describe '#add_timeline_event' do
+ subject { described_class.new(noteable: incident).add_timeline_event(timeline_event) }
+
+ it_behaves_like 'a system note' do
+ let(:noteable) { incident }
+ let(:action) { 'timeline_event' }
+ end
+
+ it 'posts the correct text to the system note' do
+ path = project_issues_incident_path(project, incident, anchor: "timeline_event_#{timeline_event.id}")
+ expect(subject.note).to match("added an [incident timeline event](#{path})")
+ end
+ end
+
+ describe '#edit_timeline_event' do
+ let(:was_changed) { :unknown }
+ let(:path) { project_issues_incident_path(project, incident, anchor: "timeline_event_#{timeline_event.id}") }
+
+ subject do
+ described_class.new(noteable: incident).edit_timeline_event(timeline_event, author, was_changed: was_changed)
+ end
+
+ it_behaves_like 'a system note' do
+ let(:noteable) { incident }
+ let(:action) { 'timeline_event' }
+ end
+
+ context "when only timeline event's occurred_at was changed" do
+ let(:was_changed) { :occurred_at }
+
+ it 'posts the correct text to the system note' do
+ expect(subject.note).to match("edited the event time/date on [incident timeline event](#{path})")
+ end
+ end
+
+ context "when only timeline event's note was changed" do
+ let(:was_changed) { :note }
+
+ it 'posts the correct text to the system note' do
+ expect(subject.note).to match("edited the text on [incident timeline event](#{path})")
+ end
+ end
+
+ context "when both timeline events occurred_at and note was changed" do
+ let(:was_changed) { :occurred_at_and_note }
+
+ it 'posts the correct text to the system note' do
+ expect(subject.note).to match("edited the event time/date and text on [incident timeline event](#{path})")
+ end
+ end
+
+ context "when was changed reason is unknown" do
+ let(:was_changed) { :unknown }
+
+ it 'posts the correct text to the system note' do
+ expect(subject.note).to match("edited [incident timeline event](#{path})")
+ end
+ end
+ end
+
+ describe '#delete_timeline_event' do
+ subject { described_class.new(noteable: incident).delete_timeline_event(author) }
+
+ it_behaves_like 'a system note' do
+ let(:noteable) { incident }
+ let(:action) { 'timeline_event' }
+ end
+
+ it 'posts the correct text to the system note' do
+ expect(subject.note).to match('deleted an incident timeline event')
+ end
+ end
+end
diff --git a/spec/services/system_notes/time_tracking_service_spec.rb b/spec/services/system_notes/time_tracking_service_spec.rb
index ec126cb5447..fdf18f4f29a 100644
--- a/spec/services/system_notes/time_tracking_service_spec.rb
+++ b/spec/services/system_notes/time_tracking_service_spec.rb
@@ -106,6 +106,30 @@ RSpec.describe ::SystemNotes::TimeTrackingService do
end
end
+ describe '#remove_timelog' do
+ subject { described_class.new(noteable: noteable, project: project, author: author).remove_timelog(timelog) }
+
+ context 'when the timelog has a positive time spent value' do
+ let_it_be(:noteable, reload: true) { create(:issue, project: project) }
+
+ let(:timelog) { create(:timelog, user: author, issue: noteable, time_spent: 1800, spent_at: '2022-03-30T00:00:00.000Z')}
+
+ it 'sets the note text' do
+ expect(subject.note).to eq "deleted 30m of spent time from 2022-03-30"
+ end
+ end
+
+ context 'when the timelog has a negative time spent value' do
+ let_it_be(:noteable, reload: true) { create(:issue, project: project) }
+
+ let(:timelog) { create(:timelog, user: author, issue: noteable, time_spent: -1800, spent_at: '2022-03-30T00:00:00.000Z')}
+
+ it 'sets the note text' do
+ expect(subject.note).to eq "deleted -30m of spent time from 2022-03-30"
+ end
+ end
+ end
+
describe '#change_time_spent' do
subject { described_class.new(noteable: noteable, project: project, author: author).change_time_spent }
diff --git a/spec/services/timelogs/delete_service_spec.rb b/spec/services/timelogs/delete_service_spec.rb
new file mode 100644
index 00000000000..c52cebdc5bf
--- /dev/null
+++ b/spec/services/timelogs/delete_service_spec.rb
@@ -0,0 +1,65 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Timelogs::DeleteService do
+ let_it_be(:author) { create(:user) }
+ let_it_be(:project) { create(:project, :public) }
+ let_it_be(:issue) { create(:issue, project: project) }
+ let_it_be(:timelog) { create(:timelog, user: author, issue: issue, time_spent: 1800)}
+
+ let(:service) { described_class.new(timelog, user) }
+
+ describe '#execute' do
+ subject { service.execute }
+
+ context 'when the timelog exists' do
+ let(:user) { author }
+
+ it 'removes the timelog' do
+ expect { subject }.to change { Timelog.count }.by(-1)
+ end
+
+ it 'returns the removed timelog' do
+ expect(subject).to be_success
+ expect(subject.payload).to eq(timelog)
+ end
+ end
+
+ context 'when the timelog does not exist' do
+ let(:user) { create(:user) }
+ let!(:timelog) { nil }
+
+ it 'returns an error' do
+ expect(subject).to be_error
+ expect(subject.message).to eq('Timelog doesn\'t exist or you don\'t have permission to delete it')
+ expect(subject.http_status).to eq(404)
+ end
+ end
+
+ context 'when the user does not have permission' do
+ let(:user) { create(:user) }
+
+ it 'returns an error' do
+ expect(subject).to be_error
+ expect(subject.message).to eq('Timelog doesn\'t exist or you don\'t have permission to delete it')
+ expect(subject.http_status).to eq(404)
+ end
+ end
+
+ context 'when the timelog deletion fails' do
+ let(:user) { author }
+ let!(:timelog) { create(:timelog, user: author, issue: issue, time_spent: 1800)}
+
+ before do
+ allow(timelog).to receive(:destroy).and_return(false)
+ end
+
+ it 'returns an error' do
+ expect(subject).to be_error
+ expect(subject.message).to eq('Failed to remove timelog')
+ expect(subject.http_status).to eq(400)
+ end
+ end
+ end
+end
diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb
index 80a506bb1d6..45dbe83b496 100644
--- a/spec/services/users/destroy_service_spec.rb
+++ b/spec/services/users/destroy_service_spec.rb
@@ -73,10 +73,10 @@ RSpec.describe Users::DestroyService do
allow(user).to receive(:personal_projects).and_return([])
expect_next_instance_of(Snippets::BulkDestroyService) do |bulk_destroy_service|
- expect(bulk_destroy_service).to receive(:execute).with(hard_delete: true).and_call_original
+ expect(bulk_destroy_service).to receive(:execute).with({ hard_delete: true }).and_call_original
end
- service.execute(user, hard_delete: true)
+ service.execute(user, { hard_delete: true })
end
it 'does not delete project snippets that the user is the author of' do
@@ -336,35 +336,24 @@ RSpec.describe Users::DestroyService do
context 'batched nullify' do
let(:other_user) { create(:user) }
- context 'when :nullify_in_batches_on_user_deletion feature flag is enabled' do
- it 'nullifies related associations in batches' do
- expect(other_user).to receive(:nullify_dependent_associations_in_batches).and_call_original
+ it 'nullifies related associations in batches' do
+ expect(other_user).to receive(:nullify_dependent_associations_in_batches).and_call_original
- described_class.new(user).execute(other_user, skip_authorization: true)
- end
-
- it 'nullifies last_updated_issues and closed_issues' do
- issue = create(:issue, closed_by: other_user, updated_by: other_user)
-
- described_class.new(user).execute(other_user, skip_authorization: true)
-
- issue.reload
-
- expect(issue.closed_by).to be_nil
- expect(issue.updated_by).to be_nil
- end
+ described_class.new(user).execute(other_user, skip_authorization: true)
end
- context 'when :nullify_in_batches_on_user_deletion feature flag is disabled' do
- before do
- stub_feature_flags(nullify_in_batches_on_user_deletion: false)
- end
+ it 'nullifies last_updated_issues, closed_issues, resource_label_events' do
+ issue = create(:issue, closed_by: other_user, updated_by: other_user)
+ resource_label_event = create(:resource_label_event, user: other_user)
- it 'does not use batching' do
- expect(other_user).not_to receive(:nullify_dependent_associations_in_batches)
+ described_class.new(user).execute(other_user, skip_authorization: true)
- described_class.new(user).execute(other_user, skip_authorization: true)
- end
+ issue.reload
+ resource_label_event.reload
+
+ expect(issue.closed_by).to be_nil
+ expect(issue.updated_by).to be_nil
+ expect(resource_label_event.user).to be_nil
end
end
end
diff --git a/spec/services/namespaces/in_product_marketing_email_records_spec.rb b/spec/services/users/in_product_marketing_email_records_spec.rb
index d80e20135d5..0b9400dcd12 100644
--- a/spec/services/namespaces/in_product_marketing_email_records_spec.rb
+++ b/spec/services/users/in_product_marketing_email_records_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe Namespaces::InProductMarketingEmailRecords do
+RSpec.describe Users::InProductMarketingEmailRecords do
let_it_be(:user) { create :user }
subject(:records) { described_class.new }
@@ -15,8 +15,9 @@ RSpec.describe Namespaces::InProductMarketingEmailRecords do
before do
allow(Users::InProductMarketingEmail).to receive(:bulk_insert!)
- records.add(user, :team_short, 0)
- records.add(user, :create, 1)
+ records.add(user, track: :team_short, series: 0)
+ records.add(user, track: :create, series: 1)
+ records.add(user, campaign: Users::InProductMarketingEmail::BUILD_IOS_APP_GUIDE)
end
it 'bulk inserts added records' do
@@ -31,24 +32,34 @@ RSpec.describe Namespaces::InProductMarketingEmailRecords do
end
describe '#add' do
- it 'adds a Users::InProductMarketingEmail record to its records' do
+ it 'adds a Users::InProductMarketingEmail record to its records', :aggregate_failures do
freeze_time do
- records.add(user, :team_short, 0)
- records.add(user, :create, 1)
+ records.add(user, track: :team_short, series: 0)
+ records.add(user, track: :create, series: 1)
+ records.add(user, campaign: Users::InProductMarketingEmail::BUILD_IOS_APP_GUIDE)
- first, second = records.records
+ first, second, third = records.records
expect(first).to be_a Users::InProductMarketingEmail
+ expect(first.campaign).to be_nil
expect(first.track.to_sym).to eq :team_short
expect(first.series).to eq 0
expect(first.created_at).to eq Time.zone.now
expect(first.updated_at).to eq Time.zone.now
expect(second).to be_a Users::InProductMarketingEmail
+ expect(second.campaign).to be_nil
expect(second.track.to_sym).to eq :create
expect(second.series).to eq 1
expect(second.created_at).to eq Time.zone.now
expect(second.updated_at).to eq Time.zone.now
+
+ expect(third).to be_a Users::InProductMarketingEmail
+ expect(third.campaign).to eq Users::InProductMarketingEmail::BUILD_IOS_APP_GUIDE
+ expect(third.track).to be_nil
+ expect(third.series).to be_nil
+ expect(third.created_at).to eq Time.zone.now
+ expect(third.updated_at).to eq Time.zone.now
end
end
end
diff --git a/spec/services/users/validate_otp_service_spec.rb b/spec/services/users/validate_manual_otp_service_spec.rb
index 46b80b2149f..d71735814f2 100644
--- a/spec/services/users/validate_otp_service_spec.rb
+++ b/spec/services/users/validate_manual_otp_service_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe Users::ValidateOtpService do
+RSpec.describe Users::ValidateManualOtpService do
let_it_be(:user) { create(:user) }
let(:otp_code) { 42 }
@@ -25,8 +25,8 @@ RSpec.describe Users::ValidateOtpService do
allow(::Gitlab.config.forti_authenticator).to receive(:enabled).and_return(true)
end
- it 'calls FortiAuthenticator strategy' do
- expect_next_instance_of(::Gitlab::Auth::Otp::Strategies::FortiAuthenticator) do |strategy|
+ it 'calls ManualOtp strategy' do
+ expect_next_instance_of(::Gitlab::Auth::Otp::Strategies::FortiAuthenticator::ManualOtp) do |strategy|
expect(strategy).to receive(:validate).with(otp_code).once
end
@@ -48,4 +48,25 @@ RSpec.describe Users::ValidateOtpService do
validate
end
end
+
+ context 'unexpected error' do
+ before do
+ stub_feature_flags(forti_authenticator: user)
+ allow(::Gitlab.config.forti_authenticator).to receive(:enabled).and_return(true)
+ end
+
+ it 'returns error' do
+ error_message = "boom!"
+
+ expect_next_instance_of(::Gitlab::Auth::Otp::Strategies::FortiAuthenticator::ManualOtp) do |strategy|
+ expect(strategy).to receive(:validate).with(otp_code).once.and_raise(StandardError, error_message)
+ end
+ expect(Gitlab::ErrorTracking).to receive(:log_exception)
+
+ result = validate
+
+ expect(result[:status]).to eq(:error)
+ expect(result[:message]).to eq(error_message)
+ end
+ end
end
diff --git a/spec/services/users/validate_push_otp_service_spec.rb b/spec/services/users/validate_push_otp_service_spec.rb
new file mode 100644
index 00000000000..960b6bcd3bb
--- /dev/null
+++ b/spec/services/users/validate_push_otp_service_spec.rb
@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Users::ValidatePushOtpService do
+ let_it_be(:user) { create(:user) }
+
+ subject(:validate) { described_class.new(user).execute }
+
+ context 'FortiAuthenticator' do
+ before do
+ stub_feature_flags(forti_authenticator: user)
+ allow(::Gitlab.config.forti_authenticator).to receive(:enabled).and_return(true)
+ end
+
+ it 'calls PushOtp strategy' do
+ expect_next_instance_of(::Gitlab::Auth::Otp::Strategies::FortiAuthenticator::PushOtp) do |strategy|
+ expect(strategy).to receive(:validate).once
+ end
+
+ validate
+ end
+ end
+
+ context 'unexpected error' do
+ before do
+ stub_feature_flags(forti_authenticator: user)
+ allow(::Gitlab.config.forti_authenticator).to receive(:enabled).and_return(true)
+ end
+
+ it 'returns error' do
+ error_message = "boom!"
+
+ expect_next_instance_of(::Gitlab::Auth::Otp::Strategies::FortiAuthenticator::PushOtp) do |strategy|
+ expect(strategy).to receive(:validate).once.and_raise(StandardError, error_message)
+ end
+ expect(Gitlab::ErrorTracking).to receive(:log_exception)
+
+ result = validate
+
+ expect(result[:status]).to eq(:error)
+ expect(result[:message]).to eq(error_message)
+ end
+ end
+end
diff --git a/spec/services/work_items/delete_task_service_spec.rb b/spec/services/work_items/delete_task_service_spec.rb
new file mode 100644
index 00000000000..04944645c9b
--- /dev/null
+++ b/spec/services/work_items/delete_task_service_spec.rb
@@ -0,0 +1,88 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe WorkItems::DeleteTaskService do
+ let_it_be(:project) { create(:project) }
+ let_it_be(:developer) { create(:user).tap { |u| project.add_developer(u) } }
+ let_it_be_with_refind(:task) { create(:work_item, project: project, author: developer) }
+ let_it_be_with_refind(:list_work_item) do
+ create(:work_item, project: project, description: "- [ ] #{task.to_reference}+")
+ end
+
+ let(:current_user) { developer }
+ let(:line_number_start) { 1 }
+ let(:params) do
+ {
+ line_number_start: line_number_start,
+ line_number_end: 1,
+ task: task
+ }
+ end
+
+ before_all do
+ create(:issue_link, source_id: list_work_item.id, target_id: task.id)
+ end
+
+ shared_examples 'failing WorkItems::DeleteTaskService' do |error_message|
+ it { is_expected.to be_error }
+
+ it 'does not remove work item or issue links' do
+ expect do
+ service_result
+ list_work_item.reload
+ end.to not_change(WorkItem, :count).and(
+ not_change(IssueLink, :count)
+ ).and(
+ not_change(list_work_item, :description)
+ )
+ end
+
+ it 'returns an error message' do
+ expect(service_result.errors).to contain_exactly(error_message)
+ end
+ end
+
+ describe '#execute' do
+ subject(:service_result) do
+ described_class.new(
+ work_item: list_work_item,
+ current_user: current_user,
+ lock_version: list_work_item.lock_version,
+ task_params: params
+ ).execute
+ end
+
+ context 'when work item params are valid' do
+ it { is_expected.to be_success }
+
+ it 'deletes the work item and the related issue link' do
+ expect do
+ service_result
+ end.to change(WorkItem, :count).by(-1).and(
+ change(IssueLink, :count).by(-1)
+ )
+ end
+
+ it 'removes the task list item with the work item reference' do
+ expect do
+ service_result
+ end.to change(list_work_item, :description).from(list_work_item.description).to('')
+ end
+ end
+
+ context 'when first operation fails' do
+ let(:line_number_start) { -1 }
+
+ it_behaves_like 'failing WorkItems::DeleteTaskService', 'line_number_start must be greater than 0'
+ end
+
+ context 'when last operation fails' do
+ let_it_be(:non_member_user) { create(:user) }
+
+ let(:current_user) { non_member_user }
+
+ it_behaves_like 'failing WorkItems::DeleteTaskService', 'User not authorized to delete work item'
+ end
+ end
+end
diff --git a/spec/services/work_items/task_list_reference_removal_service_spec.rb b/spec/services/work_items/task_list_reference_removal_service_spec.rb
new file mode 100644
index 00000000000..bca72da0efa
--- /dev/null
+++ b/spec/services/work_items/task_list_reference_removal_service_spec.rb
@@ -0,0 +1,151 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe WorkItems::TaskListReferenceRemovalService do
+ let_it_be(:developer) { create(:user) }
+ let_it_be(:project) { create(:project, :repository).tap { |project| project.add_developer(developer) } }
+ let_it_be(:task) { create(:work_item, project: project) }
+ let_it_be(:single_line_work_item, refind: true) do
+ create(:work_item, project: project, description: "- [ ] #{task.to_reference}+ single line")
+ end
+
+ let_it_be(:multiple_line_work_item, refind: true) do
+ create(
+ :work_item,
+ project: project,
+ description: <<~MARKDOWN
+ Any text
+
+ * [ ] Item to be converted
+ #{task.to_reference}+ second line
+ third line
+ * [x] task
+
+ More text
+ MARKDOWN
+ )
+ end
+
+ let(:line_number_start) { 3 }
+ let(:line_number_end) { 5 }
+ let(:work_item) { multiple_line_work_item }
+ let(:lock_version) { work_item.lock_version }
+
+ shared_examples 'successful work item task reference removal service' do |expected_description|
+ it { is_expected.to be_success }
+
+ it 'removes the task list item containing the task reference' do
+ expect do
+ result
+ end.to change(work_item, :description).from(work_item.description).to(expected_description)
+ end
+
+ it 'creates system notes' do
+ expect do
+ result
+ end.to change(Note, :count).by(1)
+
+ expect(Note.last.note).to include('changed the description')
+ end
+ end
+
+ shared_examples 'failing work item task reference removal service' do |error_message|
+ it { is_expected.to be_error }
+
+ it 'does not change the work item description' do
+ expect do
+ result
+ work_item.reload
+ end.to not_change(work_item, :description)
+ end
+
+ it 'returns an error message' do
+ expect(result.errors).to contain_exactly(error_message)
+ end
+ end
+
+ describe '#execute' do
+ subject(:result) do
+ described_class.new(
+ work_item: work_item,
+ task: task,
+ line_number_start: line_number_start,
+ line_number_end: line_number_end,
+ lock_version: lock_version,
+ current_user: developer
+ ).execute
+ end
+
+ context 'when task mardown spans a single line' do
+ let(:line_number_start) { 1 }
+ let(:line_number_end) { 1 }
+ let(:work_item) { single_line_work_item }
+
+ it_behaves_like 'successful work item task reference removal service', ''
+
+ context 'when description does not contain a task' do
+ let_it_be(:no_matching_work_item) { create(:work_item, project: project, description: 'no matching task') }
+
+ let(:work_item) { no_matching_work_item }
+
+ it_behaves_like 'failing work item task reference removal service', 'Unable to detect a task on lines 1-1'
+ end
+
+ context 'when description reference does not exactly match the task reference' do
+ before do
+ work_item.update!(description: work_item.description.gsub(task.to_reference, "#{task.to_reference}200"))
+ end
+
+ it_behaves_like 'failing work item task reference removal service', 'Unable to detect a task on lines 1-1'
+ end
+ end
+
+ context 'when task mardown spans multiple lines' do
+ it_behaves_like 'successful work item task reference removal service', "Any text\n\n* [x] task\n\nMore text"
+ end
+
+ context 'when updating the work item fails' do
+ before do
+ work_item.title = nil
+ end
+
+ it_behaves_like 'failing work item task reference removal service', "Title can't be blank"
+ end
+
+ context 'when description is empty' do
+ let_it_be(:empty_work_item) { create(:work_item, project: project, description: '') }
+
+ let(:work_item) { empty_work_item }
+
+ it_behaves_like 'failing work item task reference removal service', "Work item description can't be blank"
+ end
+
+ context 'when line_number_start is lower than 1' do
+ let(:line_number_start) { 0 }
+
+ it_behaves_like 'failing work item task reference removal service', 'line_number_start must be greater than 0'
+ end
+
+ context 'when line_number_end is lower than line_number_start' do
+ let(:line_number_end) { line_number_start - 1 }
+
+ it_behaves_like 'failing work item task reference removal service',
+ 'line_number_end must be greater or equal to line_number_start'
+ end
+
+ context 'when lock_version is older than current' do
+ let(:lock_version) { work_item.lock_version - 1 }
+
+ it_behaves_like 'failing work item task reference removal service', 'Stale work item. Check lock version'
+ end
+
+ context 'when work item is stale before updating' do
+ it_behaves_like 'failing work item task reference removal service', 'Stale work item. Check lock version' do
+ before do
+ ::WorkItem.where(id: work_item.id).update_all(lock_version: lock_version + 1)
+ end
+ end
+ end
+ end
+end