Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix(gocardless): Handle GoCardlessPro::ValidationError when creating payment #2817

Merged
merged 1 commit into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/jobs/invoices/payments/adyen_create_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class AdyenCreateJob < ApplicationJob
retry_on Faraday::ConnectionFailed, wait: :polynomially_longer, attempts: 6

def perform(invoice)
result = Invoices::Payments::AdyenService.new(invoice).create
result = Invoices::Payments::AdyenService.call(invoice)
result.raise_if_error!
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/jobs/invoices/payments/gocardless_create_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class GocardlessCreateJob < ApplicationJob
unique :until_executed

def perform(invoice)
result = Invoices::Payments::GocardlessService.new(invoice).create
result = Invoices::Payments::GocardlessService.call(invoice)
result.raise_if_error!
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/jobs/invoices/payments/stripe_create_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class StripeCreateJob < ApplicationJob
retry_on ::Stripe::APIConnectionError, wait: :polynomially_longer, attempts: 6

def perform(invoice)
result = Invoices::Payments::StripeService.new(invoice).create
result = Invoices::Payments::StripeService.call(invoice)
result.raise_if_error!
end
end
Expand Down
4 changes: 2 additions & 2 deletions app/services/invoices/payments/adyen_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ class AdyenService < BaseService
def initialize(invoice = nil)
@invoice = invoice

super(nil)
super
end

def create
def call
result.invoice = invoice
return result unless should_process_payment?

Expand Down
18 changes: 11 additions & 7 deletions app/services/invoices/payments/gocardless_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ def code
def initialize(invoice = nil)
@invoice = invoice

super(nil)
super
end

def create
def call
result.invoice = invoice
return result unless should_process_payment?

Expand Down Expand Up @@ -66,6 +66,15 @@ def create

result.service_failure!(code: e.code, message: e.message)
result
rescue GoCardlessPro::Error, GoCardlessPro::ValidationError => e
deliver_error_webhook(e)
update_invoice_payment_status(payment_status: :failed, deliver_webhook: false)

if e.is_a?(GoCardlessPro::ValidationError)
result
else
raise
end
end

def update_payment_status(provider_payment_id:, status:)
Expand Down Expand Up @@ -147,11 +156,6 @@ def create_gocardless_payment
'Idempotency-Key' => "#{invoice.id}/#{invoice.payment_attempts}"
}
)
rescue GoCardlessPro::Error => e
deliver_error_webhook(e)
update_invoice_payment_status(payment_status: :failed, deliver_webhook: false)

raise
end

def invoice_payment_status(payment_status)
Expand Down
4 changes: 2 additions & 2 deletions app/services/invoices/payments/stripe_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ class StripeService < BaseService
def initialize(invoice = nil)
@invoice = invoice

super(nil)
super
end

def create
def call
result.invoice = invoice
return result unless should_process_payment?

Expand Down
9 changes: 2 additions & 7 deletions spec/jobs/invoices/payments/adyen_create_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,13 @@
RSpec.describe Invoices::Payments::AdyenCreateJob, type: :job do
let(:invoice) { create(:invoice) }

let(:adyen_service) { instance_double(Invoices::Payments::AdyenService) }

it 'calls the stripe create service' do
allow(Invoices::Payments::AdyenService).to receive(:new)
allow(Invoices::Payments::AdyenService).to receive(:call)
.with(invoice)
.and_return(adyen_service)
allow(adyen_service).to receive(:create)
.and_return(BaseService::Result.new)

described_class.perform_now(invoice)

expect(Invoices::Payments::AdyenService).to have_received(:new)
expect(adyen_service).to have_received(:create)
expect(Invoices::Payments::AdyenService).to have_received(:call)
end
end
9 changes: 2 additions & 7 deletions spec/jobs/invoices/payments/gocardless_create_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,13 @@
RSpec.describe Invoices::Payments::GocardlessCreateJob, type: :job do
let(:invoice) { create(:invoice) }

let(:gocardless_service) { instance_double(Invoices::Payments::GocardlessService) }

it 'calls the stripe create service' do
allow(Invoices::Payments::GocardlessService).to receive(:new)
allow(Invoices::Payments::GocardlessService).to receive(:call)
.with(invoice)
.and_return(gocardless_service)
allow(gocardless_service).to receive(:create)
.and_return(BaseService::Result.new)

described_class.perform_now(invoice)

expect(Invoices::Payments::GocardlessService).to have_received(:new)
expect(gocardless_service).to have_received(:create)
expect(Invoices::Payments::GocardlessService).to have_received(:call)
end
end
9 changes: 2 additions & 7 deletions spec/jobs/invoices/payments/stripe_create_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,13 @@
RSpec.describe Invoices::Payments::StripeCreateJob, type: :job do
let(:invoice) { create(:invoice) }

let(:stripe_service) { instance_double(Invoices::Payments::StripeService) }

it 'calls the stripe create service' do
allow(Invoices::Payments::StripeService).to receive(:new)
allow(Invoices::Payments::StripeService).to receive(:call)
.with(invoice)
.and_return(stripe_service)
allow(stripe_service).to receive(:create)
.and_return(BaseService::Result.new)

