From a21167da1b84fba4200c2a9540929d799037d9e2 Mon Sep 17 00:00:00 2001 From: Rajendra Kadam <rajendrakadam249@gmail.com> Date: Mon, 27 Jul 2020 09:33:02 +0000 Subject: [PATCH] Fix Rails/SaveBang RuboCop offenses for all `spec/helpers/*` and `ee/spec/helpers/*` --- .rubocop_todo.yml | 13 ------------- changelogs/unreleased/rails-save-bang-5.yml | 5 +++++ ee/spec/helpers/application_helper_spec.rb | 2 +- ee/spec/helpers/ee/dashboard_helper_spec.rb | 2 +- ee/spec/helpers/ee/issues_helper_spec.rb | 2 +- spec/helpers/appearances_helper_spec.rb | 2 +- spec/helpers/auto_devops_helper_spec.rb | 2 +- spec/helpers/issuables_helper_spec.rb | 2 +- spec/helpers/issues_helper_spec.rb | 8 ++++---- spec/helpers/members_helper_spec.rb | 2 +- spec/helpers/notes_helper_spec.rb | 6 +++--- spec/helpers/profiles_helper_spec.rb | 2 +- .../projects/alert_management_helper_spec.rb | 2 +- spec/helpers/projects_helper_spec.rb | 8 ++++---- spec/helpers/visibility_level_helper_spec.rb | 7 ++++--- 15 files changed, 29 insertions(+), 36 deletions(-) create mode 100644 changelogs/unreleased/rails-save-bang-5.yml diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 279ef50bc3210..9152b5bdb8b27 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1273,9 +1273,6 @@ Rails/SaveBang: - 'ee/spec/finders/security/vulnerabilities_finder_spec.rb' - 'ee/spec/frontend/fixtures/analytics.rb' - 'ee/spec/graphql/resolvers/vulnerabilities_resolver_spec.rb' - - 'ee/spec/helpers/application_helper_spec.rb' - - 'ee/spec/helpers/ee/dashboard_helper_spec.rb' - - 'ee/spec/helpers/ee/issues_helper_spec.rb' - 'ee/spec/initializers/fog_google_https_private_urls_spec.rb' - 'ee/spec/lib/analytics/merge_request_metrics_calculator_spec.rb' - 'ee/spec/lib/ee/gitlab/auth/ldap/sync/group_spec.rb' @@ -1571,16 +1568,6 @@ Rails/SaveBang: - 'spec/graphql/mutations/merge_requests/set_locked_spec.rb' - 'spec/graphql/mutations/merge_requests/set_wip_spec.rb' - 'spec/graphql/resolvers/boards_resolver_spec.rb' - - 'spec/helpers/appearances_helper_spec.rb' - - 'spec/helpers/auto_devops_helper_spec.rb' - - 'spec/helpers/issuables_helper_spec.rb' - - 'spec/helpers/issues_helper_spec.rb' - - 'spec/helpers/members_helper_spec.rb' - - 'spec/helpers/notes_helper_spec.rb' - - 'spec/helpers/profiles_helper_spec.rb' - - 'spec/helpers/projects/alert_management_helper_spec.rb' - - 'spec/helpers/projects_helper_spec.rb' - - 'spec/helpers/visibility_level_helper_spec.rb' - 'spec/initializers/active_record_locking_spec.rb' - 'spec/initializers/fog_google_https_private_urls_spec.rb' - 'spec/lib/after_commit_queue_spec.rb' diff --git a/changelogs/unreleased/rails-save-bang-5.yml b/changelogs/unreleased/rails-save-bang-5.yml new file mode 100644 index 0000000000000..5fdb72b71a9e3 --- /dev/null +++ b/changelogs/unreleased/rails-save-bang-5.yml @@ -0,0 +1,5 @@ +--- +title: Refactor spec/helpers and ee/spec/helpers to fix SaveBang Cop +merge_request: 37446 +author: Rajendra Kadam +type: fixed diff --git a/ee/spec/helpers/application_helper_spec.rb b/ee/spec/helpers/application_helper_spec.rb index d11e9c8755f01..a1549f10f8d52 100644 --- a/ee/spec/helpers/application_helper_spec.rb +++ b/ee/spec/helpers/application_helper_spec.rb @@ -125,7 +125,7 @@ def expect_autocomplete_data_sources(object, noteable_type, source_keys) end it 'returns paths for autocomplete_sources_controller including epics for group projects' do - object.update(group: create(:group)) + object.update!(group: create(:group)) expect_autocomplete_data_sources(object, noteable_type, [:members, :issues, :mergeRequests, :labels, :milestones, :commands, :snippets, :epics]) end diff --git a/ee/spec/helpers/ee/dashboard_helper_spec.rb b/ee/spec/helpers/ee/dashboard_helper_spec.rb index cbe3e1dec7d60..4e7dbaa9bcc74 100644 --- a/ee/spec/helpers/ee/dashboard_helper_spec.rb +++ b/ee/spec/helpers/ee/dashboard_helper_spec.rb @@ -185,7 +185,7 @@ def stub_user_permissions_for(feature, enabled) end with_them do - let(:user) { create(current_user) } + let(:user) { create(current_user) } # rubocop:disable Rails/SaveBang let(:license) { has_license && create(:license) } subject { helper.has_start_trial? } diff --git a/ee/spec/helpers/ee/issues_helper_spec.rb b/ee/spec/helpers/ee/issues_helper_spec.rb index 62fa546b21ffa..f5268c9f103e3 100644 --- a/ee/spec/helpers/ee/issues_helper_spec.rb +++ b/ee/spec/helpers/ee/issues_helper_spec.rb @@ -13,7 +13,7 @@ context 'with linked issue' do context 'with promoted issue' do before do - issue.update(promoted_to_epic: new_epic) + issue.update!(promoted_to_epic: new_epic) end context 'when user has permission to see new epic' do diff --git a/spec/helpers/appearances_helper_spec.rb b/spec/helpers/appearances_helper_spec.rb index 179c69b2a67c6..d972ac271193a 100644 --- a/spec/helpers/appearances_helper_spec.rb +++ b/spec/helpers/appearances_helper_spec.rb @@ -70,7 +70,7 @@ context 'when there is a logo but no associated upload' do before do # Legacy attachments were not tracked in the uploads table - appearance.logo.upload.destroy + appearance.logo.upload.destroy! appearance.reload end diff --git a/spec/helpers/auto_devops_helper_spec.rb b/spec/helpers/auto_devops_helper_spec.rb index ad705dc5a7b31..4f060a0ae3b5e 100644 --- a/spec/helpers/auto_devops_helper_spec.rb +++ b/spec/helpers/auto_devops_helper_spec.rb @@ -128,7 +128,7 @@ context 'with groups' do before do - receiver.update(parent: parent) + receiver.update!(parent: parent) end context 'when auto devops is enabled on parent' do diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index 4c93a8387a9ee..581c2fb0afe28 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -160,7 +160,7 @@ end before do - user.destroy + user.destroy! end it 'returns "Ghost user" as edited_by' do diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb index f2757f0e3ed55..3f84eeb12c21e 100644 --- a/spec/helpers/issues_helper_spec.rb +++ b/spec/helpers/issues_helper_spec.rb @@ -162,7 +162,7 @@ context 'with linked issue' do context 'with moved issue' do before do - issue.update(moved_to: new_issue) + issue.update!(moved_to: new_issue) end context 'when user has permission to see new issue' do @@ -181,7 +181,7 @@ context 'with duplicated issue' do before do - issue.update(duplicated_to: new_issue) + issue.update!(duplicated_to: new_issue) end context 'when user has permission to see new issue' do @@ -203,7 +203,7 @@ let(:user) { project.owner } before do - issue.update(moved_to: nil, duplicated_to: nil) + issue.update!(moved_to: nil, duplicated_to: nil) end it_behaves_like 'does not display link' @@ -220,7 +220,7 @@ allow(Gitlab::IncomingEmail).to receive(:enabled?) { true } allow(Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true } - old_issue.update(moved_to: new_issue) + old_issue.update!(moved_to: new_issue) end it 'is true when moved issue project has service desk disabled' do diff --git a/spec/helpers/members_helper_spec.rb b/spec/helpers/members_helper_spec.rb index 99e8696e96063..84b3f99b89ad7 100644 --- a/spec/helpers/members_helper_spec.rb +++ b/spec/helpers/members_helper_spec.rb @@ -25,7 +25,7 @@ context 'an accepted user invitation with no user associated' do before do - group_member_invite.update(invite_email: "#{SecureRandom.hex}@example.com", invite_token: nil, user_id: nil) + group_member_invite.update_columns(invite_email: "#{SecureRandom.hex}@example.com", invite_token: nil, user_id: nil) end it 'logs an exception and shows orphaned status' do diff --git a/spec/helpers/notes_helper_spec.rb b/spec/helpers/notes_helper_spec.rb index 19dcd698c0b2a..506bdb65db4a4 100644 --- a/spec/helpers/notes_helper_spec.rb +++ b/spec/helpers/notes_helper_spec.rb @@ -77,9 +77,9 @@ context 'for a merge request discusion' do let(:merge_request) { create(:merge_request, source_project: project, target_project: project, importing: true) } - let!(:merge_request_diff1) { merge_request.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } - let!(:merge_request_diff2) { merge_request.merge_request_diffs.create(head_commit_sha: nil) } - let!(:merge_request_diff3) { merge_request.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') } + let!(:merge_request_diff1) { merge_request.merge_request_diffs.create!(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } + let!(:merge_request_diff2) { merge_request.merge_request_diffs.create!(head_commit_sha: nil) } + let!(:merge_request_diff3) { merge_request.merge_request_diffs.create!(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') } context 'for a diff discussion' do context 'when the discussion is active' do diff --git a/spec/helpers/profiles_helper_spec.rb b/spec/helpers/profiles_helper_spec.rb index 4a8ba2b711314..61b7ff94edb4f 100644 --- a/spec/helpers/profiles_helper_spec.rb +++ b/spec/helpers/profiles_helper_spec.rb @@ -31,7 +31,7 @@ end it 'returns DB stored commit_email' do - user.update(commit_email: Gitlab::PrivateCommitEmail::TOKEN) + user.update!(commit_email: Gitlab::PrivateCommitEmail::TOKEN) expect(helper.selected_commit_email(user)).to eq(Gitlab::PrivateCommitEmail::TOKEN) end diff --git a/spec/helpers/projects/alert_management_helper_spec.rb b/spec/helpers/projects/alert_management_helper_spec.rb index 859c08b194a12..20464d7f64d2f 100644 --- a/spec/helpers/projects/alert_management_helper_spec.rb +++ b/spec/helpers/projects/alert_management_helper_spec.rb @@ -49,7 +49,7 @@ context 'when alerts service is inactive' do it 'disables alert management' do - alerts_service.update(active: false) + alerts_service.update!(active: false) expect(data).to include( 'alert-management-enabled' => 'false' diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index a3d0673f1b3c1..2b345ff3ae614 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -126,7 +126,7 @@ it "returns false if there are permissions and origin project is PRIVATE" do allow(helper).to receive(:can?) { true } - project.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) expect(helper.can_change_visibility_level?(forked_project, user)).to be_falsey end @@ -134,7 +134,7 @@ it "returns true if there are permissions and origin project is INTERNAL" do allow(helper).to receive(:can?) { true } - project.update(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) expect(helper.can_change_visibility_level?(forked_project, user)).to be_truthy end @@ -647,7 +647,7 @@ context 'user has a configured commit email' do before do confirmed_email = create(:email, :confirmed, user: user) - user.update(commit_email: confirmed_email) + user.update!(commit_email: confirmed_email) end it 'returns the commit email' do @@ -866,7 +866,7 @@ def grant_user_access(project, user, access) when :developer, :maintainer project.add_user(user, access) when :owner - project.namespace.update(owner: user) + project.namespace.update!(owner: user) end end diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb index 9cbace3cfd097..7ef911131bad7 100644 --- a/spec/helpers/visibility_level_helper_spec.rb +++ b/spec/helpers/visibility_level_helper_spec.rb @@ -171,13 +171,14 @@ with_them do it "provides correct visibility level for forked project" do - project.update(visibility_level: max_allowed) + project.update!(visibility_level: max_allowed) expect(selected_visibility_level(forked_project, requested_level)).to eq(expected) end - it "provides correct visibiility level for project in group" do - project.group.update(visibility_level: max_allowed) + it "provides correct visibility level for project in group" do + project.update!(visibility_level: max_allowed) + project.group.update!(visibility_level: max_allowed) expect(selected_visibility_level(project, requested_level)).to eq(expected) end -- GitLab