misc(PaymentRequest): Cleanup code

app/jobs/invoices/payments/adyen_create_job.rb

@@ -10,7 +10,7 @@ class AdyenCreateJob < ApplicationJob
       retry_on Faraday::ConnectionFailed, wait: :polynomially_longer, attempts: 6
 
       def perform(invoice)
-        # NOTE: Legacy job, kept only to avoid existing jobs
+        # NOTE: Legacy job, kept only to avoid faileure with existing jobs
 
         Invoices::Payments::CreateService.call!(invoice:, payment_provider: :adyen)
       end

app/jobs/invoices/payments/gocardless_create_job.rb

@@ -8,7 +8,7 @@ class GocardlessCreateJob < ApplicationJob
       unique :until_executed, on_conflict: :log
 
       def perform(invoice)
-        # NOTE: Legacy job, kept only to avoid existing jobs
+        # NOTE: Legacy job, kept only to avoid faileure with existing jobs
 
         Invoices::Payments::CreateService.call!(invoice:, payment_provider: :gocardless)
       end

app/jobs/invoices/payments/stripe_create_job.rb

@@ -13,7 +13,7 @@ class StripeCreateJob < ApplicationJob
       retry_on Invoices::Payments::RateLimitError, wait: :polynomially_longer, attempts: 6
 
       def perform(invoice)
-        # NOTE: Legacy job, kept only to avoid existing jobs
+        # NOTE: Legacy job, kept only to avoid faileure with existing jobs
 
         Invoices::Payments::CreateService.call!(invoice:, payment_provider: :stripe)
       end

app/jobs/payment_requests/payments/adyen_create_job.rb

@@ -10,6 +10,8 @@ class AdyenCreateJob < ApplicationJob
       retry_on Faraday::ConnectionFailed, wait: :polynomially_longer, attempts: 6
 
       def perform(payable)
+        # NOTE: Legacy job, kept only to avoid faileure with existing jobs
+
         PaymentRequests::Payments::CreateService.call!(payable:, payment_provider: 'adyen')
       end
     end

app/jobs/payment_requests/payments/gocardless_create_job.rb

@@ -8,6 +8,8 @@ class GocardlessCreateJob < ApplicationJob
       unique :until_executed, on_conflict: :log
 
       def perform(payable)
+        # NOTE: Legacy job, kept only to avoid faileure with existing jobs
+
         PaymentRequests::Payments::CreateService.call!(payable:, payment_provider: 'gocardless')
       end
     end

app/jobs/payment_requests/payments/stripe_create_job.rb

@@ -11,6 +11,8 @@ class StripeCreateJob < ApplicationJob
       retry_on ::Stripe::APIConnectionError, wait: :polynomially_longer, attempts: 6
 
       def perform(payable)
+        # NOTE: Legacy job, kept only to avoid faileure with existing jobs
+
         PaymentRequests::Payments::CreateService.call!(payable:, payment_provider: 'stripe')
       end
     end

app/services/invoices/payments/create_service.rb

@@ -34,11 +34,11 @@ def call
           payment_provider_id: current_payment_provider.id,
           payment_provider_customer_id: current_payment_provider_customer.id,
           amount_cents: invoice.total_amount_cents,
-          amount_currency: invoice.currency
+          amount_currency: invoice.currency,
+          status: "pending"
         ).find_or_create_by!(
           payable: invoice,
-          payable_payment_status: "pending",
-          status: "pending"
+          payable_payment_status: "pending"
         )
 
         result.payment = payment
@@ -56,21 +56,15 @@ def call
         ).call!
 
         payment_status = payment_result.payment.payable_payment_status
-        update_invoice_payment_status(
-          payment_status: (payment_status == "processing") ? :pending : payment_status,
-          processing: payment_status == "processing"
-        )
+        update_invoice_payment_status(payment_status:)
 
         Integrations::Aggregator::Payments::CreateJob.perform_later(payment:) if result.payment.should_sync_payment?
 
         result
       rescue BaseService::ServiceFailure => e
         result.payment = e.result.payment
         deliver_error_webhook(e.result)
