From b41a3361ece1eb17adbee4eb9c65e852da768e1d Mon Sep 17 00:00:00 2001 From: Richard Wilson Date: Fri, 5 May 2017 13:53:28 -0700 Subject: [PATCH] Ensure payment_method_id cannot be null on sources There was an issue where, when checking out with paypal, the payment method was not being saved on the source. It does not make sense to have a source without a payment method. I've fixed the issue here and included code into the migration to backfill missing data. --- .../transaction_import.rb | 1 + ...05193712_add_null_constraint_to_sources.rb | 30 +++++++++++++++++++ .../solidus_paypal_braintree/gateway_spec.rb | 5 ++-- .../transaction_import_spec.rb | 4 +++ 4 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20170505193712_add_null_constraint_to_sources.rb diff --git a/app/models/solidus_paypal_braintree/transaction_import.rb b/app/models/solidus_paypal_braintree/transaction_import.rb index a2173dae..a2487f97 100644 --- a/app/models/solidus_paypal_braintree/transaction_import.rb +++ b/app/models/solidus_paypal_braintree/transaction_import.rb @@ -27,6 +27,7 @@ def initialize(order, transaction) def source SolidusPaypalBraintree::Source.new nonce: transaction.nonce, payment_type: transaction.payment_type, + payment_method: transaction.payment_method, user: user end diff --git a/db/migrate/20170505193712_add_null_constraint_to_sources.rb b/db/migrate/20170505193712_add_null_constraint_to_sources.rb new file mode 100644 index 00000000..165258f3 --- /dev/null +++ b/db/migrate/20170505193712_add_null_constraint_to_sources.rb @@ -0,0 +1,30 @@ +class AddNullConstraintToSources < ActiveRecord::Migration + def up + payments = Spree::Payment.arel_table + sources = SolidusPaypalBraintree::Source.arel_table + join_sources = payments.join(sources).on( + payments[:source_id].eq(sources[:id]).and( + payments[:source_type].eq("SolidusPaypalBraintree::Source") + ).and( + sources[:payment_method_id].eq(nil) + ) + ).join_sources + + count = Spree::Payment.joins(join_sources).count + Rails.logger.info("Updating #{count} problematic sources") + + Spree::Payment.joins(join_sources).find_each do |payment| + Rails.logger.info("Updating source #{payment.source_id} with payment method id #{payment.payment_method_id}") + SolidusPaypalBraintree::Source.where(id: payment.source_id).update_all(payment_method_id: payment.payment_method_id) + end + + # We use a foreign key constraint on the model, + # but it doesnt make sense to have this model exist without a payment method + # as two of its methods delegate to the payment method. + change_column_null(:solidus_paypal_braintree_sources, :payment_method_id, false) + end + + def down + change_column_null(:solidus_paypal_braintree_sources, :payment_method_id, true) + end +end diff --git a/spec/models/solidus_paypal_braintree/gateway_spec.rb b/spec/models/solidus_paypal_braintree/gateway_spec.rb index 76bba551..9c2a88f1 100644 --- a/spec/models/solidus_paypal_braintree/gateway_spec.rb +++ b/spec/models/solidus_paypal_braintree/gateway_spec.rb @@ -4,7 +4,7 @@ RSpec.describe SolidusPaypalBraintree::Gateway do let(:gateway) do - new_gateway + new_gateway.tap(&:save) end let(:braintree) { gateway.braintree } @@ -15,7 +15,8 @@ SolidusPaypalBraintree::Source.new( nonce: 'fake-valid-nonce', user: user, - payment_type: payment_type + payment_type: payment_type, + payment_method: gateway ) end diff --git a/spec/models/solidus_paypal_braintree/transaction_import_spec.rb b/spec/models/solidus_paypal_braintree/transaction_import_spec.rb index ce2a1dce..c4938a04 100644 --- a/spec/models/solidus_paypal_braintree/transaction_import_spec.rb +++ b/spec/models/solidus_paypal_braintree/transaction_import_spec.rb @@ -54,6 +54,10 @@ expect(subject.payment_type).to eq 'ApplePayCard' end + it 'takes the payment method from the transaction' do + expect(subject.payment_method).to eq braintree_gateway + end + context 'order has a user' do let(:user) { Spree.user_class.new } let(:order) { Spree::Order.new user: user }