Merge branch 'issue#220040-fix-rails-savebang-project-models' into 'master'

Fix Rails/SaveBang Rubocop offenses for project related models

See merge request gitlab-org/gitlab!57983

.rubocop_manual_todo.yml

@@ -199,10 +199,6 @@ Rails/SaveBang:
     - 'spec/models/operations/feature_flags/strategy_spec.rb'
     - 'spec/models/operations/feature_flags/user_list_spec.rb'
     - 'spec/models/pages_domain_spec.rb'
-    - 'spec/models/project_auto_devops_spec.rb'
-    - 'spec/models/project_feature_spec.rb'
-    - 'spec/models/project_spec.rb'
-    - 'spec/models/project_team_spec.rb'
     - 'spec/models/protectable_dropdown_spec.rb'
     - 'spec/models/redirect_route_spec.rb'
     - 'spec/models/release_spec.rb'

changelogs/unreleased/issue-220040-fix-rails-savebang-project-models.yml

+---
+title: Fix Rails/SaveBang Rubocop offenses for project related models
+merge_request: 57983
+author: Huzaifa Iftikhar @huzaifaiftikhar
+type: fixed

spec/models/project_auto_devops_spec.rb

@@ -70,7 +70,7 @@ RSpec.describe ProjectAutoDevops do
 
       it 'does not create a gitlab deploy token' do
         expect do
-          auto_devops.save
+          auto_devops.save!
         end.not_to change { DeployToken.count }
       end
     end
@@ -80,7 +80,7 @@ RSpec.describe ProjectAutoDevops do
 
       it 'creates a gitlab deploy token' do
         expect do
-          auto_devops.save
+          auto_devops.save!
         end.to change { DeployToken.count }.by(1)
       end
     end
@@ -90,7 +90,7 @@ RSpec.describe ProjectAutoDevops do
 
       it 'creates a gitlab deploy token' do
         expect do
-          auto_devops.save
+          auto_devops.save!
         end.to change { DeployToken.count }.by(1)
       end
     end
@@ -101,7 +101,7 @@ RSpec.describe ProjectAutoDevops do
 
       it 'creates a deploy token' do
         expect do
-          auto_devops.save
+          auto_devops.save!
         end.to change { DeployToken.count }.by(1)
       end
     end
@@ -114,7 +114,7 @@ RSpec.describe ProjectAutoDevops do
         allow(Gitlab::CurrentSettings).to receive(:auto_devops_enabled?).and_return(true)
 
         expect do
-          auto_devops.save
+          auto_devops.save!
         end.to change { DeployToken.count }.by(1)
       end
     end
@@ -125,7 +125,7 @@ RSpec.describe ProjectAutoDevops do
 
       it 'does not create a deploy token' do
         expect do
-          auto_devops.save
+          auto_devops.save!
         end.not_to change { DeployToken.count }
       end
     end
@@ -137,7 +137,7 @@ RSpec.describe ProjectAutoDevops do
 
       it 'does not create a deploy token' do
         expect do
-          auto_devops.save
+          auto_devops.save!
         end.not_to change { DeployToken.count }
       end
     end
@@ -149,7 +149,7 @@ RSpec.describe ProjectAutoDevops do
 
       it 'does not create a deploy token' do
         expect do
-          auto_devops.save
+          auto_devops.save!
         end.not_to change { DeployToken.count }
       end
     end

spec/models/project_feature_spec.rb

@@ -20,7 +20,7 @@ RSpec.describe ProjectFeature do
 
   context 'repository related features' do
     before do
