Merge pull request #11441 from macanudo527/fix_rails_HasManyOrHasOneDependent

Fix Rails/HasManyOrHasOneDependent

.rubocop_todo.yml

@@ -6,12 +6,13 @@
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 1
+# Offense count: 2
 # This cop supports safe autocorrection (--autocorrect).
 # Configuration parameters: Max, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns.
 # URISchemes: http, https
 Layout/LineLength:
   Exclude:
+    - 'app/models/spree/payment.rb'
     - 'spec/system/admin/tag_rules_spec.rb'
 
 # Offense count: 17
@@ -433,30 +434,15 @@ Rails/FindEach:
     - 'spec/system/admin/bulk_order_management_spec.rb'
     - 'spec/system/admin/enterprise_relationships_spec.rb'
 
-# Offense count: 46
+# Offense count: 11
 # Configuration parameters: Include.
 # Include: app/models/**/*.rb
 Rails/HasManyOrHasOneDependent:
   Exclude:
-    - 'app/models/customer.rb'
     - 'app/models/enterprise.rb'
-    - 'app/models/order_cycle.rb'
     - 'app/models/spree/address.rb'
-    - 'app/models/spree/adjustment.rb'
-    - 'app/models/spree/country.rb'
-    - 'app/models/spree/credit_card.rb'
-    - 'app/models/spree/order.rb'
-    - 'app/models/spree/payment.rb'
-    - 'app/models/spree/payment_method.rb'
-    - 'app/models/spree/property.rb'
-    - 'app/models/spree/return_authorization.rb'
-    - 'app/models/spree/shipment.rb'
-    - 'app/models/spree/shipping_category.rb'
-    - 'app/models/spree/shipping_method.rb'
     - 'app/models/spree/stock_item.rb'
     - 'app/models/spree/tax_rate.rb'
-    - 'app/models/spree/taxonomy.rb'
-    - 'app/models/spree/user.rb'
     - 'app/models/spree/variant.rb'
 
 # Offense count: 25
@@ -819,11 +805,6 @@ Style/ClassAndModuleChildren:
     - 'spec/controllers/spree/admin/base_controller_spec.rb'
     - 'spec/models/spree/payment_method_spec.rb'
 
-# Offense count: 1
-Style/ClassVars:
-  Exclude:
-    - 'lib/spree/core/delegate_belongs_to.rb'
-
 # Offense count: 2
 # This cop supports safe autocorrection (--autocorrect).
 # Configuration parameters: MaxUnannotatedPlaceholdersAllowed, AllowedMethods, AllowedPatterns.
@@ -872,13 +853,14 @@ Style/HashLikeCase:
   Exclude:
     - 'app/models/enterprise.rb'
 
-# Offense count: 356
+# Offense count: 184
 # This cop supports safe autocorrection (--autocorrect).
 # Configuration parameters: EnforcedStyle, EnforcedShorthandSyntax, UseHashRocketsWithSymbolValues, PreferHashRocketsForNonAlnumEndingSymbols.
 # SupportedStyles: ruby19, hash_rockets, no_mixed_keys, ruby19_no_mixed_keys
 # SupportedShorthandSyntax: always, never, either, consistent
 Style/HashSyntax:
   Exclude:
+    - 'spec/services/voucher_adjustments_service_spec.rb'
     - 'spec/system/admin/reports_spec.rb'
     - 'spec/system/admin/tag_rules_spec.rb'
     - 'spec/system/admin/variant_overrides_spec.rb'
@@ -958,7 +940,7 @@ Style/OpenStructUse:
     - 'spec/lib/reports/users_and_enterprises_report_spec.rb'
     - 'spec/serializers/api/enterprise_serializer_spec.rb'
 
-# Offense count: 16
+# Offense count: 15
 # Configuration parameters: AllowedMethods.
 # AllowedMethods: respond_to_missing?
 Style/OptionalBooleanParameter:
@@ -972,7 +954,6 @@ Style/OptionalBooleanParameter:
     - 'app/models/spree/shipment.rb'
     - 'engines/order_management/app/services/order_management/stock/estimator.rb'
     - 'lib/spree/core/controller_helpers/order.rb'
