Fix Rails/SaveBang offenses

Changelog: other
EE: true

.rubocop_todo/rails/save_bang.yml

@@ -2,13 +2,6 @@
 Rails/SaveBang:
   Exclude:
   - ee/spec/lib/analytics/merge_request_metrics_calculator_spec.rb
-  - ee/spec/models/application_setting_spec.rb
-  - ee/spec/models/approval_merge_request_rule_spec.rb
-  - ee/spec/models/approval_project_rule_spec.rb
-  - ee/spec/models/burndown_spec.rb
-  - ee/spec/models/elasticsearch_indexed_namespace_spec.rb
-  - ee/spec/models/gitlab_subscription_spec.rb
-  - ee/spec/models/issue_spec.rb
   - ee/spec/models/protected_environment_spec.rb
   - ee/spec/models/repository_spec.rb
   - ee/spec/models/scim_identity_spec.rb

ee/spec/models/application_setting_spec.rb

@@ -755,7 +755,7 @@ RSpec.describe ApplicationSetting do
       it "doesn't call the update lifetime service" do
         expect(::PersonalAccessTokens::Instance::UpdateLifetimeService).not_to receive(:new)
 
-        setting.save
+        setting.save!
       end
     end
 
@@ -770,7 +770,7 @@ RSpec.describe ApplicationSetting do
           expect(service).to receive(:execute)
         end
 
-        setting.save
+        setting.save!
       end
     end
   end

ee/spec/models/approval_merge_request_rule_spec.rb

@@ -78,7 +78,7 @@ RSpec.describe ApprovalMergeRequestRule, factory_default: :keep do
 
         expect(rule).not_to be_valid
         expect(rule.errors.messages).to eq(rule_type: ['any-approver for the merge request already exists'])
-        expect { rule.save(validate: false) }.to raise_error(ActiveRecord::RecordNotUnique)
+        expect { rule.save!(validate: false) }.to raise_error(ActiveRecord::RecordNotUnique)
       end
     end
   end
@@ -264,7 +264,7 @@ RSpec.describe ApprovalMergeRequestRule, factory_default: :keep do
 
     context 'when project merge_requests_author_approval is true' do
       it 'contains author' do
-        merge_request.project.update(merge_requests_author_approval: true)
+        merge_request.project.update!(merge_requests_author_approval: true)
 
         expect(subject.approvers).to contain_exactly(merge_request.author)
       end
@@ -272,7 +272,7 @@ RSpec.describe ApprovalMergeRequestRule, factory_default: :keep do
 
     context 'when project merge_requests_author_approval is false' do
       before do
-        merge_request.project.update(merge_requests_author_approval: false)
+        merge_request.project.update!(merge_requests_author_approval: false)
       end
 
       it 'does not contain author' do

ee/spec/models/approval_project_rule_spec.rb

@@ -233,7 +233,7 @@ RSpec.describe ApprovalProjectRule do
 
       expect(rule).not_to be_valid
       expect(rule.errors.messages).to eq(rule_type: ['any-approver for the project already exists'])
-      expect { rule.save(validate: false) }.to raise_error(ActiveRecord::RecordNotUnique)
+      expect { rule.save!(validate: false) }.to raise_error(ActiveRecord::RecordNotUnique)
     end
   end
 
@@ -273,28 +273,28 @@ RSpec.describe ApprovalProjectRule do
     end
 
     describe '#audit_add users after :add' do
-      let(:action!) { rule.update(users: [user, new_user]) }
+      let(:action!) { rule.update!(users: [user, new_user]) }
       let(:message) { 'Added User Spiderman to approval group on Vulnerability rule' }
 
       it_behaves_like 'auditable'
     end
 
     describe '#audit_remove users after :remove' do
-      let(:action!) { rule.update(users: []) }
+      let(:action!) { rule.update!(users: []) }
       let(:message) { 'Removed User Batman from approval group on Vulnerability rule' }
 
       it_behaves_like 'auditable'
     end
 
     describe '#audit_add groups after :add' do
-      let(:action!) { rule.update(groups: [group, new_group]) }
+      let(:action!) { rule.update!(groups: [group, new_group]) }
       let(:message) { 'Added Group Avengers to approval group on Vulnerability rule' }
 
       it_behaves_like 'auditable'
     end
 
     describe '#audit_remove groups after :remove' do
-      let(:action!) { rule.update(groups: []) }
+      let(:action!) { rule.update!(groups: []) }
       let(:message) { 'Removed Group Justice League from approval group on Vulnerability rule' }
 
       it_behaves_like 'auditable'

ee/spec/models/burndown_spec.rb

@@ -39,7 +39,7 @@ RSpec.describe Burndown do
       let_it_be(:non_member) { create(:user) }
 
       subject do
-        project.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+        project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
         described_class.new(milestone.issues_visible_to_user(non_member), milestone.start_date, milestone.due_date).as_json.each { |event| event[:created_at] = event[:created_at].to_date }
       end
 
@@ -57,13 +57,13 @@ RSpec.describe Burndown do
     end
 
     it "returns empty array if milestone start date is nil" do
-      milestone.update(start_date: nil)
+      milestone.update!(start_date: nil)
 
       expect(subject).to eq([])
     end
 
     it "returns empty array if milestone due date is nil" do
-      milestone.update(due_date: nil)
+      milestone.update!(due_date: nil)
 
       expect(subject).to eq([])
     end

ee/spec/models/elasticsearch_indexed_namespace_spec.rb

@@ -37,7 +37,7 @@ RSpec.describe ElasticsearchIndexedNamespace, :saas do
   context 'with plans' do
     Plan::PAID_HOSTED_PLANS.each do |plan|
       plan_factory = "#{plan}_plan"
