From 4e556348685c6f1c1441d695474cd74b007c38ba Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Wed, 28 May 2025 14:51:30 +0200 Subject: [PATCH 1/4] UserAddress: Add dependent/inverse_of options This is to satisfy Rubocop, but it's also just good behavior. If a user record is destroyed, we should also destroy their address book. For the default addresses, we don't add cascading deleted, because these are also present in the `has_many :user_addresses`. --- .../concerns/spree/user_address_book.rb | 30 +++++++++++++++---- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/core/app/models/concerns/spree/user_address_book.rb b/core/app/models/concerns/spree/user_address_book.rb index 3e8cb60efb5..23728edc12b 100644 --- a/core/app/models/concerns/spree/user_address_book.rb +++ b/core/app/models/concerns/spree/user_address_book.rb @@ -5,7 +5,7 @@ module UserAddressBook extend ActiveSupport::Concern included do - has_many :user_addresses, foreign_key: "user_id", class_name: "Spree::UserAddress" do + has_many :user_addresses, foreign_key: "user_id", class_name: "Spree::UserAddress", inverse_of: :user, dependent: :destroy do def find_first_by_address_values(address_attrs) detect { |ua| ua.address == Spree::Address.new(address_attrs) } end @@ -32,11 +32,29 @@ def mark_default(user_address, address_type: :shipping) has_many :addresses, through: :user_addresses - has_one :default_user_bill_address, ->{ default_billing }, class_name: 'Spree::UserAddress', foreign_key: 'user_id' - has_one :bill_address, through: :default_user_bill_address, source: :address - - has_one :default_user_ship_address, ->{ default_shipping }, class_name: 'Spree::UserAddress', foreign_key: 'user_id' - has_one :ship_address, through: :default_user_ship_address, source: :address + has_one :default_user_bill_address, + ->{ default_billing }, + class_name: 'Spree::UserAddress', + foreign_key: 'user_id', + inverse_of: false, + dependent: false + has_one :bill_address, + through: :default_user_bill_address, + source: :address, + inverse_of: false, + dependent: false + + has_one :default_user_ship_address, + ->{ default_shipping }, + class_name: 'Spree::UserAddress', + foreign_key: 'user_id', + inverse_of: false, + dependent: false + has_one :ship_address, + through: :default_user_ship_address, + source: :address, + inverse_of: false, + dependent: false accepts_nested_attributes_for :ship_address accepts_nested_attributes_for :bill_address From ecd34bc2df86bcce77cf7021d5225fdc6c3d5c43 Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Wed, 28 May 2025 17:13:59 +0200 Subject: [PATCH 2/4] UserAddress: Add :inverse_of option --- core/app/models/spree/user_address.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/app/models/spree/user_address.rb b/core/app/models/spree/user_address.rb index 41ca17ce8b3..e4f34777d85 100644 --- a/core/app/models/spree/user_address.rb +++ b/core/app/models/spree/user_address.rb @@ -2,7 +2,7 @@ module Spree class UserAddress < Spree::Base - belongs_to :user, class_name: UserClassHandle.new, foreign_key: "user_id", optional: true + belongs_to :user, class_name: UserClassHandle.new, foreign_key: "user_id", optional: true, inverse_of: :user_addresses belongs_to :address, class_name: "Spree::Address", optional: true validates_uniqueness_of :address_id, scope: :user_id From 6151985690173d5fe97b10830fe9af6d7a69c834 Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Thu, 29 May 2025 12:07:47 +0200 Subject: [PATCH 3/4] UserAddress: Make belongs_to relations mandatory Join records are absolutely useless if either of their "arms" are missing. Make them mandatory. --- core/app/models/spree/user_address.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/app/models/spree/user_address.rb b/core/app/models/spree/user_address.rb index e4f34777d85..70432f472ee 100644 --- a/core/app/models/spree/user_address.rb +++ b/core/app/models/spree/user_address.rb @@ -2,8 +2,8 @@ module Spree class UserAddress < Spree::Base - belongs_to :user, class_name: UserClassHandle.new, foreign_key: "user_id", optional: true, inverse_of: :user_addresses - belongs_to :address, class_name: "Spree::Address", optional: true + belongs_to :user, class_name: UserClassHandle.new, foreign_key: "user_id", inverse_of: :user_addresses + belongs_to :address, class_name: "Spree::Address" validates_uniqueness_of :address_id, scope: :user_id validates_uniqueness_of :user_id, conditions: -> { default_shipping }, message: :default_address_exists, if: :default? From 97df766b9f29160e6b84bfbe3c5975df0f57cf2d Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Fri, 30 May 2025 12:29:49 +0200 Subject: [PATCH 4/4] Add foreign key to spree_user_addresses I'm only adding a foreign key constraint between user_addresses and addresses, not for users, because that should be the responsibility of the user class, so we could i.e. do it in solidus_auth_devise. --- .../migrate/20250530102541_add_addressbook_foreign_key.rb | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 core/db/migrate/20250530102541_add_addressbook_foreign_key.rb diff --git a/core/db/migrate/20250530102541_add_addressbook_foreign_key.rb b/core/db/migrate/20250530102541_add_addressbook_foreign_key.rb new file mode 100644 index 00000000000..73d88369d1c --- /dev/null +++ b/core/db/migrate/20250530102541_add_addressbook_foreign_key.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddAddressbookForeignKey < ActiveRecord::Migration[7.0] + def change + add_foreign_key :spree_user_addresses, :spree_addresses, column: :address_id, null: false + end +end