Skip to content

Conversation

@jeremy
Copy link
Member

@jeremy jeremy commented Jan 25, 2026

Summary

When the account owner's email changes, the Stripe customer record now gets updated automatically. This handles two scenarios:

  1. Owner changes their email - identity_id change on the owner user
  2. Ownership transfer - a different user becomes the owner

Root cause: create_stripe_customer set email once at subscription creation time with no subsequent syncing.

Fix: Layered responsibility across User → Account → Subscription:

  • User::NotifiesAccountOfEmailChange - Triggers on owner email or role change
  • Account::Billing#owner_email_changed - Enqueues background job
  • Account::Subscription#sync_customer_email_to_stripe - Updates Stripe customer
  • Account::SyncStripeCustomerEmailJob - Async with polynomial backoff retry

Changes

  • saas/app/models/user/notifies_account_of_email_change.rb - User concern
  • saas/app/models/account/billing.rb - Account method to enqueue job
  • saas/app/models/account/subscription.rb - Stripe sync method
  • saas/app/jobs/account/sync_stripe_customer_email_job.rb - Background job
  • saas/lib/fizzy/saas/engine.rb - Include concern in User
  • saas/test/models/user/notifies_account_of_email_change_test.rb - User tests
  • saas/test/models/account/billing_test.rb - Job enqueueing tests
  • saas/test/models/account/subscription_test.rb - Sync method tests

Test plan

  • bin/rails test saas/test/ - All saas tests pass
  • Manual: Change email in dev with Stripe integration, verify customer updated
  • Manual: Transfer ownership, verify new owner's email synced

Fixes BUG-9504334396
Source: HackerOne #3522917

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 131d46b5e5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@jeremy jeremy force-pushed the fix/BUG-9504334396 branch from e6da0a3 to 5e34a60 Compare January 25, 2026 09:04
Copy link
Member Author

@jeremy jeremy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about account ownership changes? It's not just the owning user's email changed, it's the the account owner email changed.

When an account owner changes their email address, update the
corresponding Stripe customer record via a background job.

Also handles ownership transfers: when a user becomes the account
owner, their email is synced to Stripe.

Responsibility chain:
- User::NotifiesAccountOfEmailChange triggers on owner identity change
  or when a user becomes owner
- Account::Billing#owner_email_changed enqueues sync job
- Account::SyncStripeCustomerEmailJob performs the update with
  polynomial backoff retries
- Account::Subscription#sync_customer_email_to_stripe calls Stripe API
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements automatic syncing of the account owner's email to Stripe when the owner changes their email address or when ownership is transferred to a different user. Previously, the Stripe customer email was only set once at subscription creation time and never updated.

Changes:

  • Added User::NotifiesAccountOfEmailChange concern that triggers when owner identity changes or a user becomes owner
  • Added Account::Billing#owner_email_changed method to enqueue sync jobs
  • Added Account::Subscription#sync_customer_email_to_stripe to update Stripe customer records
  • Added Account::SyncStripeCustomerEmailJob to handle async Stripe updates
  • Added comprehensive test coverage for the notification flow and sync behavior

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
saas/app/models/user/notifies_account_of_email_change.rb New concern that detects when owner email changes or ownership is transferred and notifies the account
saas/lib/fizzy/saas/engine.rb Includes the new concern in the User model
saas/app/models/account/billing.rb Adds method to enqueue sync job when owner email changes
saas/app/models/account/subscription.rb Implements the Stripe customer email sync logic
saas/app/jobs/account/sync_stripe_customer_email_job.rb Background job that calls the sync method with retry logic for Stripe errors
saas/test/models/user/notifies_account_of_email_change_test.rb Tests for the user concern covering various scenarios
saas/test/models/account/billing_test.rb Tests for job enqueueing behavior
saas/test/models/account/subscription_test.rb Tests for the sync method covering edge cases

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jeremy
Copy link
Member Author

jeremy commented Jan 25, 2026

Re: ownership changes - addressed. The concern now handles both cases:

def account_owner_changed?
  owner? && identity && (saved_change_to_identity_id? || became_owner?)
end

def became_owner?
  saved_change_to_role? && role_before_last_save != "owner"
end

Test coverage:

  • "notifies account when owner changes email"
  • "notifies account when user becomes owner"
  • "does not notify account when non-owner changes email"
  • "does not notify account when owner becomes member"

- Handle Stripe::InvalidRequestError in sync_customer_email_to_stripe
  (mirrors cancel method behavior for deleted customers)
- Add test for deactivated owner (owner with nil identity)
- Add test for deleted Stripe customer scenario
@jeremy jeremy force-pushed the fix/BUG-9504334396 branch from dd45880 to 6e8f44a Compare January 25, 2026 10:35
@jeremy jeremy merged commit f2ff5af into main Jan 25, 2026
12 checks passed
@jeremy jeremy deleted the fix/BUG-9504334396 branch January 25, 2026 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants