Skip to content

feat(deleter): send the delete notice on every organization delete - #1903

Merged
whoAbhishekSah merged 4 commits into
mainfrom
org-delete-notice-always
Aug 25, 2026
Merged

feat(deleter): send the delete notice on every organization delete#1903
whoAbhishekSah merged 4 commits into
mainfrom
org-delete-notice-always

Conversation

@whoAbhishekSah

@whoAbhishekSah whoAbhishekSah commented Aug 25, 2026

Copy link
Copy Markdown
Member

Follow-up to #1880. The owner email used to go out only when the delete forfeited unused tokens. Every deleted organization now notifies all of its owners, tokens or not.

  • The owners are resolved after the blocker check passes and before teardown removes their policies — best-effort, so the delete never depends on the policy machinery. A delete whose owners cannot be resolved logs a warning instead of failing. (An earlier commit persisted the recipient set in an audit record for retries; it was reverted after review — that pushed owner ids onto the webhook feed to cover a failure window of two local statements at the tail of teardown. A delete failing exactly there and completing on retry logs that no owner could be notified.)
  • Because the mail is no longer about forfeits, the config key moves from billing.token_forfeit_notice to billing.org_delete_notice. Nothing has deployed the old key yet, so this is a rename, not a migration.
  • The built-in default template is a plain deletion notice. It names no token amounts (settlement numbers have tax implications) and adds one settlement line only when purchased tokens existed. .Amount and .Purchased stay available to config templates for deployments that want them.
  • Nothing changes for the audit trail: app.billing.tokens.forfeited still records the exact per-account amount and purchased, and the per-account recovery on retried deletes still fills the template data. The webhook event list and its docs page now name app.billing.tokens.forfeited and app.billing.checkout.deleted, which audit publishing already sends to subscribers.
  • Tests: the forfeit record and its retry recovery now run against a real audit service with an in-memory repository (record content asserted on the first attempt, notice rebuilt from the record on a retry), and every successful-delete test asserts the owner mail.

How support reconstructs a refund after the org is gone

The delete hard-deletes the org's ledger rows, so the forfeit audit record is the number support settles from: amount (balance at delete) and purchased (the refundable share) per billing account. If the record is ever in doubt, this query rebuilds what the surviving remnants still know — the platform-side halves of purchase entries carry a checkout_id, and the checkout's deletion audit record remembers the org:

WITH deleted_checkouts AS (
  SELECT a.org_id,
         a.target->>'ID'            AS checkout_id,
         a.metadata->>'customer_id' AS account_id,
         a.metadata->>'provider_id' AS stripe_session
  FROM auditlogs a
  WHERE a.action = 'app.billing.checkout.deleted'
),
bought AS (
  SELECT dc.org_id, dc.account_id,
         sum(t.amount) AS tokens_bought
  FROM deleted_checkouts dc
  JOIN billing_transactions t
    ON t.metadata->>'checkout_id' = dc.checkout_id
   AND t.source = 'system.buy'
   AND t.type   = 'debit'      -- the platform-side half survives the delete
  GROUP BY 1, 2
),
forfeited AS (
  SELECT a.org_id,
         a.target->>'ID'                    AS account_id,
         (a.metadata->>'amount')::bigint    AS balance_at_delete,
         (a.metadata->>'purchased')::bigint AS purchased_at_delete
  FROM auditlogs a
  WHERE a.action = 'app.billing.tokens.forfeited'
)
SELECT coalesce(b.org_id, f.org_id)         AS org_id,
       coalesce(b.account_id, f.account_id) AS account_id,
       b.tokens_bought,        -- ceiling, rebuilt from the ledger remnants
       f.balance_at_delete,    -- from the forfeit audit record
       f.purchased_at_delete   -- the refundable number
FROM bought b
FULL OUTER JOIN forfeited f
  ON b.org_id = f.org_id AND b.account_id = f.account_id;

tokens_bought is only a ceiling: usage rows carry no org attribution on their surviving halves, so the unspent share is not derivable from the ledger remnants — that is exactly what the forfeit record exists for. The stripe_session ids bridge to Stripe for the payment and refund history, which is the double-refund guard.

🤖 Generated with Claude Code

The owner email used to go out only when the delete forfeited tokens.
Every deleted organization now notifies all of its owners, whether it
held tokens or not. The owners are resolved after the blocker check
passes and before teardown removes their policies, still best-effort so
the delete never depends on the policy machinery.

Because the mail is no longer about forfeits, the config key moves from
billing.token_forfeit_notice to billing.org_delete_notice (nothing has
deployed the old key), and the built-in default becomes a plain
deletion notice: it names no token amounts and only mentions that
purchased tokens will be settled when some existed. The template still
receives .Amount and .Purchased for deployments that want them, and the
forfeit audit records keep the exact per-account numbers, which is
where support settles from.
@vercel

vercel Bot commented Aug 25, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
frontier Ready Ready Preview Aug 25, 2026 9:47am

@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 29be2f14-509b-41bb-a741-992440c2c886

📥 Commits

Reviewing files that changed from the base of the PR and between e1ffe69 and 3e3e876.

📒 Files selected for processing (5)
  • core/deleter/delete_notice.go
  • core/deleter/service.go
  • core/deleter/service_test.go
  • docs/content/docs/reference/webhook.mdx
  • web/sdk/admin/utils/webhook-events.ts

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Summary by CodeRabbit

  • New Features
    • Added organization deletion notices for owners after successful deletion.
    • Added support for app.billing.checkout.deleted and app.billing.tokens.forfeited webhook events.
  • Improvements
    • Deletion notices now provide more consistent information, including cases where no balance remains.
    • Owner notification delivery is handled on a best-effort basis without blocking organization deletion.
  • Documentation
    • Updated webhook event reference documentation with the new billing events.

