fix(billing): ensure all tenants have valid Stripe customer IDs #5

Merged
hodlbod merged 2 commits from userAdityaa/caravel:billing-tenants-id into master 2026-04-14 23:06:49 +00:00
Contributor

Problem

Tenants could be created with empty stripe_customer_id, breaking invoice/webhook ownership mapping and causing ambiguous billing behavior.

Solution

  • Onboarding now creates Stripe customer at tenant creation
  • Legacy tenants auto-backfilled with valid customer IDs on login
  • DB-level guardrails: unique index + triggers prevent empty/duplicate IDs
  • Command layer validates non-empty stripe_customer_id
  • Billing code fails clearly if customer_id is missing

Testing

  • SQL validation confirms zero empty stripe_customer_id values
  • Onboarding tested with new tenant creation
  • Migration triggers verified active
  • Build and tests passing

Closes #2

### Problem Tenants could be created with empty `stripe_customer_id`, breaking invoice/webhook ownership mapping and causing ambiguous billing behavior. ### Solution - Onboarding now creates Stripe customer at tenant creation - Legacy tenants auto-backfilled with valid customer IDs on login - DB-level guardrails: unique index + triggers prevent empty/duplicate IDs - Command layer validates non-empty `stripe_customer_id` - Billing code fails clearly if `customer_id` is missing ### Testing - SQL validation confirms zero empty stripe_customer_id values - Onboarding tested with new tenant creation - Migration triggers verified active - Build and tests passing Closes #2 <video src="attachments/7da43240-e7f4-4512-b48b-1d1e6d4f6fed" title="Screen Recording 2026-04-11 at 5.39.16 PM.mov" controls></video>
hodlbod requested changes 2026-04-13 16:56:29 +00:00
@@ -0,0 +16,4 @@
WHEN trim(NEW.stripe_customer_id) = ''
BEGIN
SELECT RAISE(ABORT, 'stripe_customer_id cannot be empty');
END;
Owner

This is all too heavy handed, validation is already done at the application level, the column should just be changed to non-null in migration 1.

This is all too heavy handed, validation is already done at the application level, the column should just be changed to non-null in migration 1.
@@ -372,0 +368,4 @@
// Ensure tenant exists and always has a Stripe customer id.
match state.api.query.get_tenant(&pubkey).await {
Ok(Some(existing_tenant)) => {
if existing_tenant.stripe_customer_id.trim().is_empty() {
Owner

This branch should never happen because tenant creation fails below if stripe customer fails to be created. We don't have any data that needs to be migrated. Just make the customer id not null.

This branch should never happen because tenant creation fails below if stripe customer fails to be created. We don't have any data that needs to be migrated. Just make the customer id not null.
hodlbod marked this conversation as resolved
@@ -267,0 +281,4 @@
.execute(&mut *tx)
.await?;
let activity = Self::insert_activity(&mut tx, "update_tenant", "tenant", pubkey).await?;
Owner

Skip activity logging, this is just an implementation detail

Skip activity logging, this is just an implementation detail
Author
Contributor

Addressed the feedback, and also added tests to cover the flow.

Addressed the feedback, and also added tests to cover the flow.
hodlbod reviewed 2026-04-13 21:01:57 +00:00
hodlbod left a comment
Owner

Just a couple comments, and then feel free to merge.

Re: tests, it's totally up to you, but I generally go without until the app is stable, then add them with the correct behavior so that I'm not slowing myself down maintaining incorrect tests. But again, up to you.

Just a couple comments, and then feel free to merge. Re: tests, it's totally up to you, but I generally go without until the app is stable, then add them with the correct behavior so that I'm not slowing myself down maintaining incorrect tests. But again, up to you.
@@ -131,0 +139,4 @@
"tenant {} has no stripe_customer_id",
tenant.pubkey
));
}
Owner

This can be removed, it should be impossible

This can be removed, it should be impossible
@@ -445,0 +460,4 @@
pub async fn stripe_create_customer(&self, tenant_pubkey: &str) -> Result<String> {
if self.stripe_secret_key.trim().is_empty() {
return Err(anyhow!("missing STRIPE_SECRET_KEY"));
}
Owner

This should be validated on startup (in the constructor) and fail there instead

This should be validated on startup (in the constructor) and fail there instead
Author
Contributor

I’ll push all the updates shortly, including PRs for the other open issues as well.

I’ll push all the updates shortly, including PRs for the other open issues as well.
Author
Contributor

Re: tests, it's totally up to you, but I generally go without until the app is stable, then add them with the correct behavior so that I'm not slowing myself down maintaining incorrect tests. But again, up to you.

I’ll keep the tests in the codebase for now since writing them as I go is part of my usual workflow.
I’ll make sure to update them as the behavior evolves so they stay accurate and useful.

> Re: tests, it's totally up to you, but I generally go without until the app is stable, then add them with the correct behavior so that I'm not slowing myself down maintaining incorrect tests. But again, up to you. I’ll keep the tests in the codebase for now since writing them as I go is part of my usual workflow. I’ll make sure to update them as the behavior evolves so they stay accurate and useful.
userAdityaa added 2 commits 2026-04-14 23:01:47 +00:00
userAdityaa force-pushed billing-tenants-id from bad81dbbb8 to 6faf7d3fa0 2026-04-14 23:01:47 +00:00 Compare
userAdityaa marked the pull request as work in progress 2026-04-14 23:02:12 +00:00
Owner

LGTM, is this still a WIP or ready to merge?

LGTM, is this still a WIP or ready to merge?
Author
Contributor

LGTM, is this still a WIP or ready to merge?

was just checking everything from my side, removing WIP if the changes looks good to you.

> LGTM, is this still a WIP or ready to merge? was just checking everything from my side, removing WIP if the changes looks good to you.
userAdityaa marked the pull request as ready for review 2026-04-14 23:05:12 +00:00
hodlbod merged commit ce595c8bc5 into master 2026-04-14 23:06:49 +00:00
Sign in to join this conversation.
No Reviewers
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: coracle/caravel#5