Skip to content

Commit 79a4d08

Browse files
committed
docs(billing): drop self-explanatory inline comments from the notification path
1 parent 375317b commit 79a4d08

4 files changed

Lines changed: 4 additions & 19 deletions

File tree

apps/sim/lib/billing/core/limit-notifications.test.ts

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ vi.mock('@sim/db', () => {
3030
set: () => updateBuilder,
3131
where: () => updateBuilder,
3232
returning: () => Promise.resolve(mockClaim()),
33-
// Awaited directly by the re-arm path (no `.returning()`).
3433
then: (f: (v: unknown) => unknown, r?: (e: unknown) => unknown) =>
3534
Promise.resolve(undefined).then(f, r),
3635
}
@@ -40,7 +39,6 @@ vi.mock('@sim/db', () => {
4039
innerJoin: () => selectBuilder,
4140
leftJoin: () => selectBuilder,
4241
limit: () => Promise.resolve(mockSelectRows()),
43-
// Awaited directly by the org-admins query (no `.limit()`).
4442
then: (f: (v: unknown) => unknown, r?: (e: unknown) => unknown) =>
4543
Promise.resolve(mockSelectRows()).then(f, r),
4644
}
@@ -81,8 +79,8 @@ describe('maybeSendLimitThresholdEmail', () => {
8179
beforeEach(() => {
8280
vi.clearAllMocks()
8381
billingFlag.enabled = true
84-
mockClaim.mockReturnValue([{ id: 'u1' }]) // claim wins by default
85-
mockSelectRows.mockReturnValue([]) // no settings row / no admins by default
82+
mockClaim.mockReturnValue([{ id: 'u1' }])
83+
mockSelectRows.mockReturnValue([])
8684
getEmailPreferencesMock.mockResolvedValue(null)
8785
})
8886

@@ -100,8 +98,6 @@ describe('maybeSendLimitThresholdEmail', () => {
10098
})
10199

102100
it('never sends in rearmOnly mode, even when usage is above a threshold', async () => {
103-
// A storage shrink that still leaves usage at 90% must only re-arm, not send,
104-
// even if the stored threshold is 0 (claim would otherwise win).
105101
await maybeSendLimitThresholdEmail({
106102
...baseUserParams,
107103
currentUsage: 4.5,
@@ -113,14 +109,12 @@ describe('maybeSendLimitThresholdEmail', () => {
113109
})
114110

115111
it('does not send when the atomic claim is lost (already notified)', async () => {
116-
mockClaim.mockReturnValue([]) // someone else already advanced the threshold
112+
mockClaim.mockReturnValue([])
117113
await maybeSendLimitThresholdEmail({ ...baseUserParams, currentUsage: 4.5, limit: 5 })
118114
expect(sendEmailSpy).not.toHaveBeenCalled()
119115
})
120116

121117
it('claims without re-arming on a crossing (re-arm and claim are mutually exclusive)', async () => {
122-
// 90% crossing: a single claim update, no re-arm — so there is no re-arm/claim
123-
// interleaving for a concurrent caller to exploit into a duplicate email.
124118
await maybeSendLimitThresholdEmail({ ...baseUserParams, currentUsage: 4.5, limit: 5 })
125119
expect(dbUpdateSpy).toHaveBeenCalledTimes(1)
126120
expect(mockClaim).toHaveBeenCalledTimes(1)
@@ -143,8 +137,6 @@ describe('maybeSendLimitThresholdEmail', () => {
143137
mockSelectRows.mockReturnValue([{ enabled: false }])
144138
await maybeSendLimitThresholdEmail({ ...baseUserParams, currentUsage: 4.5, limit: 5 })
145139
expect(sendEmailSpy).not.toHaveBeenCalled()
146-
// Recipient resolution gates the claim, so the threshold isn't advanced —
147-
// re-enabling notifications later still lets the email fire.
148140
expect(mockClaim).not.toHaveBeenCalled()
149141
})
150142

@@ -164,7 +156,7 @@ describe('maybeSendLimitThresholdEmail', () => {
164156

165157
it('re-arms but does not send when usage is fully cleared (zero usage)', async () => {
166158
await maybeSendLimitThresholdEmail({ ...baseUserParams, currentUsage: 0, limit: 5 })
167-
expect(dbUpdateSpy).toHaveBeenCalledTimes(1) // re-arm only
159+
expect(dbUpdateSpy).toHaveBeenCalledTimes(1)
168160
expect(mockClaim).not.toHaveBeenCalled()
169161
expect(sendEmailSpy).not.toHaveBeenCalled()
170162
})

apps/sim/lib/billing/core/limit-notifications.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,8 +290,6 @@ export async function maybeNotifyLimit(params: {
290290
: params.subscription
291291
const isOrg = Boolean(sub && isOrgScopedSubscription(sub, params.billedUserId))
292292

293-
// Only the send path (user scope, not re-arm, at/above the warn band) needs
294-
// the recipient's email — skip the read for the common far-from-limit case.
295293
const percent = params.limit > 0 ? (params.currentUsage / params.limit) * 100 : 0
296294
let userEmail: string | undefined
297295
let userName: string | undefined

apps/sim/lib/billing/storage/limits.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,6 @@ export async function getUserStorageLimit(
101101
return limits.free
102102
}
103103

104-
// Org-scoped subs use pooled org-level storage. Custom limits come from the
105-
// subscription metadata (already on `sub`); otherwise team/enterprise default.
106104
if (isOrgScopedSubscription(sub, userId)) {
107105
const metadata = sub.metadata as { customStorageLimitGB?: number } | null
108106
if (metadata?.customStorageLimitGB) {

apps/sim/lib/table/billing.test.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -161,9 +161,6 @@ describe('assertRowCapacity', () => {
161161
})
162162

163163
describe('notifyTableRowUsage — edge-crossing gate', () => {
164-
// The notify (and its first billing lookup) fire synchronously when the gate
165-
// passes, so asserting on the billed-account lookup is enough — no DB work
166-
// happens unless the insert actually crosses a threshold.
167164
beforeEach(() => mockGetWorkspaceBilledAccountUserId.mockClear())
168165

169166
it('fires when an insert crosses UP into the warn band (limit 5000)', () => {

0 commit comments

Comments
 (0)