From d8ae0b588a4e8a89a3ab89c976ac27e0a5d9fca1 Mon Sep 17 00:00:00 2001 From: George Mendoza Date: Fri, 10 Jun 2022 14:59:42 +0800 Subject: [PATCH 01/10] Install browser tools before running tests Fixes `Webdrivers::BrowserNotFound: Failed to find Chrome binary` errors in CircleCI. --- .circleci/config.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index a9ec60d0..da28152f 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,6 +1,8 @@ version: 2.1 orbs: + browser-tools: circleci/browser-tools@1.1 + # Always take the latest version of the orb, this allows us to # run specs against Solidus supported versions only without the need # to change this configuration every time a Solidus version is released @@ -11,10 +13,12 @@ jobs: run-specs-with-postgres: executor: solidusio_extensions/postgres steps: + - browser-tools/install-browser-tools - solidusio_extensions/run-tests run-specs-with-mysql: executor: solidusio_extensions/mysql steps: + - browser-tools/install-browser-tools - solidusio_extensions/run-tests lint-code: executor: solidusio_extensions/sqlite-memory From 4ea790ed1ac387d355712a589d6626e793cd3be7 Mon Sep 17 00:00:00 2001 From: George Mendoza Date: Fri, 10 Jun 2022 15:10:07 +0800 Subject: [PATCH 02/10] Autocorrect RuboCop offenses Output: ``` solidus_paypal_braintree 15:03:48 $ bundle exec rubocop --autocorrect Inspecting 76 files C..............................................CC..C........CCC.......C..... Offenses: Gemfile:23:6: C: [Corrected] Style/FetchEnvVar: Use ENV.fetch('DB') or ENV.fetch('DB', nil) instead of ENV['DB']. case ENV['DB'] ^^^^^^^^^ solidus_paypal_braintree.gemspec:22:3: C: [Corrected] Gemspec/DeprecatedAttributeAssignment: Do not set test_files in gemspec. s.test_files = Dir['spec/**/*'] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ spec/controllers/solidus_paypal_braintree/checkouts_controller_spec.rb:57:14: C: [Correctable] RSpec/ExpectChange: Prefer change(SolidusPaypalBraintree::Source, :count). to change { SolidusPaypalBraintree::Source.count }. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ spec/controllers/solidus_paypal_braintree/checkouts_controller_spec.rb:91:39: C: [Correctable] RSpec/ExpectChange: Prefer change(::Spree::Payment, :count). expect{ patch_update }.not_to(change{ ::Spree::Payment.count }) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ spec/controllers/solidus_paypal_braintree/checkouts_controller_spec.rb:95:39: C: [Correctable] RSpec/ExpectChange: Prefer change(SolidusPaypalBraintree::Source, :count). expect{ patch_update }.not_to(change{ SolidusPaypalBraintree::Source.count }) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ spec/controllers/solidus_paypal_braintree/transactions_controller_spec.rb:103:37: C: [Correctable] RSpec/ExpectChange: Prefer change(Spree::Address, :count). expect { post_create }.to change { Spree::Address.count }.by(1) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ spec/controllers/solidus_paypal_braintree/transactions_controller_spec.rb:116:37: C: [Correctable] RSpec/ExpectChange: Prefer change(Spree::Address, :count). expect { post_create }.to change { Spree::Address.count }.by(1) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ spec/controllers/solidus_paypal_braintree/transactions_controller_spec.rb:126:41: C: [Correctable] RSpec/ExpectChange: Prefer change(Spree::Address, :count). expect { post_create }.not_to(change { Spree::Address.count }) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ spec/models/solidus_paypal_braintree/avs_result_spec.rb:9:25: C: [Corrected] RSpec/VerifiedDoubleReference: Use a constant class reference for verified doubles. instance_double('Braintree::Transaction', ^^^^^^^^^^^^^^^^^^^^^^^^ spec/models/solidus_paypal_braintree/avs_result_spec.rb:30:25: C: [Corrected] RSpec/VerifiedDoubleReference: Use a constant class reference for verified doubles. instance_double('Braintree::Transaction', ^^^^^^^^^^^^^^^^^^^^^^^^ spec/models/solidus_paypal_braintree/gateway_spec.rb:423:50: C: [Corrected] RSpec/VerifiedDoubleReference: Use a constant class reference for verified doubles. subject { gateway.try_void(instance_double('Spree::Payment', response_code: source.token)) } ^^^^^^^^^^^^^^^^ spec/models/solidus_paypal_braintree/gateway_spec.rb:426:22: C: [Corrected] RSpec/VerifiedDoubleReference: Use a constant class reference for verified doubles. class_double('Braintree::Transaction', ^^^^^^^^^^^^^^^^^^^^^^^^ spec/models/solidus_paypal_braintree/gateway_spec.rb:431:34: C: [Corrected] RSpec/VerifiedDoubleReference: Use a constant class reference for verified doubles. client = instance_double('Braintree::Gateway') ^^^^^^^^^^^^^^^^^^^^ spec/models/solidus_paypal_braintree/gateway_spec.rb:438:27: C: [Corrected] RSpec/VerifiedDoubleReference: Use a constant class reference for verified doubles. instance_double('Braintree::Transaction', ^^^^^^^^^^^^^^^^^^^^^^^^ spec/models/solidus_paypal_braintree/gateway_spec.rb:476:27: C: [Corrected] RSpec/VerifiedDoubleReference: Use a constant class reference for verified doubles. instance_double('Braintree::Transaction', ^^^^^^^^^^^^^^^^^^^^^^^^ spec/models/solidus_paypal_braintree/gateway_spec.rb:488:27: C: [Corrected] RSpec/VerifiedDoubleReference: Use a constant class reference for verified doubles. instance_double('Braintree::Transaction', ^^^^^^^^^^^^^^^^^^^^^^^^ spec/models/solidus_paypal_braintree/response_spec.rb:7:7: C: [Corrected] RSpec/VerifiedDoubleReference: Use a constant class reference for verified doubles. 'Braintree::ValidationError', ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ spec/models/solidus_paypal_braintree/response_spec.rb:15:7: C: [Corrected] RSpec/VerifiedDoubleReference: Use a constant class reference for verified doubles. 'Braintree::ErrorResult', ^^^^^^^^^^^^^^^^^^^^^^^^ spec/models/solidus_paypal_braintree/response_spec.rb:29:7: C: [Corrected] RSpec/VerifiedDoubleReference: Use a constant class reference for verified doubles. 'Braintree::Transaction', ^^^^^^^^^^^^^^^^^^^^^^^^ spec/models/solidus_paypal_braintree/response_spec.rb:39:7: C: [Corrected] RSpec/VerifiedDoubleReference: Use a constant class reference for verified doubles. 'Braintree::SuccessfulResult', ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ spec/models/solidus_paypal_braintree/response_spec.rb:83:13: C: [Corrected] RSpec/VerifiedDoubleReference: Use a constant class reference for verified doubles. 'Braintree::Transaction', ^^^^^^^^^^^^^^^^^^^^^^^^ spec/models/solidus_paypal_braintree/response_spec.rb:102:13: C: [Corrected] RSpec/VerifiedDoubleReference: Use a constant class reference for verified doubles. 'Braintree::Transaction', ^^^^^^^^^^^^^^^^^^^^^^^^ spec/models/solidus_paypal_braintree/response_spec.rb:120:13: C: [Corrected] RSpec/VerifiedDoubleReference: Use a constant class reference for verified doubles. 'Braintree::Transaction', ^^^^^^^^^^^^^^^^^^^^^^^^ spec/models/solidus_paypal_braintree/response_spec.rb:139:13: C: [Corrected] RSpec/VerifiedDoubleReference: Use a constant class reference for verified doubles. 'Braintree::Transaction', ^^^^^^^^^^^^^^^^^^^^^^^^ spec/models/solidus_paypal_braintree/response_spec.rb:156:13: C: [Corrected] RSpec/VerifiedDoubleReference: Use a constant class reference for verified doubles. 'Braintree::Transaction', ^^^^^^^^^^^^^^^^^^^^^^^^ spec/models/solidus_paypal_braintree/response_spec.rb:189:11: C: [Corrected] RSpec/VerifiedDoubleReference: Use a constant class reference for verified doubles. 'Braintree::Transaction', ^^^^^^^^^^^^^^^^^^^^^^^^ spec/models/solidus_paypal_braintree/response_spec.rb:235:11: C: [Corrected] RSpec/VerifiedDoubleReference: Use a constant class reference for verified doubles. 'Braintree::Transaction', ^^^^^^^^^^^^^^^^^^^^^^^^ spec/support/capybara.rb:7:30: C: [Corrected] Style/RedundantParentheses: Don't use parentheses around a method call. Capybara.javascript_driver = (ENV.fetch('CAPYBARA_DRIVER', :selenium_chrome)).to_sym ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ spec/support/capybara.rb:7:31: C: [Corrected] Style/FetchEnvVar: Use ENV.fetch('CAPYBARA_DRIVER', :selenium_chrome) instead of ENV['CAPYBARA_DRIVER'] || :selenium_chrome. Capybara.javascript_driver = (ENV['CAPYBARA_DRIVER'] || :selenium_chrome).to_sym ^^^^^^^^^^^^^^^^^^^^^^ 76 files inspected, 29 offenses detected, 23 offenses corrected, 6 more offenses can be corrected with `rubocop -A` ``` --- Gemfile | 2 +- solidus_paypal_braintree.gemspec | 1 - .../avs_result_spec.rb | 4 ++-- .../solidus_paypal_braintree/gateway_spec.rb | 12 +++++----- .../solidus_paypal_braintree/response_spec.rb | 22 +++++++++---------- spec/support/capybara.rb | 2 +- 6 files changed, 21 insertions(+), 22 deletions(-) diff --git a/Gemfile b/Gemfile index 21a3da77..40470f89 100644 --- a/Gemfile +++ b/Gemfile @@ -20,7 +20,7 @@ gem 'sassc-rails', platforms: :mri gem 'bourbon' -case ENV['DB'] +case ENV.fetch('DB', nil) when 'mysql' gem 'mysql2' when 'postgresql' diff --git a/solidus_paypal_braintree.gemspec b/solidus_paypal_braintree.gemspec index b3d363e0..23693cd3 100644 --- a/solidus_paypal_braintree.gemspec +++ b/solidus_paypal_braintree.gemspec @@ -19,7 +19,6 @@ Gem::Specification.new do |s| s.files = Dir.chdir(File.expand_path(__dir__)) do `git ls-files -z`.split("\x0").reject { |f| f.match(%r{^(test|spec|features)/}) } end - s.test_files = Dir['spec/**/*'] s.bindir = "exe" s.executables = s.files.grep(%r{^exe/}) { |f| File.basename(f) } s.require_paths = ["lib"] diff --git a/spec/models/solidus_paypal_braintree/avs_result_spec.rb b/spec/models/solidus_paypal_braintree/avs_result_spec.rb index ba7b6cc7..4a7d267c 100644 --- a/spec/models/solidus_paypal_braintree/avs_result_spec.rb +++ b/spec/models/solidus_paypal_braintree/avs_result_spec.rb @@ -6,7 +6,7 @@ context 'with avs_error_response_code' do let(:transaction) do - instance_double('Braintree::Transaction', + instance_double(Braintree::Transaction, avs_error_response_code: error_code, avs_street_address_response_code: nil, avs_postal_code_response_code: nil) @@ -27,7 +27,7 @@ context 'without avs_error_response_code' do let(:transaction) do - instance_double('Braintree::Transaction', + instance_double(Braintree::Transaction, avs_error_response_code: nil, avs_street_address_response_code: codes.first, avs_postal_code_response_code: codes.last) diff --git a/spec/models/solidus_paypal_braintree/gateway_spec.rb b/spec/models/solidus_paypal_braintree/gateway_spec.rb index 4668cc4b..4b26a6d5 100644 --- a/spec/models/solidus_paypal_braintree/gateway_spec.rb +++ b/spec/models/solidus_paypal_braintree/gateway_spec.rb @@ -420,22 +420,22 @@ end describe '#try_void' do - subject { gateway.try_void(instance_double('Spree::Payment', response_code: source.token)) } + subject { gateway.try_void(instance_double(Spree::Payment, response_code: source.token)) } let(:transaction_request) do - class_double('Braintree::Transaction', + class_double(Braintree::Transaction, find: transaction_response) end before do - client = instance_double('Braintree::Gateway') + client = instance_double(Braintree::Gateway) allow(client).to receive(:transaction) { transaction_request } allow(gateway).to receive(:braintree) { client } end context 'with voidable payment' do let(:transaction_response) do - instance_double('Braintree::Transaction', + instance_double(Braintree::Transaction, status: Braintree::Transaction::Status::Authorized) end @@ -473,7 +473,7 @@ context 'with voidable paypal payment' do let(:transaction_response) do - instance_double('Braintree::Transaction', + instance_double(Braintree::Transaction, status: Braintree::Transaction::Status::SettlementPending) end @@ -485,7 +485,7 @@ context 'with non-voidable payment' do let(:transaction_response) do - instance_double('Braintree::Transaction', + instance_double(Braintree::Transaction, status: Braintree::Transaction::Status::Settled) end diff --git a/spec/models/solidus_paypal_braintree/response_spec.rb b/spec/models/solidus_paypal_braintree/response_spec.rb index 534bb815..8f1904c4 100644 --- a/spec/models/solidus_paypal_braintree/response_spec.rb +++ b/spec/models/solidus_paypal_braintree/response_spec.rb @@ -4,7 +4,7 @@ let(:failed_transaction) { nil } let(:error) do instance_double( - 'Braintree::ValidationError', + Braintree::ValidationError, code: '12345', message: "Cannot refund a transaction unless it is settled." ) @@ -12,7 +12,7 @@ let(:error_result) do instance_double( - 'Braintree::ErrorResult', + Braintree::ErrorResult, success?: false, errors: [error], transaction: failed_transaction, @@ -26,7 +26,7 @@ let(:successful_result) do transaction = instance_double( - 'Braintree::Transaction', + Braintree::Transaction, status: 'ok', id: 'abcdef', avs_error_response_code: nil, @@ -36,7 +36,7 @@ ) instance_double( - 'Braintree::SuccessfulResult', + Braintree::SuccessfulResult, success?: true, transaction: transaction ) @@ -80,7 +80,7 @@ let(:error) { nil } let(:failed_transaction) do instance_double( - 'Braintree::Transaction', + Braintree::Transaction, id: 'abcdef', status: "settlement_declined", processor_settlement_response_code: "4001", @@ -99,7 +99,7 @@ let(:error) { nil } let(:failed_transaction) do instance_double( - 'Braintree::Transaction', + Braintree::Transaction, id: 'abcdef', status: "gateway_rejected", gateway_rejection_reason: "cvv", @@ -117,7 +117,7 @@ let(:error) { nil } let(:failed_transaction) do instance_double( - 'Braintree::Transaction', + Braintree::Transaction, id: 'abcdef', status: "processor_declined", processor_response_code: '2001', @@ -136,7 +136,7 @@ let(:error) { nil } let(:failed_transaction) do instance_double( - 'Braintree::Transaction', + Braintree::Transaction, id: 'abcdef', status: "authorization_expired", avs_error_response_code: nil, @@ -153,7 +153,7 @@ let(:error) { nil } let(:failed_transaction) do instance_double( - 'Braintree::Transaction', + Braintree::Transaction, id: 'abcdef', status: "something_bad_happened", avs_error_response_code: nil, @@ -186,7 +186,7 @@ let(:failed_transaction) do instance_double( - 'Braintree::Transaction', + Braintree::Transaction, id: 'abcdef', avs_error_response_code: 'E', avs_street_address_response_code: nil, @@ -232,7 +232,7 @@ let(:failed_transaction) do instance_double( - 'Braintree::Transaction', + Braintree::Transaction, id: 'abcdef', avs_error_response_code: nil, avs_street_address_response_code: nil, diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb index 7081f0ad..e57d6c79 100644 --- a/spec/support/capybara.rb +++ b/spec/support/capybara.rb @@ -4,4 +4,4 @@ end end -Capybara.javascript_driver = (ENV['CAPYBARA_DRIVER'] || :selenium_chrome).to_sym +Capybara.javascript_driver = ENV.fetch('CAPYBARA_DRIVER', :selenium_chrome).to_sym From cc7367e0538754bbe6ae120ec365417d1e254726 Mon Sep 17 00:00:00 2001 From: George Mendoza Date: Fri, 10 Jun 2022 15:15:12 +0800 Subject: [PATCH 03/10] Manually correct RSpec/ExpectChange offenses For some reason, RuboCop is unable to autocorrect these. RuboCop output: ``` solidus_paypal_braintree 15:12:20 $ bundle exec rubocop --autocorrect Inspecting 76 files ................................................C..C........................ Offenses: spec/controllers/solidus_paypal_braintree/checkouts_controller_spec.rb:56:36: C: [Correctable] RSpec/ExpectChange: Prefer change(SolidusPaypalBraintree::Source, :count). expect { patch_update }.to change { SolidusPaypalBraintree::Source.count }.from(0).to(1) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ spec/controllers/solidus_paypal_braintree/checkouts_controller_spec.rb:88:39: C: [Correctable] RSpec/ExpectChange: Prefer change(::Spree::Payment, :count). expect{ patch_update }.not_to(change{ ::Spree::Payment.count }) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ spec/controllers/solidus_paypal_braintree/checkouts_controller_spec.rb:92:39: C: [Correctable] RSpec/ExpectChange: Prefer change(SolidusPaypalBraintree::Source, :count). expect{ patch_update }.not_to(change{ SolidusPaypalBraintree::Source.count }) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ spec/controllers/solidus_paypal_braintree/transactions_controller_spec.rb:103:37: C: [Correctable] RSpec/ExpectChange: Prefer change(Spree::Address, :count). expect { post_create }.to change { Spree::Address.count }.by(1) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ spec/controllers/solidus_paypal_braintree/transactions_controller_spec.rb:116:37: C: [Correctable] RSpec/ExpectChange: Prefer change(Spree::Address, :count). expect { post_create }.to change { Spree::Address.count }.by(1) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ spec/controllers/solidus_paypal_braintree/transactions_controller_spec.rb:126:41: C: [Correctable] RSpec/ExpectChange: Prefer change(Spree::Address, :count). expect { post_create }.not_to(change { Spree::Address.count }) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 76 files inspected, 6 offenses detected, 6 more offenses can be corrected with `rubocop -A` ``` --- .../solidus_paypal_braintree/checkouts_controller_spec.rb | 6 +++--- .../transactions_controller_spec.rb | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/controllers/solidus_paypal_braintree/checkouts_controller_spec.rb b/spec/controllers/solidus_paypal_braintree/checkouts_controller_spec.rb index 204904ed..716515db 100644 --- a/spec/controllers/solidus_paypal_braintree/checkouts_controller_spec.rb +++ b/spec/controllers/solidus_paypal_braintree/checkouts_controller_spec.rb @@ -54,7 +54,7 @@ it 'creates a payment source' do expect { patch_update }. - to change { SolidusPaypalBraintree::Source.count }. + to change(SolidusPaypalBraintree::Source, :count). from(0). to(1) end @@ -88,11 +88,11 @@ end it "does not change the number of payments in the system" do - expect{ patch_update }.not_to(change{ ::Spree::Payment.count }) + expect{ patch_update }.not_to(change(::Spree::Payment, :count)) end it "does not change the number of sources in the system" do - expect{ patch_update }.not_to(change{ SolidusPaypalBraintree::Source.count }) + expect{ patch_update }.not_to(change(SolidusPaypalBraintree::Source, :count)) end end end diff --git a/spec/controllers/solidus_paypal_braintree/transactions_controller_spec.rb b/spec/controllers/solidus_paypal_braintree/transactions_controller_spec.rb index 7165c7b3..c229ddd6 100644 --- a/spec/controllers/solidus_paypal_braintree/transactions_controller_spec.rb +++ b/spec/controllers/solidus_paypal_braintree/transactions_controller_spec.rb @@ -100,7 +100,7 @@ # Creating the order also creates 3 addresses, we want to make sure # the transaction import only creates 1 new one order - expect { post_create }.to change { Spree::Address.count }.by(1) + expect { post_create }.to change(Spree::Address, :count).by(1) expect(Spree::Address.last.address1).to eq "123 Fake Street" end end @@ -113,7 +113,7 @@ it "creates a new address, looking up the ISO by country name" do order - expect { post_create }.to change { Spree::Address.count }.by(1) + expect { post_create }.to change(Spree::Address, :count).by(1) expect(Spree::Address.last.country.iso).to eq "US" end end @@ -123,7 +123,7 @@ it "does not create a new address" do order - expect { post_create }.not_to(change { Spree::Address.count }) + expect { post_create }.not_to(change(Spree::Address, :count)) end end From 1682f228149b8298519850b4a4116b5bdd8e64ac Mon Sep 17 00:00:00 2001 From: George Mendoza Date: Fri, 10 Jun 2022 15:26:13 +0800 Subject: [PATCH 04/10] Allow Rails version to be configured Remove Bundler workaround. No longer needed. --- Gemfile | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index 40470f89..e4943707 100644 --- a/Gemfile +++ b/Gemfile @@ -6,10 +6,7 @@ git_source(:github) { |repo| "https://github.com/#{repo}.git" } branch = ENV.fetch('SOLIDUS_BRANCH', 'master') gem 'solidus', github: 'solidusio/solidus', branch: branch -# Needed to help Bundler figure out how to resolve dependencies, -# otherwise it takes forever to resolve them. -# See https://github.com/bundler/bundler/issues/6677 -gem 'rails', '>0.a' +gem 'rails', ENV.fetch('RAILS_VERSION', nil) # Provides basic authentication functionality for testing parts of your engine gem 'solidus_auth_devise' From 8e8a6c075f41630b96955361380bb4c3c50f6cff Mon Sep 17 00:00:00 2001 From: George Mendoza Date: Tue, 14 Jun 2022 14:52:52 +0800 Subject: [PATCH 05/10] Ignore local Gemfile --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index e5737986..0e523682 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,7 @@ .project .sass-cache coverage +Gemfile-local Gemfile.lock tmp nbproject From 396f5e8b63f5ebe2a4932a1ac0c5ff9cbf4e0ef2 Mon Sep 17 00:00:00 2001 From: George Mendoza Date: Tue, 14 Jun 2022 17:14:16 +0800 Subject: [PATCH 06/10] Temporarily disable failing Venmo specs Will be reenabled once Venmo is enabled on the Braintree sandbox. --- spec/features/frontend/venmo_checkout_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/features/frontend/venmo_checkout_spec.rb b/spec/features/frontend/venmo_checkout_spec.rb index 5360ba17..c6149ff0 100644 --- a/spec/features/frontend/venmo_checkout_spec.rb +++ b/spec/features/frontend/venmo_checkout_spec.rb @@ -70,7 +70,8 @@ end end - context 'with Venmo transactions', vcr: { cassette_name: 'checkout/valid_venmo_transaction' } do + # TODO: Reenable these specs once Venmo is enabled on the Braintree sandbox. + xcontext 'with Venmo transactions', vcr: { cassette_name: 'checkout/valid_venmo_transaction' } do before do fake_venmo_successful_tokenization end From f8bcc3c35a6123b343bba5f1cd368cec069ba0fb Mon Sep 17 00:00:00 2001 From: George Mendoza Date: Fri, 10 Jun 2022 14:52:43 +0800 Subject: [PATCH 07/10] Wrap register_solidus_paypal_braintree_gateway in a to_prepare block Fixes `NameError: uninitialized constant SolidusPaypalBraintree::Gateway` when running `bundle exec rake` --- lib/solidus_paypal_braintree/engine.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/solidus_paypal_braintree/engine.rb b/lib/solidus_paypal_braintree/engine.rb index e09102bc..d8457d8a 100644 --- a/lib/solidus_paypal_braintree/engine.rb +++ b/lib/solidus_paypal_braintree/engine.rb @@ -19,9 +19,11 @@ class Engine < Rails::Engine end initializer "register_solidus_paypal_braintree_gateway", after: "spree.register.payment_methods" do |app| - app.config.spree.payment_methods << SolidusPaypalBraintree::Gateway - SolidusPaypalBraintree::Gateway.allowed_admin_form_preference_types << :preference_select - Spree::PermittedAttributes.source_attributes.concat [:nonce, :payment_type, :paypal_funding_source] + config.to_prepare do + app.config.spree.payment_methods << SolidusPaypalBraintree::Gateway + SolidusPaypalBraintree::Gateway.allowed_admin_form_preference_types << :preference_select + Spree::PermittedAttributes.source_attributes.concat [:nonce, :payment_type, :paypal_funding_source] + end end if SolidusSupport.frontend_available? From 7b175df18adf2cbadff9fa0fb95041e330235c8f Mon Sep 17 00:00:00 2001 From: George Mendoza Date: Fri, 10 Jun 2022 14:57:01 +0800 Subject: [PATCH 08/10] Explicitly refer to Spree Fixes `NameError: uninitialized constant SolidusPaypalBraintree::Spree::PermittedAttributes` when running `bundle exec rake`. --- lib/solidus_paypal_braintree/engine.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/solidus_paypal_braintree/engine.rb b/lib/solidus_paypal_braintree/engine.rb index d8457d8a..1ce9d970 100644 --- a/lib/solidus_paypal_braintree/engine.rb +++ b/lib/solidus_paypal_braintree/engine.rb @@ -22,7 +22,7 @@ class Engine < Rails::Engine config.to_prepare do app.config.spree.payment_methods << SolidusPaypalBraintree::Gateway SolidusPaypalBraintree::Gateway.allowed_admin_form_preference_types << :preference_select - Spree::PermittedAttributes.source_attributes.concat [:nonce, :payment_type, :paypal_funding_source] + ::Spree::PermittedAttributes.source_attributes.concat [:nonce, :payment_type, :paypal_funding_source] end end From 3ac8d493085393d48305c5f11fc93c7fcef70a17 Mon Sep 17 00:00:00 2001 From: George Mendoza Date: Tue, 14 Jun 2022 17:29:09 +0800 Subject: [PATCH 09/10] Fix transaction address validation error handling --- app/models/solidus_paypal_braintree/transaction.rb | 4 ++-- app/models/solidus_paypal_braintree/transaction_import.rb | 4 ++-- .../solidus_paypal_braintree/transactions_controller_spec.rb | 2 +- .../solidus_paypal_braintree/transaction_import_spec.rb | 2 +- spec/models/solidus_paypal_braintree/transaction_spec.rb | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/models/solidus_paypal_braintree/transaction.rb b/app/models/solidus_paypal_braintree/transaction.rb index 0e50c1de..815434a7 100644 --- a/app/models/solidus_paypal_braintree/transaction.rb +++ b/app/models/solidus_paypal_braintree/transaction.rb @@ -18,8 +18,8 @@ class Transaction errors.add(:payment_method, 'Must be braintree') end if address && !address.valid? - address.errors.each do |field, error| - errors.add(:address, "#{field} #{error}") + address.errors.each do |error| + errors.add(:address, error.full_message) end end end diff --git a/app/models/solidus_paypal_braintree/transaction_import.rb b/app/models/solidus_paypal_braintree/transaction_import.rb index c4cfc91a..616f1734 100644 --- a/app/models/solidus_paypal_braintree/transaction_import.rb +++ b/app/models/solidus_paypal_braintree/transaction_import.rb @@ -12,8 +12,8 @@ class InvalidImportError < StandardError; end errors.add("Address", "is invalid") if address && !address.valid? if !transaction.valid? - transaction.errors.each do |field, error| - errors.add(field, error) + transaction.errors.each do |error| + errors.add(error.attribute, error.message) end end errors.none? diff --git a/spec/controllers/solidus_paypal_braintree/transactions_controller_spec.rb b/spec/controllers/solidus_paypal_braintree/transactions_controller_spec.rb index c229ddd6..a99c7428 100644 --- a/spec/controllers/solidus_paypal_braintree/transactions_controller_spec.rb +++ b/spec/controllers/solidus_paypal_braintree/transactions_controller_spec.rb @@ -65,7 +65,7 @@ SolidusPaypalBraintree::TransactionsController::InvalidImportError, "Import invalid: " \ "Address is invalid, " \ - "Address city can't be blank" + "Address City can't be blank" ) end end diff --git a/spec/models/solidus_paypal_braintree/transaction_import_spec.rb b/spec/models/solidus_paypal_braintree/transaction_import_spec.rb index 448f0c08..f14e8acc 100644 --- a/spec/models/solidus_paypal_braintree/transaction_import_spec.rb +++ b/spec/models/solidus_paypal_braintree/transaction_import_spec.rb @@ -44,7 +44,7 @@ it "sets useful error messages" do transaction_import.valid? expect(transaction_import.errors.full_messages). - to eq ["Address is invalid", "Address zip can't be blank"] + to eq ["Address is invalid", "Address Zip can't be blank"] end end end diff --git a/spec/models/solidus_paypal_braintree/transaction_spec.rb b/spec/models/solidus_paypal_braintree/transaction_spec.rb index 801b0b1b..3965b9e5 100644 --- a/spec/models/solidus_paypal_braintree/transaction_spec.rb +++ b/spec/models/solidus_paypal_braintree/transaction_spec.rb @@ -78,7 +78,7 @@ it "sets useful error messages" do transaction.valid? expect(transaction.errors.full_messages). - to eq ["Address zip can't be blank"] + to eq ["Address Zip can't be blank"] end end end From f96066d78c1930017202f13383e332bf0ebb2259 Mon Sep 17 00:00:00 2001 From: George Mendoza Date: Wed, 15 Jun 2022 18:15:01 +0800 Subject: [PATCH 10/10] Ensure that register_solidus_paypal_braintree_gateway initializer is idempotent For historical reasons, the `to_prepare` callback may run twice. The code it executes must be idempotent. See https://guides.rubyonrails.org/autoloading_and_reloading_constants.html#autoload-on-boot-and-on-each-reload. --- lib/solidus_paypal_braintree/engine.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/solidus_paypal_braintree/engine.rb b/lib/solidus_paypal_braintree/engine.rb index 1ce9d970..df19ba25 100644 --- a/lib/solidus_paypal_braintree/engine.rb +++ b/lib/solidus_paypal_braintree/engine.rb @@ -21,8 +21,8 @@ class Engine < Rails::Engine initializer "register_solidus_paypal_braintree_gateway", after: "spree.register.payment_methods" do |app| config.to_prepare do app.config.spree.payment_methods << SolidusPaypalBraintree::Gateway - SolidusPaypalBraintree::Gateway.allowed_admin_form_preference_types << :preference_select - ::Spree::PermittedAttributes.source_attributes.concat [:nonce, :payment_type, :paypal_funding_source] + SolidusPaypalBraintree::Gateway.allowed_admin_form_preference_types.push(:preference_select).uniq! + ::Spree::PermittedAttributes.source_attributes.concat([:nonce, :payment_type, :paypal_funding_source]).uniq! end end