-      let_it_be(plan_factory) { create(plan_factory) }
+      let_it_be(plan_factory) { create(plan_factory) } # rubocop:disable Rails/SaveBang
     end
 
     let_it_be(:namespaces) { create_list(:namespace, 3) }

ee/spec/models/gitlab_subscription_spec.rb

@@ -6,7 +6,7 @@ RSpec.describe GitlabSubscription, :saas do
   using RSpec::Parameterized::TableSyntax
 
   %i[free_plan bronze_plan premium_plan ultimate_plan].each do |plan|
-    let_it_be(plan) { create(plan) }
+    let_it_be(plan) { create(plan) } # rubocop:disable Rails/SaveBang
   end
 
   describe 'default values', :freeze_time do

ee/spec/models/issue_spec.rb

@@ -546,7 +546,7 @@ RSpec.describe Issue do
 
           award_emoji = create(:award_emoji, :upvote, awardable: issue)
 
-          award_emoji.destroy
+          award_emoji.destroy!
         end
       end
     end
@@ -620,7 +620,7 @@ RSpec.describe Issue do
       end
 
       it 'positions issues even when after and before positions are the same' do
-        issue1.update relative_position: issue.relative_position
+        issue1.update! relative_position: issue.relative_position
 
         new_issue.move_between(issue, issue1)
         [issue, issue1].each(&:reset)
@@ -630,7 +630,7 @@ RSpec.describe Issue do
       end
 
       it 'positions issues between other two if distance is 1' do
-        issue1.update relative_position: issue.relative_position + 1
+        issue1.update! relative_position: issue.relative_position + 1
 
         new_issue.move_between(issue, issue1)
         [issue, issue1].each(&:reset)
@@ -640,8 +640,8 @@ RSpec.describe Issue do
       end
 
       it 'positions issue in the middle of other two if distance is big enough' do
-        issue.update relative_position: 6000
-        issue1.update relative_position: 10000
+        issue.update! relative_position: 6000
+        issue1.update! relative_position: 10000
 
         new_issue.move_between(issue, issue1)
 
@@ -662,8 +662,8 @@ RSpec.describe Issue do
       end
 
       it 'positions issue in the middle of other two if distance is not big enough' do
-        issue.update relative_position: 100
-        issue1.update relative_position: 400
+        issue.update! relative_position: 100
+        issue1.update! relative_position: 400
 
         new_issue.move_between(issue, issue1)
 
@@ -671,8 +671,8 @@ RSpec.describe Issue do
       end
 
       it 'positions issue in the middle of other two is there is no place' do
-        issue.update relative_position: 100
-        issue1.update relative_position: 101
+        issue.update! relative_position: 100
+        issue1.update! relative_position: 101
 
         new_issue.move_between(issue, issue1)
         [issue, issue1].each(&:reset)
@@ -682,10 +682,10 @@ RSpec.describe Issue do
       end
 
       it 'uses rebalancing if there is no place' do
-        issue.update relative_position: 100
-        issue1.update relative_position: 101
+        issue.update! relative_position: 100
+        issue1.update! relative_position: 101
         issue2 = create(:issue, relative_position: 102, project: project)
-        new_issue.update relative_position: 103
+        new_issue.update! relative_position: 103
 
         new_issue.move_between(issue1, issue2)
         new_issue.save!
@@ -698,10 +698,10 @@ RSpec.describe Issue do
       end
 
       it 'positions issue right if we pass non-sequential parameters' do
-        issue.update relative_position: 99
-        issue1.update relative_position: 101
+        issue.update! relative_position: 99
+        issue1.update! relative_position: 101
         issue2 = create(:issue, relative_position: 102, project: project)
-        new_issue.update relative_position: 103
+        new_issue.update! relative_position: 103
 
         new_issue.move_between(issue, issue2)
         new_issue.save!
@@ -849,7 +849,7 @@ RSpec.describe Issue do
         create(:issue_link, source: issue, target: blocked_issue_2, link_type: IssueLink::TYPE_BLOCKS)
         create(:issue_link, source: issue, target: blocked_issue_3, link_type: IssueLink::TYPE_BLOCKS)
         # Set to 0 for proper testing, this is being set by IssueLink callbacks.
-        issue.update(blocking_issues_count: 0)
+        issue.update!(blocking_issues_count: 0)
 
         expect { issue.update_blocking_issues_count! }
           .to change { issue.blocking_issues_count }.from(0).to(3)
@@ -871,7 +871,7 @@ RSpec.describe Issue do
     end
 
     before do
-      blocked_issue.update(blocking_issues_count: 0)
+      blocked_issue.update!(blocking_issues_count: 0)
     end
 
     context 'when blocked issue is closed' do
@@ -887,9 +887,9 @@ RSpec.describe Issue do
     context 'when blocked issue is reopened' do
       before do
         blocked_issue.close
-        blocked_issue.update(blocking_issues_count: 0)
-        blocking_issue1.update(blocking_issues_count: 0)
-        blocking_issue2.update(blocking_issues_count: 0)
+        blocked_issue.update!(blocking_issues_count: 0)
+        blocking_issue1.update!(blocking_issues_count: 0)
+        blocking_issue2.update!(blocking_issues_count: 0)
       end
 
       it 'updates blocking and blocked issues cache' do
@@ -1021,7 +1021,7 @@ RSpec.describe Issue do
       before do
         stub_licensed_features(incident_sla: license_available)
         issue_type = incident_type ? 'incident' : 'issue'
-        issue.update(issue_type: issue_type)
+        issue.update!(issue_type: issue_type)
       end
 
       it 'returns the expected value' do