From 1de5763709c4e85f5c8ffd6bb7f5b7bf4eafbafd Mon Sep 17 00:00:00 2001
From: Rajendra Kadam <rajendrakadam249@gmail.com>
Date: Mon, 27 Jul 2020 20:07:21 +0530
Subject: [PATCH] Fix rubocop offenses for ee/spec/policies/* and
spec/policies/*
This MR fixes Rails/SaveBang cop for spec files in policies folder.
Add changelog for the fix
Fix specs that were failing due to change
---
.rubocop_todo.yml | 12 -----------
changelogs/unreleased/rails-save-bang-6.yml | 5 +++++
ee/spec/policies/group_policy_spec.rb | 2 +-
ee/spec/policies/note_policy_spec.rb | 2 +-
ee/spec/policies/project_policy_spec.rb | 7 ++++++-
.../policies/protected_branch_policy_spec.rb | 2 +-
.../vulnerabilities/feedback_policy_spec.rb | 4 ++--
spec/policies/ci/build_policy_spec.rb | 2 +-
spec/policies/ci/pipeline_policy_spec.rb | 2 +-
.../ci/pipeline_schedule_policy_spec.rb | 6 +++---
spec/policies/group_policy_spec.rb | 20 +++++++++----------
spec/policies/issue_policy_spec.rb | 2 +-
spec/policies/merge_request_policy_spec.rb | 6 +++---
spec/policies/project_policy_spec.rb | 9 +++++++--
14 files changed, 42 insertions(+), 39 deletions(-)
create mode 100644 changelogs/unreleased/rails-save-bang-6.yml
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index 9152b5bdb8b27..3e0c2838252c3 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 0000000000000..d0731fd34a960
--- /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 b4f16d0d32deb..fc5758a256f77 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 f1a7226f7d564..43696cfab80f1 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 850b880e86f93..026472479444c 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 ade98dd46445d..63312104dfd20 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 09457e549f857..d8708223a8b1d 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 d25473388553e..7ea180af9d0c0 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 fcd96bc66532f..9a65823c950c1 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 b455384d17a84..1e36f455f6fcd 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 733cc9bd9cb0a..4debaf59eb9f4 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 b917def66aa19..e352b9901593b 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 2f3cb2e998aa7..3a46d5b922674 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 dc6ed94309b31..2c246bd475b50 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) }
--
GitLab