fix(payment): Ensure payment status is checkd as a string (#3104)

## Description

This PR is related to #3088 it
ensure that none of the payment `status`, `payable_payment_status` and
related payables's `payment_status` are updated if an unknown status is
received from a payment provider

app/models/payment.rb

@@ -23,7 +23,7 @@ class Payment < ApplicationRecord
 
   delegate :customer, to: :payable
 
-  enum :payable_payment_status, PAYABLE_PAYMENT_STATUS.map { |s| [s, s] }.to_h
+  enum :payable_payment_status, PAYABLE_PAYMENT_STATUS.map { |s| [s, s] }.to_h, validate: {allow_nil: true}
 
   scope :for_organization, lambda { |organization|
     payables_join = ActiveRecord::Base.sanitize_sql_array([

app/services/invoices/payments/adyen_service.rb

@@ -27,14 +27,14 @@ def update_payment_status(provider_payment_id:, status:, metadata: {})
         payment.status = status
 
         payable_payment_status = payment.payment_provider&.determine_payment_status(payment.status)
-        if Payment::PAYABLE_PAYMENT_STATUS.include?(payable_payment_status)
-          payment.payable_payment_status = payable_payment_status
-        end
+        payment.payable_payment_status = payable_payment_status
         payment.save!
 
         update_invoice_payment_status(payment_status: payable_payment_status)
 
         result
+      rescue ActiveRecord::RecordInvalid => e
+        result.record_validation_failure!(record: e.record)
       rescue BaseService::FailedResult => e
         result.fail_with_error!(e)
       end

app/services/invoices/payments/cashfree_service.rb

@@ -26,14 +26,14 @@ def update_payment_status(organization_id:, status:, cashfree_payment:)
         payment.status = status
 
         payable_payment_status = payment.payment_provider&.determine_payment_status(payment.status)
-        if Payment::PAYABLE_PAYMENT_STATUS.include?(payable_payment_status)
-          payment.payable_payment_status = payable_payment_status
-        end
+        payment.payable_payment_status = payable_payment_status
         payment.save!
 
         update_invoice_payment_status(payment_status: payable_payment_status)
 
         result
+      rescue ActiveRecord::RecordInvalid => e
+        result.record_validation_failure!(record: e.record)
       rescue BaseService::FailedResult => e
         result.fail_with_error!(e)
       end

app/services/invoices/payments/gocardless_service.rb

@@ -22,14 +22,14 @@ def update_payment_status(provider_payment_id:, status:)
         payment.status = status
 
         payable_payment_status = payment.payment_provider&.determine_payment_status(payment.status)
-        if Payment::PAYABLE_PAYMENT_STATUS.include?(payable_payment_status)
-          payment.payable_payment_status = payable_payment_status
-        end
+        payment.payable_payment_status = payable_payment_status
         payment.save!
 
         update_invoice_payment_status(payment_status: payable_payment_status)
 
         result
+      rescue ActiveRecord::RecordInvalid => e
+        result.record_validation_failure!(record: e.record)
       rescue BaseService::FailedResult => e
         result.fail_with_error!(e)
       end

app/services/invoices/payments/stripe_service.rb

@@ -32,9 +32,7 @@ def update_payment_status(organization_id:, status:, stripe_payment:)
         payment.status = status
 
         payable_payment_status = payment.payment_provider&.determine_payment_status(payment.status)
-        if Payment::PAYABLE_PAYMENT_STATUS.include?(payable_payment_status)
-          payment.payable_payment_status = payable_payment_status
-        end
+        payment.payable_payment_status = payable_payment_status
         payment.save!
 
         update_invoice_payment_status(
@@ -43,6 +41,8 @@ def update_payment_status(organization_id:, status:, stripe_payment:)
         )
 
         result
+      rescue ActiveRecord::RecordInvalid => e
+        result.record_validation_failure!(record: e.record)
       rescue BaseService::FailedResult => e
         result.fail_with_error!(e)
       end

app/services/payment_requests/payments/adyen_service.rb

@@ -46,9 +46,7 @@ def update_payment_status(provider_payment_id:, status:, metadata: {})
         payment.status = status
 
         payable_payment_status = payment.payment_provider&.determine_payment_status(payment.status)
-        if Payment::PAYABLE_PAYMENT_STATUS.include?(payable_payment_status)
-          payment.payable_payment_status = payable_payment_status
-        end
+        payment.payable_payment_status = payable_payment_status
         payment.save!
 
         update_payable_payment_status(payment_status: payable_payment_status)
@@ -58,6 +56,8 @@ def update_payment_status(provider_payment_id:, status:, metadata: {})
         PaymentRequestMailer.with(payment_request: payment.payable).requested.deliver_later if result.payable.payment_failed?
 
         result
+      rescue ActiveRecord::RecordInvalid => e
+        result.record_validation_failure!(record: e.record)
       rescue BaseService::FailedResult => e
         result.fail_with_error!(e)
       end

app/services/payment_requests/payments/cashfree_service.rb

@@ -42,9 +42,7 @@ def update_payment_status(organization_id:, status:, cashfree_payment:)
         payment.status = status
 
         payable_payment_status = payment.payment_provider&.determine_payment_status(payment.status)
-        if Payment::PAYABLE_PAYMENT_STATUS.include?(payable_payment_status)
-          payment.payable_payment_status = payable_payment_status
-        end
+        payment.payable_payment_status = payable_payment_status
         payment.save!
 
         update_payable_payment_status(payment_status: payable_payment_status)
@@ -54,6 +52,8 @@ def update_payment_status(organization_id:, status:, cashfree_payment:)
         PaymentRequestMailer.with(payment_request: payment.payable).requested.deliver_later if result.payable.payment_failed?
 
         result
+      rescue ActiveRecord::RecordInvalid => e
+        result.record_validation_failure!(record: e.record)
       rescue BaseService::FailedResult => e
         result.fail_with_error!(e)
       end

app/services/payment_requests/payments/gocardless_service.rb

@@ -35,9 +35,7 @@ def update_payment_status(provider_payment_id:, status:)
         payment.status = status
 
         payable_payment_status = payment.payment_provider&.determine_payment_status(payment.status)
-        if Payment::PAYABLE_PAYMENT_STATUS.include?(payable_payment_status)
-          payment.payable_payment_status = payable_payment_status
-        end
+        payment.payable_payment_status = payable_payment_status
         payment.save!
 
         update_payable_payment_status(payment_status: payable_payment_status)
@@ -47,6 +45,8 @@ def update_payment_status(provider_payment_id:, status:)
         PaymentRequestMailer.with(payment_request: payment.payable).requested.deliver_later if result.payable.payment_failed?
 
         result
+      rescue ActiveRecord::RecordInvalid => e
+        result.record_validation_failure!(record: e.record)
       rescue BaseService::FailedResult => e
         result.fail_with_error!(e)
       end

app/services/payment_requests/payments/stripe_service.rb

@@ -49,15 +49,11 @@ def update_payment_status(organization_id:, status:, stripe_payment:)
         result.payable = payment.payable
         return result if payment.payable.payment_succeeded?
 
-        payment.update!(status:)
-
         processing = status == "processing"
         payment.status = status
 
         payable_payment_status = payment.payment_provider&.determine_payment_status(payment.status)
-        if Payment::PAYABLE_PAYMENT_STATUS.include?(payable_payment_status)
-          payment.payable_payment_status = payable_payment_status
-        end
+        payment.payable_payment_status = payable_payment_status
         payment.save!
 
         update_payable_payment_status(payment_status: payable_payment_status, processing:)
@@ -67,6 +63,8 @@ def update_payment_status(organization_id:, status:, stripe_payment:)
         PaymentRequestMailer.with(payment_request: payment.payable).requested.deliver_later if result.payable.payment_failed?
 
         result
+      rescue ActiveRecord::RecordInvalid => e
+        result.record_validation_failure!(record: e.record)
       rescue BaseService::FailedResult => e
         result.fail_with_error!(e)
       end

spec/services/invoices/payments/adyen_service_spec.rb

@@ -73,6 +73,7 @@
 
       expect(result).to be_success
       expect(result.payment.status).to eq("Authorised")
+      expect(result.payment.payable_payment_status).to eq("succeeded")
       expect(result.invoice.reload).to have_attributes(
         payment_status: "succeeded",
         ready_for_payment_processing: false
@@ -88,6 +89,7 @@
 
         expect(result).to be_success
         expect(result.payment.status).to eq("Refused")
+        expect(result.payment.payable_payment_status).to eq("failed")
         expect(result.invoice.reload).to have_attributes(
           payment_status: "failed",
           ready_for_payment_processing: true
@@ -119,8 +121,8 @@
         aggregate_failures do
           expect(result).not_to be_success
           expect(result.error).to be_a(BaseService::ValidationFailure)
-          expect(result.error.messages.keys).to include(:payment_status)
-          expect(result.error.messages[:payment_status]).to include("value_is_invalid")
+          expect(result.error.messages.keys).to include(:payable_payment_status)
+          expect(result.error.messages[:payable_payment_status]).to include("value_is_invalid")
         end
       end
     end
@@ -143,6 +145,7 @@
         aggregate_failures do
           expect(result).to be_success
           expect(result.payment.status).to eq("succeeded")
+          expect(result.payment.payable_payment_status).to eq("succeeded")
           expect(result.invoice.reload).to have_attributes(
             payment_status: "succeeded",
             ready_for_payment_processing: false

spec/services/invoices/payments/cashfree_service_spec.rb

@@ -57,6 +57,7 @@
 
       expect(result).to be_success
       expect(result.payment.status).to eq("PAID")
+      expect(result.payment.payable_payment_status).to eq("succeeded")
       expect(result.invoice.reload).to have_attributes(
         payment_status: "succeeded",
         ready_for_payment_processing: false
@@ -81,6 +82,7 @@
 
         expect(result).to be_success
         expect(result.payment.status).to eq("EXPIRED")
+        expect(result.payment.payable_payment_status).to eq("failed")
         expect(result.invoice.reload).to have_attributes(
           payment_status: "failed",
           ready_for_payment_processing: true
@@ -129,8 +131,8 @@
 
         expect(result).not_to be_success
         expect(result.error).to be_a(BaseService::ValidationFailure)
-        expect(result.error.messages.keys).to include(:payment_status)
-        expect(result.error.messages[:payment_status]).to include("value_is_invalid")
+        expect(result.error.messages.keys).to include(:payable_payment_status)
+        expect(result.error.messages[:payable_payment_status]).to include("value_is_invalid")
       end
     end
 
@@ -159,6 +161,7 @@
 
         expect(result).to be_success
         expect(result.payment.status).to eq("PAID")
+        expect(result.payment.payable_payment_status).to eq("succeeded")
         expect(result.invoice.reload).to have_attributes(
           payment_status: "succeeded",
           ready_for_payment_processing: false

spec/services/invoices/payments/gocardless_service_spec.rb

@@ -51,6 +51,7 @@
 
       expect(result).to be_success
       expect(result.payment.status).to eq("paid_out")
+      expect(result.payment.payable_payment_status).to eq("succeeded")
       expect(result.invoice.reload).to have_attributes(
         payment_status: "succeeded",
         ready_for_payment_processing: false
@@ -66,6 +67,7 @@
 
         expect(result).to be_success
         expect(result.payment.status).to eq("failed")
+        expect(result.payment.payable_payment_status).to eq("failed")
         expect(result.invoice.reload).to have_attributes(
           payment_status: "failed",
           ready_for_payment_processing: true
@@ -97,8 +99,8 @@
         aggregate_failures do
           expect(result).not_to be_success
           expect(result.error).to be_a(BaseService::ValidationFailure)
-          expect(result.error.messages.keys).to include(:payment_status)
-          expect(result.error.messages[:payment_status]).to include("value_is_invalid")
+          expect(result.error.messages.keys).to include(:payable_payment_status)
+          expect(result.error.messages[:payable_payment_status]).to include("value_is_invalid")
         end
       end
     end
@@ -114,6 +116,7 @@
 
         expect(result).to be_success
         expect(result.payment.status).to eq("paid_out")
+        expect(result.payment.payable_payment_status).to eq("succeeded")
         expect(result.invoice.reload).to have_attributes(
           payment_status: "succeeded",
           ready_for_payment_processing: false

spec/services/invoices/payments/stripe_service_spec.rb

@@ -128,6 +128,7 @@
 
       expect(result).to be_success
       expect(result.payment.status).to eq("succeeded")
+      expect(result.payment.payable_payment_status).to eq("succeeded")
       expect(result.invoice.reload).to have_attributes(
         payment_status: "succeeded",
         ready_for_payment_processing: false
@@ -152,6 +153,7 @@
 
         expect(result).to be_success
         expect(result.payment.status).to eq("failed")
+        expect(result.payment.payable_payment_status).to eq("failed")
         expect(result.invoice.reload).to have_attributes(
           payment_status: "failed",
           ready_for_payment_processing: true
@@ -185,8 +187,8 @@
         aggregate_failures do
           expect(result).not_to be_success
           expect(result.error).to be_a(BaseService::ValidationFailure)
-          expect(result.error.messages.keys).to include(:payment_status)
-          expect(result.error.messages[:payment_status]).to include("value_is_invalid")
+          expect(result.error.messages.keys).to include(:payable_payment_status)
+          expect(result.error.messages[:payable_payment_status]).to include("value_is_invalid")
         end
       end
     end
@@ -217,6 +219,7 @@
         aggregate_failures do
           expect(result).to be_success
           expect(result.payment.status).to eq("succeeded")
+          expect(result.payment.payable_payment_status).to eq("succeeded")
           expect(result.invoice.reload).to have_attributes(
             payment_status: "succeeded",
             ready_for_payment_processing: false
@@ -310,6 +313,7 @@
 
             expect(result).to be_success
             expect(result.payment.status).to eq("succeeded")
+            expect(result.payment.payable_payment_status).to eq("succeeded")
             expect(result.invoice.reload).to have_attributes(
               payment_status: "succeeded",
               ready_for_payment_processing: false

spec/services/payment_requests/payments/adyen_service_spec.rb

@@ -143,6 +143,7 @@
       expect(result.payment.status).to eq(status)
 
       expect(result.payable.reload).to be_payment_succeeded
+      expect(result.payment.payable_payment_status).to eq("succeeded")
       expect(result.payable.ready_for_payment_processing).to eq(false)
 
       expect(invoice_1.reload).to be_payment_succeeded
@@ -204,6 +205,7 @@
       it "updates the payment, payment_request and invoices status", :aggregate_failures do
         expect(result).to be_success
         expect(result.payment.status).to eq(status)
+        expect(result.payment.payable_payment_status).to eq("failed")
 
         expect(result.payable.reload).to be_payment_failed
         expect(result.payable.ready_for_payment_processing).to eq(true)
@@ -255,14 +257,14 @@
           .to not_change { payment_request.reload.payment_status }
           .and not_change { invoice_1.reload.payment_status }
           .and not_change { invoice_2.reload.payment_status }
-          .and change { payment.reload.status }.to(status)
+          .and not_change { payment.reload.status }
       end
 
       it "returns an error", :aggregate_failures do
         expect(result).not_to be_success
         expect(result.error).to be_a(BaseService::ValidationFailure)
-        expect(result.error.messages.keys).to include(:payment_status)
-        expect(result.error.messages[:payment_status]).to include("value_is_invalid")
+        expect(result.error.messages.keys).to include(:payable_payment_status)
+        expect(result.error.messages[:payable_payment_status]).to include("value_is_invalid")
       end
 
       it "does not send payment requested email" do
@@ -292,6 +294,7 @@
 
         expect(result).to be_success
         expect(result.payment.status).to eq(status)
+        expect(result.payment.payable_payment_status).to eq("succeeded")
 
         expect(result.payable).to be_payment_succeeded
         expect(result.payable.ready_for_payment_processing).to eq(false)