From 5be4dca80e1dc4283e61f1ff8e80cf22629be63c Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Fri, 12 Dec 2025 09:25:41 +0100 Subject: [PATCH] Refresh the subscription instead of relying on event payloads Events can land out of order. See https://github.com/basecamp/fizzy-saas/pull/23#discussion_r2609909558 --- .../controllers/stripe/webhooks_controller.rb | 56 +++++++------------ .../stripe/webhooks_controller_test.rb | 32 +++++------ 2 files changed, 37 insertions(+), 51 deletions(-) diff --git a/saas/app/controllers/stripe/webhooks_controller.rb b/saas/app/controllers/stripe/webhooks_controller.rb index 31839e089..14c50c74d 100644 --- a/saas/app/controllers/stripe/webhooks_controller.rb +++ b/saas/app/controllers/stripe/webhooks_controller.rb @@ -15,14 +15,10 @@ class Stripe::WebhooksController < ApplicationController private def dispatch_stripe_event(event) case event.type - when "checkout.session.completed" - handle_checkout_completed(event.data.object) - when "customer.subscription.updated" - handle_subscription_updated(event.data.object) - when "customer.subscription.deleted" - handle_subscription_deleted(event.data.object) - when "invoice.payment_failed" - handle_payment_failed(event.data.object) + when "checkout.session.completed" + sync_new_subscription(event.data.object.subscription, plan_key: event.data.object.metadata["plan_key"]) if event.data.object.mode == "subscription" + when "customer.subscription.updated", "customer.subscription.deleted" + sync_subscription(event.data.object.id) end end @@ -36,39 +32,29 @@ class Stripe::WebhooksController < ApplicationController nil end - def handle_checkout_completed(session) - return unless session.mode == "subscription" - - subscription = find_subscription_by_customer(session.customer) - return unless subscription - - stripe_subscription = Stripe::Subscription.retrieve(session.subscription) - - subscription.update! \ - stripe_subscription_id: stripe_subscription.id, - plan_key: session.metadata["plan_key"], - status: stripe_subscription.status, - current_period_end: extract_current_period_end(stripe_subscription) + def sync_new_subscription(stripe_subscription_id, plan_key:) + sync_subscription(stripe_subscription_id) do |subscription_properties| + subscription_properties[:plan_key] = plan_key if plan_key + end end - def handle_subscription_updated(stripe_subscription) + # Always fetch fresh subscription data from Stripe to handle out-of-order + # event delivery. Not relying on payload data. + def sync_subscription(stripe_subscription_id) + stripe_subscription = Stripe::Subscription.retrieve(stripe_subscription_id) + if subscription = find_subscription_by_customer(stripe_subscription.customer) - subscription.update! \ + subscription_properties = { + stripe_subscription_id: stripe_subscription.id, status: stripe_subscription.status, - current_period_end: extract_current_period_end(stripe_subscription), + current_period_end: current_period_end_for(stripe_subscription), cancel_at: stripe_subscription.cancel_at ? Time.at(stripe_subscription.cancel_at) : nil - end - end + } - def handle_subscription_deleted(stripe_subscription) - if subscription = find_subscription_by_customer(stripe_subscription.customer) - subscription.update!(status: "canceled", stripe_subscription_id: nil) - end - end + yield subscription_properties if block_given? + subscription_properties[:stripe_subscription_id] = nil if stripe_subscription.status == "canceled" - def handle_payment_failed(invoice) - if subscription = find_subscription_by_customer(invoice.customer) - subscription.update!(status: "past_due") + subscription.update!(subscription_properties) end end @@ -76,7 +62,7 @@ class Stripe::WebhooksController < ApplicationController Account::Subscription.find_by(stripe_customer_id: customer_id) end - def extract_current_period_end(stripe_subscription) + def current_period_end_for(stripe_subscription) timestamp = stripe_subscription.items.data.first&.current_period_end Time.at(timestamp) if timestamp end diff --git a/saas/test/controllers/stripe/webhooks_controller_test.rb b/saas/test/controllers/stripe/webhooks_controller_test.rb index 7bc4897dc..c1c8ac4d0 100644 --- a/saas/test/controllers/stripe/webhooks_controller_test.rb +++ b/saas/test/controllers/stripe/webhooks_controller_test.rb @@ -18,7 +18,7 @@ class Stripe::WebhooksControllerTest < ActionDispatch::IntegrationTest end test "checkout session completed activates subscription" do - stripe_sub = OpenStruct.new(id: "sub_123", status: "active", items: stub_items(1.month.from_now.to_i)) + stripe_sub = OpenStruct.new(id: "sub_123", customer: "cus_test123", status: "active", cancel_at: nil, items: stub_items(1.month.from_now.to_i)) event = stripe_event("checkout.session.completed", mode: "subscription", @@ -41,14 +41,18 @@ class Stripe::WebhooksControllerTest < ActionDispatch::IntegrationTest test "subscription updated changes status" do @subscription.update!(stripe_subscription_id: "sub_123", status: "active") - event = stripe_event("customer.subscription.updated", + stripe_sub = OpenStruct.new( + id: "sub_123", customer: "cus_test123", status: "past_due", cancel_at: nil, items: stub_items(1.month.from_now.to_i) ) + event = stripe_event("customer.subscription.updated", id: "sub_123") + Stripe::Webhook.stubs(:construct_event).returns(event) + Stripe::Subscription.stubs(:retrieve).returns(stripe_sub) post stripe_webhooks_path @@ -59,9 +63,18 @@ class Stripe::WebhooksControllerTest < ActionDispatch::IntegrationTest test "subscription deleted cancels subscription" do @subscription.update!(stripe_subscription_id: "sub_123", status: "active") - event = stripe_event("customer.subscription.deleted", customer: "cus_test123") + stripe_sub = OpenStruct.new( + id: "sub_123", + customer: "cus_test123", + status: "canceled", + cancel_at: nil, + items: stub_items(1.month.from_now.to_i) + ) + + event = stripe_event("customer.subscription.deleted", id: "sub_123") Stripe::Webhook.stubs(:construct_event).returns(event) + Stripe::Subscription.stubs(:retrieve).returns(stripe_sub) post stripe_webhooks_path @@ -71,19 +84,6 @@ class Stripe::WebhooksControllerTest < ActionDispatch::IntegrationTest assert_nil @subscription.stripe_subscription_id end - test "payment failed marks subscription as past due" do - @subscription.update!(stripe_subscription_id: "sub_123", status: "active") - - event = stripe_event("invoice.payment_failed", customer: "cus_test123") - - Stripe::Webhook.stubs(:construct_event).returns(event) - - post stripe_webhooks_path - - assert_response :ok - assert_equal "past_due", @subscription.reload.status - end - private def stripe_event(type, **attributes) OpenStruct.new(type: type, data: OpenStruct.new(object: OpenStruct.new(attributes)))