Merge pull request #6271 from mamhoff/foreign-key-inverse-of-product-taxons

Add dependent/inverse_of options to product-related models

Author: tvdeyen

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2025-06-18 13:16:56 UTC using RuboCop version 1.76.0.
+# on 2025-06-18 14:45:47 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: 57
+# Offense count: 53
 # Configuration parameters: Include.
 # Include: **/app/models/**/*.rb
 Rails/HasManyOrHasOneDependent:
@@ -374,7 +374,7 @@ Rails/IndexWith:
     - 'core/lib/spree/core/search/variant.rb'
     - 'core/lib/spree/preferences/preferable.rb'
 
-# Offense count: 27
+# Offense count: 26
 # Configuration parameters: IgnoreScopes, Include.
 # Include: **/app/models/**/*.rb
 Rails/InverseOf:

core/app/models/spree/classification.rb

@@ -4,8 +4,8 @@ module Spree
   class Classification < Spree::Base
     self.table_name = 'spree_products_taxons'
     acts_as_list scope: :taxon
-    belongs_to :product, class_name: "Spree::Product", inverse_of: :classifications, touch: true, optional: true
-    belongs_to :taxon, class_name: "Spree::Taxon", inverse_of: :classifications, touch: true, optional: true
+    belongs_to :product, class_name: "Spree::Product", inverse_of: :classifications, touch: true
+    belongs_to :taxon, class_name: "Spree::Taxon", inverse_of: :classifications, touch: true
 
     # For https://github.com/spree/spree/issues/3494
     validates :taxon_id, uniqueness: { scope: :product_id, message: :already_linked }

core/app/models/spree/product.rb

@@ -23,7 +23,7 @@ class Product < Spree::Base
 
     has_many :product_properties, dependent: :destroy, inverse_of: :product
     has_many :properties, through: :product_properties
-    has_many :variant_property_rules
+    has_many :variant_property_rules, dependent: :destroy
     has_many :variant_property_rule_values, through: :variant_property_rules, source: :values
     has_many :variant_property_rule_conditions, through: :variant_property_rules, source: :conditions
 
@@ -38,12 +38,14 @@ class Product < Spree::Base
       -> { where(is_master: true).with_discarded },
       inverse_of: :product,
       class_name: 'Spree::Variant',
-      autosave: true
+      autosave: true,
+      dependent: false
 
     has_many :variants,
       -> { where(is_master: false).order(:position) },
       inverse_of: :product,
-      class_name: 'Spree::Variant'
+      class_name: 'Spree::Variant',
+      dependent: false
 
     has_many :variants_including_master,
       -> { order(:position) },

core/app/models/spree/product_option_type.rb

@@ -2,8 +2,8 @@
 
 module Spree
   class ProductOptionType < Spree::Base
-    belongs_to :product, class_name: 'Spree::Product', inverse_of: :product_option_types, touch: true, optional: true
-    belongs_to :option_type, class_name: 'Spree::OptionType', inverse_of: :product_option_types, optional: true
+    belongs_to :product, class_name: 'Spree::Product', inverse_of: :product_option_types, touch: true
+    belongs_to :option_type, class_name: 'Spree::OptionType', inverse_of: :product_option_types
     acts_as_list scope: :product
   end
 end

core/app/models/spree/product_property.rb

@@ -6,8 +6,8 @@ class ProductProperty < Spree::Base
 
     acts_as_list scope: :product
 
-    belongs_to :product, touch: true, class_name: 'Spree::Product', inverse_of: :product_properties, optional: true
-    belongs_to :property, class_name: 'Spree::Property', inverse_of: :product_properties, optional: true
+    belongs_to :product, touch: true, class_name: 'Spree::Product', inverse_of: :product_properties
+    belongs_to :property, class_name: 'Spree::Property', inverse_of: :product_properties
 
     self.allowed_ransackable_attributes = ['value']
   end

core/app/models/spree/taxonomy.rb

@@ -7,8 +7,8 @@ class Taxonomy < Spree::Base
     validates :name, presence: true
     validates :name, uniqueness: true
 
-    has_many :taxons, inverse_of: :taxonomy
-    has_one :root, -> { where parent_id: nil }, class_name: "Spree::Taxon", dependent: :destroy
+    has_many :taxons, inverse_of: :taxonomy, dependent: false
+    has_one :root, -> { where parent_id: nil }, class_name: "Spree::Taxon", dependent: :destroy, inverse_of: false
 
     after_save :set_name
 

core/app/models/spree/variant_property_rule.rb

@@ -13,7 +13,7 @@
 # targeted by the rule, the properties will automatically apply to the variant.
 module Spree
   class VariantPropertyRule < Spree::Base
