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

Fix Rails/SaveBang offenses for spec/services/merge_requests/*

See merge request gitlab-org/gitlab!41315

.rubocop_todo.yml

@@ -793,9 +793,6 @@ Rails/SaveBang:
     - 'ee/spec/services/groups/autocomplete_service_spec.rb'
     - 'ee/spec/services/ldap_group_reset_service_spec.rb'
     - 'ee/spec/services/lfs/unlock_file_service_spec.rb'
-    - 'ee/spec/services/merge_requests/approval_service_spec.rb'
-    - 'ee/spec/services/merge_requests/remove_approval_service_spec.rb'
-    - 'ee/spec/services/merge_requests/update_blocks_service_spec.rb'
     - 'ee/spec/services/merge_trains/refresh_merge_request_service_spec.rb'
     - 'ee/spec/services/quick_actions/interpret_service_spec.rb'
     - 'ee/spec/services/slash_commands/global_slack_handler_spec.rb'
@@ -1181,14 +1178,6 @@ Rails/SaveBang:
     - 'spec/services/issuable/clone/attributes_rewriter_spec.rb'
     - 'spec/services/issuable/common_system_notes_service_spec.rb'
     - 'spec/services/labels/promote_service_spec.rb'
-    - 'spec/services/members/destroy_service_spec.rb'
-    - 'spec/services/merge_requests/build_service_spec.rb'
-    - 'spec/services/merge_requests/conflicts/list_service_spec.rb'
-    - 'spec/services/merge_requests/create_service_spec.rb'
-    - 'spec/services/merge_requests/merge_service_spec.rb'
-    - 'spec/services/merge_requests/post_merge_service_spec.rb'
-    - 'spec/services/merge_requests/refresh_service_spec.rb'
-    - 'spec/services/merge_requests/update_service_spec.rb'
     - 'spec/services/milestones/destroy_service_spec.rb'
     - 'spec/services/milestones/promote_service_spec.rb'
     - 'spec/services/milestones/transfer_service_spec.rb'

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

+---
+title: Fix Rails/SaveBang offenses for */spec/services/merge_requests/*
+merge_request: 41315
+author: Rajendra Kadam
+type: other

ee/spec/services/merge_requests/approval_service_spec.rb

@@ -133,8 +133,8 @@ RSpec.describe MergeRequests::ApprovalService do
 
     context 'when project requires force auth for approval' do
       before do
-        project.update(require_password_to_approve: true)
-        user.update(password: 'password')
+        project.update!(require_password_to_approve: true)
+        user.update!(password: 'password')
       end
       context 'when password not specified' do
         it 'does not update the approvals' do

ee/spec/services/merge_requests/remove_approval_service_spec.rb

@@ -18,7 +18,7 @@ RSpec.describe MergeRequests::RemoveApprovalService do
       before do
         project.add_developer(create(:user))
         merge_request.update!(approvals_before_merge: 2)
-        merge_request.approvals.create(user: user)
+        merge_request.approvals.create!(user: user)
       end
 
       it 'removes the approval' do
@@ -54,7 +54,7 @@ RSpec.describe MergeRequests::RemoveApprovalService do
       let(:notification_service) { NotificationService.new }
 
       before do
-        merge_request.approvals.create(user: user)
+        merge_request.approvals.create!(user: user)
         allow(service).to receive(:notification_service).and_return(notification_service)
       end
 

ee/spec/services/merge_requests/update_blocks_service_spec.rb

@@ -42,7 +42,7 @@ RSpec.describe MergeRequests::UpdateBlocksService do
       {
         remove_hidden: remove_hidden,
         references: refs,
-        update: update
+        update: update # rubocop: disable Rails/SaveBang
       }
     end
 

spec/services/members/destroy_service_spec.rb

@@ -192,8 +192,8 @@ RSpec.describe Members::DestroyService do
 
   context 'with an access requester' do
     before do
-      group_project.update(request_access_enabled: true)
-      group.update(request_access_enabled: true)
+      group_project.update!(request_access_enabled: true)
+      group.update!(request_access_enabled: true)
       group_project.request_access(member_user)
       group.request_access(member_user)
     end

spec/services/merge_requests/build_service_spec.rb

@@ -514,7 +514,7 @@ RSpec.describe MergeRequests::BuildService do
       let(:target_project) { create(:project, :public, :repository) }
 
       before do
-        target_project.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+        target_project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
       end
 
       it 'sets the target_project correctly' do

spec/services/merge_requests/conflicts/list_service_spec.rb

@@ -36,7 +36,7 @@ RSpec.describe MergeRequests::Conflicts::ListService do
 
     it 'returns a falsey value when the MR does not support new diff notes' do
       merge_request = create_merge_request('conflict-resolvable')
-      merge_request.merge_request_diff.update(start_commit_sha: nil)
+      merge_request.merge_request_diff.update!(start_commit_sha: nil)
 
       expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_falsey
     end

spec/services/merge_requests/create_service_spec.rb

@@ -440,7 +440,7 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
 
         context "when issuable feature is private" do
           before do
-            project.project_feature.update(issues_access_level: ProjectFeature::PRIVATE,
+            project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE,
                                            merge_requests_access_level: ProjectFeature::PRIVATE)
           end
 
@@ -448,7 +448,7 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
 
           levels.each do |level|
             it "removes not authorized assignee when project is #{Gitlab::VisibilityLevel.level_name(level)}" do
-              project.update(visibility_level: level)
+              project.update!(visibility_level: level)
               opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] }
 
               merge_request = described_class.new(project, user, opts).execute

spec/services/merge_requests/merge_service_spec.rb

@@ -176,7 +176,7 @@ RSpec.describe MergeRequests::MergeService do
           end
 
           it 'does not close issue' do
-            jira_tracker.update(jira_issue_transition_id: nil)
+            jira_tracker.update!(jira_issue_transition_id: nil)
 
             expect_any_instance_of(JiraService).not_to receive(:transition_issue)
 
@@ -389,7 +389,7 @@ RSpec.describe MergeRequests::MergeService do
           error_message = 'Failed to squash. Should be done manually'
 
           allow_any_instance_of(MergeRequests::SquashService).to receive(:squash!).and_return(nil)
-          merge_request.update(squash: true)
+          merge_request.update!(squash: true)
 
           service.execute(merge_request)
 
@@ -403,7 +403,7 @@ RSpec.describe MergeRequests::MergeService do
           error_message = 'another squash is already in progress'
 
           allow_any_instance_of(MergeRequest).to receive(:squash_in_progress?).and_return(true)
-          merge_request.update(squash: true)
+          merge_request.update!(squash: true)
 
           service.execute(merge_request)
 
@@ -421,7 +421,7 @@ RSpec.describe MergeRequests::MergeService do
           %w(semi-linear ff).each do |merge_method|
             it "logs and saves error if merge is #{merge_method} only" do
               merge_method = 'rebase_merge' if merge_method == 'semi-linear'
-              merge_request.project.update(merge_method: merge_method)
+              merge_request.project.update!(merge_method: merge_method)
               error_message = 'Only fast-forward merge is allowed for your project. Please update your source branch'
               allow(service).to receive(:execute_hooks)
 

spec/services/merge_requests/post_merge_service_spec.rb

@@ -50,7 +50,7 @@ RSpec.describe MergeRequests::PostMergeService do
     end
 
     it 'marks MR as merged regardless of errors when closing issues' do
-      merge_request.update(target_branch: 'foo')
+      merge_request.update!(target_branch: 'foo')
       allow(project).to receive(:default_branch).and_return('foo')
 
       issue = create(:issue, project: project)

spec/services/merge_requests/refresh_service_spec.rb

@@ -621,7 +621,7 @@ RSpec.describe MergeRequests::RefreshService do
         before do
           stub_feature_flags(track_resource_state_change_events: state_tracking_enabled)
 
-          @fork_project.destroy
+          @fork_project.destroy!
           service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/feature')
           reload_mrs
         end

spec/services/merge_requests/update_service_spec.rb

@@ -415,7 +415,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
         it 'sends notifications for subscribers of changed milestone', :sidekiq_might_not_need_inline do
           merge_request.milestone = create(:milestone, project: project)
 
-          merge_request.save
+          merge_request.save!
 
           perform_enqueued_jobs do
             update_merge_request(milestone_id: "")
@@ -639,7 +639,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
 
     context 'updating asssignee_ids' do
       it 'does not update assignee when assignee_id is invalid' do
-        merge_request.update(assignee_ids: [user.id])
+        merge_request.update!(assignee_ids: [user.id])
 
         update_merge_request(assignee_ids: [-1])
 
@@ -647,7 +647,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
       end
 
       it 'unassigns assignee when user id is 0' do
-        merge_request.update(assignee_ids: [user.id])
+        merge_request.update!(assignee_ids: [user.id])
 
         update_merge_request(assignee_ids: [0])
 
@@ -675,7 +675,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
         levels.each do |level|
           it "does not update with unauthorized assignee when project is #{Gitlab::VisibilityLevel.level_name(level)}" do
             assignee = create(:user)
-            project.update(visibility_level: level)
+            project.update!(visibility_level: level)
             feature_visibility_attr = :"#{merge_request.model_name.plural}_access_level"
             project.project_feature.update_attribute(feature_visibility_attr, ProjectFeature::PRIVATE)