From 3da0c2e386eda46bb60a02b6bcc88423d07846c9 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Mon, 14 Dec 2020 14:22:23 -0800 Subject: [PATCH 01/18] send authorization emails when running the subscriptionconfirmjob --- app/jobs/subscription_confirm_job.rb | 9 +++++++ spec/jobs/subscription_confirm_job_spec.rb | 28 ++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/app/jobs/subscription_confirm_job.rb b/app/jobs/subscription_confirm_job.rb index 29ecd6b6d0..216b2de0a7 100644 --- a/app/jobs/subscription_confirm_job.rb +++ b/app/jobs/subscription_confirm_job.rb @@ -47,6 +47,7 @@ class SubscriptionConfirmJob < ActiveJob::Base process_payment!(order) send_confirmation_email(order) + send_payment_authorization_emails(order) rescue StandardError => e if order.errors.any? send_failed_payment_email(order) @@ -93,6 +94,14 @@ class SubscriptionConfirmJob < ActiveJob::Base SubscriptionMailer.confirmation_email(order).deliver_now end + def send_payment_authorization_emails(order) + order.payments.each do |payment| + if payment.cvv_response_message.present? + Spree::PaymentMailer.authorize_payment(payment).deliver + end + end + end + def send_failed_payment_email(order, error_message = nil) order.update! record_and_log_error(:failed_payment, order, error_message) diff --git a/spec/jobs/subscription_confirm_job_spec.rb b/spec/jobs/subscription_confirm_job_spec.rb index c06e65cf39..8da100c7a5 100644 --- a/spec/jobs/subscription_confirm_job_spec.rb +++ b/spec/jobs/subscription_confirm_job_spec.rb @@ -130,6 +130,7 @@ describe SubscriptionConfirmJob do before do OrderWorkflow.new(order).complete! allow(job).to receive(:send_confirmation_email).and_call_original + allow(job).to receive(:send_payment_authorization_emails).and_call_original setup_email expect(job).to receive(:record_order) end @@ -213,6 +214,7 @@ describe SubscriptionConfirmJob do it "sends a failed payment email" do expect(job).to receive(:send_failed_payment_email) expect(job).to_not receive(:send_confirmation_email) + expect(job).to_not receive(:send_payment_authorization_emails) job.send(:confirm_order!, order) end end @@ -231,6 +233,7 @@ describe SubscriptionConfirmJob do ActionMailer::Base.deliveries.clear expect{ job.send(:confirm_order!, order) }.to_not enqueue_job ConfirmOrderJob expect(job).to have_received(:send_confirmation_email).once + expect(job).to have_received(:send_payment_authorization_emails).once expect(ActionMailer::Base.deliveries.count).to be 1 end end @@ -271,4 +274,29 @@ describe SubscriptionConfirmJob do expect(mail_mock).to have_received(:deliver_now) end end + + describe "#send_payment_authorization_emails" do + let(:order) { instance_double(Spree::Order) } + let(:mail_mock) { double(:mailer_mock, deliver: true) } + let(:payment) { create(:payment, amount: 10) } + + before do + allow(order).to receive(:payments) { [payment] } + allow(Spree::PaymentMailer).to receive(:authorize_payment) { mail_mock } + end + + it "sends authorization email if a payment requires it" do + allow(payment).to receive(:cvv_response_message) { "http://redirect_url" } + job.send(:send_payment_authorization_emails, order) + expect(Spree::PaymentMailer).to have_received(:authorize_payment) + expect(mail_mock).to have_received(:deliver) + end + + it "does not send authorization email if no payment requires it" do + allow(payment).to receive(:cvv_response_message) { nil } + job.send(:send_payment_authorization_emails, order) + expect(Spree::PaymentMailer).not_to have_received(:authorize_payment) + expect(mail_mock).not_to have_received(:deliver) + end + end end From e694449dccbdde7a4ca404ee7a3ccfc74765960d Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Thu, 21 Jan 2021 09:09:41 -0800 Subject: [PATCH 02/18] move subs jobs out of spree namespace --- app/jobs/subscription_confirm_job.rb | 2 +- spec/jobs/subscription_confirm_job_spec.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/jobs/subscription_confirm_job.rb b/app/jobs/subscription_confirm_job.rb index 216b2de0a7..7c83fac8bb 100644 --- a/app/jobs/subscription_confirm_job.rb +++ b/app/jobs/subscription_confirm_job.rb @@ -97,7 +97,7 @@ class SubscriptionConfirmJob < ActiveJob::Base def send_payment_authorization_emails(order) order.payments.each do |payment| if payment.cvv_response_message.present? - Spree::PaymentMailer.authorize_payment(payment).deliver + PaymentMailer.authorize_payment(payment).deliver end end end diff --git a/spec/jobs/subscription_confirm_job_spec.rb b/spec/jobs/subscription_confirm_job_spec.rb index 8da100c7a5..dcecb5a6ac 100644 --- a/spec/jobs/subscription_confirm_job_spec.rb +++ b/spec/jobs/subscription_confirm_job_spec.rb @@ -282,20 +282,20 @@ describe SubscriptionConfirmJob do before do allow(order).to receive(:payments) { [payment] } - allow(Spree::PaymentMailer).to receive(:authorize_payment) { mail_mock } + allow(PaymentMailer).to receive(:authorize_payment) { mail_mock } end it "sends authorization email if a payment requires it" do allow(payment).to receive(:cvv_response_message) { "http://redirect_url" } job.send(:send_payment_authorization_emails, order) - expect(Spree::PaymentMailer).to have_received(:authorize_payment) + expect(PaymentMailer).to have_received(:authorize_payment) expect(mail_mock).to have_received(:deliver) end it "does not send authorization email if no payment requires it" do allow(payment).to receive(:cvv_response_message) { nil } job.send(:send_payment_authorization_emails, order) - expect(Spree::PaymentMailer).not_to have_received(:authorize_payment) + expect(PaymentMailer).not_to have_received(:authorize_payment) expect(mail_mock).not_to have_received(:deliver) end end From c28b65f772aa676a447eb04e7d05c472c2a238dd Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Thu, 21 Jan 2021 09:12:40 -0800 Subject: [PATCH 03/18] update subs jobs delivery methods --- app/jobs/subscription_confirm_job.rb | 2 +- .../services/order_management/subscriptions/summarizer.rb | 2 ++ spec/jobs/subscription_confirm_job_spec.rb | 8 +++----- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/jobs/subscription_confirm_job.rb b/app/jobs/subscription_confirm_job.rb index 7c83fac8bb..09105ba0e7 100644 --- a/app/jobs/subscription_confirm_job.rb +++ b/app/jobs/subscription_confirm_job.rb @@ -97,7 +97,7 @@ class SubscriptionConfirmJob < ActiveJob::Base def send_payment_authorization_emails(order) order.payments.each do |payment| if payment.cvv_response_message.present? - PaymentMailer.authorize_payment(payment).deliver + PaymentMailer.authorize_payment(payment).deliver_now end end end diff --git a/engines/order_management/app/services/order_management/subscriptions/summarizer.rb b/engines/order_management/app/services/order_management/subscriptions/summarizer.rb index fe02a9ad9c..86c7deadea 100644 --- a/engines/order_management/app/services/order_management/subscriptions/summarizer.rb +++ b/engines/order_management/app/services/order_management/subscriptions/summarizer.rb @@ -35,12 +35,14 @@ module OrderManagement record_issue(type, order, line2) end + # This uses `deliver_now` since it's called from inside a job def send_placement_summary_emails @summaries.values.each do |summary| SubscriptionMailer.placement_summary_email(summary).deliver_now end end + # This uses `deliver_now` since it's called from inside a job def send_confirmation_summary_emails @summaries.values.each do |summary| SubscriptionMailer.confirmation_summary_email(summary).deliver_now diff --git a/spec/jobs/subscription_confirm_job_spec.rb b/spec/jobs/subscription_confirm_job_spec.rb index dcecb5a6ac..52f6324dcc 100644 --- a/spec/jobs/subscription_confirm_job_spec.rb +++ b/spec/jobs/subscription_confirm_job_spec.rb @@ -230,11 +230,9 @@ describe SubscriptionConfirmJob do end it "sends only a subscription confirm email, no regular confirmation emails" do - ActionMailer::Base.deliveries.clear expect{ job.send(:confirm_order!, order) }.to_not enqueue_job ConfirmOrderJob expect(job).to have_received(:send_confirmation_email).once expect(job).to have_received(:send_payment_authorization_emails).once - expect(ActionMailer::Base.deliveries.count).to be 1 end end end @@ -277,7 +275,7 @@ describe SubscriptionConfirmJob do describe "#send_payment_authorization_emails" do let(:order) { instance_double(Spree::Order) } - let(:mail_mock) { double(:mailer_mock, deliver: true) } + let(:mail_mock) { double(:mailer_mock, deliver_now: true) } let(:payment) { create(:payment, amount: 10) } before do @@ -289,14 +287,14 @@ describe SubscriptionConfirmJob do allow(payment).to receive(:cvv_response_message) { "http://redirect_url" } job.send(:send_payment_authorization_emails, order) expect(PaymentMailer).to have_received(:authorize_payment) - expect(mail_mock).to have_received(:deliver) + expect(mail_mock).to have_received(:deliver_now) end it "does not send authorization email if no payment requires it" do allow(payment).to receive(:cvv_response_message) { nil } job.send(:send_payment_authorization_emails, order) expect(PaymentMailer).not_to have_received(:authorize_payment) - expect(mail_mock).not_to have_received(:deliver) + expect(mail_mock).not_to have_received(:deliver_now) end end end From 83d7d49e44321397e2897f2191b0cb621c0c97bd Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Tue, 2 Feb 2021 10:43:37 -0800 Subject: [PATCH 04/18] refactor sub confirm job; move email to service --- app/jobs/subscription_confirm_job.rb | 9 ------- .../stripe_sca_payment_authorize.rb | 4 +++ .../stripe_sca_payment_authorize_spec.rb | 20 ++++++++++++++ spec/jobs/subscription_confirm_job_spec.rb | 26 ------------------- 4 files changed, 24 insertions(+), 35 deletions(-) diff --git a/app/jobs/subscription_confirm_job.rb b/app/jobs/subscription_confirm_job.rb index 09105ba0e7..29ecd6b6d0 100644 --- a/app/jobs/subscription_confirm_job.rb +++ b/app/jobs/subscription_confirm_job.rb @@ -47,7 +47,6 @@ class SubscriptionConfirmJob < ActiveJob::Base process_payment!(order) send_confirmation_email(order) - send_payment_authorization_emails(order) rescue StandardError => e if order.errors.any? send_failed_payment_email(order) @@ -94,14 +93,6 @@ class SubscriptionConfirmJob < ActiveJob::Base SubscriptionMailer.confirmation_email(order).deliver_now end - def send_payment_authorization_emails(order) - order.payments.each do |payment| - if payment.cvv_response_message.present? - PaymentMailer.authorize_payment(payment).deliver_now - end - end - end - def send_failed_payment_email(order, error_message = nil) order.update! record_and_log_error(:failed_payment, order, error_message) diff --git a/engines/order_management/app/services/order_management/subscriptions/stripe_sca_payment_authorize.rb b/engines/order_management/app/services/order_management/subscriptions/stripe_sca_payment_authorize.rb index ea4fbe5ce7..87b55889e8 100644 --- a/engines/order_management/app/services/order_management/subscriptions/stripe_sca_payment_authorize.rb +++ b/engines/order_management/app/services/order_management/subscriptions/stripe_sca_payment_authorize.rb @@ -15,6 +15,10 @@ module OrderManagement @order.errors.add(:base, I18n.t('authorization_failure')) unless @payment.pending? + if @payment.cvv_response_message.present? + PaymentMailer.authorize_payment(@payment).deliver_now + end + @payment end end diff --git a/engines/order_management/spec/services/order_management/subscriptions/stripe_sca_payment_authorize_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/stripe_sca_payment_authorize_spec.rb index 17ece20be4..4240ecc0e8 100644 --- a/engines/order_management/spec/services/order_management/subscriptions/stripe_sca_payment_authorize_spec.rb +++ b/engines/order_management/spec/services/order_management/subscriptions/stripe_sca_payment_authorize_spec.rb @@ -57,6 +57,26 @@ module OrderManagement expect(order.errors[:base].first).to eq "Authorization Failure" end end + + context "and payment authorize requires additional authorization" do + let(:mail_mock) { double(:mailer_mock, deliver_now: true) } + + before do + allow(PaymentMailer).to receive(:authorize_payment) { mail_mock } + allow(payment).to receive(:authorize!) { + payment.state = "pending" + payment.cvv_response_message = "https://stripe.com/redirect" + } + end + + it "adds sends an email requesting authorization" do + payment_authorize.call! + + expect(order.errors.size).to eq 0 + expect(PaymentMailer).to have_received(:authorize_payment) + expect(mail_mock).to have_received(:deliver_now) + end + end end end end diff --git a/spec/jobs/subscription_confirm_job_spec.rb b/spec/jobs/subscription_confirm_job_spec.rb index 52f6324dcc..39bec020b1 100644 --- a/spec/jobs/subscription_confirm_job_spec.rb +++ b/spec/jobs/subscription_confirm_job_spec.rb @@ -232,7 +232,6 @@ describe SubscriptionConfirmJob do it "sends only a subscription confirm email, no regular confirmation emails" do expect{ job.send(:confirm_order!, order) }.to_not enqueue_job ConfirmOrderJob expect(job).to have_received(:send_confirmation_email).once - expect(job).to have_received(:send_payment_authorization_emails).once end end end @@ -272,29 +271,4 @@ describe SubscriptionConfirmJob do expect(mail_mock).to have_received(:deliver_now) end end - - describe "#send_payment_authorization_emails" do - let(:order) { instance_double(Spree::Order) } - let(:mail_mock) { double(:mailer_mock, deliver_now: true) } - let(:payment) { create(:payment, amount: 10) } - - before do - allow(order).to receive(:payments) { [payment] } - allow(PaymentMailer).to receive(:authorize_payment) { mail_mock } - end - - it "sends authorization email if a payment requires it" do - allow(payment).to receive(:cvv_response_message) { "http://redirect_url" } - job.send(:send_payment_authorization_emails, order) - expect(PaymentMailer).to have_received(:authorize_payment) - expect(mail_mock).to have_received(:deliver_now) - end - - it "does not send authorization email if no payment requires it" do - allow(payment).to receive(:cvv_response_message) { nil } - job.send(:send_payment_authorization_emails, order) - expect(PaymentMailer).not_to have_received(:authorize_payment) - expect(mail_mock).not_to have_received(:deliver_now) - end - end end From 84db1c9bb48a11e2ac84f416fbf72e4f9274c598 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Wed, 3 Feb 2021 09:07:16 -0800 Subject: [PATCH 05/18] update text of auth emails --- app/views/payment_mailer/authorize_payment.html.haml | 4 ++-- app/views/payment_mailer/authorize_payment.text.haml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/payment_mailer/authorize_payment.html.haml b/app/views/payment_mailer/authorize_payment.html.haml index 438a51c416..ea25e0e7b4 100644 --- a/app/views/payment_mailer/authorize_payment.html.haml +++ b/app/views/payment_mailer/authorize_payment.html.haml @@ -1,2 +1,2 @@ -= t('.instructions', distributor: @payment.order.distributor.name, amount: @payment.display_amount) -= main_app.authorize_payment_url(@payment) += t('spree.payment_mailer.authorize_payment.instructions', distributor: @payment.order.distributor.name, amount: @payment.display_amount) += link_to main_app.authorize_payment_url(@payment), main_app.authorize_payment_url(@payment) diff --git a/app/views/payment_mailer/authorize_payment.text.haml b/app/views/payment_mailer/authorize_payment.text.haml index 956e63ec9f..39ca5e3dbc 100644 --- a/app/views/payment_mailer/authorize_payment.text.haml +++ b/app/views/payment_mailer/authorize_payment.text.haml @@ -1,3 +1,3 @@ -= t('.instructions', distributor: @payment.order.distributor.name, amount: @payment.display_amount) += t('spree.payment_mailer.authorize_payment.instructions', distributor: @payment.order.distributor.name, amount: @payment.display_amount) -= main_app.authorize_payment_url(@payment) += link_to main_app.authorize_payment_url(@payment), main_app.authorize_payment_url(@payment) From 4e3594b8f8d0bb71101be06ba82d14389491aef7 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Wed, 3 Feb 2021 09:09:05 -0800 Subject: [PATCH 06/18] remove redundant url check method --- app/services/checkout/stripe_redirect.rb | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/app/services/checkout/stripe_redirect.rb b/app/services/checkout/stripe_redirect.rb index e7c40a4b9e..81ed0e9313 100644 --- a/app/services/checkout/stripe_redirect.rb +++ b/app/services/checkout/stripe_redirect.rb @@ -15,7 +15,7 @@ module Checkout payment = OrderManagement::Subscriptions::StripeScaPaymentAuthorize.new(@order).call! raise if @order.errors.any? - field_with_url(payment) if url?(field_with_url(payment)) + field_with_url(payment) end private @@ -28,14 +28,8 @@ module Checkout payment_method.is_a?(Spree::Gateway::StripeSCA) end - def url?(string) - return false if string.blank? - - string.starts_with?("http") - end - # Stripe::AuthorizeResponsePatcher patches the Stripe authorization response - # so that this field stores the redirect URL + # so that this field stores the redirect URL. It also verifies that it is a Stripe URL. def field_with_url(payment) payment.cvv_response_message end From c0b3fc301e371e68e7516dac83dd5bb5d2587f3a Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Wed, 3 Feb 2021 09:15:40 -0800 Subject: [PATCH 07/18] add email template to notify hub that auth is required --- app/mailers/payment_mailer.rb | 20 ++++++++++++++++--- .../authorization_required.html.haml | 2 ++ .../authorization_required.text.haml | 3 +++ config/locales/en.yml | 3 +++ .../stripe_sca_payment_authorize.rb | 1 + .../stripe_sca_payment_authorize_spec.rb | 6 ++++-- .../services/checkout/stripe_redirect_spec.rb | 1 + 7 files changed, 31 insertions(+), 5 deletions(-) create mode 100644 app/views/payment_mailer/authorization_required.html.haml create mode 100644 app/views/payment_mailer/authorization_required.text.haml diff --git a/app/mailers/payment_mailer.rb b/app/mailers/payment_mailer.rb index b0c4a4bbff..253d7b794a 100644 --- a/app/mailers/payment_mailer.rb +++ b/app/mailers/payment_mailer.rb @@ -7,8 +7,22 @@ class PaymentMailer < Spree::BaseMailer @payment = payment subject = I18n.t('spree.payment_mailer.authorize_payment.subject', distributor: @payment.order.distributor.name) - mail(to: payment.order.user.email, - from: from_address, - subject: subject) + I18n.with_locale valid_locale(@payment.order.user) do + mail(to: payment.order.user.email, + from: from_address, + subject: subject) + end + end + + def authorization_required(payment) + @payment = payment + shop_owner = @payment.order.distributor.owner + subject = I18n.t('spree.payment_mailer.authorization_required.subject', + order: @payment.order) + I18n.with_locale valid_locale(shop_owner) do + mail(to: shop_owner.email, + from: from_address, + subject: subject) + end end end diff --git a/app/views/payment_mailer/authorization_required.html.haml b/app/views/payment_mailer/authorization_required.html.haml new file mode 100644 index 0000000000..080a88de06 --- /dev/null +++ b/app/views/payment_mailer/authorization_required.html.haml @@ -0,0 +1,2 @@ += t('spree.payment_mailer.authorization_required.message', order_number: @payment.order.number) += link_to spree.edit_admin_order_url(@payment.order), spree.edit_admin_order_url(@payment.order) diff --git a/app/views/payment_mailer/authorization_required.text.haml b/app/views/payment_mailer/authorization_required.text.haml new file mode 100644 index 0000000000..b084ca2755 --- /dev/null +++ b/app/views/payment_mailer/authorization_required.text.haml @@ -0,0 +1,3 @@ += t('spree.payment_mailer.authorization_required.message', order_number: @payment.order.number) + += link_to spree.edit_admin_order_url(@payment.order), spree.edit_admin_order_url(@payment.order) diff --git a/config/locales/en.yml b/config/locales/en.yml index a39ac74bd8..d714884095 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3658,6 +3658,9 @@ See the %{link} to find out more about %{sitename}'s features and to start using authorize_payment: subject: "Please authorize your payment to %{distributor} on OFN" instructions: "Your payment of %{amount} to %{distributor} requires additional authentication. Please visit the following URL to authorize your payment:" + authorization_required: + subject: "A payment requires authorization from the customer" + message: "A payment for order %{order_number} requires additional authorization from the customer. The customer has been notified via email and the payment will appear as pending until it is authorized." shipment_mailer: shipped_email: dear_customer: "Dear Customer," diff --git a/engines/order_management/app/services/order_management/subscriptions/stripe_sca_payment_authorize.rb b/engines/order_management/app/services/order_management/subscriptions/stripe_sca_payment_authorize.rb index 87b55889e8..5f6dea721c 100644 --- a/engines/order_management/app/services/order_management/subscriptions/stripe_sca_payment_authorize.rb +++ b/engines/order_management/app/services/order_management/subscriptions/stripe_sca_payment_authorize.rb @@ -17,6 +17,7 @@ module OrderManagement if @payment.cvv_response_message.present? PaymentMailer.authorize_payment(@payment).deliver_now + PaymentMailer.authorization_required(@payment).deliver_now end @payment diff --git a/engines/order_management/spec/services/order_management/subscriptions/stripe_sca_payment_authorize_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/stripe_sca_payment_authorize_spec.rb index 4240ecc0e8..3bae4da0f4 100644 --- a/engines/order_management/spec/services/order_management/subscriptions/stripe_sca_payment_authorize_spec.rb +++ b/engines/order_management/spec/services/order_management/subscriptions/stripe_sca_payment_authorize_spec.rb @@ -63,18 +63,20 @@ module OrderManagement before do allow(PaymentMailer).to receive(:authorize_payment) { mail_mock } + allow(PaymentMailer).to receive(:authorization_required) { mail_mock } allow(payment).to receive(:authorize!) { payment.state = "pending" payment.cvv_response_message = "https://stripe.com/redirect" } end - it "adds sends an email requesting authorization" do + it "sends an email requesting authorization and an email notifying the shop owner" do payment_authorize.call! expect(order.errors.size).to eq 0 expect(PaymentMailer).to have_received(:authorize_payment) - expect(mail_mock).to have_received(:deliver_now) + expect(PaymentMailer).to have_received(:authorization_required) + expect(mail_mock).to have_received(:deliver_now).twice end end end diff --git a/spec/services/checkout/stripe_redirect_spec.rb b/spec/services/checkout/stripe_redirect_spec.rb index d85152ecbf..05b46dfe25 100644 --- a/spec/services/checkout/stripe_redirect_spec.rb +++ b/spec/services/checkout/stripe_redirect_spec.rb @@ -43,6 +43,7 @@ describe Checkout::StripeRedirect do stripe_payment.state = 'pending' true end + allow(stripe_payment.order).to receive(:distributor) { distributor } test_redirect_url = "http://stripe_auth_url/" allow(stripe_payment).to receive(:cvv_response_message).and_return(test_redirect_url) From 558b01896f069add02cbdc14aa2f35b8b3cdafea Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Wed, 3 Feb 2021 09:26:32 -0800 Subject: [PATCH 08/18] extract url helpers to helper module --- app/helpers/full_url_helper.rb | 17 +++++++++++++++++ app/models/spree/gateway/stripe_sca.rb | 12 ++---------- 2 files changed, 19 insertions(+), 10 deletions(-) create mode 100644 app/helpers/full_url_helper.rb diff --git a/app/helpers/full_url_helper.rb b/app/helpers/full_url_helper.rb new file mode 100644 index 0000000000..e3bd833346 --- /dev/null +++ b/app/helpers/full_url_helper.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module FullUrlHelper + def url_helpers + # This is how we can get the helpers with a usable root_url outside the controllers + Rails.application.routes.default_url_options = ActionMailer::Base.default_url_options + Rails.application.routes.url_helpers + end + + def full_checkout_path + URI.join(url_helpers.root_url, url_helpers.checkout_path).to_s + end + + def full_order_path(order) + URI.join(url_helpers.root_url, url_helpers.order_path(order)).to_s + end +end diff --git a/app/models/spree/gateway/stripe_sca.rb b/app/models/spree/gateway/stripe_sca.rb index 351adf019d..a2fcc94717 100644 --- a/app/models/spree/gateway/stripe_sca.rb +++ b/app/models/spree/gateway/stripe_sca.rb @@ -10,6 +10,8 @@ require 'active_merchant/billing/gateways/stripe_decorator' module Spree class Gateway class StripeSCA < Gateway + include FullUrlHelper + preference :enterprise_id, :integer validate :ensure_enterprise_selected @@ -145,16 +147,6 @@ module Spree errors.add(:stripe_account_owner, I18n.t(:error_required)) end - - def full_checkout_path - URI.join(url_helpers.root_url, url_helpers.checkout_path).to_s - end - - def url_helpers - # This is how we can get the helpers with a usable root_url outside the controllers - Rails.application.routes.default_url_options = ActionMailer::Base.default_url_options - Rails.application.routes.url_helpers - end end end end From 6e735739a5ce1bb21ac3c9030fb5553c78f39815 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Wed, 3 Feb 2021 09:33:08 -0800 Subject: [PATCH 09/18] after customer auth + redirect, process payment and clear cvv_response_message --- app/controllers/spree/orders_controller.rb | 13 +++++++++++++ app/models/spree/payment/processing.rb | 1 + 2 files changed, 14 insertions(+) diff --git a/app/controllers/spree/orders_controller.rb b/app/controllers/spree/orders_controller.rb index bf4bd814e9..92c7d4ad81 100644 --- a/app/controllers/spree/orders_controller.rb +++ b/app/controllers/spree/orders_controller.rb @@ -25,6 +25,7 @@ module Spree before_action :check_at_least_one_line_item, only: :update def show + process_payment_intent!(params["payment_intent"]) @order = Spree::Order.find_by!(number: params[:id]) end @@ -215,5 +216,17 @@ module Spree line_items_attributes: [:id, :quantity] ) end + + def process_payment_intent!(payment_intent) + return unless payment_intent&.starts_with?("pi_") + return unless order = Spree::Order.find_by!(number: params[:id]) + + last_payment = OrderPaymentFinder.new(order).last_payment + return unless last_payment&.state == "pending" && + last_payment&.response_code == payment_intent + + last_payment.update_attribute(:cvv_response_message, nil) + last_payment.complete! + end end end diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb index aabafeac37..d881389a65 100644 --- a/app/models/spree/payment/processing.rb +++ b/app/models/spree/payment/processing.rb @@ -15,6 +15,7 @@ module Spree def process_offline! return unless validate! + return if cvv_response_message.present? if payment_method.auto_capture? charge_offline! From 5f84cd9f13970216a64e2cfd87ac3746bfe98c1a Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Wed, 3 Feb 2021 09:37:03 -0800 Subject: [PATCH 10/18] add param to ScaAuthorize call for redirect url --- app/services/checkout/stripe_redirect.rb | 4 +++- .../subscriptions/stripe_sca_payment_authorize.rb | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/services/checkout/stripe_redirect.rb b/app/services/checkout/stripe_redirect.rb index 81ed0e9313..d75a3f4f33 100644 --- a/app/services/checkout/stripe_redirect.rb +++ b/app/services/checkout/stripe_redirect.rb @@ -3,6 +3,8 @@ # Provides the redirect path if a redirect to the payment gateway is needed module Checkout class StripeRedirect + include FullUrlHelper + def initialize(params, order) @params = params @order = order @@ -12,7 +14,7 @@ module Checkout def path return unless stripe_payment_method? - payment = OrderManagement::Subscriptions::StripeScaPaymentAuthorize.new(@order).call! + payment = OrderManagement::Subscriptions::StripeScaPaymentAuthorize.new(@order).call!(full_checkout_path) raise if @order.errors.any? field_with_url(payment) diff --git a/engines/order_management/app/services/order_management/subscriptions/stripe_sca_payment_authorize.rb b/engines/order_management/app/services/order_management/subscriptions/stripe_sca_payment_authorize.rb index 5f6dea721c..dabbd27e24 100644 --- a/engines/order_management/app/services/order_management/subscriptions/stripe_sca_payment_authorize.rb +++ b/engines/order_management/app/services/order_management/subscriptions/stripe_sca_payment_authorize.rb @@ -3,15 +3,17 @@ module OrderManagement module Subscriptions class StripeScaPaymentAuthorize + include FullUrlHelper + def initialize(order) @order = order @payment = OrderPaymentFinder.new(@order).last_pending_payment end - def call! + def call!(redirect_url = full_order_path(@order)) return unless @payment&.checkout? - @payment.authorize! + @payment.authorize!(redirect_url) @order.errors.add(:base, I18n.t('authorization_failure')) unless @payment.pending? From 5160140d8804599416a3508efd16558f157ba45e Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Wed, 3 Feb 2021 09:39:19 -0800 Subject: [PATCH 11/18] move SCA Auth module out of Subscriptions since we reuse it in Checkout --- app/jobs/subscription_confirm_job.rb | 2 +- app/services/checkout/stripe_redirect.rb | 2 +- .../{subscriptions => order}/stripe_sca_payment_authorize.rb | 2 +- .../stripe_sca_payment_authorize_spec.rb | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) rename engines/order_management/app/services/order_management/{subscriptions => order}/stripe_sca_payment_authorize.rb (96%) rename engines/order_management/spec/services/order_management/{subscriptions => order}/stripe_sca_payment_authorize_spec.rb (96%) diff --git a/app/jobs/subscription_confirm_job.rb b/app/jobs/subscription_confirm_job.rb index 29ecd6b6d0..6a91dfd7f5 100644 --- a/app/jobs/subscription_confirm_job.rb +++ b/app/jobs/subscription_confirm_job.rb @@ -84,7 +84,7 @@ class SubscriptionConfirmJob < ActiveJob::Base def authorize_payment!(order) return if order.subscription.payment_method.class != Spree::Gateway::StripeSCA - OrderManagement::Subscriptions::StripeScaPaymentAuthorize.new(order).call! + OrderManagement::Order::StripeScaPaymentAuthorize.new(order).call! end def send_confirmation_email(order) diff --git a/app/services/checkout/stripe_redirect.rb b/app/services/checkout/stripe_redirect.rb index d75a3f4f33..b0c0d42053 100644 --- a/app/services/checkout/stripe_redirect.rb +++ b/app/services/checkout/stripe_redirect.rb @@ -14,7 +14,7 @@ module Checkout def path return unless stripe_payment_method? - payment = OrderManagement::Subscriptions::StripeScaPaymentAuthorize.new(@order).call!(full_checkout_path) + payment = OrderManagement::Order::StripeScaPaymentAuthorize.new(@order).call!(full_checkout_path) raise if @order.errors.any? field_with_url(payment) diff --git a/engines/order_management/app/services/order_management/subscriptions/stripe_sca_payment_authorize.rb b/engines/order_management/app/services/order_management/order/stripe_sca_payment_authorize.rb similarity index 96% rename from engines/order_management/app/services/order_management/subscriptions/stripe_sca_payment_authorize.rb rename to engines/order_management/app/services/order_management/order/stripe_sca_payment_authorize.rb index dabbd27e24..474967ac5c 100644 --- a/engines/order_management/app/services/order_management/subscriptions/stripe_sca_payment_authorize.rb +++ b/engines/order_management/app/services/order_management/order/stripe_sca_payment_authorize.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module OrderManagement - module Subscriptions + module Order class StripeScaPaymentAuthorize include FullUrlHelper diff --git a/engines/order_management/spec/services/order_management/subscriptions/stripe_sca_payment_authorize_spec.rb b/engines/order_management/spec/services/order_management/order/stripe_sca_payment_authorize_spec.rb similarity index 96% rename from engines/order_management/spec/services/order_management/subscriptions/stripe_sca_payment_authorize_spec.rb rename to engines/order_management/spec/services/order_management/order/stripe_sca_payment_authorize_spec.rb index 3bae4da0f4..08e54e4297 100644 --- a/engines/order_management/spec/services/order_management/subscriptions/stripe_sca_payment_authorize_spec.rb +++ b/engines/order_management/spec/services/order_management/order/stripe_sca_payment_authorize_spec.rb @@ -3,11 +3,11 @@ require 'spec_helper' module OrderManagement - module Subscriptions + module Order describe StripeScaPaymentAuthorize do let(:order) { create(:order) } let(:payment_authorize) { - OrderManagement::Subscriptions::StripeScaPaymentAuthorize.new(order) + OrderManagement::Order::StripeScaPaymentAuthorize.new(order) } describe "#call!" do From 5d2c612839864f068a7a5dd611ff25fb1ae55bb1 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Wed, 3 Feb 2021 10:00:45 -0800 Subject: [PATCH 12/18] don't send emails if auth required during checkout --- app/services/checkout/stripe_redirect.rb | 3 ++- .../order/stripe_sca_payment_authorize.rb | 4 ++-- .../order/stripe_sca_payment_authorize_spec.rb | 9 +++++++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/app/services/checkout/stripe_redirect.rb b/app/services/checkout/stripe_redirect.rb index b0c0d42053..41bcbec0a1 100644 --- a/app/services/checkout/stripe_redirect.rb +++ b/app/services/checkout/stripe_redirect.rb @@ -14,7 +14,8 @@ module Checkout def path return unless stripe_payment_method? - payment = OrderManagement::Order::StripeScaPaymentAuthorize.new(@order).call!(full_checkout_path) + payment = OrderManagement::Order::StripeScaPaymentAuthorize.new(@order). + call!(full_checkout_path, false) raise if @order.errors.any? field_with_url(payment) diff --git a/engines/order_management/app/services/order_management/order/stripe_sca_payment_authorize.rb b/engines/order_management/app/services/order_management/order/stripe_sca_payment_authorize.rb index 474967ac5c..82a79b5139 100644 --- a/engines/order_management/app/services/order_management/order/stripe_sca_payment_authorize.rb +++ b/engines/order_management/app/services/order_management/order/stripe_sca_payment_authorize.rb @@ -10,14 +10,14 @@ module OrderManagement @payment = OrderPaymentFinder.new(@order).last_pending_payment end - def call!(redirect_url = full_order_path(@order)) + def call!(redirect_url = full_order_path(@order), send_emails = true) return unless @payment&.checkout? @payment.authorize!(redirect_url) @order.errors.add(:base, I18n.t('authorization_failure')) unless @payment.pending? - if @payment.cvv_response_message.present? + if send_emails && @payment.cvv_response_message.present? PaymentMailer.authorize_payment(@payment).deliver_now PaymentMailer.authorization_required(@payment).deliver_now end diff --git a/engines/order_management/spec/services/order_management/order/stripe_sca_payment_authorize_spec.rb b/engines/order_management/spec/services/order_management/order/stripe_sca_payment_authorize_spec.rb index 08e54e4297..81adcf72df 100644 --- a/engines/order_management/spec/services/order_management/order/stripe_sca_payment_authorize_spec.rb +++ b/engines/order_management/spec/services/order_management/order/stripe_sca_payment_authorize_spec.rb @@ -78,6 +78,15 @@ module OrderManagement expect(PaymentMailer).to have_received(:authorization_required) expect(mail_mock).to have_received(:deliver_now).twice end + + it "doesn't send emails if param is set to false" do + payment_authorize.call!(nil, false) + + expect(order.errors.size).to eq 0 + expect(PaymentMailer).to_not have_received(:authorize_payment) + expect(PaymentMailer).to_not have_received(:authorization_required) + expect(mail_mock).to_not have_received(:deliver_now) + end end end end From 9104ca72a9b52c94f506d200a5bfeace16280b8e Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Wed, 10 Feb 2021 09:36:35 -0800 Subject: [PATCH 13/18] refactor to descriptive method authorization_action_required? --- app/controllers/spree/admin/payments_controller.rb | 2 +- app/models/spree/credit_card.rb | 4 ++-- app/models/spree/payment.rb | 2 +- app/models/spree/payment/processing.rb | 6 +++++- .../order_management/order/stripe_sca_payment_authorize.rb | 2 +- 5 files changed, 10 insertions(+), 6 deletions(-) diff --git a/app/controllers/spree/admin/payments_controller.rb b/app/controllers/spree/admin/payments_controller.rb index b93034750a..bdc610e8a7 100644 --- a/app/controllers/spree/admin/payments_controller.rb +++ b/app/controllers/spree/admin/payments_controller.rb @@ -157,7 +157,7 @@ module Spree raise Spree::Core::GatewayError, I18n.t('authorization_failure') unless @payment.pending? - return unless @payment.cvv_response_message.present? + return unless @payment.authorization_action_required? PaymentMailer.authorize_payment(@payment).deliver_later raise Spree::Core::GatewayError, I18n.t('action_required') diff --git a/app/models/spree/credit_card.rb b/app/models/spree/credit_card.rb index 2a10e78794..5a4efea485 100644 --- a/app/models/spree/credit_card.rb +++ b/app/models/spree/credit_card.rb @@ -83,12 +83,12 @@ module Spree end def can_resend_authorization_email?(payment) - payment.pending? && payment.cvv_response_message.present? + payment.pending? && payment.authorization_action_required? end # Indicates whether its possible to capture the payment def can_capture?(payment) - return false if payment.cvv_response_message.present? + return false if payment.authorization_action_required? payment.pending? || payment.checkout? end diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index 3bd1bcd4a3..d61f8f5676 100644 --- a/app/models/spree/payment.rb +++ b/app/models/spree/payment.rb @@ -122,7 +122,7 @@ module Spree end def resend_authorization_email! - return unless cvv_response_message.present? + return unless authorization_action_required? PaymentMailer.authorize_payment(self).deliver_later end diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb index d881389a65..80d9d2dc0f 100644 --- a/app/models/spree/payment/processing.rb +++ b/app/models/spree/payment/processing.rb @@ -15,7 +15,7 @@ module Spree def process_offline! return unless validate! - return if cvv_response_message.present? + return if authorization_action_required? if payment_method.auto_capture? charge_offline! @@ -186,6 +186,10 @@ module Spree options end + def authorization_action_required? + cvv_response_message.present? + end + private def validate! diff --git a/engines/order_management/app/services/order_management/order/stripe_sca_payment_authorize.rb b/engines/order_management/app/services/order_management/order/stripe_sca_payment_authorize.rb index 82a79b5139..f955ed2be8 100644 --- a/engines/order_management/app/services/order_management/order/stripe_sca_payment_authorize.rb +++ b/engines/order_management/app/services/order_management/order/stripe_sca_payment_authorize.rb @@ -17,7 +17,7 @@ module OrderManagement @order.errors.add(:base, I18n.t('authorization_failure')) unless @payment.pending? - if send_emails && @payment.cvv_response_message.present? + if send_emails && @payment.authorization_action_required? PaymentMailer.authorize_payment(@payment).deliver_now PaymentMailer.authorization_required(@payment).deliver_now end From ef6d1a3afb74854d73b74d94cb0ab8d1ab923ce1 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Wed, 10 Feb 2021 11:06:01 -0800 Subject: [PATCH 14/18] refactor flag param to module --- app/jobs/subscription_confirm_job.rb | 4 +++- app/services/checkout/stripe_redirect.rb | 2 +- .../order/send_authorization_emails.rb | 16 ++++++++++++++++ .../order/stripe_sca_payment_authorize.rb | 7 +------ 4 files changed, 21 insertions(+), 8 deletions(-) create mode 100644 engines/order_management/app/services/order_management/order/send_authorization_emails.rb diff --git a/app/jobs/subscription_confirm_job.rb b/app/jobs/subscription_confirm_job.rb index 6a91dfd7f5..53adc4725b 100644 --- a/app/jobs/subscription_confirm_job.rb +++ b/app/jobs/subscription_confirm_job.rb @@ -84,7 +84,9 @@ class SubscriptionConfirmJob < ActiveJob::Base def authorize_payment!(order) return if order.subscription.payment_method.class != Spree::Gateway::StripeSCA - OrderManagement::Order::StripeScaPaymentAuthorize.new(order).call! + OrderManagement::Order::StripeScaPaymentAuthorize.new(order). + extend(OrderManagement::Order::SendAuthorizationEmails). + call! end def send_confirmation_email(order) diff --git a/app/services/checkout/stripe_redirect.rb b/app/services/checkout/stripe_redirect.rb index 41bcbec0a1..401f03b23c 100644 --- a/app/services/checkout/stripe_redirect.rb +++ b/app/services/checkout/stripe_redirect.rb @@ -15,7 +15,7 @@ module Checkout return unless stripe_payment_method? payment = OrderManagement::Order::StripeScaPaymentAuthorize.new(@order). - call!(full_checkout_path, false) + call!(full_checkout_path) raise if @order.errors.any? field_with_url(payment) diff --git a/engines/order_management/app/services/order_management/order/send_authorization_emails.rb b/engines/order_management/app/services/order_management/order/send_authorization_emails.rb new file mode 100644 index 0000000000..8e4d78fe49 --- /dev/null +++ b/engines/order_management/app/services/order_management/order/send_authorization_emails.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module OrderManagement + module Order + module SendAuthorizationEmails + def call!(redirect_url = full_order_path(@order)) + super(redirect_url) + + return unless @payment.authorization_action_required? + + PaymentMailer.authorize_payment(@payment).deliver_now + PaymentMailer.authorization_required(@payment).deliver_now + end + end + end +end diff --git a/engines/order_management/app/services/order_management/order/stripe_sca_payment_authorize.rb b/engines/order_management/app/services/order_management/order/stripe_sca_payment_authorize.rb index f955ed2be8..4739c1a357 100644 --- a/engines/order_management/app/services/order_management/order/stripe_sca_payment_authorize.rb +++ b/engines/order_management/app/services/order_management/order/stripe_sca_payment_authorize.rb @@ -10,18 +10,13 @@ module OrderManagement @payment = OrderPaymentFinder.new(@order).last_pending_payment end - def call!(redirect_url = full_order_path(@order), send_emails = true) + def call!(redirect_url = full_order_path(@order)) return unless @payment&.checkout? @payment.authorize!(redirect_url) @order.errors.add(:base, I18n.t('authorization_failure')) unless @payment.pending? - if send_emails && @payment.authorization_action_required? - PaymentMailer.authorize_payment(@payment).deliver_now - PaymentMailer.authorization_required(@payment).deliver_now - end - @payment end end From 891874995bd3cddf310b6e73ec851789227f4d33 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Wed, 10 Feb 2021 11:22:29 -0800 Subject: [PATCH 15/18] refactor ProcessPaymentIntent to service --- app/controllers/spree/orders_controller.rb | 14 +--------- app/services/process_payment_intent.rb | 30 ++++++++++++++++++++++ 2 files changed, 31 insertions(+), 13 deletions(-) create mode 100644 app/services/process_payment_intent.rb diff --git a/app/controllers/spree/orders_controller.rb b/app/controllers/spree/orders_controller.rb index 92c7d4ad81..345ca4a2c3 100644 --- a/app/controllers/spree/orders_controller.rb +++ b/app/controllers/spree/orders_controller.rb @@ -25,7 +25,7 @@ module Spree before_action :check_at_least_one_line_item, only: :update def show - process_payment_intent!(params["payment_intent"]) + ProcessPaymentIntent.new(params["payment_intent"], params[:id]).call! @order = Spree::Order.find_by!(number: params[:id]) end @@ -216,17 +216,5 @@ module Spree line_items_attributes: [:id, :quantity] ) end - - def process_payment_intent!(payment_intent) - return unless payment_intent&.starts_with?("pi_") - return unless order = Spree::Order.find_by!(number: params[:id]) - - last_payment = OrderPaymentFinder.new(order).last_payment - return unless last_payment&.state == "pending" && - last_payment&.response_code == payment_intent - - last_payment.update_attribute(:cvv_response_message, nil) - last_payment.complete! - end end end diff --git a/app/services/process_payment_intent.rb b/app/services/process_payment_intent.rb new file mode 100644 index 0000000000..a4932159ca --- /dev/null +++ b/app/services/process_payment_intent.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +class ProcessPaymentIntent + def initialize(payment_intent, order_number) + @payment_intent = payment_intent + @order = Spree::Order.find_by!(number: order_number) + @last_payment = OrderPaymentFinder.new(@order).last_payment + end + + def call! + return unless valid? + + @last_payment.update_attribute(:cvv_response_message, nil) + @last_payment.complete! + end + + private + + def valid? + @order.present? && valid_intent_string? && matches_last_payment? + end + + def valid_intent_string? + @payment_intent&.starts_with?("pi_") + end + + def matches_last_payment? + @last_payment&.state == "pending" && @last_payment&.response_code == @payment_intent + end +end From e2853b9afbecc9ef9bfbab497a705304b42b6016 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Thu, 11 Feb 2021 10:53:16 +0100 Subject: [PATCH 16/18] Do not load order twice The controller already does so, then, we can pass it to the service and avoid that extra round-trip to the DB and save some memory. Spree::Order is a rather bulky object (God object code smell perhaps) and it'll surely make a difference. --- app/controllers/spree/orders_controller.rb | 2 +- app/services/process_payment_intent.rb | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/controllers/spree/orders_controller.rb b/app/controllers/spree/orders_controller.rb index 345ca4a2c3..40c315400c 100644 --- a/app/controllers/spree/orders_controller.rb +++ b/app/controllers/spree/orders_controller.rb @@ -25,8 +25,8 @@ module Spree before_action :check_at_least_one_line_item, only: :update def show - ProcessPaymentIntent.new(params["payment_intent"], params[:id]).call! @order = Spree::Order.find_by!(number: params[:id]) + ProcessPaymentIntent.new(params["payment_intent"], @order).call! end def empty diff --git a/app/services/process_payment_intent.rb b/app/services/process_payment_intent.rb index a4932159ca..28b7636868 100644 --- a/app/services/process_payment_intent.rb +++ b/app/services/process_payment_intent.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true class ProcessPaymentIntent - def initialize(payment_intent, order_number) + def initialize(payment_intent, order) @payment_intent = payment_intent - @order = Spree::Order.find_by!(number: order_number) - @last_payment = OrderPaymentFinder.new(@order).last_payment + @order = order + @last_payment = OrderPaymentFinder.new(order).last_payment end def call! @@ -16,8 +16,10 @@ class ProcessPaymentIntent private + attr_reader :order + def valid? - @order.present? && valid_intent_string? && matches_last_payment? + order.present? && valid_intent_string? && matches_last_payment? end def valid_intent_string? From 23b0885f4baed3148f3cfd7971af00faf8121d8a Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Thu, 11 Feb 2021 10:55:45 +0100 Subject: [PATCH 17/18] Turn ivars into private attr_readers This makes them more changeable and robust. Ruby will raise NoMethodError on typos while it'll silently create a new ivar without us noticing. Also, in my experience, a reader method gives more room to future refactorings and eases testing because methods are easier to stub. --- app/services/process_payment_intent.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/services/process_payment_intent.rb b/app/services/process_payment_intent.rb index 28b7636868..4f2aeec7df 100644 --- a/app/services/process_payment_intent.rb +++ b/app/services/process_payment_intent.rb @@ -10,23 +10,23 @@ class ProcessPaymentIntent def call! return unless valid? - @last_payment.update_attribute(:cvv_response_message, nil) - @last_payment.complete! + last_payment.update_attribute(:cvv_response_message, nil) + last_payment.complete! end private - attr_reader :order + attr_reader :order, :payment_intent, :last_payment def valid? order.present? && valid_intent_string? && matches_last_payment? end def valid_intent_string? - @payment_intent&.starts_with?("pi_") + payment_intent&.starts_with?("pi_") end def matches_last_payment? - @last_payment&.state == "pending" && @last_payment&.response_code == @payment_intent + last_payment&.state == "pending" && last_payment&.response_code == payment_intent end end From 96a746175dd2215878a2781b893efaed077ca5c3 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Thu, 11 Feb 2021 10:36:50 -0800 Subject: [PATCH 18/18] update spec with new behavior --- .../order/stripe_sca_payment_authorize_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/engines/order_management/spec/services/order_management/order/stripe_sca_payment_authorize_spec.rb b/engines/order_management/spec/services/order_management/order/stripe_sca_payment_authorize_spec.rb index 81adcf72df..74a03dfcf4 100644 --- a/engines/order_management/spec/services/order_management/order/stripe_sca_payment_authorize_spec.rb +++ b/engines/order_management/spec/services/order_management/order/stripe_sca_payment_authorize_spec.rb @@ -70,8 +70,8 @@ module OrderManagement } end - it "sends an email requesting authorization and an email notifying the shop owner" do - payment_authorize.call! + it "sends an email requesting authorization and an email notifying the shop owner when requested" do + payment_authorize.extend(OrderManagement::Order::SendAuthorizationEmails).call! expect(order.errors.size).to eq 0 expect(PaymentMailer).to have_received(:authorize_payment) @@ -79,8 +79,8 @@ module OrderManagement expect(mail_mock).to have_received(:deliver_now).twice end - it "doesn't send emails if param is set to false" do - payment_authorize.call!(nil, false) + it "doesn't send emails by default" do + payment_authorize.call! expect(order.errors.size).to eq 0 expect(PaymentMailer).to_not have_received(:authorize_payment)