described_class.perform_now(invoice)

expect(Invoices::Payments::StripeService).to have_received(:new)
expect(stripe_service).to have_received(:create)
expect(Invoices::Payments::StripeService).to have_received(:call)
end
end
20 changes: 10 additions & 10 deletions spec/services/invoices/payments/adyen_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
)
end

describe '#create' do
describe '#call' do
before do
adyen_payment_provider
adyen_customer
Expand All @@ -48,7 +48,7 @@
end

it 'creates an adyen payment' do
result = adyen_service.create
result = adyen_service.call

expect(result).to be_success

Expand All @@ -73,14 +73,14 @@
end

it_behaves_like 'syncs payment' do
let(:service_call) { adyen_service.create }
let(:service_call) { adyen_service.call }
end

context 'with no payment provider' do
let(:adyen_payment_provider) { nil }

it 'does not creates a adyen payment' do
result = adyen_service.create
result = adyen_service.call

expect(result).to be_success

Expand All @@ -105,7 +105,7 @@
end

it 'does not creates a adyen payment' do
result = adyen_service.create
result = adyen_service.call

expect(result).to be_success

Expand All @@ -124,7 +124,7 @@
before { adyen_customer.update!(provider_customer_id: nil) }

it 'does not creates a adyen payment' do
result = adyen_service.create
result = adyen_service.call

expect(result).to be_success

Expand All @@ -145,7 +145,7 @@
end

it 'delivers an error webhook' do
expect { adyen_service.create }.to enqueue_job(SendWebhookJob)
expect { adyen_service.call }.to enqueue_job(SendWebhookJob)
.with(
'invoice.payment_failure',
invoice,
Expand Down Expand Up @@ -182,7 +182,7 @@
it 'delivers an error webhook' do
allow(Invoices::Payments::DeliverErrorWebhookService).to receive(:call_async).and_call_original

adyen_service.create
adyen_service.call

expect(Invoices::Payments::DeliverErrorWebhookService).to have_received(:call_async)
expect(SendWebhookJob).to have_been_enqueued
Expand All @@ -204,7 +204,7 @@
invoice.update! status: :open, invoice_type: :credit
allow(Invoices::Payments::DeliverErrorWebhookService).to receive(:call_async).and_call_original

adyen_service.create
adyen_service.call

expect(Invoices::Payments::DeliverErrorWebhookService).to have_received(:call_async)
expect(SendWebhookJob).to have_been_enqueued
Expand All @@ -228,7 +228,7 @@
end

it 'delivers an error webhook' do
expect { adyen_service.create }.to enqueue_job(SendWebhookJob)
expect { adyen_service.call }.to enqueue_job(SendWebhookJob)
.with(
'invoice.payment_failure',
invoice,
Expand Down
18 changes: 9 additions & 9 deletions spec/services/invoices/payments/gocardless_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
)
end

describe '.create' do
describe '.call' do
before do
gocardless_payment_provider
gocardless_customer
Expand All @@ -53,7 +53,7 @@
end

it 'creates a gocardless payment' do
result = gocardless_service.create
result = gocardless_service.call

expect(result).to be_success

Expand All @@ -76,14 +76,14 @@
end

it_behaves_like 'syncs payment' do
let(:service_call) { gocardless_service.create }
let(:service_call) { gocardless_service.call }
end

context 'with no payment provider' do
let(:gocardless_payment_provider) { nil }

it 'does not creates a gocardless payment' do
result = gocardless_service.create
result = gocardless_service.call

expect(result).to be_success

Expand All @@ -108,7 +108,7 @@
end

it 'does not creates a gocardless payment' do
result = gocardless_service.create
result = gocardless_service.call

expect(result).to be_success

Expand All @@ -127,7 +127,7 @@
before { gocardless_customer.update!(provider_customer_id: nil) }

it 'does not creates a gocardless payment' do
result = gocardless_service.create
result = gocardless_service.call

expect(result).to be_success

Expand Down Expand Up @@ -161,7 +161,7 @@
it 'delivers an error webhook' do
allow(Invoices::Payments::DeliverErrorWebhookService).to receive(:call_async).and_call_original

expect { gocardless_service.create }.to raise_error(GoCardlessPro::Error)
expect { gocardless_service.call }.to raise_error(GoCardlessPro::Error)

expect(Invoices::Payments::DeliverErrorWebhookService).to have_received(:call_async)
expect(SendWebhookJob).to have_been_enqueued
Expand All @@ -183,7 +183,7 @@
invoice.update! status: :open, invoice_type: :credit
allow(Invoices::Payments::DeliverErrorWebhookService).to receive(:call_async).and_call_original

expect { gocardless_service.create }.to raise_error(GoCardlessPro::Error)
expect { gocardless_service.call }.to raise_error(GoCardlessPro::Error)

expect(Invoices::Payments::DeliverErrorWebhookService).to have_received(:call_async)
expect(SendWebhookJob).to have_been_enqueued
Expand Down Expand Up @@ -213,7 +213,7 @@
end

it "delivers an error webhook" do
result = gocardless_service.create
result = gocardless_service.call

aggregate_failures do
expect(result).not_to be_success
Expand Down
Loading
Loading