diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index 9152b5bdb8b2714fea2df6622fe2ab905946ff58..3e0c2838252c358c1c9c42732d78dd82676aecca 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -1351,11 +1351,6 @@ Rails/SaveBang:
- 'ee/spec/models/visible_approvable_spec.rb'
- 'ee/spec/models/vulnerabilities/feedback_spec.rb'
- 'ee/spec/models/vulnerabilities/issue_link_spec.rb'
- - 'ee/spec/policies/group_policy_spec.rb'
- - 'ee/spec/policies/note_policy_spec.rb'
- - 'ee/spec/policies/project_policy_spec.rb'
- - 'ee/spec/policies/protected_branch_policy_spec.rb'
- - 'ee/spec/policies/vulnerabilities/feedback_policy_spec.rb'
- 'ee/spec/presenters/audit_event_presenter_spec.rb'
- 'ee/spec/presenters/epic_presenter_spec.rb'
- 'ee/spec/requests/api/boards_spec.rb'
@@ -1778,13 +1773,6 @@ Rails/SaveBang:
- 'spec/models/user_status_spec.rb'
- 'spec/models/wiki_page/meta_spec.rb'
- 'spec/models/wiki_page_spec.rb'
- - 'spec/policies/ci/build_policy_spec.rb'
- - 'spec/policies/ci/pipeline_policy_spec.rb'
- - 'spec/policies/ci/pipeline_schedule_policy_spec.rb'
- - 'spec/policies/group_policy_spec.rb'
- - 'spec/policies/issue_policy_spec.rb'
- - 'spec/policies/merge_request_policy_spec.rb'
- - 'spec/policies/project_policy_spec.rb'
- 'spec/presenters/ci/build_runner_presenter_spec.rb'
- 'spec/presenters/ci/trigger_presenter_spec.rb'
- 'spec/presenters/packages/conan/package_presenter_spec.rb'
diff --git a/changelogs/unreleased/rails-save-bang-6.yml b/changelogs/unreleased/rails-save-bang-6.yml
new file mode 100644
index 0000000000000000000000000000000000000000..d0731fd34a9602876f15ae80e2c012c8ad1a5403
--- /dev/null
+++ b/changelogs/unreleased/rails-save-bang-6.yml
@@ -0,0 +1,5 @@
+---
+title: Refactor spec/policies and ee/spec/policies to fix SaveBang Cop
+merge_request: 37956
+author: Rajendra Kadam
+type: fixed
diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb
index b4f16d0d32deb5f16c4018643fff8e8729d3e128..fc5758a256f7760915ea123d448acaef1a1d4a58 100644
--- a/ee/spec/policies/group_policy_spec.rb
+++ b/ee/spec/policies/group_policy_spec.rb
@@ -881,7 +881,7 @@
context 'without Group SAML enabled' do
before do
- saml_provider.update(enabled: false)
+ saml_provider.update!(enabled: false)
end
it { is_expected.to be_disallowed(:read_group_saml_identity) }
diff --git a/ee/spec/policies/note_policy_spec.rb b/ee/spec/policies/note_policy_spec.rb
index f1a7226f7d564d85f94916e2d7276b99679bf7d0..43696cfab80f16490d5c5c0aac72db180ea00f9f 100644
--- a/ee/spec/policies/note_policy_spec.rb
+++ b/ee/spec/policies/note_policy_spec.rb
@@ -101,7 +101,7 @@ def permissions(user)
context 'for epics in a private group' do
before do
- group.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+ group.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
end
it_behaves_like 'private notes'
diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb
index 850b880e86f935dbfe5c6900ba101845344d47ae..026472479444ce3692706cd96fa1dee5f17964ce 100644
--- a/ee/spec/policies/project_policy_spec.rb
+++ b/ee/spec/policies/project_policy_spec.rb
@@ -649,7 +649,12 @@
context 'when repository is disabled' do
before do
- project.project_feature.update(repository_access_level: ProjectFeature::DISABLED)
+ project.project_feature.update!(
+ # Disable merge_requests and builds as well, since merge_requests and
+ # builds cannot have higher visibility than repository.
+ merge_requests_access_level: ProjectFeature::DISABLED,
+ builds_access_level: ProjectFeature::DISABLED,
+ repository_access_level: ProjectFeature::DISABLED)
end
it { is_expected.to be_disallowed(:read_feature_flag) }
diff --git a/ee/spec/policies/protected_branch_policy_spec.rb b/ee/spec/policies/protected_branch_policy_spec.rb
index ade98dd46445d19eb53a2acbd65f87eb37c8e68f..63312104dfd202ac88132f9645e3673266506246 100644
--- a/ee/spec/policies/protected_branch_policy_spec.rb
+++ b/ee/spec/policies/protected_branch_policy_spec.rb
@@ -13,7 +13,7 @@
before do
project.add_maintainer(user)
- project.project_group_links.create(group: allowed_group)
+ project.project_group_links.create!(group: allowed_group)
end
context 'when unprotection is limited by access levels' do
diff --git a/ee/spec/policies/vulnerabilities/feedback_policy_spec.rb b/ee/spec/policies/vulnerabilities/feedback_policy_spec.rb
index 09457e549f85739c55ad11c3f5f79eda25e6f617..d8708223a8b1d9d5e49b62a9eb66fee5ea80de89 100644
--- a/ee/spec/policies/vulnerabilities/feedback_policy_spec.rb
+++ b/ee/spec/policies/vulnerabilities/feedback_policy_spec.rb
@@ -16,7 +16,7 @@
context 'when issues feature is disabled' do
before do
- project.project_feature.update(issues_access_level: ProjectFeature::DISABLED)
+ project.project_feature.update!(issues_access_level: ProjectFeature::DISABLED)
end
it 'does not allow to create issue feedback' do
@@ -39,7 +39,7 @@
context 'when merge request feature is disabled' do
before do
- project.project_feature.update(merge_requests_access_level: ProjectFeature::DISABLED)
+ project.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED)
end
it 'does not allow to create merge request feedback' do
diff --git a/spec/policies/ci/build_policy_spec.rb b/spec/policies/ci/build_policy_spec.rb
index d25473388553e9d863447ea04dd17b45e34f84d0..7ea180af9d0c084e58cd232746c0fe2002a9ac9c 100644
--- a/spec/policies/ci/build_policy_spec.rb
+++ b/spec/policies/ci/build_policy_spec.rb
@@ -146,7 +146,7 @@
create(:protected_tag, :no_one_can_create,
name: build.ref, project: project)
- build.update(tag: true)
+ build.update!(tag: true)
end
it 'does not include ability to update build' do
diff --git a/spec/policies/ci/pipeline_policy_spec.rb b/spec/policies/ci/pipeline_policy_spec.rb
index fcd96bc66532f4fe21ae471f7f603d029095ac92..9a65823c950c1b1a18414c0c0bda9cf60d6cccfe 100644
--- a/spec/policies/ci/pipeline_policy_spec.rb
+++ b/spec/policies/ci/pipeline_policy_spec.rb
@@ -45,7 +45,7 @@
create(:protected_tag, :no_one_can_create,
name: pipeline.ref, project: project)
- pipeline.update(tag: true)
+ pipeline.update!(tag: true)
end
it 'does not include ability to update pipeline' do
diff --git a/spec/policies/ci/pipeline_schedule_policy_spec.rb b/spec/policies/ci/pipeline_schedule_policy_spec.rb
index b455384d17a84288bde45c935c0e5747a372e361..1e36f455f6fcd610794d248fc1259fed4c357d46 100644
--- a/spec/policies/ci/pipeline_schedule_policy_spec.rb
+++ b/spec/policies/ci/pipeline_schedule_policy_spec.rb
@@ -43,7 +43,7 @@
let(:tag) { 'v1.0.0' }
before do
- pipeline_schedule.update(ref: tag)
+ pipeline_schedule.update!(ref: tag)
create(:protected_tag, :no_one_can_create,
name: pipeline_schedule.ref, project: project)
@@ -69,7 +69,7 @@
describe 'rules for owner of schedule' do
before do
project.add_developer(user)
- pipeline_schedule.update(owner: user)
+ pipeline_schedule.update!(owner: user)
end
it 'includes abilities to do all operations on pipeline schedule' do
@@ -97,7 +97,7 @@
before do
project.add_maintainer(owner)
project.add_maintainer(user)
- pipeline_schedule.update(owner: owner)
+ pipeline_schedule.update!(owner: owner)
end
it 'includes abilities to take ownership' do
diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb
index 733cc9bd9cb0a65fabd4ebbd79d2ee32fbe22151..4debaf59eb9f4501391e40d90b2fdf4dbe33ffb8 100644
--- a/spec/policies/group_policy_spec.rb
+++ b/spec/policies/group_policy_spec.rb
@@ -107,7 +107,7 @@
context 'with subgroup_creation level set to maintainer' do
before_all do
- group.update(subgroup_creation_level: ::Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS)
+ group.update!(subgroup_creation_level: ::Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS)
end
it 'allows every maintainer permission plus creating subgroups' do
@@ -363,7 +363,7 @@
context 'transfer_projects' do
shared_examples_for 'allowed to transfer projects' do
before do
- group.update(project_creation_level: project_creation_level)
+ group.update!(project_creation_level: project_creation_level)
end
it { is_expected.to be_allowed(:transfer_projects) }
@@ -371,7 +371,7 @@
shared_examples_for 'not allowed to transfer projects' do
before do
- group.update(project_creation_level: project_creation_level)
+ group.update!(project_creation_level: project_creation_level)
end
it { is_expected.to be_disallowed(:transfer_projects) }
@@ -445,7 +445,7 @@
context 'create_projects' do
context 'when group has no project creation level set' do
before_all do
- group.update(project_creation_level: nil)
+ group.update!(project_creation_level: nil)
end
context 'reporter' do
@@ -475,7 +475,7 @@
context 'when group has project creation level set to no one' do
before_all do
- group.update(project_creation_level: ::Gitlab::Access::NO_ONE_PROJECT_ACCESS)
+ group.update!(project_creation_level: ::Gitlab::Access::NO_ONE_PROJECT_ACCESS)
end
context 'reporter' do
@@ -505,7 +505,7 @@
context 'when group has project creation level set to maintainer only' do
before_all do
- group.update(project_creation_level: ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS)
+ group.update!(project_creation_level: ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS)
end
context 'reporter' do
@@ -535,7 +535,7 @@
context 'when group has project creation level set to developers + maintainer' do
before_all do
- group.update(project_creation_level: ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS)
+ group.update!(project_creation_level: ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS)
end
context 'reporter' do
@@ -567,7 +567,7 @@
context 'create_subgroup' do
context 'when group has subgroup creation level set to owner' do
before_all do
- group.update(subgroup_creation_level: ::Gitlab::Access::OWNER_SUBGROUP_ACCESS)
+ group.update!(subgroup_creation_level: ::Gitlab::Access::OWNER_SUBGROUP_ACCESS)
end
context 'reporter' do
@@ -597,7 +597,7 @@
context 'when group has subgroup creation level set to maintainer' do
before_all do
- group.update(subgroup_creation_level: ::Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS)
+ group.update!(subgroup_creation_level: ::Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS)
end
context 'reporter' do
@@ -706,7 +706,7 @@
context 'which does not have design management enabled' do
before do
- project.update(lfs_enabled: false)
+ project.update!(lfs_enabled: false)
end
it { is_expected.not_to be_allowed(:read_design_activity) }
diff --git a/spec/policies/issue_policy_spec.rb b/spec/policies/issue_policy_spec.rb
index b917def66aa19b24be927b79365e1753c1d94e1c..e352b9901593b00ede0b8c41d8cf7ba754b4d17e 100644
--- a/spec/policies/issue_policy_spec.rb
+++ b/spec/policies/issue_policy_spec.rb
@@ -188,7 +188,7 @@ def permissions(user, issue)
context 'when issues are private' do
before do
- project.project_feature.update(issues_access_level: ProjectFeature::PRIVATE)
+ project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE)
end
let(:issue) { create(:issue, project: project, author: author) }
let(:visitor) { create(:user) }
diff --git a/spec/policies/merge_request_policy_spec.rb b/spec/policies/merge_request_policy_spec.rb
index 2f3cb2e998aa73b8f861e45bc774affbc68c60c7..3a46d5b922674579932347c517c53ec0b1b84c4f 100644
--- a/spec/policies/merge_request_policy_spec.rb
+++ b/spec/policies/merge_request_policy_spec.rb
@@ -51,7 +51,7 @@ def permissions(user, merge_request)
let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: author) }
before do
- project.project_feature.update(merge_requests_access_level: ProjectFeature::DISABLED)
+ project.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED)
end
describe 'the author' do
@@ -83,8 +83,8 @@ def permissions(user, merge_request)
let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: author) }
before do
- project.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
- project.project_feature.update(merge_requests_access_level: ProjectFeature::PRIVATE)
+ project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+ project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE)
end
describe 'a non-team-member' do
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index dc6ed94309b31a8b988e270b65a77ca7faa81e37..2c246bd475b50add6dcc706c36220251fa350e79 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -105,7 +105,7 @@ def expect_disallowed(*permissions)
subject { described_class.new(owner, project) }
before do
- project.project_feature.destroy
+ project.project_feature.destroy!
project.reload
end
@@ -953,7 +953,12 @@ def set_access_level(access_level)
context 'when repository is disabled' do
before do
- project.project_feature.update(repository_access_level: ProjectFeature::DISABLED)
+ project.project_feature.update!(
+ # Disable merge_requests and builds as well, since merge_requests and
+ # builds cannot have higher visibility than repository.
+ merge_requests_access_level: ProjectFeature::DISABLED,
+ builds_access_level: ProjectFeature::DISABLED,
+ repository_access_level: ProjectFeature::DISABLED)
end
it { is_expected.to be_disallowed(:read_package) }