diff --git a/backend/spec/features/admin/orders/payments_spec.rb b/backend/spec/features/admin/orders/payments_spec.rb index dc3813fed3d..5f897917f42 100644 --- a/backend/spec/features/admin/orders/payments_spec.rb +++ b/backend/spec/features/admin/orders/payments_spec.rb @@ -228,7 +228,7 @@ def create_payment(opts = {}) context "user existing card" do let!(:cc) do - create(:credit_card, payment_method: payment_method, gateway_customer_profile_id: "BGS-RFRE") + create(:credit_card, payment_method: payment_method, gateway_customer_profile_id: "BGS-RFRE", user: order.user) end before do diff --git a/core/app/models/spree/wallet_payment_source.rb b/core/app/models/spree/wallet_payment_source.rb index 32c834f5568..d79e0e2d907 100644 --- a/core/app/models/spree/wallet_payment_source.rb +++ b/core/app/models/spree/wallet_payment_source.rb @@ -1,19 +1,35 @@ # frozen_string_literal: true -class Spree::WalletPaymentSource < ActiveRecord::Base - belongs_to :user, class_name: Spree::UserClassHandle.new, foreign_key: 'user_id', inverse_of: :wallet_payment_sources - belongs_to :payment_source, polymorphic: true, inverse_of: :wallet_payment_sources +module Spree + class WalletPaymentSource < Spree::Base + belongs_to :user, class_name: Spree::UserClassHandle.new, foreign_key: 'user_id', inverse_of: :wallet_payment_sources + belongs_to :payment_source, polymorphic: true, inverse_of: :wallet_payment_sources - validates_presence_of :user - validates_presence_of :payment_source + validates_presence_of :user + validates_presence_of :payment_source + validates :user_id, uniqueness: { + scope: [:payment_source_type, :payment_source_id], + message: :payment_source_already_exists + } - validate :check_for_payment_source_class + validate :check_for_payment_source_class + validate :validate_payment_source_ownership - private + private - def check_for_payment_source_class - if !payment_source.is_a?(Spree::PaymentSource) - errors.add(:payment_source, :has_to_be_payment_source_class) + def check_for_payment_source_class + if !payment_source.is_a?(Spree::PaymentSource) + errors.add(:payment_source, :has_to_be_payment_source_class) + end + end + + def validate_payment_source_ownership + return unless payment_source.present? + + if payment_source.respond_to?(:user_id) && + payment_source.user_id != user_id + errors.add(:payment_source, :not_owned_by_user) + end end end end diff --git a/core/config/locales/en.yml b/core/config/locales/en.yml index e2967e25a8b..a81e18ed118 100644 --- a/core/config/locales/en.yml +++ b/core/config/locales/en.yml @@ -492,6 +492,9 @@ en: attributes: payment_source: has_to_be_payment_source_class: has to be a Spree::PaymentSource + not_owned_by_user: does not belong to user + user_id: + payment_source_already_exists: already has the Spree::PaymentSource in their wallet models: spree/address: one: Address diff --git a/core/spec/models/spree/order/checkout_spec.rb b/core/spec/models/spree/order/checkout_spec.rb index 8adbc03b618..6c5d2457e90 100644 --- a/core/spec/models/spree/order/checkout_spec.rb +++ b/core/spec/models/spree/order/checkout_spec.rb @@ -359,6 +359,7 @@ def assert_state_changed(order, from, to) before do user = create(:user, email: 'spree@example.org', bill_address: user_bill_address) + default_credit_card.update(user: user) wallet_payment_source = user.wallet.add(default_credit_card) user.wallet.default_wallet_payment_source = wallet_payment_source order.user = user @@ -497,12 +498,7 @@ def assert_state_changed(order, from, to) context "with a payment in the pending state" do let(:order) { create :order_ready_to_complete } - let(:payment) { create :payment, state: "pending", amount: order.total } - - before do - order.payments = [payment] - order.save! - end + let(:payment) { create :payment, order: order, state: "pending", amount: order.total } it "allows the order to complete" do expect { order.complete! }. @@ -542,7 +538,7 @@ def assert_state_changed(order, from, to) order.user = FactoryBot.create(:user) order.store = FactoryBot.create(:store) order.email = 'spree@example.org' - order.payments << FactoryBot.create(:payment) + order.payments << FactoryBot.create(:payment, order: order) # make sure we will actually capture a payment allow(order).to receive_messages(payment_required?: true) diff --git a/core/spec/models/spree/wallet_payment_source_spec.rb b/core/spec/models/spree/wallet_payment_source_spec.rb index 45f02d31b1a..5eb97f56b8e 100644 --- a/core/spec/models/spree/wallet_payment_source_spec.rb +++ b/core/spec/models/spree/wallet_payment_source_spec.rb @@ -6,6 +6,8 @@ subject { Spree::WalletPaymentSource } describe "validation" do + let(:user) { create(:user) } + context 'with a non-PaymentSource model' do with_model 'NonPaymentSource', scope: :all do model do @@ -19,7 +21,7 @@ it "errors when `payment_source` is not a `Spree::PaymentSource`" do wallet_payment_source = Spree::WalletPaymentSource.new( payment_source: payment_source, - user: create(:user) + user: user ) expect(wallet_payment_source).not_to be_valid @@ -29,18 +31,45 @@ end end + it "is invalid if `payment_source` is already in the user's wallet" do + credit_card = create(:credit_card, user: user) + Spree::WalletPaymentSource.create( + payment_source: credit_card, + user: user + ) + wallet_payment_source = subject.new( + payment_source: credit_card, + user: user + ) + expect(wallet_payment_source).not_to be_valid + expect(wallet_payment_source.errors.messages).to eq( + { user_id: ["already has the Spree::PaymentSource in their wallet"] } + ) + end + + it "is invalid when `payment_source` is not owned by the user" do + wallet_payment_source = subject.new( + payment_source: create(:credit_card), + user: user + ) + expect(wallet_payment_source).not_to be_valid + expect(wallet_payment_source.errors.messages).to eq( + { payment_source: ["does not belong to user"] } + ) + end + it "is valid with a `credit_card` as `payment_source`" do valid_attrs = { - payment_source: create(:credit_card), - user: create(:user) + payment_source: create(:credit_card, user: user), + user: user } expect(subject.new(valid_attrs)).to be_valid end it "is valid with `store_credit` as `payment_source`" do valid_attrs = { - payment_source: create(:store_credit), - user: create(:user) + payment_source: create(:store_credit, user: user), + user: user } expect(subject.new(valid_attrs)).to be_valid end diff --git a/core/spec/models/spree/wallet_spec.rb b/core/spec/models/spree/wallet_spec.rb index 4235f65cf1c..f198a3e76fb 100644 --- a/core/spec/models/spree/wallet_spec.rb +++ b/core/spec/models/spree/wallet_spec.rb @@ -123,7 +123,7 @@ end context 'with a wallet payment source that does not belong to the wallet' do - let(:other_wallet_credit_card) { other_wallet.add(credit_card) } + let(:other_wallet_credit_card) { other_wallet.add(other_credit_card) } let(:other_wallet) { Spree::Wallet.new(other_user) } let(:other_credit_card) { create(:credit_card, user_id: other_user.id) } let(:other_user) { create(:user) }