From 1d1e581c85c22937a808f9873259161eb184db87 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 15 Jun 2017 14:50:52 +1000 Subject: [PATCH] Refactoring CreditCardController, rendering card processing errors as json --- .../spree/credit_cards_controller.rb | 52 +++++++------- app/serializers/api/credit_card_serializer.rb | 2 +- config/locales/en.yml | 1 + .../spree/credit_cards_controller_spec.rb | 70 ++++++++++++++----- 4 files changed, 80 insertions(+), 45 deletions(-) diff --git a/app/controllers/spree/credit_cards_controller.rb b/app/controllers/spree/credit_cards_controller.rb index bca24fdaad..53fb641f5c 100644 --- a/app/controllers/spree/credit_cards_controller.rb +++ b/app/controllers/spree/credit_cards_controller.rb @@ -5,29 +5,19 @@ module Spree before_filter :destroy_at_stripe, only: [:destroy] def new_from_token - set_user - # At the moment a new Customer is created for every credit card (even via ActiveMerchant), - # so doing the same here (for now). - if @customer = create_customer(params[:token]) - # Since it's a new customer, the default_source is the card that our original token represented - credit_card_params = format_credit_card_params(params) - .merge({gateway_payment_profile_id: @customer.default_source, - gateway_customer_profile_id: @customer.id, - cc_type: params[:cc_type] - }) - - @credit_card = Spree::CreditCard.new(credit_card_params) - # Can't mass assign these: - @credit_card.cc_type = credit_card_params[:cc_type] - @credit_card.last_digits = credit_card_params[:last_digits] - @credit_card.user_id = @user.id + # A new Customer is created for every credit card (same as via ActiveMerchant) + # Note that default_source is the card represented by the token + begin + @customer = create_customer(params[:token]) + @credit_card = build_card_from(stored_card_attributes) if @credit_card.save render json: @credit_card, serializer: ::Api::CreditCardSerializer, status: :ok else - render json: "error saving credit card", status: 500 + message = t(:card_could_not_be_saved) + render json: { flash: { error: I18n.t(:spree_gateway_error_flash_for_checkout, error: message) } }, status: 400 end - else - render json: "error creating Stripe customer", status: 500 + rescue Stripe::CardError => e + return render json: { flash: { error: I18n.t(:spree_gateway_error_flash_for_checkout, error: e.message) } }, status: 400 end end @@ -47,18 +37,28 @@ module Spree private def create_customer(token) - Stripe::Customer.create(email: @user.email, source: token) + Stripe::Customer.create(email: spree_current_user.email, source: token) end - def format_credit_card_params(params_hash) - { month: params_hash[:exp_month], - year: params_hash[:exp_year], - last_digits: params_hash[:last4] + def stored_card_attributes + return {} unless @customer.try(:default_source) + { + month: params[:exp_month], + year: params[:exp_year], + last_digits: params[:last4], + gateway_payment_profile_id: @customer.default_source, + gateway_customer_profile_id: @customer.id, + cc_type: params[:cc_type] } end - def set_user - @user = spree_current_user + def build_card_from(attrs) + card = Spree::CreditCard.new(attrs) + # Can't mass assign these: + card.cc_type = attrs[:cc_type] + card.last_digits = attrs[:last_digits] + card.user_id = spree_current_user.id + card end def set_credit_card diff --git a/app/serializers/api/credit_card_serializer.rb b/app/serializers/api/credit_card_serializer.rb index 34b3bd124f..f53e9f4e23 100644 --- a/app/serializers/api/credit_card_serializer.rb +++ b/app/serializers/api/credit_card_serializer.rb @@ -6,7 +6,7 @@ class Api::CreditCardSerializer < ActiveModel::Serializer end def number - 'x-' + object.last_digits + "x-#{object.last_digits}" end def expiry diff --git a/config/locales/en.yml b/config/locales/en.yml index 03be617bfc..66be05de9f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -768,6 +768,7 @@ en: # Front-end controller translations + card_could_not_be_saved: card could not be saved spree_gateway_error_flash_for_checkout: "There was a problem with your payment information: %{error}" # Printable Invoice Columns diff --git a/spec/controllers/spree/credit_cards_controller_spec.rb b/spec/controllers/spree/credit_cards_controller_spec.rb index 4b707c6114..4c183cb9d1 100644 --- a/spec/controllers/spree/credit_cards_controller_spec.rb +++ b/spec/controllers/spree/credit_cards_controller_spec.rb @@ -5,29 +5,63 @@ describe Spree::CreditCardsController do include AuthenticationWorkflow let(:user) { create_enterprise_user } let(:token) { "tok_234bd2c22" } - - before do - Stripe.api_key = "sk_test_12345" - end - - it "Creates a credit card from token + params" do - controller.stub(:spree_current_user) { user } - - stub_request(:post, "https://api.stripe.com/v1/customers") - .with(:body => { email: user.email, source: token }) - .to_return(status: 200, body: JSON.generate({ id: "cus_AZNMJ", default_source: "card_1AEEb" })) - - expect{ post :new_from_token, { + let(:params) do + { + format: :json, "exp_month" => 12, "exp_year" => 2020, "last4" => 4242, "token" => token, "cc_type" => "visa" - } }.to change(Spree::CreditCard, :count).by(1) + } + end - Spree::CreditCard.last.gateway_payment_profile_id.should eq "card_1AEEb" - Spree::CreditCard.last.gateway_customer_profile_id.should eq "cus_AZNMJ" - Spree::CreditCard.last.user_id.should eq user.id - Spree::CreditCard.last.last_digits.should eq "4242" + before do + Stripe.api_key = "sk_test_12345" + controller.stub(:spree_current_user) { user } + stub_request(:post, "https://api.stripe.com/v1/customers") + .with(:body => { email: user.email, source: token }) + .to_return(response_mock) + end + + describe "#new_from_token" do + context "when the request to store the customer/card with Stripe is successful" do + let(:response_mock) { { status: 200, body: JSON.generate({ id: "cus_AZNMJ", default_source: "card_1AEEb" }) } } + + it "saves the card locally" do + expect{ post :new_from_token, params }.to change(Spree::CreditCard, :count).by(1) + + card = Spree::CreditCard.last + card.gateway_payment_profile_id.should eq "card_1AEEb" + card.gateway_customer_profile_id.should eq "cus_AZNMJ" + card.user_id.should eq user.id + card.last_digits.should eq "4242" + end + + context "when saving the card locally fails" do + before do + allow(controller).to receive(:stored_card_attributes) { {} } + end + + it "renders a flash error" do + expect{ post :new_from_token, params }.to_not change(Spree::CreditCard, :count) + + json_response = JSON.parse(response.body) + flash_message = I18n.t(:spree_gateway_error_flash_for_checkout, error: I18n.t(:card_could_not_be_saved)) + expect(json_response["flash"]["error"]).to eq flash_message + end + end + end + + context "when the request to store the customer/card with Stripe fails" do + let(:response_mock) { { status: 402, body: JSON.generate({ error: { message: "Bup-bow..." }}) } } + it "doesn't save the card locally, and renders a flash error" do + expect{ post :new_from_token, params }.to_not change(Spree::CreditCard, :count) + + json_response = JSON.parse(response.body) + flash_message = I18n.t(:spree_gateway_error_flash_for_checkout, error: "Bup-bow...") + expect(json_response["flash"]["error"]).to eq flash_message + end + end end end