-      project.project_feature.update(
+      project.project_feature.update!(
         merge_requests_access_level: ProjectFeature::DISABLED,
         builds_access_level: ProjectFeature::DISABLED,
         repository_access_level: ProjectFeature::PRIVATE

spec/models/project_spec.rb

@@ -215,7 +215,7 @@ RSpec.describe Project, factory_default: :keep do
       it 'does not raise an error' do
         project = create(:project)
 
-        expect { project.update(ci_cd_settings: nil) }.not_to raise_exception
+        expect { project.update!(ci_cd_settings: nil) }.not_to raise_exception
       end
     end
 
@@ -873,13 +873,13 @@ RSpec.describe Project, factory_default: :keep do
       end
 
       it 'returns the most recent timestamp' do
-        project.update(updated_at: nil,
+        project.update!(updated_at: nil,
                        last_activity_at: timestamp,
                        last_repository_updated_at: timestamp - 1.hour)
 
         expect(project.last_activity_date).to be_like_time(timestamp)
 
-        project.update(updated_at: timestamp,
+        project.update!(updated_at: timestamp,
                        last_activity_at: timestamp - 1.hour,
                        last_repository_updated_at: nil)
 
@@ -2672,7 +2672,7 @@ RSpec.describe Project, factory_default: :keep do
 
     context 'with pending pipeline' do
       it 'returns empty relation' do
-        pipeline.update(status: 'pending')
+        pipeline.update!(status: 'pending')
         pending_build = create_build(pipeline)
 
         expect { project.latest_successful_build_for_ref!(pending_build.name) }
@@ -2865,7 +2865,7 @@ RSpec.describe Project, factory_default: :keep do
     end
 
     it 'returns false when remote mirror is disabled' do
-      project.remote_mirrors.first.update(enabled: false)
+      project.remote_mirrors.first.update!(enabled: false)
 
       is_expected.to be_falsy
     end
@@ -2896,7 +2896,7 @@ RSpec.describe Project, factory_default: :keep do
     end
 
     it 'does not sync disabled remote mirrors' do
-      project.remote_mirrors.first.update(enabled: false)
+      project.remote_mirrors.first.update!(enabled: false)
 
       expect_any_instance_of(RemoteMirror).not_to receive(:sync)
 
@@ -2934,7 +2934,7 @@ RSpec.describe Project, factory_default: :keep do
     it 'fails stuck remote mirrors' do
       project = create(:project, :repository, :remote_mirror)
 
-      project.remote_mirrors.first.update(
+      project.remote_mirrors.first.update!(
         update_status: :started,
         last_update_started_at: 2.days.ago
       )
@@ -3192,7 +3192,7 @@ RSpec.describe Project, factory_default: :keep do
       end
 
       it 'returns the root of the fork network when the directs source was deleted' do
-        forked_project.destroy
+        forked_project.destroy!
 
         expect(second_fork.fork_source).to eq(project)
       end
@@ -3436,7 +3436,7 @@ RSpec.describe Project, factory_default: :keep do
           let(:environment) { 'foo%bar/test' }
 
           it 'matches literally for _' do
-            ci_variable.update(environment_scope: 'foo%bar/*')
+            ci_variable.environment_scope = 'foo%bar/*'
 
             is_expected.to contain_exactly(ci_variable)
           end
@@ -3677,7 +3677,7 @@ RSpec.describe Project, factory_default: :keep do
 
     it "updates the namespace_id when changed" do
       namespace = create(:namespace)
-      project.update(namespace: namespace)
+      project.update!(namespace: namespace)
 
       expect(project.statistics.namespace_id).to eq namespace.id
     end
@@ -3970,14 +3970,14 @@ RSpec.describe Project, factory_default: :keep do
       expect(project).to receive(:visibility_level_allowed_as_fork).and_call_original
       expect(project).to receive(:visibility_level_allowed_by_group).and_call_original
 
-      project.update(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
+      project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
     end
 
     it 'does not validate the visibility' do
       expect(project).not_to receive(:visibility_level_allowed_as_fork).and_call_original
       expect(project).not_to receive(:visibility_level_allowed_by_group).and_call_original
 
-      project.update(updated_at: Time.current)
+      project.update!(updated_at: Time.current)
     end
   end
 
@@ -4061,7 +4061,7 @@ RSpec.describe Project, factory_default: :keep do
       project_2 = create(:project, :public, :merge_requests_disabled)
       project_3 = create(:project, :public, :issues_disabled)
       project_4 = create(:project, :public)
-      project_4.project_feature.update(issues_access_level: ProjectFeature::PRIVATE, merge_requests_access_level: ProjectFeature::PRIVATE )
+      project_4.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE, merge_requests_access_level: ProjectFeature::PRIVATE )
 
       project_ids = described_class.ids_with_issuables_available_for(user).pluck(:id)
 
@@ -4104,7 +4104,7 @@ RSpec.describe Project, factory_default: :keep do
       let(:project) { create(:project, :public) }
 
       it 'returns projects with the project feature access level nil' do
-        project.project_feature.update(merge_requests_access_level: nil)
+        project.project_feature.update!(merge_requests_access_level: nil)
 
         is_expected.to include(project)
       end
@@ -4392,7 +4392,7 @@ RSpec.describe Project, factory_default: :keep do
     it 'is run when the project is destroyed' do
       expect(project).to receive(:legacy_remove_pages).and_call_original
 
-      expect { project.destroy }.not_to raise_error
+      expect { project.destroy! }.not_to raise_error
     end
   end
 
@@ -4922,7 +4922,7 @@ RSpec.describe Project, factory_default: :keep do
 
     context 'when enabled on group' do
       it 'has auto devops implicitly enabled' do
-        project.update(namespace: create(:group, :auto_devops_enabled))
+        project.update!(namespace: create(:group, :auto_devops_enabled))
 
         expect(project).to have_auto_devops_implicitly_enabled
       end
@@ -4931,7 +4931,7 @@ RSpec.describe Project, factory_default: :keep do
     context 'when enabled on parent group' do
       it 'has auto devops implicitly enabled' do
         subgroup = create(:group, parent: create(:group, :auto_devops_enabled))
-        project.update(namespace: subgroup)
+        project.update!(namespace: subgroup)
 
         expect(project).to have_auto_devops_implicitly_enabled
       end
@@ -5405,7 +5405,7 @@ RSpec.describe Project, factory_default: :keep do
 
       before do
         create_list(:group_badge, 2, group: project_group)
-        project_group.update(parent: parent_group)
+        project_group.update!(parent: parent_group)
       end
 
       it 'returns the project and the project nested groups badges' do
@@ -6489,7 +6489,7 @@ RSpec.describe Project, factory_default: :keep do
     end
 
     it 'removes chat names on removal' do
-      expect { subject.destroy }.to change { ChatName.count }.by(-5)
+      expect { subject.destroy! }.to change { ChatName.count }.by(-5)
     end
   end
 

spec/models/project_team_spec.rb

@@ -294,7 +294,7 @@ RSpec.describe ProjectTeam do
       context 'when project is shared with group' do
         before do
           group = create(:group)
-          project.project_group_links.create(
+          project.project_group_links.create!(
             group: group,
             group_access: Gitlab::Access::DEVELOPER)
 
@@ -309,7 +309,7 @@ RSpec.describe ProjectTeam do
 
         context 'but share_with_group_lock is true' do
           before do
-            project.namespace.update(share_with_group_lock: true)
+            project.namespace.update!(share_with_group_lock: true)
           end
 
           it { expect(project.team.max_member_access(maintainer.id)).to eq(Gitlab::Access::NO_ACCESS) }
@@ -496,7 +496,7 @@ RSpec.describe ProjectTeam do
       project.add_guest(promoted_guest)
       project.add_guest(guest)
 
-      project.project_group_links.create(
+      project.project_group_links.create!(
         group: group,
         group_access: Gitlab::Access::DEVELOPER
       )
@@ -505,7 +505,7 @@ RSpec.describe ProjectTeam do
       group.add_developer(group_developer)
       group.add_developer(second_developer)
 
-      project.project_group_links.create(
+      project.project_group_links.create!(
         group: second_group,
         group_access: Gitlab::Access::MAINTAINER
       )