Merge branch 'rails-save-bang-6' into 'master'

Fix Rails/SaveBang cop for all ee/spec/policies/* and spec/policies/*

See merge request gitlab-org/gitlab!37956

.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'

changelogs/unreleased/rails-save-bang-6.yml

+---
+title: Refactor spec/policies and ee/spec/policies to fix SaveBang Cop
+merge_request: 37956
+author: Rajendra Kadam
+type: fixed

ee/spec/policies/group_policy_spec.rb

@@ -881,7 +881,7 @@ RSpec.describe GroupPolicy do
 
       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) }

ee/spec/policies/note_policy_spec.rb

@@ -101,7 +101,7 @@ RSpec.describe NotePolicy do
 
     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'

ee/spec/policies/project_policy_spec.rb

@@ -649,7 +649,12 @@ RSpec.describe ProjectPolicy do
 
       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) }

ee/spec/policies/protected_branch_policy_spec.rb

@@ -13,7 +13,7 @@ RSpec.describe ProtectedBranchPolicy do
 
   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

ee/spec/policies/vulnerabilities/feedback_policy_spec.rb

@@ -16,7 +16,7 @@ RSpec.describe Vulnerabilities::FeedbackPolicy do
 
       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 @@ RSpec.describe Vulnerabilities::FeedbackPolicy do
 
       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

spec/policies/ci/build_policy_spec.rb

@@ -146,7 +146,7 @@ RSpec.describe Ci::BuildPolicy do
           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

spec/policies/ci/pipeline_policy_spec.rb

@@ -45,7 +45,7 @@ RSpec.describe Ci::PipelinePolicy, :models do
           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

spec/policies/ci/pipeline_schedule_policy_spec.rb

@@ -43,7 +43,7 @@ RSpec.describe Ci::PipelineSchedulePolicy, :models do
         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 @@ RSpec.describe Ci::PipelineSchedulePolicy, :models do
     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 @@ RSpec.describe Ci::PipelineSchedulePolicy, :models do
       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

spec/policies/group_policy_spec.rb

@@ -107,7 +107,7 @@ RSpec.describe GroupPolicy do
 
     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 @@ RSpec.describe GroupPolicy do
   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 @@ RSpec.describe GroupPolicy do
 
     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 @@ RSpec.describe GroupPolicy do
   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 @@ RSpec.describe GroupPolicy do
 
     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 @@ RSpec.describe GroupPolicy do
 
     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 @@ RSpec.describe GroupPolicy do
 
     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 @@ RSpec.describe GroupPolicy do
   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 @@ RSpec.describe GroupPolicy do
 
     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 @@ RSpec.describe GroupPolicy do
 
         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) }

spec/policies/issue_policy_spec.rb

@@ -188,7 +188,7 @@ RSpec.describe IssuePolicy do
 
     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) }

spec/policies/merge_request_policy_spec.rb

@@ -51,7 +51,7 @@ RSpec.describe MergeRequestPolicy do
     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 @@ RSpec.describe MergeRequestPolicy do
     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

spec/policies/project_policy_spec.rb

@@ -105,7 +105,7 @@ RSpec.describe ProjectPolicy do
     subject { described_class.new(owner, project) }
 
     before do
-      project.project_feature.destroy
+      project.project_feature.destroy!
       project.reload
     end
 
@@ -953,7 +953,12 @@ RSpec.describe ProjectPolicy do
 
       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) }