misc(Payment): Refactor payment processing preconditions (#2935)

## Context

This PR follow #2930 in the
refactor of the payment processing.

The main goal of this refactor will be to:
- **Avoid double payment at all cost**
- Avoid code duplication between payment processor integration to make
maintenance easier

## Description

This PR:
- Updates the legacy `Invoices::Payments::*CreateJob` to delegate the
logic to the new `Invoices::Payments::CreateService#call` rather than
directly to `Invoices::Payments::*Service#call`
- Share the precondition on invoice status, payment provider presence
and invoice amount into the `Invoices::Payments::CreateService#call` to
avoid duplication

In a next step this last method will be updated to create a payment
before delegating the payment processing to the providers and to ensure
that no processing payment already exists.

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

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

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

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

app/services/invoices/payments/create_service.rb

@@ -3,43 +3,56 @@
 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
 
       private
 
       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

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

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

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

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

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

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

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) }