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

Fix Rails/SaveBang RuboCop offenses for all factories

See merge request gitlab-org/gitlab!37268

.rubocop_todo.yml

@@ -854,10 +854,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'
@@ -1099,19 +1095,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'

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

+---
+title: Refactor all factories to fix SaveBang Cop
+merge_request: 37268
+author: Rajendra Kadam
+type: fixed

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 }

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

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

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

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
 

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

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

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
 

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

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

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

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

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

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

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
 

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)

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