diff --git a/app/assets/javascripts/admin/payments/services/stripe_elements.js.coffee b/app/assets/javascripts/admin/payments/services/stripe_elements.js.coffee index d5c20e8668..03808bbbfc 100644 --- a/app/assets/javascripts/admin/payments/services/stripe_elements.js.coffee +++ b/app/assets/javascripts/admin/payments/services/stripe_elements.js.coffee @@ -17,7 +17,7 @@ angular.module("admin.payments").factory 'AdminStripeElements', ($rootScope, Sta console.error(JSON.stringify(response.error)) else secrets.token = response.token.id - secrets.cc_type = @mapTokenApiCardBrand(response.token.card.brand) + secrets.cc_type = response.token.card.brand secrets.card = response.token.card submit() @@ -33,28 +33,10 @@ angular.module("admin.payments").factory 'AdminStripeElements', ($rootScope, Sta console.error(JSON.stringify(response.error)) else secrets.token = response.paymentMethod.id - secrets.cc_type = @mapPaymentMethodsApiCardBrand(response.paymentMethod.card.brand) + secrets.cc_type = response.paymentMethod.card.brand secrets.card = response.paymentMethod.card submit() - # Maps the brand returned by Stripe's tokenAPI to that required by activemerchant - mapTokenApiCardBrand: (cardBrand) -> - switch cardBrand - when 'MasterCard' then return 'master' - when 'Visa' then return 'visa' - when 'American Express' then return 'american_express' - when 'Discover' then return 'discover' - when 'JCB' then return 'jcb' - when 'Diners Club' then return 'diners_club' - - # Maps the brand returned by Stripe's paymentMethodsAPI to that required by activemerchant - mapPaymentMethodsApiCardBrand: (cardBrand) -> - switch cardBrand - when 'mastercard' then return 'master' - when 'amex' then return 'american_express' - when 'diners' then return 'diners_club' - else return cardBrand # a few brands are equal, for example, visa - # It doesn't matter if any of these are nil, all are optional. makeCardData: (secrets) -> {'name': secrets.name, diff --git a/app/assets/javascripts/darkswarm/services/stripe_elements.js.coffee b/app/assets/javascripts/darkswarm/services/stripe_elements.js.coffee index 9d66876716..fba76cdf00 100644 --- a/app/assets/javascripts/darkswarm/services/stripe_elements.js.coffee +++ b/app/assets/javascripts/darkswarm/services/stripe_elements.js.coffee @@ -16,7 +16,7 @@ angular.module('Darkswarm').factory 'StripeElements', ($rootScope, Messages) -> @reportError(response.error, t("error") + ": #{response.error.message}") else secrets.token = response.token.id - secrets.cc_type = @mapTokenApiCardBrand(response.token.card.brand) + secrets.cc_type = response.token.card.brand secrets.card = response.token.card submit() .catch (response) => @@ -37,7 +37,7 @@ angular.module('Darkswarm').factory 'StripeElements', ($rootScope, Messages) -> @reportError(response.error, t("error") + ": #{response.error.message}") else secrets.token = response.paymentMethod.id - secrets.cc_type = @mapPaymentMethodsApiCardBrand(response.paymentMethod.card.brand) + secrets.cc_type = response.paymentMethod.card.brand secrets.card = response.paymentMethod.card submit() .catch (response) => @@ -56,24 +56,6 @@ angular.module('Darkswarm').factory 'StripeElements', ($rootScope, Messages) -> # $evalAsync is improved way of triggering a digest without calling $apply $rootScope.$evalAsync() - # Maps the brand returned by Stripe's tokenAPI to that required by activemerchant - mapTokenApiCardBrand: (cardBrand) -> - switch cardBrand - when 'MasterCard' then return 'master' - when 'Visa' then return 'visa' - when 'American Express' then return 'american_express' - when 'Discover' then return 'discover' - when 'JCB' then return 'jcb' - when 'Diners Club' then return 'diners_club' - - # Maps the brand returned by Stripe's paymentMethodsAPI to that required by activemerchant - mapPaymentMethodsApiCardBrand: (cardBrand) -> - switch cardBrand - when 'mastercard' then return 'master' - when 'amex' then return 'american_express' - when 'diners' then return 'diners_club' - else return cardBrand # a few brands are equal, for example, visa - # It doesn't matter if any of these are nil, all are optional. makeCardData: (secrets) -> {'name': secrets.name, diff --git a/app/models/spree/credit_card.rb b/app/models/spree/credit_card.rb index 1145cb06d2..9bf584ea46 100644 --- a/app/models/spree/credit_card.rb +++ b/app/models/spree/credit_card.rb @@ -7,6 +7,7 @@ module Spree has_many :payments, as: :source + before_validation :reformat_card_type! before_save :set_last_digits attr_accessor :verification_value @@ -39,20 +40,8 @@ module Spree end end - # cc_type is set by jquery.payment, which helpfully provides different - # types from Active Merchant. Converting them is necessary. def cc_type=(type) - real_type = case type - when 'mastercard', 'maestro' - 'master' - when 'amex' - 'american_express' - when 'dinersclub' - 'diners_club' - else - type - end - self[:cc_type] = real_type + reformat_card_type!(type) end def set_last_digits @@ -129,6 +118,27 @@ module Spree private + def reformat_card_type!(type = nil) + self[:cc_type] = active_merchant_card_type(type || cc_type) + end + + # ActiveMerchant requires certain credit card brand names to be stored in a specific format. + # See: https://github.com/activemerchant/active_merchant/blob/master/lib/active_merchant/billing/credit_card.rb#L89 + def active_merchant_card_type(type) + card_type = type.to_s.downcase + + case card_type + when "mastercard", "maestro", "master card" + "master" + when "amex", "american express" + "american_express" + when "dinersclub", "diners club" + "diners_club" + else + card_type + end + end + def expiry_not_in_the_past return unless year.present? && month.present? diff --git a/spec/javascripts/unit/darkswarm/services/stripe_elements_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/stripe_elements_spec.js.coffee index d7e7fa249a..d901537ba2 100644 --- a/spec/javascripts/unit/darkswarm/services/stripe_elements_spec.js.coffee +++ b/spec/javascripts/unit/darkswarm/services/stripe_elements_spec.js.coffee @@ -34,7 +34,7 @@ describe 'StripeElements Service', -> StripeElements.requestToken(secrets, submit) $rootScope.$digest() # required for #then to by called expect(secrets.token).toEqual "token" - expect(secrets.cc_type).toEqual "master" + expect(secrets.cc_type).toEqual "MasterCard" expect(submit).toHaveBeenCalled() describe "with unsatifactory data", -> @@ -52,19 +52,3 @@ describe 'StripeElements Service', -> expect(Loading.clear).toHaveBeenCalled() expect(RailsFlashLoader.loadFlash).toHaveBeenCalledWith({error: "Error: There was a problem"}) expect(Bugsnag.notify).toHaveBeenCalled() - - describe 'mapTokenApiCardBrand', -> - it "maps the brand returned by Stripe's tokenAPI to that required by activemerchant", -> - expect(StripeElements.mapTokenApiCardBrand('MasterCard')).toEqual "master" - expect(StripeElements.mapTokenApiCardBrand('Visa')).toEqual "visa" - expect(StripeElements.mapTokenApiCardBrand('American Express')).toEqual "american_express" - expect(StripeElements.mapTokenApiCardBrand('Discover')).toEqual "discover" - expect(StripeElements.mapTokenApiCardBrand('JCB')).toEqual "jcb" - expect(StripeElements.mapTokenApiCardBrand('Diners Club')).toEqual "diners_club" - - describe 'mapPaymentMethodsApiCardBrand', -> - it "maps the brand returned by Stripe's paymentMethodsAPI to that required by activemerchant", -> - expect(StripeElements.mapPaymentMethodsApiCardBrand('mastercard')).toEqual "master" - expect(StripeElements.mapPaymentMethodsApiCardBrand('amex')).toEqual "american_express" - expect(StripeElements.mapPaymentMethodsApiCardBrand('diners')).toEqual "diners_club" - expect(StripeElements.mapPaymentMethodsApiCardBrand('visa')).toEqual "visa" diff --git a/spec/models/spree/credit_card_spec.rb b/spec/models/spree/credit_card_spec.rb index 3a8bd54ec1..23f40b3489 100644 --- a/spec/models/spree/credit_card_spec.rb +++ b/spec/models/spree/credit_card_spec.rb @@ -4,16 +4,16 @@ require 'spec_helper' module Spree describe CreditCard do - describe "original specs from Spree" do - let(:valid_credit_card_attributes) { - { - number: '4111111111111111', - verification_value: '123', - month: 12, - year: Time.zone.now.year + 1 - } + let(:valid_credit_card_attributes) { + { + number: '4111111111111111', + verification_value: '123', + month: 12, + year: Time.zone.now.year + 1 } + } + describe "original specs from Spree" do def self.payment_states Spree::Payment.state_machine.states.keys end @@ -151,25 +151,6 @@ module Spree end end - context "#cc_type=" do - it "converts between the different types" do - credit_card.cc_type = 'mastercard' - expect(credit_card.cc_type).to eq 'master' - - credit_card.cc_type = 'maestro' - expect(credit_card.cc_type).to eq 'master' - - credit_card.cc_type = 'amex' - expect(credit_card.cc_type).to eq 'american_express' - - credit_card.cc_type = 'dinersclub' - expect(credit_card.cc_type).to eq 'diners_club' - - credit_card.cc_type = 'some_outlandish_cc_type' - expect(credit_card.cc_type).to eq 'some_outlandish_cc_type' - end - end - context "#associations" do it "should be able to access its payments" do expect { credit_card.payments.to_a }.not_to raise_error @@ -198,6 +179,42 @@ module Spree end end + describe "formatting the card type for ActiveMerchant" do + context "#cc_type=" do + let(:credit_card) { build(:credit_card) } + + it "converts the card type format" do + credit_card.cc_type = 'mastercard' + expect(credit_card.cc_type).to eq 'master' + + credit_card.cc_type = 'maestro' + expect(credit_card.cc_type).to eq 'master' + + credit_card.cc_type = 'amex' + expect(credit_card.cc_type).to eq 'american_express' + + credit_card.cc_type = 'dinersclub' + expect(credit_card.cc_type).to eq 'diners_club' + + credit_card.cc_type = 'some_outlandish_cc_type' + expect(credit_card.cc_type).to eq 'some_outlandish_cc_type' + end + end + + context "on save" do + it "converts the card type format" do + expect_any_instance_of(Spree::CreditCard).to receive(:reformat_card_type!). + at_least(:once).and_call_original + + credit_card = Spree::CreditCard.create( + valid_credit_card_attributes.merge(cc_type: "Master Card") + ) + + expect(credit_card.cc_type).to eq "master" + end + end + end + describe "setting default credit card for a user" do let(:user) { create(:user) } let(:onetime_card_attrs) do