-    - 'lib/spree/core/delegate_belongs_to.rb'
     - 'spec/support/request/web_helper.rb'
 
 # Offense count: 1

app/models/customer.rb

@@ -16,7 +16,7 @@ class Customer < ApplicationRecord
 
   belongs_to :enterprise
   belongs_to :user, class_name: "Spree::User", optional: true
-  has_many :orders, class_name: "Spree::Order"
+  has_many :orders, class_name: "Spree::Order", dependent: :nullify
   before_validation :downcase_email
   before_validation :empty_code
   before_create :associate_user
@@ -70,6 +70,5 @@ class Customer < ApplicationRecord
       throw :abort
     end
     Subscription.where(customer_id: id).destroy_all
-    orders.update_all(customer_id: nil)
   end
 end

app/models/enterprise.rb

@@ -37,7 +37,7 @@ class Enterprise < ApplicationRecord
                                     dependent: :destroy
   has_and_belongs_to_many :groups, join_table: 'enterprise_groups_enterprises',
                                    class_name: 'EnterpriseGroup'
-  has_many :producer_properties, foreign_key: 'producer_id'
+  has_many :producer_properties, foreign_key: 'producer_id', dependent: :destroy
   has_many :properties, through: :producer_properties
   has_many :supplied_products, class_name: 'Spree::Product',
                                foreign_key: 'supplier_id',
@@ -57,9 +57,9 @@ class Enterprise < ApplicationRecord
            inverse_of: :distributor, foreign_key: :distributor_id
   has_many :payment_methods, through: :distributor_payment_methods
   has_many :shipping_methods, through: :distributor_shipping_methods
-  has_many :customers
-  has_many :inventory_items
-  has_many :tag_rules
+  has_many :customers, dependent: :destroy
+  has_many :inventory_items, dependent: :destroy
+  has_many :tag_rules, dependent: :destroy
   has_one :stripe_account, dependent: :destroy
   has_many :vouchers
   has_one :custom_tab, dependent: :destroy

app/models/order_cycle.rb

@@ -11,7 +11,7 @@ class OrderCycle < ApplicationRecord
 
   belongs_to :coordinator, class_name: 'Enterprise'
 
-  has_many :coordinator_fee_refs, class_name: 'CoordinatorFee'
+  has_many :coordinator_fee_refs, class_name: 'CoordinatorFee', dependent: :destroy
   has_many :coordinator_fees, through: :coordinator_fee_refs, source: :enterprise_fee,
                               dependent: :destroy
 
@@ -19,12 +19,16 @@ class OrderCycle < ApplicationRecord
 
   # These scope names are prepended with "cached_" because there are existing accessor methods
   # :incoming_exchanges and :outgoing_exchanges.
-  has_many :cached_incoming_exchanges, -> { where incoming: true }, class_name: "Exchange"
-  has_many :cached_outgoing_exchanges, -> { where incoming: false }, class_name: "Exchange"
+  has_many :cached_incoming_exchanges, -> {
+                                         where incoming: true
+                                       }, class_name: "Exchange", dependent: :destroy
+  has_many :cached_outgoing_exchanges, -> {
+                                         where incoming: false
+                                       }, class_name: "Exchange", dependent: :destroy
 
   has_many :suppliers, -> { distinct }, source: :sender, through: :cached_incoming_exchanges
   has_many :distributors, -> { distinct }, source: :receiver, through: :cached_outgoing_exchanges
-  has_many :order_cycle_schedules
+  has_many :order_cycle_schedules, dependent: :destroy
   has_many :schedules, through: :order_cycle_schedules
   has_and_belongs_to_many :selected_distributor_payment_methods,
                           class_name: 'DistributorPaymentMethod',

app/models/spree/adjustment.rb

@@ -36,7 +36,7 @@ module Spree
     # Deletion of metadata is handled in the database.
     # So we don't need the option `dependent: :destroy` as long as
     # AdjustmentMetadata has no destroy logic itself.
-    has_one :metadata, class_name: 'AdjustmentMetadata'
+    has_one :metadata, class_name: 'AdjustmentMetadata', dependent: nil
     has_many :adjustments, as: :adjustable, dependent: :destroy
 
     belongs_to :adjustable, polymorphic: true

