diff --git a/core/app/models/spree/credit_card.rb b/core/app/models/spree/credit_card.rb index b412abdb631..7aad9b70ef0 100644 --- a/core/app/models/spree/credit_card.rb +++ b/core/app/models/spree/credit_card.rb @@ -52,8 +52,8 @@ def default def default=(set_as_default) Spree::Deprecation.warn("CreditCard.default= is deprecated. Please use user.wallet.default_wallet_payment_source= instead.", caller) if set_as_default # setting this card as default - user.wallet.add(self) - user.wallet.default_wallet_payment_source = self + wallet_payment_source = user.wallet.add(self) + user.wallet.default_wallet_payment_source = wallet_payment_source true else # removing this card as default if user.wallet.default_wallet_payment_source.try!(:payment_source) == self diff --git a/core/app/models/spree/wallet.rb b/core/app/models/spree/wallet.rb index f07c3f6dfc6..078c4c00174 100644 --- a/core/app/models/spree/wallet.rb +++ b/core/app/models/spree/wallet.rb @@ -6,6 +6,8 @@ # links a PaymentSource (e.g. a CreditCard) to a User. One of a user's # WalletPaymentSources may be the 'default' WalletPaymentSource. class Spree::Wallet + class Unauthorized < StandardError; end + attr_reader :user def initialize(user) @@ -51,18 +53,19 @@ def default_wallet_payment_source end # Change the default WalletPaymentSource for this wallet. - # @param source [PaymentSource] The payment source to set as the default. + # @param source [WalletPaymentSource] The payment source to set as the default. # It must be in the wallet already. Pass nil to clear the default. - # @return [WalletPaymentSource] the associated WalletPaymentSource, or nil if clearing - # the default. - def default_wallet_payment_source=(payment_source) - wallet_payment_source = payment_source && user.wallet_payment_sources.find_by!(payment_source: payment_source) + # @return [void] + def default_wallet_payment_source=(wallet_payment_source) + if wallet_payment_source && !find(wallet_payment_source.id) + raise Unauthorized, "wallet_payment_source #{wallet_payment_source.id} does not belong to wallet of user #{user.id}" + end + wallet_payment_source.transaction do # Unset old default default_wallet_payment_source.try!(:update!, default: false) # Set new default wallet_payment_source.try!(:update!, default: true) end - wallet_payment_source end end diff --git a/core/app/models/spree/wallet/add_payment_sources_to_wallet.rb b/core/app/models/spree/wallet/add_payment_sources_to_wallet.rb index 9df370fbf98..af6bece7b66 100644 --- a/core/app/models/spree/wallet/add_payment_sources_to_wallet.rb +++ b/core/app/models/spree/wallet/add_payment_sources_to_wallet.rb @@ -21,13 +21,13 @@ def add_to_wallet # add valid sources to wallet and optionally set a default if sources.any? - sources.each do |source| + # arbitrarily sort by id for picking a default + wallet_payment_sources = sources.sort_by(&:id).map do |source| order.user.wallet.add(source) end - # arbitrarily pick the last one for the default - default_source = sources.sort_by(&:id).last - order.user.wallet.default_wallet_payment_source = default_source + order.user.wallet.default_wallet_payment_source = + wallet_payment_sources.last end end end diff --git a/core/spec/models/spree/order/checkout_spec.rb b/core/spec/models/spree/order/checkout_spec.rb index 01ca20717a2..1474aa31ea5 100644 --- a/core/spec/models/spree/order/checkout_spec.rb +++ b/core/spec/models/spree/order/checkout_spec.rb @@ -363,8 +363,8 @@ def assert_state_changed(order, from, to) before do user = create(:user, email: 'spree@example.org', bill_address: user_bill_address) - user.wallet.add(default_credit_card) - user.wallet.default_wallet_payment_source = default_credit_card + wallet_payment_source = user.wallet.add(default_credit_card) + user.wallet.default_wallet_payment_source = wallet_payment_source order.user = user allow(order).to receive_messages(payment_required?: true) diff --git a/core/spec/models/spree/wallet_spec.rb b/core/spec/models/spree/wallet_spec.rb index fd26eb069f8..293253fe63a 100644 --- a/core/spec/models/spree/wallet_spec.rb +++ b/core/spec/models/spree/wallet_spec.rb @@ -4,7 +4,7 @@ let(:user) { create(:user) } let(:credit_card) { create(:credit_card, user_id: user.id) } let(:store_credit) { create(:store_credit, user_id: user.id) } - subject { Spree::Wallet.new(user) } + subject(:wallet) { Spree::Wallet.new(user) } describe "#add" do context "with valid payment source" do @@ -72,7 +72,7 @@ let!(:wallet_credit_card) { subject.add(credit_card) } it "marks the passed in payment source as default" do - expect { subject.default_wallet_payment_source = credit_card }.to( + expect { subject.default_wallet_payment_source = wallet_credit_card }.to( change(subject, :default_wallet_payment_source). from(nil). to(wallet_credit_card) @@ -84,16 +84,29 @@ let!(:wallet_credit_card) { subject.add(credit_card) } let!(:wallet_store_credit){ subject.add(store_credit) } - before { subject.default_wallet_payment_source = credit_card } + before { subject.default_wallet_payment_source = wallet_credit_card } it "sets the new payment source as the default" do - expect { subject.default_wallet_payment_source = store_credit }.to( + expect { subject.default_wallet_payment_source = wallet_store_credit }.to( change(subject, :default_wallet_payment_source). from(wallet_credit_card). to(wallet_store_credit) ) end 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) { Spree::Wallet.new(other_user) } + let(:other_credit_card) { create(:credit_card, user_id: other_user.id) } + let(:other_user) { create(:user) } + + it 'raises an error' do + expect { + wallet.default_wallet_payment_source = other_wallet_credit_card + }.to raise_error(Spree::Wallet::Unauthorized) + end + end end describe "#default_wallet_payment_source" do @@ -105,7 +118,7 @@ context "with a default payment source" do let!(:wallet_credit_card) { subject.add(credit_card) } - before { subject.default_wallet_payment_source = credit_card } + before { subject.default_wallet_payment_source = wallet_credit_card } it "will return the wallet payment source" do expect(subject.default_wallet_payment_source).to eql wallet_credit_card