diff --git a/.rubocop_styleguide.yml b/.rubocop_styleguide.yml index 13214b0aed..a96c96b306 100644 --- a/.rubocop_styleguide.yml +++ b/.rubocop_styleguide.yml @@ -187,7 +187,7 @@ Metrics/AbcSize: Max: 15 Metrics/BlockLength: - ExcludedMethods: ["collection", "context", "describe", "it", "member", "namespace", "resource", "resources"] + ExcludedMethods: ["class_eval", "collection", "context", "describe", "it", "member", "namespace", "resource", "resources"] Metrics/BlockNesting: Max: 3 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/factories.rb b/spec/factories.rb index 5f2d2c809f..6a8b2ef8b9 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -510,6 +510,12 @@ FactoryBot.define do end end + # A card that has been added to the user's profile and can be re-used. + factory :stored_credit_card, parent: :credit_card do + gateway_customer_profile_id "cus_F2T..." + gateway_payment_profile_id "card_1EY..." + end + factory :stripe_payment_method, :class => Spree::Gateway::StripeConnect do name 'Stripe' environment 'test' diff --git a/spec/features/admin/subscriptions_spec.rb b/spec/features/admin/subscriptions_spec.rb index c8e127a955..42ab7ea2c4 100644 --- a/spec/features/admin/subscriptions_spec.rb +++ b/spec/features/admin/subscriptions_spec.rb @@ -143,7 +143,7 @@ feature 'Subscriptions' do context 'creating a new subscription' do let(:address) { create(:address) } let!(:customer_user) { create(:user) } - let!(:credit_card1) { create(:credit_card, user: customer_user, cc_type: 'visa', last_digits: 1111, month: 10, year: 2030) } + let!(:credit_card1) { create(:stored_credit_card, user: customer_user, cc_type: 'visa', last_digits: 1111, month: 10, year: 2030) } let!(:customer) { create(:customer, enterprise: shop, bill_address: address, user: customer_user, allow_charges: true) } let!(:test_product) { create(:product, supplier: shop) } let!(:test_variant) { create(:variant, product: test_product, unit_value: "100", price: 12.00, option_values: []) } @@ -413,7 +413,7 @@ feature 'Subscriptions' do describe "allowed variants" do let!(:customer) { create(:customer, enterprise: shop, allow_charges: true) } - let!(:credit_card) { create(:credit_card, user: customer.user) } + let!(:credit_card) { create(:stored_credit_card, user: customer.user) } let!(:shop_product) { create(:product, supplier: shop) } let!(:shop_variant) { create(:variant, product: shop_product, unit_value: "2000") } let!(:permitted_supplier) do 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 diff --git a/spec/models/spree/user_spec.rb b/spec/models/spree/user_spec.rb index 31b6dbbfd8..bd1d7ad78c 100644 --- a/spec/models/spree/user_spec.rb +++ b/spec/models/spree/user_spec.rb @@ -145,7 +145,7 @@ describe Spree.user_class do end context "when the user has one credit card" do - let!(:card) { create(:credit_card, user: user) } + let!(:card) { create(:stored_credit_card, user: user) } it "should be assigned as the default and be returned" do expect(card.reload.is_default).to be true @@ -154,8 +154,8 @@ describe Spree.user_class do end context "when the user has more than one card" do - let!(:non_default_card) { create(:credit_card, user: user) } - let!(:default_card) { create(:credit_card, user: user, is_default: true) } + let!(:non_default_card) { create(:stored_credit_card, user: user) } + let!(:default_card) { create(:stored_credit_card, user: user, is_default: true) } it "returns the card which is specified as the default" do expect(user.default_card.id).to be default_card.id