diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index b38b39daec..31339ee76b 100644 --- a/.rubocop_todo.yml +++ b/.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 diff --git a/app/models/customer.rb b/app/models/customer.rb index 732c15c1ff..a60c0d8013 100644 --- a/app/models/customer.rb +++ b/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 diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index a7c68b0e4b..bd9aedbc93 100644 --- a/app/models/enterprise.rb +++ b/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 diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index d9f8205ef3..b6e7d80913 100644 --- a/app/models/order_cycle.rb +++ b/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', diff --git a/app/models/spree/adjustment.rb b/app/models/spree/adjustment.rb index f07201539c..7625b34962 100644 --- a/app/models/spree/adjustment.rb +++ b/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 diff --git a/app/models/spree/country.rb b/app/models/spree/country.rb index 6647b8d698..cc9e3dbed9 100644 --- a/app/models/spree/country.rb +++ b/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 diff --git a/app/models/spree/credit_card.rb b/app/models/spree/credit_card.rb index 529f657014..c4ec4f0967 100644 --- a/app/models/spree/credit_card.rb +++ b/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 diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index c2ce1404bd..9b7790c3fe 100644 --- a/app/models/spree/order.rb +++ b/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 diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index 51d19d72ad..6f32de6f9b 100644 --- a/app/models/spree/payment.rb +++ b/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 diff --git a/app/models/spree/payment_method.rb b/app/models/spree/payment_method.rb index 891f418eed..f3011b222c 100644 --- a/app/models/spree/payment_method.rb +++ b/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 diff --git a/app/models/spree/property.rb b/app/models/spree/property.rb index 3762c3b393..9f45270a78 100644 --- a/app/models/spree/property.rb +++ b/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 diff --git a/app/models/spree/return_authorization.rb b/app/models/spree/return_authorization.rb index 61a9e219ae..ad291f2bf3 100644 --- a/app/models/spree/return_authorization.rb +++ b/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 diff --git a/app/models/spree/shipment.rb b/app/models/spree/shipment.rb index 1f528bc4a8..476658b13a 100644 --- a/app/models/spree/shipment.rb +++ b/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 diff --git a/app/models/spree/shipping_category.rb b/app/models/spree/shipping_category.rb index 6bbfb7f603..deec192c8f 100644 --- a/app/models/spree/shipping_category.rb +++ b/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 diff --git a/app/models/spree/shipping_method.rb b/app/models/spree/shipping_method.rb index d84cac35a8..42ab253ef7 100644 --- a/app/models/spree/shipping_method.rb +++ b/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' diff --git a/app/models/spree/taxonomy.rb b/app/models/spree/taxonomy.rb index 50778e37cf..a8600caa27 100644 --- a/app/models/spree/taxonomy.rb +++ b/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 diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index 84b2d56919..fce0828b4c 100644 --- a/app/models/spree/user.rb +++ b/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 diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb index 9768f00ced..528790313b 100644 --- a/app/models/spree/variant.rb +++ b/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 diff --git a/spec/controllers/spree/admin/return_authorizations_controller_spec.rb b/spec/controllers/spree/admin/return_authorizations_controller_spec.rb index aaf7bc81d6..d8b0a1ca42 100644 --- a/spec/controllers/spree/admin/return_authorizations_controller_spec.rb +++ b/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