From e6196717f6bd268e90f4e7d575f75b3d68362ac0 Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Wed, 28 May 2025 15:02:52 +0200 Subject: [PATCH 1/2] AdjustmentReason: Add :dependent option We should not delete adjustment reasons if there are adjustments created with them. --- .rubocop_todo.yml | 4 ++-- core/app/models/spree/adjustment_reason.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index bdc46d9a167..00c8965f1fd 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2025-06-26 11:28:28 UTC using RuboCop version 1.76.0. +# on 2025-07-08 12:12:57 UTC using RuboCop version 1.76.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -332,7 +332,7 @@ Rails/FilePath: - "core/lib/spree/testing_support/dummy_app.rb" - "sample/lib/spree/sample.rb" -# Offense count: 43 +# Offense count: 42 # Configuration parameters: Include. # Include: **/app/models/**/*.rb Rails/HasManyOrHasOneDependent: diff --git a/core/app/models/spree/adjustment_reason.rb b/core/app/models/spree/adjustment_reason.rb index 81d74c1044f..d9f46d9f253 100644 --- a/core/app/models/spree/adjustment_reason.rb +++ b/core/app/models/spree/adjustment_reason.rb @@ -2,7 +2,7 @@ module Spree class AdjustmentReason < Spree::Base - has_many :adjustments, inverse_of: :adjustment_reason + has_many :adjustments, inverse_of: :adjustment_reason, dependent: :restrict_with_error validates :name, presence: true, uniqueness: { case_sensitive: false, allow_blank: true } validates :code, presence: true, uniqueness: { case_sensitive: false, allow_blank: true } From 9e1ca7f195f4c7fe9c0cee0153d420e41a8f40dc Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Tue, 8 Jul 2025 14:10:42 +0200 Subject: [PATCH 2/2] Add FK between Adjustments and Adjustment Reasons This migration will fail if there are any adjustments with invalid adjustment reason IDs. The suggested path of action for store owners is to change those adjustment reason IDs to NULL, because we don't want to accidentally lose adjustments. --- ...0317_add_adjustment_reason_foreign_keys.rb | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 core/db/migrate/20250708120317_add_adjustment_reason_foreign_keys.rb diff --git a/core/db/migrate/20250708120317_add_adjustment_reason_foreign_keys.rb b/core/db/migrate/20250708120317_add_adjustment_reason_foreign_keys.rb new file mode 100644 index 00000000000..e0eac2f8a93 --- /dev/null +++ b/core/db/migrate/20250708120317_add_adjustment_reason_foreign_keys.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +class AddAdjustmentReasonForeignKeys < ActiveRecord::Migration[7.0] + def up + # Uncomment the following code to remove orphaned records if this migration fails + # + # say_with_time "Removing invalid adjustment reason IDs from adjustments table" do + # Spree::Adjustment.where.not(adjustment_reason_id: nil).left_joins(:adjustment_reason).where(spree_adjustment_reasons: { id: nil }).update_all(adjustment_reason_id: nil) + # end + + add_foreign_key :spree_adjustments, :spree_adjustment_reasons, column: :adjustment_reason_id, null: true, on_delete: :restrict + rescue ActiveRecord::StatementInvalid => e + if e.cause.is_a?(PG::ForeignKeyViolation) || e.cause.is_a?(Mysql2::Error) || e.cause.is_a?(SQLite3::ConstraintException) + Rails.logger.warn <<~MSG + ⚠️ Foreign key constraint failed when adding :spree_adjustments => :spree_adjustment_reasons. + To fix this: + 1. Uncomment the code that removes invalid adjustment reason IDs from the spree_adjustments table. + 2. Rerun the migration. + Offending error: #{e.cause.class} - #{e.cause.message} + MSG + end + raise + end + + def down + remove_foreign_key :spree_adjustments, :spree_adjustment_reasons, column: :adjustment_reason_id, null: true, on_delete: :restrict + end +end