app/models/spree/country.rb

@@ -2,7 +2,7 @@
 
 module Spree
   class Country < ApplicationRecord
-    has_many :states, -> { order('name ASC') }
+    has_many :states, -> { order('name ASC') }, dependent: :destroy
 
     validates :name, :iso_name, presence: true
 

app/models/spree/credit_card.rb

@@ -7,7 +7,7 @@ module Spree
     belongs_to :payment_method
     belongs_to :user
 
-    has_many :payments, as: :source
+    has_many :payments, as: :source, dependent: :nullify
 
     before_save :set_last_digits
 

app/models/spree/order.rb

@@ -45,7 +45,7 @@ module Spree
     belongs_to :ship_address, class_name: 'Spree::Address'
     alias_attribute :shipping_address, :ship_address
 
-    has_many :state_changes, as: :stateful
+    has_many :state_changes, as: :stateful, dependent: :destroy
     has_many :line_items, -> {
                             order('created_at ASC')
                           }, class_name: "Spree::LineItem", dependent: :destroy
@@ -71,12 +71,12 @@ module Spree
              },
              class_name: 'Spree::Adjustment',
              dependent: :destroy
-    has_many :invoices
+    has_many :invoices, dependent: :restrict_with_exception
 
     belongs_to :order_cycle
     belongs_to :distributor, class_name: 'Enterprise'
     belongs_to :customer
-    has_one :proxy_order
+    has_one :proxy_order, dependent: :destroy
     has_one :subscription, through: :proxy_order
 
     accepts_nested_attributes_for :line_items

app/models/spree/payment.rb

@@ -21,7 +21,7 @@ module Spree
     belongs_to :payment_method, class_name: 'Spree::PaymentMethod'
 
     has_many :offsets, -> { where("source_type = 'Spree::Payment' AND amount < 0").completed },
-             class_name: "Spree::Payment", foreign_key: :source_id
+             class_name: "Spree::Payment", foreign_key: :source_id, dependent: :restrict_with_exception
     has_many :log_entries, as: :source, dependent: :destroy
 
     has_one :adjustment, as: :adjustable, dependent: :destroy

app/models/spree/payment_method.rb

@@ -15,7 +15,7 @@ module Spree
     DISPLAY = [:both, :back_end].freeze
     default_scope -> { where(deleted_at: nil) }
 
-    has_many :credit_cards, class_name: "Spree::CreditCard"
+    has_many :credit_cards, class_name: "Spree::CreditCard", dependent: :destroy
 
     validates :name, presence: true
     validate :distributor_validation

app/models/spree/property.rb

@@ -4,7 +4,7 @@ module Spree
   class Property < ApplicationRecord
     has_many :product_properties, dependent: :destroy
     has_many :products, through: :product_properties
-    has_many :producer_properties
+    has_many :producer_properties, dependent: :destroy
 
     validates :name, :presentation, presence: true
 

app/models/spree/return_authorization.rb

@@ -8,8 +8,8 @@ module Spree
 
     belongs_to :order, class_name: 'Spree::Order', inverse_of: :return_authorizations
 
-    has_many :inventory_units, inverse_of: :return_authorization
-    has_one :stock_location
+    has_many :inventory_units, inverse_of: :return_authorization, dependent: :nullify
+    has_one :stock_location, dependent: nil
     before_save :force_positive_amount
     before_create :generate_number
 

app/models/spree/shipment.rb

@@ -12,7 +12,7 @@ module Spree
 
     has_many :shipping_rates, dependent: :delete_all
     has_many :shipping_methods, through: :shipping_rates
-    has_many :state_changes, as: :stateful
+    has_many :state_changes, as: :stateful, dependent: :destroy
     has_many :inventory_units, dependent: :delete_all
     has_many :adjustments, as: :adjustable, dependent: :destroy
 

app/models/spree/shipping_category.rb

@@ -3,8 +3,8 @@
 module Spree
   class ShippingCategory < ApplicationRecord
     validates :name, presence: true