Walkthrough

The PR renames billing notice configuration, updates cascade deletion wiring, changes owner notification behavior, preserves audit amount validation, and registers two billing webhook events.

Changes

Organization deletion notice flow

Layer / File(s) Summary
Notice configuration and constructor wiring
billing/config.go, core/deleter/service.go, cmd/serve.go, core/deleter/service_test.go
The configuration and cascade deleter now use OrgDeleteNoticeConfig. Runtime wiring passes cfg.Billing.OrgDeleteNotice.
Notice collection and delivery
core/deleter/delete_notice.go, core/deleter/service.go, core/deleter/service_test.go
Deletion collects notice data before blocker checks, resolves owners best-effort, and sends deletion notices after successful deletion. Tests cover successful and failure paths.
Audit and webhook validation
core/deleter/service_test.go, docs/content/docs/reference/webhook.mdx, web/sdk/admin/utils/webhook-events.ts
Tests verify forfeiture audit amounts and retry recovery. Documentation and the SDK list app.billing.checkout.deleted and app.billing.tokens.forfeited.

Estimated code review effort: 4 (Complex) | ~40 minutes

Merge Risk: ⚪ Minimal · up to 3e3e8

The change makes organization deletion notices consistent and updates the related configuration and documentation without any identified current-head merge-blocking risk.

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 9df21cb3-a6bd-4ce1-b9b1-0869551ef988

📥 Commits

Reviewing files that changed from the base of the PR and between c6c57cf and dd97cc4.

📒 Files selected for processing (5)
  • billing/config.go
  • cmd/serve.go
  • core/deleter/delete_notice.go
  • core/deleter/service.go
  • core/deleter/service_test.go

Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.

Comment thread core/deleter/service.go
@coveralls

coveralls commented Aug 25, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 32833795403

Coverage increased (+0.03%) to 49.157%

Details

  • Coverage increased (+0.03%) from the base build.
  • Patch coverage: 12 uncovered changes across 2 files (38 of 50 lines covered, 76.0%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
core/deleter/delete_notice.go 24 13 54.17%
cmd/serve.go 1 0 0.0%
Total (3 files) 50 38 76.0%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 40515
Covered Lines: 19916
Line Coverage: 49.16%
Coverage Strength: 15.73 hits per line

💛 - Coveralls

@whoAbhishekSah

Copy link
Copy Markdown
Member Author

Tested this live on a local build of the branch, with a real SMTP sink catching the mails and a real Stripe test checkout for the purchased-token case.

Case Result
Zero-token org, owner deletes it Mail sent with subject "Your organization was deleted". No settlement line, no amounts.
Complimentary (awarded) tokens, 100 forfeited Mail has no amounts and no settlement line. The app.billing.tokens.forfeited audit row keeps amount=100, purchased=0.
Purchased tokens (paid a Stripe test checkout, then deleted) Mail adds "Unused purchased tokens will be settled by the support team", still no numbers. Audit keeps amount=100, purchased=100.
Two owners Both owners received their own copy.
Org deleted by a platform admin (service account) The owner was mailed. The body names the deleter by its title. This org also had no billing account, so the empty-customers path works.
Mail server down Delete still succeeds. The log warns "failed to send the delete notice ... connection refused".
Custom template via billing.org_delete_notice Custom subject and body rendered. .Amount and .Purchased are available to config templates as described.
Blocked delete (active paid subscription) Delete fails with the blocker error, the org survives, and no notice is attempted. After cancelling the subscription, the retried delete succeeded and mailed the owner.

The default mails never contain token counts, which matches the settlement concern in the description.

Teardown deletes the owner policies before the roles and the org row.
A delete failing in that window left the retry unable to resolve the
owners, so the completing retry emailed no one. The owner ids now go
into an audit record before teardown starts, and a retry that finds no
resolvable owners loads them from there — the users themselves outlive
the org, so ids are all it needs. Best-effort on both sides, like the
notice itself: without a readable audit store the send path logs that
no owner could be notified.

@rohilsurana rohilsurana left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A few notes on the audit record, the tests, and some leftovers from the rename.

Comment thread core/audit/audit.go Outdated
Comment thread core/deleter/delete_notice.go Outdated
Comment thread core/deleter/service_test.go Outdated
Comment thread core/deleter/delete_notice.go Outdated
Comment thread core/deleter/delete_notice.go
The recipients record turned internal retry state into an audit event,
and every audit log fans out to the registered webhooks — pushing owner
user ids to third-party endpoints for a window that spans two local
statements at the very tail of teardown. The audit log stays what it
was meant to be here: the record of the forfeited amounts. A delete
that fails in that narrow window and completes on retry logs that no
owner could be notified, which is the visible, honest outcome.
The forfeit record and its retry recovery now have real coverage: the
tests put an audit service with an in-memory repository into the
context, assert the record carries the amount and the purchased share,
and assert a retry that finds no billing accounts rebuilds the notice
from the record. The two successful-delete subscription tests assert
the owner mail instead of silencing it through a failed owner lookup.
The dead zero-total return in collectDeleteNotice is gone, the last
"transferable" comments say "settled" like the mail copy, and the
webhook event list plus its docs page now name app.billing.tokens.forfeited
and app.billing.checkout.deleted, which audit publishing already sends
to subscribers.
@whoAbhishekSah
whoAbhishekSah merged commit 0cde896 into main Aug 25, 2026
8 checks passed
@whoAbhishekSah
whoAbhishekSah deleted the org-delete-notice-always branch August 25, 2026 10:35
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.

3 participants