-    belongs_to :product, touch: true, optional: true
+    belongs_to :product, touch: true
 
     has_many :values, class_name: 'Spree::VariantPropertyRuleValue', dependent: :destroy
     has_many :properties, through: :values

core/db/migrate/20250604072105_add_fk_products_variant_property_rules.rb

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+class AddFkProductsVariantPropertyRules < ActiveRecord::Migration[7.0]
+  def up
+    # Uncomment the following code to remove orphaned records if this migration fails
+    #
+    # say_with_time "Removing orphaned variant property rules (no corresponding product)" do
+    #   Spree::VariantPropertyRule.left_joins(:product).where(spree_products: { id: nil }).delete_all
+    # end
+
+    add_foreign_key :spree_variant_property_rules, :spree_products, column: :product_id, null: false
+  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_variant_property_rules => :spree_products.
+        To fix this:
+          1. Uncomment the code that removes orphaned records.
+          2. Rerun the migration.
+        Offending error: #{e.cause.class} - #{e.cause.message}
+      MSG
+    end
+    raise
+  end
+
+  def down
+    remove_foreign_key :spree_variant_property_rules, :spree_products, column: :product_id, null: false
+  end
+end

core/db/migrate/20250604072555_add_fk_to_product_properties.rb

@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+class AddFkToProductProperties < ActiveRecord::Migration[7.0]
+  def up
+    # Uncomment the following code to remove orphaned records if this migration fails
+    #
+    # say_with_time "Removing orphaned product properties (no corresponding product)" do
+    #   Spree::ProductProperty.left_joins(:product).where(spree_products: { id: nil }).delete_all
+    # end
+    begin
+      add_foreign_key :spree_product_properties, :spree_products, column: :product_id, null: false
+    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_product_properties => :spree_products.
+          To fix this:
+            1. Uncomment the code that removes orphaned records.
+            2. Rerun the migration.
+          Offending error: #{e.cause.class} - #{e.cause.message}
+        MSG
+      end
+      raise
+    end
+
+    # Uncomment the following code to remove orphaned records if this migration fails
+    #
+    # say_with_time "Removing orphaned product properties (no corresponding property)" do
+    #   Spree::ProductProperty.left_joins(:property).where(spree_properties: { id: nil }).delete_all
+    # end
+    begin
+      add_foreign_key :spree_product_properties, :spree_properties, column: :property_id, null: false
+    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_product_properties => :spree_properties.
+          To fix this:
+            1. Uncomment the code that removes orphaned records.
+            2. Rerun the migration.
+          Offending error: #{e.cause.class} - #{e.cause.message}
+        MSG
+      end
+      raise
+    end
+  end
+
+  def down
+    remove_foreign_key :spree_product_properties, :spree_products, column: :product_id, null: false
+    remove_foreign_key :spree_product_properties, :spree_properties, column: :property_id, null: false
+  end
+end

core/db/migrate/20250604072948_add_fk_to_product_option_types.rb

@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+class AddFkToProductOptionTypes < ActiveRecord::Migration[7.0]
+  def up
+    # Uncomment the following code to remove orphaned records if this migration fails
+    #
+    # say_with_time "Removing orphaned product option types (no corresponding product)" do
+    #   Spree::ProductOptionType.left_joins(:product).where(spree_products: { id: nil }).delete_all
+    # end
+    begin
+      add_foreign_key :spree_product_option_types, :spree_products, column: :product_id
+    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_product_option_types => :spree_products.
+          To fix this:
+            1. Uncomment the code that removes orphaned records.
+            2. Rerun the migration.
+          Offending error: #{e.cause.class} - #{e.cause.message}
+        MSG
+      end
+      raise
+    end
+
+    # Uncomment the following code to remove orphaned records if this migration fails
+    #
+    # say_with_time "Removing orphaned product option types (no corresponding option type)" do
+    #   Spree::ProductOptionType.left_joins(:option_type).where(spree_option_types: { id: nil }).delete_all
+    # end
+    begin
+      add_foreign_key :spree_product_option_types, :spree_option_types, column: :option_type_id
+    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_product_option_types => :spree_option_types.
+          To fix this:
+            1. Uncomment the code that removes orphaned records.
+            2. Rerun the migration.
+          Offending error: #{e.cause.class} - #{e.cause.message}
+        MSG
+      end
+      raise
+    end
+  end
+
+  def down
+    remove_foreign_key :spree_product_option_types, :spree_products, column: :product_id
+    remove_foreign_key :spree_product_option_types, :spree_option_types, column: :option_type_id
+  end
+end