-    has_many :products
-    has_many :shipping_method_categories, inverse_of: :shipping_method
+    has_many :products, dependent: :nullify
+    has_many :shipping_method_categories, inverse_of: :shipping_method, dependent: :destroy
     has_many :shipping_methods, through: :shipping_method_categories
   end
 end

app/models/spree/shipping_method.rb

@@ -13,11 +13,11 @@ module Spree
 
     default_scope -> { where(deleted_at: nil) }
 
-    has_many :shipping_rates, inverse_of: :shipping_method
+    has_many :shipping_rates, inverse_of: :shipping_method, dependent: :destroy
     has_many :shipments, through: :shipping_rates
-    has_many :shipping_method_categories
+    has_many :shipping_method_categories, dependent: :destroy
     has_many :shipping_categories, through: :shipping_method_categories
-    has_many :distributor_shipping_methods
+    has_many :distributor_shipping_methods, dependent: :destroy
     has_many :distributors, through: :distributor_shipping_methods,
                             class_name: 'Enterprise',
                             foreign_key: 'distributor_id'

app/models/spree/taxonomy.rb

@@ -4,7 +4,7 @@ module Spree
   class Taxonomy < ApplicationRecord
     validates :name, presence: true
 
-    has_many :taxons
+    has_many :taxons, dependent: :nullify
     has_one :root, -> { where parent_id: nil }, class_name: "Spree::Taxon", dependent: :destroy
 
     after_save :set_name

app/models/spree/user.rb

@@ -14,7 +14,7 @@ module Spree
            encryptor: 'authlogic_sha512', reconfirmable: true,
            omniauth_providers: [:openid_connect]
 
-    has_many :orders
+    has_many :orders, dependent: nil
     belongs_to :ship_address, class_name: 'Spree::Address'
     belongs_to :bill_address, class_name: 'Spree::Address'
 
@@ -33,11 +33,13 @@ module Spree
     has_many :enterprise_roles, dependent: :destroy
     has_many :enterprises, through: :enterprise_roles
     has_many :owned_enterprises, class_name: 'Enterprise',
-                                 foreign_key: :owner_id, inverse_of: :owner
+                                 foreign_key: :owner_id, inverse_of: :owner,
+                                 dependent: :restrict_with_exception
     has_many :owned_groups, class_name: 'EnterpriseGroup',
-                            foreign_key: :owner_id, inverse_of: :owner
-    has_many :customers
-    has_many :credit_cards
+                            foreign_key: :owner_id, inverse_of: :owner,
+                            dependent: :restrict_with_exception
+    has_many :customers, dependent: :destroy
+    has_many :credit_cards, dependent: :destroy
     has_many :report_rendering_options, class_name: "::ReportRenderingOptions", dependent: :destroy
     has_many :webhook_endpoints, dependent: :destroy
 

app/models/spree/variant.rb

@@ -38,7 +38,6 @@ module Spree
 
     has_many :stock_items, dependent: :destroy, inverse_of: :variant
     has_many :stock_locations, through: :stock_items
-    has_many :stock_movements
     has_many :images, -> { order(:position) }, as: :viewable,
                                                dependent: :destroy,
                                                class_name: "Spree::Image"
@@ -56,8 +55,8 @@ module Spree
 
     has_many :exchange_variants
     has_many :exchanges, through: :exchange_variants
-    has_many :variant_overrides
-    has_many :inventory_items
+    has_many :variant_overrides, dependent: :destroy
+    has_many :inventory_items, dependent: :destroy
 
     localize_number :price, :weight
 

spec/controllers/spree/admin/return_authorizations_controller_spec.rb

@@ -33,6 +33,18 @@ module Spree
         expect(return_authorization.amount.to_s).to eq "10.2"
         expect(return_authorization.reason.to_s).to eq "half broken"
       end
+
+      context "with a return authorization" do
+        let!(:return_authorization) { create(:return_authorization, order:) }
+
+        it "deletes a return authorization" do
+          expect{
+            spree_delete :destroy, id: return_authorization.id, order_id: order.number
+          }.to change { order.return_authorizations.without_deleted.count }.by(-1)
+
+          expect(response).to redirect_to spree.admin_order_return_authorizations_url(order.number)
+        end
+      end
     end
   end
 end