-
-        if e.result.payment.payable_payment_status&.to_sym != :pending
-          update_invoice_payment_status(payment_status: e.result.payment.payable_payment_status)
-        end
+        update_invoice_payment_status(payment_status: e.result.payment.payable_payment_status)
 
         # Some errors should be investigated and need to be raised
         raise if e.result.reraise
@@ -113,13 +107,13 @@ def current_payment_provider_customer
           .find_by(payment_provider_id: current_payment_provider.id)
       end
 
-      def update_invoice_payment_status(payment_status:, processing: false)
+      def update_invoice_payment_status(payment_status:)
         Invoices::UpdateService.call!(
           invoice: invoice,
           params: {
-            payment_status:,
             # NOTE: A proper `processing` payment status should be introduced for invoices
-            ready_for_payment_processing: !processing && payment_status.to_sym != :failed # TODO
+            payment_status: (payment_status.to_s == "processing") ? :pending : payment_status,
+            ready_for_payment_processing: %w[pending failed].include?(payment_status.to_s)
           },
           webhook_notification: payment_status.to_sym == :succeeded
         )

app/services/payment_providers/create_payment_factory.rb

@@ -3,7 +3,7 @@
 module PaymentProviders
   class CreatePaymentFactory
     def self.new_instance(provider:, payment:, reference:, metadata:)
-      service_class(provider:).new(payment:, referehce:, metadata:)
+      service_class(provider:).new(payment:, reference:, metadata:)
     end
 
     def self.service_class(provider:)

app/services/payment_requests/payments/create_service.rb

@@ -36,11 +36,11 @@ def call
           payment_provider_id: current_payment_provider.id,
           payment_provider_customer_id: current_payment_provider_customer.id,
           amount_cents: payable.total_amount_cents,
-          amount_currency: payable.currency
+          amount_currency: payable.currency,
+          status: "pending"
         ).find_or_create_by!(
           payable:,
-          payable_payment_status: "pending",
-          status: "pending"
+          payable_payment_status: "pending"
         )
 
         result.payment = payment
@@ -56,23 +56,16 @@ def call
           }
         ).call!
 
-        # TODO: payment status should be failed and payable_payment_status should be pending
-        # Keep payment in a pending state. Used manly for `amount_too_small` in stripe service
-        return result if payment.payable_payment_status.nil?
-
-        update_payable_payment_status(payment_status: payment.payable_payment_status)
-        update_invoices_payment_status(payment_status: payment.payable_payment_status)
+        update_payable_payment_status(payment_status: payment_result.payment.payable_payment_status)
+        update_invoices_payment_status(payment_status: payment_result.payment.payable_payment_status)
 
         PaymentRequestMailer.with(payment_request: payable).requested.deliver_later if payable.payment_failed?
-        Integrations::Aggregator::Payments::CreateJob.perform_later(payment:) if result.payment.should_sync_payment?
 
         result
       rescue BaseService::ServiceFailure => e
+        result.payment = e.result.payment
         deliver_error_webhook(e.result)
-
-        if e.result.payment.payable_payment_status.present?
-          update_payable_payment_status(payment_status: e.result.payment.payable_payment_status)
-        end
+        update_payable_payment_status(payment_status: e.result.payment.payable_payment_status)
 
         # Some errors should be investigated and need to be raised
         raise if e.result.reraise
@@ -93,7 +86,7 @@ def call_async
 
       attr_reader :payable
 
-      delegate :customer, to: :payable
+      delegate :customer, :organization, to: :payable
 
       def provider
         @provider ||= payable.customer.payment_provider&.to_sym
