Unify update_status_and_charges for generic provider support

app/controllers/registrations_controller.rb

@@ -314,6 +314,7 @@ def stripe_webhook
 
     # Create a default audit that marks the event as "unhandled".
     audit_event = StripeWebhookEvent.create_from_api(event)
+    audit_remote_timestamp = audit_event.created_at_remote
 
     stripe_intent = event.data.object # contains a polymorphic type that depends on the event
     stored_record = StripeRecord.find_by(stripe_id: stripe_intent.id)
@@ -327,14 +328,21 @@ def stripe_webhook
       end
     end
 
+    connected_account = ConnectedStripeAccount.find_by(account_id: stored_record.account_id)
+
+    unless connected_account.present?
+      logger.error "Stripe webhook reported event for account '#{stored_record.account_id}' but we are not connected to that account."
+      return head :not_found
+    end
+
     # Handle the event
     case event.type
     when StripeWebhookEvent::PAYMENT_INTENT_SUCCEEDED
       # stripe_intent contains a Stripe::PaymentIntent as per Stripe documentation
 
       stored_intent = stored_record.payment_intent
 
-      stored_intent.update_status_and_charges(stripe_intent, audit_event, audit_event.created_at_remote) do |charge_transaction|
+      stored_intent.update_status_and_charges(connected_account, stripe_intent, audit_event, audit_remote_timestamp) do |charge_transaction|
         ruby_money = charge_transaction.money_amount
         stored_holder = stored_intent.holder
 
@@ -350,14 +358,14 @@ def stripe_webhook
           # Context: When our servers die due to traffic spikes, the Stripe webhook cannot be processed
           #   and Stripe tries again after an exponential backoff. So we (erroneously!) record the creation timestamp
           #   in our DB _after_ the backed-off event has been processed. This can lead to a wrong registration order :(
-          stored_payment.update!(created_at: audit_event.created_at_remote)
+          stored_payment.update!(created_at: audit_remote_timestamp)
         end
       end
     when StripeWebhookEvent::PAYMENT_INTENT_CANCELED
       # stripe_intent contains a Stripe::PaymentIntent as per Stripe documentation
 
       stored_intent = stored_record.payment_intent
-      stored_intent.update_status_and_charges(stripe_intent, audit_event, audit_event.created_at_remote)
+      stored_intent.update_status_and_charges(connected_account, stripe_intent, audit_event, audit_remote_timestamp)
     else
       logger.info "Unhandled Stripe event type: #{event.type}"
     end
@@ -412,7 +420,7 @@ def payment_completion
 
     registration = stored_intent.holder
 
-    stored_intent.update_status_and_charges(remote_intent, current_user) do |charge_transaction|
+    stored_intent.update_status_and_charges(payment_account, remote_intent, current_user) do |charge_transaction|
       ruby_money = charge_transaction.money_amount
 
       registration.record_payment(

app/models/connected_stripe_account.rb

@@ -78,6 +78,28 @@ def prepare_intent(registration, amount_iso, currency_iso, paying_user)
     )
   end
 
+  def retrieve_payments(payment_intent)
+    intent_record = payment_intent.payment_record
+
+    intent_charges = Stripe::Charge.list(
+      { payment_intent: intent_record.stripe_id },
+      stripe_account: self.account_id,
+    )
+
+    intent_charges.data.map do |charge|
+      stripe_record = StripeRecord.find_by(stripe_id: charge.id)
+
+      if stripe_record.present?
+        stripe_record.update_status(charge)
+      else
+        stripe_record = StripeRecord.create_from_api(charge, {}, self.account_id, intent_record)
+        yield stripe_record if block_given?
+      end
+
+      stripe_record
+    end
+  end
+
   def find_payment(record_id)
     StripeRecord.charge.find(record_id)
   end

app/models/payment_intent.rb

@@ -12,7 +12,7 @@ class PaymentIntent < ApplicationRecord
   scope :started, -> { where.not(wca_status: 'created') }
   scope :incomplete, -> { where.not(wca_status: ['succeeded', 'canceled']) }
 
-  delegate :retrieve_remote, :money_amount, :account_id, to: :payment_record
+  delegate :retrieve_remote, :money_amount, :account_id, :determine_wca_status, to: :payment_record
 
   # Stripe secrets are case-sensitive. Make sure that this information is not lost during encryption.
   encrypts :client_secret, downcase: false
@@ -34,74 +34,67 @@ class PaymentIntent < ApplicationRecord
   scope :paypal, -> { where(payment_record_type: 'PaypalRecord') }
   scope :stripe, -> { where(payment_record_type: 'StripeRecord') }
 
-  def update_status_and_charges(api_intent, action_source, source_datetime = DateTime.current, &block)
-    if payment_record_type == 'StripeRecord'
-      update_stripe_status_and_charges(api_intent, action_source, source_datetime, &block)
-    else
-      raise "Trying to update status and charges for a PaymentIntent with unmatched payment_record_type of: #{payment_record_type}"
-    end
-  end
+  def update_status_and_charges(payment_account, api_intent, action_source, source_datetime = DateTime.current)
+    self.with_lock do
+      # The order of operations here is critical:
+      #   We need to update the underlying raw record first, so that `determine_wca_status` works correctly
+      self.payment_record.update_status(api_intent)
+      updated_wca_status = self.determine_wca_status.to_s
 
