diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index 65ef4c0d0c..82a0ad2bac 100644 --- a/app/controllers/registrations_controller.rb +++ b/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,6 +328,13 @@ 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 @@ -334,7 +342,7 @@ def stripe_webhook 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( diff --git a/app/models/connected_stripe_account.rb b/app/models/connected_stripe_account.rb index cf0aa646f1..f3c2cfcca7 100644 --- a/app/models/connected_stripe_account.rb +++ b/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 diff --git a/app/models/payment_intent.rb b/app/models/payment_intent.rb index aa6741ef8a..337153a25c 100644 --- a/app/models/payment_intent.rb +++ b/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 diff --git a/app/models/stripe_record.rb b/app/models/stripe_record.rb index 5dfcc5480d..52c87734fe 100644 --- a/app/models/stripe_record.rb +++ b/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