@@ -119,7 +112,8 @@ def update_payable_payment_status(payment_status:)
         PaymentRequests::UpdateService.call!(
           payable: payable,
           params: {
-            payment_status:,
+            # NOTE: A proper `processing` payment status should be introduced for invoices
+            payment_status: (payment_status.to_s == "processing") ? :pending : payment_status,
             ready_for_payment_processing: payment_status.to_sym == :failed
           },
           webhook_notification: payment_status.to_sym == :succeeded
@@ -128,14 +122,15 @@ def update_payable_payment_status(payment_status:)
 
       def update_invoices_payment_status(payment_status:)
         payable.invoices.each do |invoice|
-          Invoices::UpdateService.call(
+          Invoices::UpdateService.call!(
             invoice:,
             params: {
-              payment_status:,
+              # NOTE: A proper `processing` payment status should be introduced for invoices
+              payment_status: (payment_status.to_s == "processing") ? :pending : payment_status,
               ready_for_payment_processing: payment_status.to_sym == :failed
             },
             webhook_notification: payment_status.to_sym == :succeeded
-          ).raise_if_error!
+          )
         end
       end
 

spec/services/invoices/payments/create_service_spec.rb

@@ -27,8 +27,17 @@
       provider_customer
 
       allow(provider_class)
-        .to receive(:new).with(payment: an_instance_of(Payment))
-        .and_return(provider_service)
+        .to receive(:new)
+        .with(
+          payment: an_instance_of(Payment),
+          reference: "#{invoice.organization.name} - Invoice #{invoice.number}",
+          metadata: {
+            lago_invoice_id: invoice.id,
+            lago_customer_id: customer.id,
+            invoice_issuing_date: invoice.issuing_date.iso8601,
+            invoice_type: invoice.invoice_type
+          }
+        ).and_return(provider_service)
       allow(provider_service).to receive(:call!)
         .and_return(result)
     end
@@ -69,7 +78,7 @@
       it "calls the gocardless service" do
         create_service.call
 
-        expect(provider_class).to have_received(:new).with(payment: an_instance_of(Payment))
+        expect(provider_class).to have_received(:new)
         expect(provider_service).to have_received(:call!)
       end
     end
@@ -215,16 +224,18 @@
       end
 
       context "when invoice is credit? and open?" do
+        let(:invoice) { create(:invoice, :credit, :open, customer:, organization:, total_amount_cents: 100) }
+        let(:wallet_transaction) { create(:wallet_transaction) }
+        let(:fee) { create(:fee, fee_type: :credit, invoice: invoice, invoiceable: wallet_transaction) }
+
         before do
+          fee
+
           allow(Invoices::Payments::DeliverErrorWebhookService)
             .to receive(:call_async).and_call_original
         end
 
         it "delivers an error webhook" do
-          wallet_transaction = create(:wallet_transaction)
-          create(:fee, fee_type: :credit, invoice: invoice, invoiceable: wallet_transaction)
-          invoice.update!(status: :open, invoice_type: :credit)
-
           expect { create_service.call }.to raise_error(BaseService::ServiceFailure)
 
           expect(Invoices::Payments::DeliverErrorWebhookService).to have_received(:call_async)

spec/services/payment_providers/adyen/payments/create_service_spec.rb

@@ -3,7 +3,7 @@
 require "rails_helper"
 
 RSpec.describe PaymentProviders::Adyen::Payments::CreateService, type: :service do
-  subject(:create_service) { described_class.new(payment:) }
+  subject(:create_service) { described_class.new(payment:, reference:, metadata:) }
 
   let(:customer) { create(:customer, payment_provider_code: code) }
   let(:organization) { customer.organization }
@@ -17,6 +17,15 @@
   let(:payments_response) { generate(:adyen_payments_response) }
   let(:payment_methods_response) { generate(:adyen_payment_methods_response) }
   let(:code) { "adyen_1" }
+  let(:reference) { "organization.name - Invoice #{invoice.number}" }
+  let(:metadata) do
+    {
+      lago_customer_id: customer.id,
+      lago_invoice_id: invoice.id,
+      invoice_issuing_date: invoice.issuing_date.iso8601,
+      invoice_type: invoice.invoice_type
+    }
+  end
 
   let(:invoice) do
     create(

spec/services/payment_providers/create_payment_factory_spec.rb

@@ -3,7 +3,7 @@
 require "rails_helper"
 
 RSpec.describe PaymentProviders::CreatePaymentFactory, type: :service do
-  subject(:new_instance) { described_class.new_instance(provider:, payment:) }
+  subject(:new_instance) { described_class.new_instance(provider:, payment:, reference: '', metadata: {}) }
 
   let(:provider) { "stripe" }
   let(:payment) { create(:payment) }

spec/services/payment_providers/gocardless/payments/create_service_spec.rb

@@ -3,7 +3,7 @@
 require "rails_helper"
 
 RSpec.describe PaymentProviders::Gocardless::Payments::CreateService, type: :service do
-  subject(:create_service) { described_class.new(payment:) }
+  subject(:create_service) { described_class.new(payment:, reference:, metadata:) }
 
   let(:customer) { create(:customer, payment_provider_code: code) }
   let(:organization) { customer.organization }
@@ -14,6 +14,15 @@
   let(:gocardless_mandates_service) { instance_double(GoCardlessPro::Services::MandatesService) }
   let(:gocardless_list_response) { instance_double(GoCardlessPro::ListResponse) }
   let(:code) { "gocardless_1" }
+  let(:reference) { "organization.name - Invoice #{invoice.number}" }
+  let(:metadata) do
+    {
+      lago_customer_id: customer.id,
+      lago_invoice_id: invoice.id,
+      invoice_issuing_date: invoice.issuing_date.iso8601,
+      invoice_type: invoice.invoice_type
+    }
+  end
 
   let(:invoice) do
     create(

spec/services/payment_providers/stripe/payments/create_service_spec.rb

@@ -3,13 +3,22 @@
 require "rails_helper"
 
 RSpec.describe PaymentProviders::Stripe::Payments::CreateService, type: :service do
-  subject(:create_service) { described_class.new(payment:) }
+  subject(:create_service) { described_class.new(payment:, reference:, metadata:) }
 
   let(:customer) { create(:customer, payment_provider_code: code) }
   let(:organization) { customer.organization }
   let(:stripe_payment_provider) { create(:stripe_provider, organization:, code:) }
   let(:stripe_customer) { create(:stripe_customer, customer:, payment_method_id: "pm_123456", payment_provider: stripe_payment_provider) }
   let(:code) { "stripe_1" }
+  let(:reference) { "organization.name - Invoice #{invoice.number}" }
+  let(:metadata) do
+    {
+      lago_customer_id: customer.id,
+      lago_invoice_id: invoice.id,
+      invoice_issuing_date: invoice.issuing_date.iso8601,
+      invoice_type: invoice.invoice_type
+    }
+  end
 
   let(:invoice) do
     create(
@@ -311,13 +320,8 @@
           off_session: true,
           return_url: create_service.__send__(:success_redirect_url),
           error_on_requires_action: true,
-          description: create_service.__send__(:description),
-          metadata: {
-            lago_customer_id: customer.id,
-            lago_invoice_id: invoice.id,
-            invoice_issuing_date: invoice.issuing_date.iso8601,
-            invoice_type: invoice.invoice_type
-          }
+          description: reference,
+          metadata: metadata
         }
       end
 
@@ -337,14 +341,5 @@
         end
       end
     end
-
-    context "with #description" do
-      let(:description_call) { create_service.__send__(:description) }
-      let(:description) { "#{organization.name} - Invoice #{invoice.number}" }
-
-      it "returns the description" do
-        expect(description_call).to eq(description)
-      end
-    end
   end
 end