From b3b8cb778fffee79c84cf339fc3a4e4ebf51ad45 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 10 May 2019 15:41:24 +1000 Subject: [PATCH] Make only reusable credit cards the default card https://github.com/openfoodfoundation/openfoodnetwork/issues/3727 When a user didn't have a default credit card and then checked out with a credit card it became the default even when the user didn't intend to store it. That lead to subscriptions trying to charge a one-time card which fails. --- app/models/spree/credit_card_decorator.rb | 6 ++- spec/models/spree/credit_card_spec.rb | 48 +++++++++++++++-------- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/app/models/spree/credit_card_decorator.rb b/app/models/spree/credit_card_decorator.rb index e82dea3443..630e0fab4f 100644 --- a/app/models/spree/credit_card_decorator.rb +++ b/app/models/spree/credit_card_decorator.rb @@ -31,13 +31,17 @@ Spree::CreditCard.class_eval do private + def reusable? + gateway_customer_profile_id.present? + end + def default_missing? !user.credit_cards.exists?(is_default: true) end def ensure_single_default_card return unless user - return unless is_default? || default_missing? + return unless is_default? || (reusable? && default_missing?) user.credit_cards.update_all(['is_default=(id=?)', id]) self.is_default = true end diff --git a/spec/models/spree/credit_card_spec.rb b/spec/models/spree/credit_card_spec.rb index 9d246944f6..35633309ac 100644 --- a/spec/models/spree/credit_card_spec.rb +++ b/spec/models/spree/credit_card_spec.rb @@ -4,15 +4,27 @@ module Spree describe CreditCard do describe "setting default credit card for a user" do let(:user) { create(:user) } + let(:onetime_card_attrs) do + {user: user, gateway_payment_profile_id: "tok_1EY..."} + end + let(:stored_card_attrs) do + { + user: user, + gateway_customer_profile_id: "cus_F2T...", + gateway_payment_profile_id: "card_1EY..." + } + end + let(:stored_default_card_attrs) do + stored_card_attrs.merge(is_default: true) + end context "when a card is already set as the default" do - let!(:card1) { create(:credit_card, user: user, is_default: true) } + let!(:card1) { create(:credit_card, stored_default_card_attrs) } context "and I create a new card" do - let(:attrs) { { user: user } } - let!(:card2) { create(:credit_card, attrs) } - context "without specifying it as the default" do + let!(:card2) { create(:credit_card, stored_card_attrs) } + it "keeps the existing default" do expect(card1.reload.is_default).to be true expect(card2.reload.is_default).to be false @@ -20,7 +32,7 @@ module Spree end context "and I specify it as the default" do - let(:attrs) { { user: user, is_default: true } } + let!(:card2) { create(:credit_card, stored_default_card_attrs) } it "switches the default to the new card" do expect(card1.reload.is_default).to be false @@ -30,24 +42,21 @@ module Spree end context "and I update another card" do - let(:attrs) { { user: user } } let!(:card2) { create(:credit_card, user: user) } - before do - card2.update_attributes!(attrs) - end - context "without specifying it as the default" do it "keeps the existing default" do + card2.update_attributes!(stored_card_attrs) + expect(card1.reload.is_default).to be true expect(card2.reload.is_default).to be false end end context "and I specify it as the default" do - let(:attrs) { { user: user, is_default: true } } - it "switches the default to the updated card" do + card2.update_attributes!(stored_default_card_attrs) + expect(card1.reload.is_default).to be false expect(card2.reload.is_default).to be true end @@ -57,23 +66,30 @@ module Spree context "when no card is currently set as the default for a user" do context "and I create a new card" do - let(:attrs) { { user: user } } - let!(:card1) { create(:credit_card, attrs) } - context "without specifying it as the default" do + let!(:card1) { create(:credit_card, stored_card_attrs) } + it "sets it as the default anyway" do expect(card1.reload.is_default).to be true end end context "and I specify it as the default" do - let(:attrs) { { user: user, is_default: true } } + let!(:card1) { create(:credit_card, stored_default_card_attrs) } it "sets it as the default" do expect(card1.reload.is_default).to be true end end end + + context "and the checkout creates a one-time-use card" do + let!(:card1) { create(:credit_card, onetime_card_attrs) } + + it "sets it as the default anyway" do + expect(card1.reload.is_default).to be false + end + end end end end