Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Maintain the same Stripe Customer across multiple cards #77

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ else
gem 'sqlite3'
end

group :development, :test do
gem 'stripe'
end

gemspec

# Use a local Gemfile to include development dependencies that might not be
Expand Down
12 changes: 10 additions & 2 deletions app/models/spree/payment_method/stripe_credit_card.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,14 @@ def create_profile(payment)
creditcard = source
end

user_stripe_payment_sources = payment.source.user&.wallet&.wallet_payment_sources&.select do |wps|
wps.payment_source.payment_method.type == self.class.name
end
if user_stripe_payment_sources.present?
customer_id = user_stripe_payment_sources.map {|ps| ps.payment_source&.gateway_customer_profile_id }.compact.last
options[:customer] = customer_id
end

response = gateway.store(creditcard, options)
if response.success?
if v3_intents?
Expand All @@ -133,8 +141,8 @@ def create_profile(payment)
else
payment.source.update!(
cc_type: payment.source.cc_type,
gateway_customer_profile_id: response.params['id'],
gateway_payment_profile_id: response.params['default_source'] || response.params['default_card']
gateway_customer_profile_id: options[:customer] ? response.params['customer'] : response.params['id'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the ternary can become: response.params['customer'] || response.params['id']. Currently response.params['customer'] is nil when not reusing a customer, but we can make it a little more future proof with response.params['customer'].presence, so it will work properly also if we start receiving empty-string values.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general I would agree with you, but I think in context with the line that follows, it's a bit more complicated. As you can see, both ternaries use the same setup with options[:customer] ? .

So I think there's an argument for keeping the logic parallel between the two, even if the first one could be made shorter.

gateway_payment_profile_id: options[:customer] ? response.params['id'] : response.params['default_source'] || response.params['default_card']
)
end
else
Expand Down
57 changes: 57 additions & 0 deletions lib/solidus_stripe/testing_support/stripe_checkout_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# frozen_string_literal: true

module StripeCheckoutHelper
def initialize_checkout
FactoryBot.create(:store)
zone.members << Spree::ZoneMember.create!(zoneable: country)
FactoryBot.create(:free_shipping_method)

Spree::PaymentMethod::StripeCreditCard.create!(
name: "Stripe",
preferred_secret_key: "sk_test_VCZnDv3GLU15TRvn8i2EsaAN",
preferred_publishable_key: "pk_test_Cuf0PNtiAkkMpTVC2gwYDMIg",
preferred_v3_elements: preferred_v3_elements,
preferred_v3_intents: preferred_v3_intents
)

FactoryBot.create(:product, name: "DL-44")

visit spree.root_path
click_link "DL-44"
click_button "Add To Cart"

expect(page).to have_current_path("/cart")
click_button "Checkout"

expect(page).to have_current_path("/checkout/registration")
click_link "Create a new account"
within("#new_spree_user") do
fill_in "Email", with: "[email protected]"
fill_in "Password", with: "superStrongPassword"
fill_in "Password Confirmation", with: "superStrongPassword"
end
click_button "Create"

# Address
expect(page).to have_current_path("/checkout/address")

within("#billing") do
fill_in_name
fill_in "Street Address", with: "YT-1300"
fill_in "City", with: "Mos Eisley"
select "United States of America", from: "Country"
select country.states.first.name, from: "order_bill_address_attributes_state_id"
fill_in "Zip", with: "12010"
fill_in "Phone", with: "(555) 555-5555"
end
click_on "Save and Continue"

# Delivery
expect(page).to have_current_path("/checkout/delivery")
expect(page).to have_content("UPS Ground")
click_on "Save and Continue"

# Payment
expect(page).to have_current_path("/checkout/payment")
end
end
52 changes: 1 addition & 51 deletions spec/features/stripe_checkout_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,57 +9,7 @@
let(:card_3d_secure) { "4000 0025 0000 3155" }

before do
FactoryBot.create(:store)
zone.members << Spree::ZoneMember.create!(zoneable: country)
FactoryBot.create(:free_shipping_method)

Spree::PaymentMethod::StripeCreditCard.create!(
name: "Stripe",
preferred_secret_key: "sk_test_VCZnDv3GLU15TRvn8i2EsaAN",
preferred_publishable_key: "pk_test_Cuf0PNtiAkkMpTVC2gwYDMIg",
preferred_v3_elements: preferred_v3_elements,
preferred_v3_intents: preferred_v3_intents
)

FactoryBot.create(:product, name: "DL-44")

visit spree.root_path
click_link "DL-44"
click_button "Add To Cart"

expect(page).to have_current_path("/cart")
click_button "Checkout"

expect(page).to have_current_path("/checkout/registration")
click_link "Create a new account"
within("#new_spree_user") do
fill_in "Email", with: "[email protected]"
fill_in "Password", with: "superStrongPassword"
fill_in "Password Confirmation", with: "superStrongPassword"
end
click_button "Create"

# Address
expect(page).to have_current_path("/checkout/address")

within("#billing") do
fill_in_name
fill_in "Street Address", with: "YT-1300"
fill_in "City", with: "Mos Eisley"
select "United States of America", from: "Country"
select country.states.first.name, from: "order_bill_address_attributes_state_id"
fill_in "Zip", with: "12010"
fill_in "Phone", with: "(555) 555-5555"
end
click_on "Save and Continue"

# Delivery
expect(page).to have_current_path("/checkout/delivery")
expect(page).to have_content("UPS Ground")
click_on "Save and Continue"

# Payment
expect(page).to have_current_path("/checkout/payment")
initialize_checkout
end

# This will fetch a token from Stripe.com and then pass that to the webserver.
Expand Down
127 changes: 127 additions & 0 deletions spec/features/stripe_customer_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
# frozen_string_literal: true

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first 100 lines here are still almost identical to the ones in stripe_checkout.spec. I think that all this duplication may become an issue in the future, as changes in one file will need to be ported to the other, for example if something changes on Solidus FE flow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I’ve moved this to a checkout_helper, into a method called initialize_checkout. This deduplicates almost all of the code that was shared between stripe_checkout_spec and stripe_customer_spec.

require 'spec_helper'
require 'stripe'
Stripe.api_key = "sk_test_VCZnDv3GLU15TRvn8i2EsaAN"

RSpec.describe "Stripe checkout", type: :feature do
let(:zone) { FactoryBot.create(:zone) }
let(:country) { FactoryBot.create(:country) }

before do
initialize_checkout

fill_in_card
click_button "Save and Continue"

# Confirmation
expect(page).to have_current_path("/checkout/confirm")
click_button "Place Order"
expect(page).to have_content("Your order has been processed successfully")

# Begin Second Purchase
visit spree.root_path
click_link "DL-44"
click_button "Add To Cart"

expect(page).to have_current_path("/cart")
click_button "Checkout"

# Address
expect(page).to have_current_path("/checkout/address")

within("#billing") do
fill_in_name
fill_in "Street Address", with: "YT-1300"
fill_in "City", with: "Mos Eisley"
select "United States of America", from: "Country"
select country.states.first.name, from: "order_bill_address_attributes_state_id"
fill_in "Zip", with: "12010"
fill_in "Phone", with: "(555) 555-5555"
end
click_on "Save and Continue"

# Delivery
expect(page).to have_current_path("/checkout/delivery")
expect(page).to have_content("UPS Ground")
click_on "Save and Continue"

# Payment
expect(page).to have_current_path("/checkout/payment")
end

shared_examples "Maintain Consistent Stripe Customer Across Purchases" do
it "can re-use saved cards and maintain the same Stripe payment ID and customer ID", js: true do

choose "Use an existing card on file"
click_button "Save and Continue"

# Confirm
expect(page).to have_current_path("/checkout/confirm")
click_button "Place Order"
expect(page).to have_content("Your order has been processed successfully")

user = Spree::User.find_by_email("[email protected]")
user_sources = user.wallet.wallet_payment_sources
expect(user_sources.size).to eq(1)

user_card = user_sources.first.payment_source
expect(user_card.gateway_customer_profile_id).to start_with 'cus_'
expect(user_card.gateway_payment_profile_id).to start_with 'card_'

stripe_customer = Stripe::Customer.retrieve(user_card.gateway_customer_profile_id)
expect(stripe_customer[:email]).to eq(user.email)
expect(stripe_customer[:sources][:total_count]).to eq(1)
expect(stripe_customer[:sources][:data].first[:customer]).to eq(user_card.gateway_customer_profile_id)
expect(stripe_customer[:sources][:data].first[:id]).to eq(user_card.gateway_payment_profile_id)

expect(user.orders.map { |o| o.payments.valid.first.source.gateway_payment_profile_id }.uniq.size).to eq(1)
expect(user.orders.map { |o| o.payments.valid.first.source.gateway_customer_profile_id }.uniq.size).to eq(1)
end

it "can use a new card and maintain the same Stripe customer ID", js: true do

choose "Use a new card / payment method"
fill_in_card({ number: '5555 5555 5555 4444' })
click_button "Save and Continue"

# Confirm
expect(page).to have_current_path("/checkout/confirm")

user = Spree::User.find_by_email("[email protected]")
user_cards = user.credit_cards
expect(user_cards.size).to eq(2)
expect(user_cards.pluck(:gateway_customer_profile_id)).to all( start_with 'cus_' )
expect(user_cards.pluck(:gateway_payment_profile_id)).to all( start_with 'card_' )
expect(user_cards.last.gateway_customer_profile_id).to eq(user_cards.first.gateway_customer_profile_id)
expect(user_cards.pluck(:gateway_customer_profile_id).uniq.size).to eq(1)

click_button "Place Order"
expect(page).to have_content("Your order has been processed successfully")

expect(user.wallet.wallet_payment_sources.size).to eq(2)
expect(user.orders.map { |o| o.payments.valid.first.source.gateway_payment_profile_id }.uniq.size).to eq(2)
expect(user.orders.map { |o| o.payments.valid.first.source.gateway_customer_profile_id }.uniq.size).to eq(1)

stripe_customer = Stripe::Customer.retrieve(user_cards.last.gateway_customer_profile_id)
stripe_customer_cards = Stripe::PaymentMethod.list({ customer: stripe_customer.id, type: 'card' })
expect(stripe_customer_cards.count).to eq(2)
expect(stripe_customer_cards.data.map { |card| card.id }).to match_array(user.orders.map { |o| o.payments.valid.first.source.gateway_payment_profile_id }.uniq)
expect(stripe_customer_cards.data.map { |card| card.id }).to match_array(user_cards.pluck(:gateway_payment_profile_id))
end
end

context 'when using Stripe V2 API library' do
let(:preferred_v3_elements) { false }
let(:preferred_v3_intents) { false }

it_behaves_like "Maintain Consistent Stripe Customer Across Purchases"
end

context 'when using Stripe V3 API library with Elements' do
let(:preferred_v3_elements) { true }
let(:preferred_v3_intents) { false }

it_behaves_like "Maintain Consistent Stripe Customer Across Purchases"
end
end
4 changes: 3 additions & 1 deletion spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,17 @@
# Requires factories defined in lib/solidus_stripe/testing_support/factories.rb
SolidusDevSupport::TestingSupport::Factories.load_for(SolidusStripe::Engine)

# Requires card input helper defined in lib/solidus_stripe/testing_support/card_input_helper.rb
# Requires helpers defined in lib/solidus_stripe/testing_support
require 'solidus_stripe/testing_support/card_input_helper'
require 'solidus_stripe/testing_support/stripe_checkout_helper'

RSpec.configure do |config|
config.infer_spec_type_from_file_location!
config.use_transactional_fixtures = false

config.include SolidusAddressNameHelper, type: :feature
config.include SolidusCardInputHelper, type: :feature
config.include StripeCheckoutHelper, type: :feature

if Spree.solidus_gem_version < Gem::Version.new('2.11')
config.extend Spree::TestingSupport::AuthorizationHelpers::Request, type: :system
Expand Down