From cf41ffb5a9dd527ccd564a4a10c1268ff9bbc648 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Sep 2017 17:16:50 +0200 Subject: [PATCH 1/4] Rename object under test in source spec --- .../solidus_paypal_braintree/source_spec.rb | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/spec/models/solidus_paypal_braintree/source_spec.rb b/spec/models/solidus_paypal_braintree/source_spec.rb index afd2b589..a8fdfcde 100644 --- a/spec/models/solidus_paypal_braintree/source_spec.rb +++ b/spec/models/solidus_paypal_braintree/source_spec.rb @@ -173,10 +173,10 @@ describe "#last_4", vcr: { cassette_name: "source/last4" } do let(:method) { new_gateway.tap(&:save!) } - let(:instance) { described_class.create!(payment_type: "CreditCard", payment_method: method) } + let(:payment_source) { described_class.create!(payment_type: "CreditCard", payment_method: method) } let(:braintree_client) { method.braintree } - subject { instance.last_4 } + subject { payment_source.last_4 } before do customer = braintree_client.customer.create @@ -187,21 +187,21 @@ }) expect(method.payment_method.token).to be - instance.update_attributes!(token: method.payment_method.token) + payment_source.update_attributes!(token: method.payment_method.token) end it "delegates to the braintree payment method" do - method = braintree_client.payment_method.find(instance.token) + method = braintree_client.payment_method.find(payment_source.token) expect(subject).to eql(method.last_4) end end describe "#display_number" do - let(:instance) { described_class.new } - subject { instance.display_number } + let(:payment_source) { described_class.new } + subject { payment_source.display_number } before do - allow(instance).to receive(:last_digits).and_return('1234') + allow(payment_source).to receive(:last_digits).and_return('1234') end it { is_expected.to eq 'XXXX-XXXX-XXXX-1234' } @@ -209,10 +209,10 @@ describe "#card_type", vcr: { cassette_name: "source/card_type" } do let(:method) { new_gateway.tap(&:save!) } - let(:instance) { described_class.create!(payment_type: "CreditCard", payment_method: method) } + let(:payment_source) { described_class.create!(payment_type: "CreditCard", payment_method: method) } let(:braintree_client) { method.braintree } - subject { instance.card_type } + subject { payment_source.card_type } before do customer = braintree_client.customer.create @@ -223,11 +223,11 @@ }) expect(method.payment_method.token).to be - instance.update_attributes!(token: method.payment_method.token) + payment_source.update_attributes!(token: method.payment_method.token) end it "delegates to the braintree payment method" do - method = braintree_client.payment_method.find(instance.token) + method = braintree_client.payment_method.find(payment_source.token) expect(subject).to eql(method.card_type) end end From 2947426f10f63ac7c29d28e34b34a9caf264053e Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Sep 2017 17:30:34 +0200 Subject: [PATCH 2/4] Add failing specs for unkown tokens in source display methods --- .../solidus_paypal_braintree/source_spec.rb | 102 +++++++++++++----- 1 file changed, 78 insertions(+), 24 deletions(-) diff --git a/spec/models/solidus_paypal_braintree/source_spec.rb b/spec/models/solidus_paypal_braintree/source_spec.rb index a8fdfcde..dd90c5b4 100644 --- a/spec/models/solidus_paypal_braintree/source_spec.rb +++ b/spec/models/solidus_paypal_braintree/source_spec.rb @@ -171,28 +171,68 @@ end end - describe "#last_4", vcr: { cassette_name: "source/last4" } do + shared_context 'unknown source token' do + let(:braintree_payment_method) { double } + + before do + allow(braintree_payment_method).to receive(:find) do + raise Braintree::NotFoundError + end + allow(payment_source).to receive(:braintree_client) do + double(payment_method: braintree_payment_method) + end + end + end + + shared_context 'nil source token' do + let(:braintree_payment_method) { double } + + before do + allow(braintree_payment_method).to receive(:find) do + raise ArgumentError + end + allow(payment_source).to receive(:braintree_client) do + double(payment_method: braintree_payment_method) + end + end + end + + describe "#last_4" do let(:method) { new_gateway.tap(&:save!) } let(:payment_source) { described_class.create!(payment_type: "CreditCard", payment_method: method) } let(:braintree_client) { method.braintree } subject { payment_source.last_4 } - before do - customer = braintree_client.customer.create - expect(customer.customer.id).to be + context 'when token is known at braintree', vcr: { cassette_name: "source/last4" } do + before do + customer = braintree_client.customer.create + expect(customer.customer.id).to be - method = braintree_client.payment_method.create({ - payment_method_nonce: "fake-valid-country-of-issuance-usa-nonce", customer_id: customer.customer.id - }) - expect(method.payment_method.token).to be + method = braintree_client.payment_method.create({ + payment_method_nonce: "fake-valid-country-of-issuance-usa-nonce", customer_id: customer.customer.id + }) + expect(method.payment_method.token).to be - payment_source.update_attributes!(token: method.payment_method.token) + payment_source.update_attributes!(token: method.payment_method.token) + end + + it "delegates to the braintree payment method" do + method = braintree_client.payment_method.find(payment_source.token) + expect(subject).to eql(method.last_4) + end + end + + context 'when the source token is not known at Braintree' do + include_context 'unknown source token' + + it('should be nil', :pending) { is_expected.to be(nil) } end - it "delegates to the braintree payment method" do - method = braintree_client.payment_method.find(payment_source.token) - expect(subject).to eql(method.last_4) + context 'when the source token is nil' do + include_context 'nil source token' + + it('should be nil', :pending) { is_expected.to be(nil) } end end @@ -207,28 +247,42 @@ it { is_expected.to eq 'XXXX-XXXX-XXXX-1234' } end - describe "#card_type", vcr: { cassette_name: "source/card_type" } do + describe "#card_type" do let(:method) { new_gateway.tap(&:save!) } let(:payment_source) { described_class.create!(payment_type: "CreditCard", payment_method: method) } let(:braintree_client) { method.braintree } subject { payment_source.card_type } - before do - customer = braintree_client.customer.create - expect(customer.customer.id).to be + context "when the token is known at braintree", vcr: { cassette_name: "source/card_type" } do + before do + customer = braintree_client.customer.create + expect(customer.customer.id).to be - method = braintree_client.payment_method.create({ - payment_method_nonce: "fake-valid-country-of-issuance-usa-nonce", customer_id: customer.customer.id - }) - expect(method.payment_method.token).to be + method = braintree_client.payment_method.create({ + payment_method_nonce: "fake-valid-country-of-issuance-usa-nonce", customer_id: customer.customer.id + }) + expect(method.payment_method.token).to be - payment_source.update_attributes!(token: method.payment_method.token) + payment_source.update_attributes!(token: method.payment_method.token) + end + + it "delegates to the braintree payment method" do + method = braintree_client.payment_method.find(payment_source.token) + expect(subject).to eql(method.card_type) + end end - it "delegates to the braintree payment method" do - method = braintree_client.payment_method.find(payment_source.token) - expect(subject).to eql(method.card_type) + context 'when the source token is not known at Braintree' do + include_context 'unknown source token' + + it('should be nil', :pending) { is_expected.to be_nil } + end + + context 'when the source token is nil' do + include_context 'nil source token' + + it('should be nil', :pending) { is_expected.to be_nil } end end end From 2bc1a6c5556bdc888465f6cb6a8ea765ebb8f5e9 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Sep 2017 17:35:14 +0200 Subject: [PATCH 3/4] Rescue Braintree::NotFound errors oin source display methods Whenever we want to display a payment source during checkout or admin payments screen it may happen that the token is not known at Braintree or the token is nil. In order to not blow up these pages we should rescue these errors instead. --- app/models/solidus_paypal_braintree/source.rb | 3 +++ spec/models/solidus_paypal_braintree/source_spec.rb | 8 ++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/app/models/solidus_paypal_braintree/source.rb b/app/models/solidus_paypal_braintree/source.rb index 517dfcf3..22f65006 100644 --- a/app/models/solidus_paypal_braintree/source.rb +++ b/app/models/solidus_paypal_braintree/source.rb @@ -64,6 +64,9 @@ def display_number def braintree_payment_method return unless braintree_client && credit_card? @braintree_payment_method ||= braintree_client.payment_method.find(token) + rescue Braintree::NotFoundError, ArgumentError => e + Rails.logger.warn("#{e}: token unknown or missing for #{inspect}") + nil end def braintree_client diff --git a/spec/models/solidus_paypal_braintree/source_spec.rb b/spec/models/solidus_paypal_braintree/source_spec.rb index dd90c5b4..02bfcf55 100644 --- a/spec/models/solidus_paypal_braintree/source_spec.rb +++ b/spec/models/solidus_paypal_braintree/source_spec.rb @@ -226,13 +226,13 @@ context 'when the source token is not known at Braintree' do include_context 'unknown source token' - it('should be nil', :pending) { is_expected.to be(nil) } + it { is_expected.to be(nil) } end context 'when the source token is nil' do include_context 'nil source token' - it('should be nil', :pending) { is_expected.to be(nil) } + it { is_expected.to be(nil) } end end @@ -276,13 +276,13 @@ context 'when the source token is not known at Braintree' do include_context 'unknown source token' - it('should be nil', :pending) { is_expected.to be_nil } + it { is_expected.to be_nil } end context 'when the source token is nil' do include_context 'nil source token' - it('should be nil', :pending) { is_expected.to be_nil } + it { is_expected.to be_nil } end end end From df75c1d3389bf664a76430123dd7c64fa442fa8e Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 12 Sep 2017 17:39:32 +0200 Subject: [PATCH 4/4] Display XXXX if last_digits returns nil If last_digits returns nil (because the token is unknown) we want to display this correctly to the user as well. --- app/models/solidus_paypal_braintree/source.rb | 2 +- .../solidus_paypal_braintree/source_spec.rb | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/app/models/solidus_paypal_braintree/source.rb b/app/models/solidus_paypal_braintree/source.rb index 22f65006..15620861 100644 --- a/app/models/solidus_paypal_braintree/source.rb +++ b/app/models/solidus_paypal_braintree/source.rb @@ -56,7 +56,7 @@ def credit_card? end def display_number - "XXXX-XXXX-XXXX-#{last_digits}" + "XXXX-XXXX-XXXX-#{last_digits.to_s.rjust(4, 'X')}" end private diff --git a/spec/models/solidus_paypal_braintree/source_spec.rb b/spec/models/solidus_paypal_braintree/source_spec.rb index 02bfcf55..3dda79fc 100644 --- a/spec/models/solidus_paypal_braintree/source_spec.rb +++ b/spec/models/solidus_paypal_braintree/source_spec.rb @@ -240,11 +240,21 @@ let(:payment_source) { described_class.new } subject { payment_source.display_number } - before do - allow(payment_source).to receive(:last_digits).and_return('1234') + context "when last_digits is a number" do + before do + allow(payment_source).to receive(:last_digits).and_return('1234') + end + + it { is_expected.to eq 'XXXX-XXXX-XXXX-1234' } end - it { is_expected.to eq 'XXXX-XXXX-XXXX-1234' } + context "when last_digits is nil" do + before do + allow(payment_source).to receive(:last_digits).and_return(nil) + end + + it { is_expected.to eq 'XXXX-XXXX-XXXX-XXXX' } + end end describe "#card_type" do