-  private
+      case updated_wca_status
+      when PaymentIntent.wca_statuses[:succeeded]
+        # The payment didn't need any additional actions and is completed!
 
-    def update_stripe_status_and_charges(api_intent, action_source, source_datetime)
-      self.with_lock do
-        self.update!(error_details: api_intent.last_payment_error)
-        self.payment_record.update_status(api_intent)
-
-        # Payment Intent lifecycle as per https://stripe.com/docs/payments/intents#intent-statuses
-        case api_intent.status
-        when 'succeeded'
-          # The payment didn't need any additional actions and is completed!
-
-          # Record the success timestamp if not already done
-          unless self.succeeded?
-            self.update!(
-              confirmed_at: source_datetime,
-              confirmation_source: action_source,
-              wca_status: payment_record.determine_wca_status,
-            )
-          end
-
-          intent_charges = Stripe::Charge.list(
-            { payment_intent: self.payment_record.stripe_id },
-            stripe_account: self.account_id,
+        # Record the success timestamp if not already done
+        unless self.succeeded?
+          self.update!(
+            confirmed_at: source_datetime,
+            confirmation_source: action_source,
+            wca_status: updated_wca_status,
           )
+        end
 
-          intent_charges.data.each do |charge|
-            recorded_transaction = StripeRecord.find_by(stripe_id: charge.id)
-
-            if recorded_transaction.present?
-              recorded_transaction.update_status(charge)
-            else
-              fresh_transaction = StripeRecord.create_from_api(charge, {}, self.account_id, self.payment_record)
+        payment_account.retrieve_payments(self) do |payment|
+          # Only trigger outer update blocks for charges that are actually successful. This is reasonable
+          # because we only ever trigger this block for PIs that are marked "successful" in the first place
+          charge_successful = payment.determine_wca_status.to_s == PaymentIntent.wca_statuses[:succeeded]
 
-              # Only trigger outer update blocks for charges that are actually successful. This is reasonable
-              # because we only ever trigger this block for PIs that are marked "successful" in the first place
-              charge_successful = fresh_transaction.stripe_status == "succeeded"
+          yield payment if block_given? && charge_successful
+        end
+      when PaymentIntent.wca_statuses[:canceled]
+        # Canceled by the gateway
 
-              yield fresh_transaction if block_given? && charge_successful
-            end
-          end
-        when 'canceled'
-          # Canceled by Stripe
+        # Record the cancellation timestamp if not already done
+        unless self.canceled?
           self.update!(
             canceled_at: source_datetime,
             cancellation_source: action_source,
-            wca_status: payment_record.determine_wca_status,
-          )
-        when 'requires_payment_method'
-          # Reset by Stripe
-          self.update!(
-            confirmed_at: nil,
-            confirmation_source: nil,
-            canceled_at: nil,
-            cancellation_source: nil,
-            wca_status: payment_record.determine_wca_status,
+            wca_status: updated_wca_status,
           )
-        else
-          self.update!(wca_status: payment_record.determine_wca_status)
         end
+      when PaymentIntent.wca_statuses[:created],
+        PaymentIntent.wca_statuses[:pending]
+        # Reset by the gateway
+        self.update!(
+          confirmed_at: nil,
+          confirmation_source: nil,
+          canceled_at: nil,
+          cancellation_source: nil,
+          wca_status: updated_wca_status,
+        )
+      end
+
+      # Write any additional details now that the main status cycle is complete
+      self.update_intent_details(api_intent)
+    end
+  end
+
+  private
+
+    def update_intent_details(api_record)
+      case self.payment_record_type
+      when "StripeRecord"
+        self.update!(error_details: api_record.last_payment_error)
       end
     end
 

app/models/stripe_record.rb

@@ -65,9 +65,9 @@ def update_status(api_transaction)
     stripe_error = nil
 
     case self.stripe_record_type
-    when 'payment_intent'
+    when StripeRecord.stripe_record_types[:payment_intent]
       stripe_error = api_transaction.last_payment_error&.code
-    when 'charge'
+    when StripeRecord.stripe_record_types[:charge]
       stripe_error = api_transaction.failure_message
     end
 
@@ -79,11 +79,11 @@ def update_status(api_transaction)
 
   def retrieve_stripe
     case self.stripe_record_type
-    when 'payment_intent'
+    when StripeRecord.stripe_record_types[:payment_intent]
       Stripe::PaymentIntent.retrieve(self.stripe_id, stripe_account: self.account_id)
-    when 'charge'
+    when StripeRecord.stripe_record_types[:charge]
       Stripe::Charge.retrieve(self.stripe_id, stripe_account: self.account_id)
-    when 'refund'
+    when StripeRecord.stripe_record_types[:refund]
       Stripe::Refund.retrieve(self.stripe_id, stripe_account: self.account_id)
     end
   end