diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index e54d970c6710725ca32d8485dea0376f84f59d18..e7fca73bcbae8de2674d1be3a2f44d7576230263 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -855,10 +855,6 @@ Rails/SaveBang: Exclude: - 'ee/spec/controllers/projects/merge_requests_controller_spec.rb' - 'ee/spec/controllers/subscriptions_controller_spec.rb' - - 'ee/spec/factories/ci/job_artifacts.rb' - - 'ee/spec/factories/epics.rb' - - 'ee/spec/factories/licenses.rb' - - 'ee/spec/factories/merge_requests.rb' - 'ee/spec/features/admin/admin_users_spec.rb' - 'ee/spec/features/admin/geo/admin_geo_nodes_spec.rb' - 'ee/spec/features/admin/licenses/admin_views_license_spec.rb' @@ -1100,19 +1096,6 @@ Rails/SaveBang: - 'spec/controllers/sent_notifications_controller_spec.rb' - 'spec/controllers/sessions_controller_spec.rb' - 'spec/controllers/users_controller_spec.rb' - - 'spec/factories/alert_management/alerts.rb' - - 'spec/factories/boards.rb' - - 'spec/factories/ci/pipelines.rb' - - 'spec/factories/design_management/designs.rb' - - 'spec/factories/design_management/versions.rb' - - 'spec/factories/emails.rb' - - 'spec/factories/issues.rb' - - 'spec/factories/labels.rb' - - 'spec/factories/merge_requests.rb' - - 'spec/factories/plans.rb' - - 'spec/factories/projects.rb' - - 'spec/factories/services.rb' - - 'spec/factories/wiki_pages.rb' - 'spec/factories_spec.rb' - 'spec/features/admin/admin_appearance_spec.rb' - 'spec/features/admin/admin_labels_spec.rb' diff --git a/changelogs/unreleased/rails-save-bang-4.yml b/changelogs/unreleased/rails-save-bang-4.yml new file mode 100644 index 0000000000000000000000000000000000000000..51b8b0a754c2e51446d1879a847ff11bd22619ed --- /dev/null +++ b/changelogs/unreleased/rails-save-bang-4.yml @@ -0,0 +1,5 @@ +--- +title: Refactor all factories to fix SaveBang Cop +merge_request: 37268 +author: Rajendra Kadam +type: fixed diff --git a/ee/spec/factories/ci/job_artifacts.rb b/ee/spec/factories/ci/job_artifacts.rb index e55cc357cde9c377c8fd8224ca384c2d84b8b39a..84e8de6901d49603eee251c82eb237d3451278f9 100644 --- a/ee/spec/factories/ci/job_artifacts.rb +++ b/ee/spec/factories/ci/job_artifacts.rb @@ -180,7 +180,7 @@ FactoryBot.define do end trait :license_management do - to_create { |instance| instance.save(validate: false) } + to_create { |instance| instance.save!(validate: false) } file_type { :license_management } file_format { :raw } diff --git a/ee/spec/factories/epics.rb b/ee/spec/factories/epics.rb index 8cc377fe3d35b08f01349de37b4529225249569d..da479dafc1adfcca40aa476c40b494d832b313e2 100644 --- a/ee/spec/factories/epics.rb +++ b/ee/spec/factories/epics.rb @@ -34,7 +34,7 @@ FactoryBot.define do end after(:create) do |epic, evaluator| - epic.update(labels: evaluator.labels) + epic.update!(labels: evaluator.labels) end end end diff --git a/ee/spec/factories/licenses.rb b/ee/spec/factories/licenses.rb index 81a19e53f810e42e3b384412e19fba96f35f3c66..39148c3ad94281f2a988134ac6729c3e6ba8fd85 100644 --- a/ee/spec/factories/licenses.rb +++ b/ee/spec/factories/licenses.rb @@ -57,6 +57,6 @@ FactoryBot.define do end # Disable validations when creating an expired license key - to_create {|instance| instance.save(validate: !expired) } + to_create {|instance| instance.save!(validate: !expired) } end end diff --git a/ee/spec/factories/merge_requests.rb b/ee/spec/factories/merge_requests.rb index 8d7485314716354511733e13a3bf7cefa464d6ab..29d89c962e81a971150571acdaafd65db0af9c71 100644 --- a/ee/spec/factories/merge_requests.rb +++ b/ee/spec/factories/merge_requests.rb @@ -31,7 +31,7 @@ FactoryBot.modify do after(:create) do |merge_request, evaluator| merge_request.pipelines_for_merge_request.last - .update(ref: merge_request.train_ref_path) + .update!(ref: merge_request.train_ref_path) end end @@ -62,7 +62,7 @@ FactoryBot.modify do after :create do |merge_request, evaluator| next if evaluator.approval_users.blank? && evaluator.approval_groups.blank? - rule = merge_request.approval_rules.first_or_create(attributes_for(:approval_merge_request_rule)) + rule = merge_request.approval_rules.first_or_create!(attributes_for(:approval_merge_request_rule)) rule.users = evaluator.approval_users if evaluator.approval_users.present? rule.groups = evaluator.approval_groups if evaluator.approval_groups.present? end diff --git a/spec/factories/alert_management/alerts.rb b/spec/factories/alert_management/alerts.rb index ef511aa54b8cdeb949ac8599ccdc8d3da1c93f98..0f05d62b889750cec5958613aa43bacb9e1a60ad 100644 --- a/spec/factories/alert_management/alerts.rb +++ b/spec/factories/alert_management/alerts.rb @@ -23,7 +23,7 @@ FactoryBot.define do trait :with_assignee do |alert| after(:create) do |alert| - alert.alert_assignees.create(assignee: create(:user)) + alert.alert_assignees.create!(assignee: create(:user)) end end diff --git a/spec/factories/boards.rb b/spec/factories/boards.rb index a201ca94380ce6df78bd987ed1f58405d7a62cae..cef7ec37f07aa630e96fcc4dfbbdb0f9343a78fe 100644 --- a/spec/factories/boards.rb +++ b/spec/factories/boards.rb @@ -28,7 +28,7 @@ FactoryBot.define do end after(:create) do |board| - board.lists.create(list_type: :closed) + board.lists.create!(list_type: :closed) end end end diff --git a/spec/factories/ci/pipelines.rb b/spec/factories/ci/pipelines.rb index 5bd5ab7d67acd0fc87bf7c1d9291924d90c9c1c1..2790be8b70d8518bdc7c330a7af0d36edb982e97 100644 --- a/spec/factories/ci/pipelines.rb +++ b/spec/factories/ci/pipelines.rb @@ -17,7 +17,7 @@ FactoryBot.define do after(:create) do |pipeline, evaluator| merge_request = evaluator.head_pipeline_of - merge_request&.update(head_pipeline: pipeline) + merge_request&.update!(head_pipeline: pipeline) end factory :ci_pipeline do diff --git a/spec/factories/design_management/designs.rb b/spec/factories/design_management/designs.rb index 6d1229063d87720be49332fa3cc2df6ced70bbb6..ee672a083d15e172f5e9425312e397285744acbc 100644 --- a/spec/factories/design_management/designs.rb +++ b/spec/factories/design_management/designs.rb @@ -34,7 +34,7 @@ FactoryBot.define do run_action = ->(action) do sha = commit_version[action] version = DesignManagement::Version.new(sha: sha, issue: issue, author: evaluator.author) - version.save(validate: false) # We need it to have an ID, validate later + version.save!(validate: false) # We need it to have an ID, validate later Gitlab::Database.bulk_insert(dv_table_name, [action.row_attrs(version)]) # rubocop:disable Gitlab/BulkInsert end diff --git a/spec/factories/design_management/versions.rb b/spec/factories/design_management/versions.rb index e6d17ba691cc4b00021e631d8f1807598bb74b9b..6a751ac4eda87d0ca05aa82564feb27dea59efce 100644 --- a/spec/factories/design_management/versions.rb +++ b/spec/factories/design_management/versions.rb @@ -135,7 +135,7 @@ FactoryBot.define do actions: version_actions ) - version.update(sha: sha) + version.update!(sha: sha) end end end diff --git a/spec/factories/emails.rb b/spec/factories/emails.rb index 284ba631c376413479750867b09d8647f1d78391..b30fa8a58969c7b16669899b394af419253f4792 100644 --- a/spec/factories/emails.rb +++ b/spec/factories/emails.rb @@ -6,6 +6,6 @@ FactoryBot.define do email { generate(:email_alias) } trait(:confirmed) { confirmed_at { Time.now } } - trait(:skip_validate) { to_create {|instance| instance.save(validate: false) } } + trait(:skip_validate) { to_create {|instance| instance.save!(validate: false) } } end end diff --git a/spec/factories/issues.rb b/spec/factories/issues.rb index 4d0924a94125e324355860c66e8a5e80289b22c1..f2a18434cbb7760c9c1da4a1c7ecaa4806a68680 100644 --- a/spec/factories/issues.rb +++ b/spec/factories/issues.rb @@ -38,7 +38,7 @@ FactoryBot.define do end after(:create) do |issue, evaluator| - issue.update(labels: evaluator.labels) + issue.update!(labels: evaluator.labels) end end end diff --git a/spec/factories/labels.rb b/spec/factories/labels.rb index 2e783adcc94616c435b2ae59b773215f0503a653..6725b571f199e2ead73994b2aa89a637c98163c6 100644 --- a/spec/factories/labels.rb +++ b/spec/factories/labels.rb @@ -27,7 +27,7 @@ FactoryBot.define do after(:create) do |label, evaluator| if evaluator.priority - label.priorities.create(project: label.project, priority: evaluator.priority) + label.priorities.create!(project: label.project, priority: evaluator.priority) end end end diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index 2a06690f894ce3205a858138db0d84e0d63596cc..6fe5c9e0ff90d396e0f76e030ea98c88db0c4e0c 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -268,7 +268,7 @@ FactoryBot.define do end after(:create) do |merge_request, evaluator| - merge_request.update(labels: evaluator.labels) + merge_request.update!(labels: evaluator.labels) end end end diff --git a/spec/factories/plans.rb b/spec/factories/plans.rb index 81506edcf1610de30b1b51e0b8788718836b8c19..903c176ec2aa79a043957263fd572b6d1d380501 100644 --- a/spec/factories/plans.rb +++ b/spec/factories/plans.rb @@ -6,7 +6,7 @@ FactoryBot.define do factory :"#{plan}_plan" do name { plan } title { name.titleize } - initialize_with { Plan.find_or_create_by(name: plan) } + initialize_with { Plan.find_or_create_by!(name: plan) } end end end diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index e4b53186ea8386422cdba3c6a13de2dc0fda1fca..b06ccb6f65cddf0b23770b232ff6033a46065fa1 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -61,7 +61,7 @@ FactoryBot.define do hash.store("pages_access_level", evaluator.pages_access_level) end - project.project_feature.update(hash) + project.project_feature.update!(hash) # Normally the class Projects::CreateService is used for creating # projects, and this class takes care of making sure the owner and current @@ -82,7 +82,7 @@ FactoryBot.define do import_state.jid = evaluator.import_jid import_state.correlation_id_value = evaluator.import_correlation_id import_state.last_error = evaluator.import_last_error - import_state.save + import_state.save! end end diff --git a/spec/factories/services.rb b/spec/factories/services.rb index 9a521336feee570ddb8051235bedb2b121ab6298..3dafc3404b26726082d642be0bd9715a7b0a99c7 100644 --- a/spec/factories/services.rb +++ b/spec/factories/services.rb @@ -196,7 +196,7 @@ FactoryBot.define do IssueTrackerService.skip_callback(:validation, :before, :handle_properties) end - to_create { |instance| instance.save(validate: false) } + to_create { |instance| instance.save!(validate: false) } after(:create) do IssueTrackerService.set_callback(:validation, :before, :handle_properties) diff --git a/spec/factories/wiki_pages.rb b/spec/factories/wiki_pages.rb index e7fcc19bbfe2d316da26612a8690a4b76864fa64..cc866d336a4134648c89edbad3996f66533b2f9b 100644 --- a/spec/factories/wiki_pages.rb +++ b/spec/factories/wiki_pages.rb @@ -31,7 +31,8 @@ FactoryBot.define do end to_create do |page, evaluator| - page.create(message: evaluator.message) + # WikiPages is ActiveModel which doesn't support `create!`. + page.create(message: evaluator.message) # rubocop:disable Rails/SaveBang end end