diff --git a/app/jobs/invoices/payments/adyen_create_job.rb b/app/jobs/invoices/payments/adyen_create_job.rb index 19ceced4a18..d20810833ec 100644 --- a/app/jobs/invoices/payments/adyen_create_job.rb +++ b/app/jobs/invoices/payments/adyen_create_job.rb @@ -12,8 +12,7 @@ class AdyenCreateJob < ApplicationJob def perform(invoice) # NOTE: Legacy job, kept only to avoid existing jobs - result = Invoices::Payments::AdyenService.call(invoice) - result.raise_if_error! + Invoices::Payments::CreateService.call!(invoice:, payment_provider: :adyen) end end end diff --git a/app/jobs/invoices/payments/gocardless_create_job.rb b/app/jobs/invoices/payments/gocardless_create_job.rb index 5c2e4cf176d..70f821ea797 100644 --- a/app/jobs/invoices/payments/gocardless_create_job.rb +++ b/app/jobs/invoices/payments/gocardless_create_job.rb @@ -10,8 +10,7 @@ class GocardlessCreateJob < ApplicationJob def perform(invoice) # NOTE: Legacy job, kept only to avoid existing jobs - result = Invoices::Payments::GocardlessService.call(invoice) - result.raise_if_error! + Invoices::Payments::CreateService.call!(invoice:, payment_provider: :gocardless) end end end diff --git a/app/jobs/invoices/payments/stripe_create_job.rb b/app/jobs/invoices/payments/stripe_create_job.rb index 1748606b3b0..413755e5374 100644 --- a/app/jobs/invoices/payments/stripe_create_job.rb +++ b/app/jobs/invoices/payments/stripe_create_job.rb @@ -9,12 +9,13 @@ class StripeCreateJob < ApplicationJob retry_on ::Stripe::RateLimitError, wait: :polynomially_longer, attempts: 6 retry_on ::Stripe::APIConnectionError, wait: :polynomially_longer, attempts: 6 + retry_on Invoices::Payments::ConnectionError, wait: :polynomially_longer, attempts: 6 + retry_on Invoices::Payments::RateLimitError, wait: :polynomially_longer, attempts: 6 def perform(invoice) # NOTE: Legacy job, kept only to avoid existing jobs - result = Invoices::Payments::StripeService.call(invoice) - result.raise_if_error! + Invoices::Payments::CreateService.call!(invoice:, payment_provider: :stripe) end end end diff --git a/app/services/invoices/payments/adyen_service.rb b/app/services/invoices/payments/adyen_service.rb index abacccd6709..4f8aef0283d 100644 --- a/app/services/invoices/payments/adyen_service.rb +++ b/app/services/invoices/payments/adyen_service.rb @@ -20,11 +20,6 @@ def call result.invoice = invoice return result unless should_process_payment? - unless invoice.total_amount_cents.positive? - update_invoice_payment_status(payment_status: :succeeded) - return result - end - increment_payment_attempts res = create_adyen_payment diff --git a/app/services/invoices/payments/create_service.rb b/app/services/invoices/payments/create_service.rb index f887257cb0f..84e13546e03 100644 --- a/app/services/invoices/payments/create_service.rb +++ b/app/services/invoices/payments/create_service.rb @@ -3,34 +3,39 @@ module Invoices module Payments class CreateService < BaseService + include Customers::PaymentProviderFinder + def initialize(invoice:, payment_provider: nil) @invoice = invoice - @payment_provider = payment_provider&.to_sym + @provider = payment_provider&.to_sym super end def call - # TODO: Refactor to avoid duplicated logic in provider services. - # Lago Payment related logic should be handled here and the - # payment execution should be delegated to the provider services - - case payment_provider - when :stripe - Invoices::Payments::StripeService.call(invoice) - when :gocardless - Invoices::Payments::GocardlessService.call(invoice) - when :adyen - Invoices::Payments::AdyenService.call(invoice) + result.invoice = invoice + return result unless should_process_payment? + + unless invoice.total_amount_cents.positive? + Invoices::UpdateService.call!( + invoice:, + params: {payment_status: :succeeded, ready_for_payment_processing: false}, + webhook_notification: true + ) + return result end + + # TODO(payments): Create a pending paymnent record with a DB uniqueness constraint on invoice_id + # and inject it to the payment services to avoid duplicated payments + ::PaymentProviders::CreatePaymentFactory.new_instance(provider:, invoice:).call end def call_async - return result unless payment_provider + return result unless provider - Invoices::Payments::CreateJob.perform_later(invoice:, payment_provider:) + Invoices::Payments::CreateJob.perform_later(invoice:, payment_provider: provider) - result.payment_provider = payment_provider + result.payment_provider = provider result end @@ -38,8 +43,16 @@ def call_async attr_reader :invoice - def payment_provider - @payment_provider ||= invoice.customer.payment_provider&.to_sym + delegate :customer, to: :invoice + + def provider + @provider ||= invoice.customer.payment_provider&.to_sym + end + + def should_process_payment? + return false if invoice.payment_succeeded? || invoice.voided? + + payment_provider(customer).present? end end end diff --git a/app/services/invoices/payments/gocardless_service.rb b/app/services/invoices/payments/gocardless_service.rb index 920588391f5..db14e575f40 100644 --- a/app/services/invoices/payments/gocardless_service.rb +++ b/app/services/invoices/payments/gocardless_service.rb @@ -33,11 +33,6 @@ def call result.invoice = invoice return result unless should_process_payment? - unless invoice.total_amount_cents.positive? - update_invoice_payment_status(payment_status: :succeeded) - return result - end - increment_payment_attempts gocardless_result = create_gocardless_payment diff --git a/app/services/invoices/payments/stripe_service.rb b/app/services/invoices/payments/stripe_service.rb index 0803f841f34..d7f0debf026 100644 --- a/app/services/invoices/payments/stripe_service.rb +++ b/app/services/invoices/payments/stripe_service.rb @@ -20,11 +20,6 @@ def call result.invoice = invoice return result unless should_process_payment? - unless invoice.total_amount_cents.positive? - update_invoice_payment_status(payment_status: :succeeded) - return result - end - increment_payment_attempts stripe_result = create_payment_intent diff --git a/app/services/payment_providers/create_payment_factory.rb b/app/services/payment_providers/create_payment_factory.rb new file mode 100644 index 00000000000..70b8ea22914 --- /dev/null +++ b/app/services/payment_providers/create_payment_factory.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module PaymentProviders + class CreatePaymentFactory + def self.new_instance(provider:, invoice:) + service_class(provider:).new(invoice) + end + + def self.service_class(provider:) + # TODO(payment): refactor Invoices::Payments::*Service#call + # into PaymentProviders::*::Payments::CreateService#call + case provider.to_sym + when :adyen + Invoices::Payments::AdyenService + when :gocardless + Invoices::Payments::GocardlessService + when :stripe + Invoices::Payments::StripeService + end + end + end +end diff --git a/spec/jobs/invoices/payments/adyen_create_job_spec.rb b/spec/jobs/invoices/payments/adyen_create_job_spec.rb index 9e6820fa44c..abe04f12f17 100644 --- a/spec/jobs/invoices/payments/adyen_create_job_spec.rb +++ b/spec/jobs/invoices/payments/adyen_create_job_spec.rb @@ -6,12 +6,12 @@ let(:invoice) { create(:invoice) } it 'calls the stripe create service' do - allow(Invoices::Payments::AdyenService).to receive(:call) - .with(invoice) + allow(Invoices::Payments::CreateService).to receive(:call!) + .with(invoice:, payment_provider: :adyen) .and_return(BaseService::Result.new) described_class.perform_now(invoice) - expect(Invoices::Payments::AdyenService).to have_received(:call) + expect(Invoices::Payments::CreateService).to have_received(:call!) end end diff --git a/spec/jobs/invoices/payments/gocardless_create_job_spec.rb b/spec/jobs/invoices/payments/gocardless_create_job_spec.rb index b05835ac76e..8f28e100c9f 100644 --- a/spec/jobs/invoices/payments/gocardless_create_job_spec.rb +++ b/spec/jobs/invoices/payments/gocardless_create_job_spec.rb @@ -6,12 +6,12 @@ let(:invoice) { create(:invoice) } it 'calls the stripe create service' do - allow(Invoices::Payments::GocardlessService).to receive(:call) - .with(invoice) + allow(Invoices::Payments::CreateService).to receive(:call!) + .with(invoice:, payment_provider: :gocardless) .and_return(BaseService::Result.new) described_class.perform_now(invoice) - expect(Invoices::Payments::GocardlessService).to have_received(:call) + expect(Invoices::Payments::CreateService).to have_received(:call!) end end diff --git a/spec/jobs/invoices/payments/stripe_create_job_spec.rb b/spec/jobs/invoices/payments/stripe_create_job_spec.rb index 359e8446df1..8ab6999168e 100644 --- a/spec/jobs/invoices/payments/stripe_create_job_spec.rb +++ b/spec/jobs/invoices/payments/stripe_create_job_spec.rb @@ -6,12 +6,12 @@ let(:invoice) { create(:invoice) } it 'calls the stripe create service' do - allow(Invoices::Payments::StripeService).to receive(:call) - .with(invoice) + allow(Invoices::Payments::CreateService).to receive(:call!) + .with(invoice:, payment_provider: :stripe) .and_return(BaseService::Result.new) described_class.perform_now(invoice) - expect(Invoices::Payments::StripeService).to have_received(:call) + expect(Invoices::Payments::CreateService).to have_received(:call!) end end diff --git a/spec/services/invoices/payments/adyen_service_spec.rb b/spec/services/invoices/payments/adyen_service_spec.rb index 08b60c50ec7..046688f3a0e 100644 --- a/spec/services/invoices/payments/adyen_service_spec.rb +++ b/spec/services/invoices/payments/adyen_service_spec.rb @@ -93,33 +93,6 @@ end end - context 'with 0 amount' do - let(:invoice) do - create( - :invoice, - organization:, - customer:, - total_amount_cents: 0, - currency: 'EUR' - ) - end - - it 'does not creates a adyen payment' do - result = adyen_service.call - - expect(result).to be_success - - aggregate_failures do - expect(result.invoice).to eq(invoice) - expect(result.payment).to be_nil - - expect(result.invoice).to be_payment_succeeded - - expect(payments_api).not_to have_received(:payments) - end - end - end - context 'when customer does not have a provider customer id' do before { adyen_customer.update!(provider_customer_id: nil) } diff --git a/spec/services/invoices/payments/create_service_spec.rb b/spec/services/invoices/payments/create_service_spec.rb index 5233c9e499a..ae4a8ab0880 100644 --- a/spec/services/invoices/payments/create_service_spec.rb +++ b/spec/services/invoices/payments/create_service_spec.rb @@ -3,50 +3,121 @@ require 'rails_helper' RSpec.describe Invoices::Payments::CreateService, type: :service do - subject(:create_service) { described_class.new(invoice:, payment_provider:) } + subject(:create_service) { described_class.new(invoice:, payment_provider: provider) } - let(:invoice) { create(:invoice, customer:, organization: customer.organization) } - let(:customer) { create(:customer, payment_provider:) } - let(:payment_provider) { 'stripe' } + let(:organization) { create(:organization) } + let(:invoice) { create(:invoice, customer:, organization:, total_amount_cents: 100) } + let(:customer) { create(:customer, organization:, payment_provider: provider, payment_provider_code:) } + let(:provider) { 'stripe' } + let(:payment_provider_code) { 'stripe_1' } + let(:payment_provider) { create(:stripe_provider, code: payment_provider_code, organization:) } describe '#call' do let(:result) { BaseService::Result.new } + let(:provider_class) { Invoices::Payments::StripeService } + let(:provider_service) { instance_double(provider_class) } - it 'calls the stripe service' do - allow(Invoices::Payments::StripeService) - .to receive(:call).with(invoice) + before do + payment_provider + + allow(provider_class) + .to receive(:new).with(invoice) + .and_return(provider_service) + allow(provider_service).to receive(:call) .and_return(result) + end + it 'calls the stripe service' do create_service.call - expect(Invoices::Payments::StripeService).to have_received(:call).with(invoice) + expect(provider_class).to have_received(:new).with(invoice) + expect(provider_service).to have_received(:call) end context 'with gocardless payment provider' do - let(:payment_provider) { 'gocardless' } + let(:provider) { 'gocardless' } + let(:provider_class) { Invoices::Payments::GocardlessService } + let(:payment_provider) { create(:gocardless_provider, code: payment_provider_code, organization:) } it 'calls the gocardless service' do - allow(Invoices::Payments::GocardlessService) - .to receive(:call).with(invoice) - .and_return(result) - create_service.call - expect(Invoices::Payments::GocardlessService).to have_received(:call).with(invoice) + expect(provider_class).to have_received(:new).with(invoice) + expect(provider_service).to have_received(:call) end end context 'with adyen payment provider' do - let(:payment_provider) { 'adyen' } + let(:provider) { 'adyen' } + let(:provider_class) { Invoices::Payments::AdyenService } + let(:payment_provider) { create(:adyen_provider, code: payment_provider_code, organization:) } it 'calls the adyen service' do - allow(Invoices::Payments::AdyenService) - .to receive(:call).with(invoice) - .and_return(result) - create_service.call - expect(Invoices::Payments::AdyenService).to have_received(:call).with(invoice) + expect(provider_class).to have_received(:new).with(invoice) + expect(provider_service).to have_received(:call) + end + end + + context 'when invoice is payment_succeeded' do + before { invoice.payment_succeeded! } + + it 'does not creates a payment' do + result = create_service.call + + expect(result).to be_success + expect(result.invoice).to eq(invoice) + expect(result.payment).to be_nil + expect(provider_class).not_to have_received(:new) + end + end + + context 'when invoice is voided' do + before { invoice.voided! } + + it 'does not creates a payment' do + result = create_service.call + + expect(result).to be_success + expect(result.invoice).to eq(invoice) + expect(result.payment).to be_nil + expect(provider_class).not_to have_received(:new) + end + end + + context 'when invoice amount is 0' do + let(:invoice) do + create( + :invoice, + organization:, + customer:, + total_amount_cents: 0, + currency: 'EUR' + ) + end + + it 'does not creates a payment' do + result = create_service.call + + expect(result).to be_success + expect(result.invoice).to eq(invoice) + expect(result.payment).to be_nil + expect(result.invoice).to be_payment_succeeded + expect(provider_class).not_to have_received(:new) + end + end + + context 'with missing payment provider' do + let(:payment_provider) { nil } + + it 'does not creates a payment' do + result = create_service.call + + expect(result).to be_success + expect(result.invoice).to eq(invoice) + expect(result.payment).to be_nil + expect(provider_class).not_to have_received(:new) end end end @@ -59,7 +130,7 @@ end context 'with gocardless payment provider' do - let(:payment_provider) { 'gocardless' } + let(:provider) { 'gocardless' } it 'enqueues a job to create a gocardless payment' do expect { create_service.call_async } @@ -69,7 +140,7 @@ end context 'with adyen payment provider' do - let(:payment_provider) { 'adyen' } + let(:provider) { 'adyen' } it 'enqueues a job to create a gocardless payment' do expect { create_service.call_async } diff --git a/spec/services/invoices/payments/gocardless_service_spec.rb b/spec/services/invoices/payments/gocardless_service_spec.rb index 7ba3ea73ea1..241e2e13a36 100644 --- a/spec/services/invoices/payments/gocardless_service_spec.rb +++ b/spec/services/invoices/payments/gocardless_service_spec.rb @@ -96,33 +96,6 @@ end end - context 'with 0 amount' do - let(:invoice) do - create( - :invoice, - organization:, - customer:, - total_amount_cents: 0, - currency: 'EUR' - ) - end - - it 'does not creates a gocardless payment' do - result = gocardless_service.call - - expect(result).to be_success - - aggregate_failures do - expect(result.invoice).to eq(invoice) - expect(result.payment).to be_nil - - expect(result.invoice).to be_payment_succeeded - - expect(gocardless_payments_service).not_to have_received(:create) - end - end - end - context 'when customer does not have a provider customer id' do before { gocardless_customer.update!(provider_customer_id: nil) } diff --git a/spec/services/invoices/payments/stripe_service_spec.rb b/spec/services/invoices/payments/stripe_service_spec.rb index dd26e249741..c2ac7d8c002 100644 --- a/spec/services/invoices/payments/stripe_service_spec.rb +++ b/spec/services/invoices/payments/stripe_service_spec.rb @@ -107,33 +107,6 @@ end end - context 'with 0 amount' do - let(:invoice) do - create( - :invoice, - organization:, - customer:, - total_amount_cents: 0, - currency: 'EUR' - ) - end - - it 'does not creates a stripe payment' do - result = stripe_service.call - - expect(result).to be_success - - aggregate_failures do - expect(result.invoice).to eq(invoice) - expect(result.payment).to be_nil - - expect(result.invoice).to be_payment_succeeded - - expect(Stripe::PaymentIntent).not_to have_received(:create) - end - end - end - context 'when customer does not have a provider customer id' do before { stripe_customer.update!(provider_customer_id: nil) } diff --git a/spec/services/payment_providers/create_payment_factory_spec.rb b/spec/services/payment_providers/create_payment_factory_spec.rb new file mode 100644 index 00000000000..c4e6d1c2208 --- /dev/null +++ b/spec/services/payment_providers/create_payment_factory_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe PaymentProviders::CreatePaymentFactory, type: :service do + subject(:new_instance) { described_class.new_instance(provider:, invoice:) } + + let(:provider) { "stripe" } + let(:invoice) { create(:invoice) } + + describe ".new_instance" do + it "creates an instance of the stripe service" do + expect(new_instance).to be_instance_of(Invoices::Payments::StripeService) + end + + context "when provider is adyen" do + let(:provider) { "adyen" } + + it "creates an instance of the adyen service" do + expect(new_instance).to be_instance_of(Invoices::Payments::AdyenService) + end + end + + context "when provider is gocardless" do + let(:provider) { "gocardless" } + + it "creates an instance of the gocardless service" do + expect(new_instance).to be_instance_of(Invoices::Payments::GocardlessService) + end + end + end +end