diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index e85eb1bfd2dbdceffd44b71251804459f4990a57..16b88d33ac468682adaeeddf2f78901228162350 100644 --- a/.rubocop_todo.yml +++ b/.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' diff --git a/changelogs/unreleased/rails-save-bang-19.yml b/changelogs/unreleased/rails-save-bang-19.yml new file mode 100644 index 0000000000000000000000000000000000000000..b7cdd00c88de1d8e671f6ad983bfbe251d2b4dd6 --- /dev/null +++ b/changelogs/unreleased/rails-save-bang-19.yml @@ -0,0 +1,5 @@ +--- +title: Fix Rails/SaveBang offenses for */spec/services/merge_requests/* +merge_request: 41315 +author: Rajendra Kadam +type: other diff --git a/ee/spec/services/merge_requests/approval_service_spec.rb b/ee/spec/services/merge_requests/approval_service_spec.rb index 1a3e37f6e4fbec4c1dafcfbff195c6ad47d2dbea..ab8092749f0cab21f9b413c85985439fbf77205a 100644 --- a/ee/spec/services/merge_requests/approval_service_spec.rb +++ b/ee/spec/services/merge_requests/approval_service_spec.rb @@ -133,8 +133,8 @@ 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 diff --git a/ee/spec/services/merge_requests/remove_approval_service_spec.rb b/ee/spec/services/merge_requests/remove_approval_service_spec.rb index 02f0a02b8a86164f75259c6a4f71b2aafe1eafa7..bfef42932e0ce3a86c01798fde09dbe356548743 100644 --- a/ee/spec/services/merge_requests/remove_approval_service_spec.rb +++ b/ee/spec/services/merge_requests/remove_approval_service_spec.rb @@ -18,7 +18,7 @@ def execute! 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 @@ def execute! 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 diff --git a/ee/spec/services/merge_requests/update_blocks_service_spec.rb b/ee/spec/services/merge_requests/update_blocks_service_spec.rb index b73cc520a12641a8088154c7c5a9991c0e089f16..d0158d391302854dce4aac993150389ae367cd67 100644 --- a/ee/spec/services/merge_requests/update_blocks_service_spec.rb +++ b/ee/spec/services/merge_requests/update_blocks_service_spec.rb @@ -42,7 +42,7 @@ { remove_hidden: remove_hidden, references: refs, - update: update + update: update # rubocop: disable Rails/SaveBang } end diff --git a/spec/services/members/destroy_service_spec.rb b/spec/services/members/destroy_service_spec.rb index 5c90f1f54ea875b65d23916b153cf80190da1a4c..3b3f2f3b95af10e4f038f5441cb17da424715cb0 100644 --- a/spec/services/members/destroy_service_spec.rb +++ b/spec/services/members/destroy_service_spec.rb @@ -192,8 +192,8 @@ 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 diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb index 20ed57c6e51a80e8783e0a5d9045e251ef48c1d5..f83b8d98ce86245148a434098098081f0f2b198e 100644 --- a/spec/services/merge_requests/build_service_spec.rb +++ b/spec/services/merge_requests/build_service_spec.rb @@ -514,7 +514,7 @@ def stub_compare 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 diff --git a/spec/services/merge_requests/conflicts/list_service_spec.rb b/spec/services/merge_requests/conflicts/list_service_spec.rb index cd9eae2f2eed9ef46ac512b0d48ee11bbe311d89..5132eac0158d5cf33a41703860cc06b8f3003512 100644 --- a/spec/services/merge_requests/conflicts/list_service_spec.rb +++ b/spec/services/merge_requests/conflicts/list_service_spec.rb @@ -36,7 +36,7 @@ def conflicts_service(merge_request) 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 diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index 941e1f20999d489c9ea17a3f31b7cfd9bc1215e0..05be3c1946b9e83de226114197de11cb42bcf138 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -440,7 +440,7 @@ 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 @@ 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 diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index 2b0adbd7b5d76db2472f3393cdd81bee27717d8f..8328f461029556d73761bfd182e91febfe8b038a 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -176,7 +176,7 @@ 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 @@ 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 @@ 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 @@ %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) diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb index a51a896ca96481b7785cad9595cdc8a519e832f9..0e7a75f2f175613f7c4ccde8561b5b5f39de3a62 100644 --- a/spec/services/merge_requests/post_merge_service_spec.rb +++ b/spec/services/merge_requests/post_merge_service_spec.rb @@ -50,7 +50,7 @@ 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) diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 4603133075028899546ab55a9af2d6000f47eedd..cace1e0bf09fbcaa7bfd60d5dadab4bd653b0c2e 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -621,7 +621,7 @@ def refresh 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 diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 097115af4ef432121de4020203563fc91bbf7b1c..5de1aaa65f6b455074da890cae2e828bc447cc74 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -415,7 +415,7 @@ def update_merge_request(opts) 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 @@ def update_merge_request(opts) 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 @@ def update_merge_request(opts) 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 @@ def update_merge_request(opts) 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)