Skip to content

Commit e83bb50

Browse files
fix(reads): page the nine remaining unbounded sweeps and the digest RPC (#548) (#556)
PostgREST caps every response at the API "Max rows" limit and returns the capped result as an ordinary 200, so truncation is indistinguishable from a complete read. #533 shipped `fetchAllRows` but wired it to only two call sites; nine reads were left unbounded. Two of them failed as wrong user-visible behaviour rather than a wrong number: - The digest's `notification_preferences` read. `resolveChannels(undefined)` treats a missing row as the schema default `{ inApp: true, email: true }`, so every student the truncated read dropped got emailed — including the ones who had turned email off. A short read could not mean "send more email"; now the read is chunked and paged, and a failure skips the tenant instead of falling through with an empty preferences map. - The digest's idempotency read. One notification row is written per recipient, so past the cap it truncated and a cron retry re-sent the day's digests, inverting the module's documented "re-runs never double-send" guarantee exactly when a retry happens. `get_daily_digest_candidates` could not be fixed from the caller side: it is a SETOF function with no ORDER BY, so `.range()` was not a stable window and there was no exact count to verify against. It now takes a keyset cursor on (tenant_id, user_id) — UNIQUE via tenant_users_unique — with ORDER BY and a clamped LIMIT. Keyset rather than OFFSET because the candidate set is computed live and shifts under a multi-second sweep. The caller stops on an EMPTY page, never a short one: a short page is ambiguous between "end of set" and "the row cap clamped us", and reading it as "done" is the original bug in a new place. Also adds `fetchAllRowsIn` for `.in()` lookups. Those have a second ceiling on the REQUEST side — a few hundred uuids overflow the gateway's URL limit — and response paging cannot help with a request that is never made. Verified by forcing the cap, not by a green run on a small dataset: - Live, against real PostgREST with the row cap lowered to 5 and 37 transactions / 23 payouts / 37 candidates seeded past it (scripts/qa-548-cap-forcing.mts). Unpaged reads come back with 5 rows while count reports 37; every paged read returns all 37 and the sums are exact. The opted-out student at index 30 is invisible to the unpaged read and found by the paged one. - In unit tests, via a fake PostgREST that clamps every response to 7 rows over a 40-student fixture. Reverting the fix fails 6 of them with exactly the expected numbers (7 of 40 sent; the opted-out student emailed). `tests/unit/unbounded-read-guard.test.ts` statically scans the tree for reads against growth tables that never bound themselves, so the next unbounded sweep fails in review. Pre-existing sites are recorded in KNOWN_UNBOUNDED as a ratchet, split into `scoped` (bounded by a user or a course) and `gap` (real platform-wide sweeps still outstanding under #540); a stale entry also fails, so the list cannot rot. `rollover_all_leagues`' tenant loop is untouched — it is PL/pgSQL and never crosses PostgREST, so it is not subject to the cap. Gates: typecheck, lint, build, test:unit 446/446 (was 411). Claude-Session: https://claude.ai/code/session_0185GrowMKwTrLQE1it2HNGN Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent d2cef7c commit e83bb50

14 files changed

Lines changed: 1383 additions & 100 deletions

File tree

app/[locale]/dashboard/admin/payouts/page.tsx

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { getTranslations } from 'next-intl/server'
44
import { format } from 'date-fns'
55
import { es, enUS } from 'date-fns/locale'
66
import { getCurrentTenantId, getCurrentUserId } from '@/lib/supabase/tenant'
7+
import { fetchAllRows } from '@/lib/supabase/fetch-all-rows'
78
import { formatByCurrency, formatMoney, sumByCurrency } from '@/lib/payments/format-money'
89
import { AdminBreadcrumb } from '@/components/admin/admin-breadcrumb'
910
import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'
@@ -62,16 +63,23 @@ export default async function AdminPayoutsPage({
6263
)
6364
}
6465

65-
// Fetch payouts for this tenant
66-
const { data: payouts } = await supabase
67-
.from('payouts')
68-
.select(
69-
'payout_id, amount, currency, status, period_start, period_end, stripe_payout_id, paid_at, failure_reason, created_at, note, payout_method'
70-
)
71-
.eq('tenant_id', tenantId)
72-
.order('created_at', { ascending: false })
73-
74-
const rows = payouts || []
66+
// Fetch payouts for this tenant. Paged and count-verified (#548): the
67+
// summary below sums this list, so a read truncated at the API row cap would
68+
// understate the school's own "Total paid" without any error to notice.
69+
// `created_at` is not unique, so `payout_id` breaks ties — an unstable sort
70+
// makes `.range()` windows overlap or skip.
71+
const rows = await fetchAllRows('payouts', (from, to) =>
72+
supabase
73+
.from('payouts')
74+
.select(
75+
'payout_id, amount, currency, status, period_start, period_end, stripe_payout_id, paid_at, failure_reason, created_at, note, payout_method',
76+
{ count: 'exact' }
77+
)
78+
.eq('tenant_id', tenantId)
79+
.order('created_at', { ascending: false })
80+
.order('payout_id', { ascending: false })
81+
.range(from, to)
82+
)
7583

7684
// Summary calculations. Paid payouts are totalled per currency and rendered as
7785
// one figure each — a school selling in USD and EUR reads two figures, not

app/[locale]/dashboard/teacher/revenue/page.tsx

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { createAdminClient } from '@/lib/supabase/admin'
22
import { getCurrentTenantId } from '@/lib/supabase/tenant'
3+
import { fetchAllRows } from '@/lib/supabase/fetch-all-rows'
34
import { getUserRole } from '@/lib/supabase/get-user-role'
45
import { redirect } from 'next/navigation'
56
import { getTranslations } from 'next-intl/server'
@@ -23,12 +24,23 @@ export default async function RevenuePage() {
2324
const tenantId = await getCurrentTenantId()
2425
const t = await getTranslations('dashboard.teacher.revenue')
2526

26-
// Parallelize all 4 independent queries
27-
const [{ data: tenant }, { data: split }, { data: transactions }, { data: payouts }] = await Promise.all([
27+
// Parallelize all 4 independent queries.
28+
//
29+
// The transaction read is paged and count-verified (#548) — every figure on
30+
// this page is a sum over it, so truncation at the API row cap would quietly
31+
// understate the school's revenue rather than fail. `created_at` is not
32+
// unique; `transaction_id` breaks ties so the paging windows are stable.
33+
// The payout read is already bounded by `.limit(10)`.
34+
const [{ data: tenant }, { data: split }, transactions, { data: payouts }] = await Promise.all([
2835
supabase.from('tenants').select('name, stripe_account_id').eq('id', tenantId).single(),
2936
supabase.from('revenue_splits').select('platform_percentage, school_percentage').eq('tenant_id', tenantId).single(),
30-
supabase.from('transactions').select('amount, status, payment_provider, created_at')
31-
.eq('tenant_id', tenantId).eq('status', 'successful').order('created_at', { ascending: false }),
37+
fetchAllRows('transactions', (from, to) =>
38+
supabase.from('transactions').select('amount, status, payment_provider, created_at', { count: 'exact' })
39+
.eq('tenant_id', tenantId).eq('status', 'successful')
40+
.order('created_at', { ascending: false })
41+
.order('transaction_id', { ascending: false })
42+
.range(from, to)
43+
),
3244
supabase.from('payouts').select('*').eq('tenant_id', tenantId).order('created_at', { ascending: false }).limit(10),
3345
])
3446

@@ -114,6 +126,10 @@ export default async function RevenuePage() {
114126
<p className="mt-0.5 text-xs text-amber-700 dark:text-amber-400">
115127
{t('stripeNotConnected.description')}
116128
</p>
129+
{/* Stays a plain anchor: /api/stripe/connect is a route handler that
130+
mints a Stripe onboarding link and redirects. next/link would
131+
prefetch it and fire that side effect on hover. */}
132+
{/* eslint-disable-next-line @next/next/no-html-link-for-pages */}
117133
<a
118134
href="/api/stripe/connect"
119135
className="mt-3 inline-flex items-center justify-center rounded-lg text-xs font-medium bg-amber-600 text-white hover:bg-amber-700 h-8 px-4 transition-colors"

app/actions/admin/revenue.ts

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import { createClient } from '@/lib/supabase/server'
44
import { createAdminClient } from '@/lib/supabase/admin'
55
import {getCurrentTenantId, getCurrentUserId } from '@/lib/supabase/tenant'
6+
import { fetchAllRows } from '@/lib/supabase/fetch-all-rows'
7+
import { fetchAllRowsIn } from '@/lib/supabase/fetch-all-rows-in'
68

79
async function verifyAdminAccess() {
810
const supabase = await createClient()
@@ -29,14 +31,26 @@ export async function getRevenueOverview() {
2931
const { tenantId } = await verifyAdminAccess()
3032
const adminClient = await createAdminClient()
3133

32-
// Get all successful transactions for this tenant
33-
const { data: transactions } = await adminClient
34-
.from('transactions')
35-
.select('amount, currency, transaction_date, product_id, plan_id, stripe_payment_intent_id')
36-
.eq('tenant_id', tenantId)
37-
.eq('status', 'successful')
38-
39-
if (!transactions || transactions.length === 0) {
34+
// Get all successful transactions for this tenant. Paged and count-verified
35+
// (#548): everything this action returns — total revenue, platform fees,
36+
// per-product breakdown, the monthly trend — is a sum over this list, so a
37+
// read truncated at the API row cap would report a confidently wrong number
38+
// instead of an error. Ordered by the primary key so the paging windows
39+
// neither overlap nor skip.
40+
const transactions = await fetchAllRows('transactions', (from, to) =>
41+
adminClient
42+
.from('transactions')
43+
.select(
44+
'amount, currency, transaction_date, product_id, plan_id, stripe_payment_intent_id',
45+
{ count: 'exact' }
46+
)
47+
.eq('tenant_id', tenantId)
48+
.eq('status', 'successful')
49+
.order('transaction_id')
50+
.range(from, to)
51+
)
52+
53+
if (transactions.length === 0) {
4054
return {
4155
totalRevenue: 0,
4256
platformFees: 0,
@@ -79,16 +93,20 @@ export async function getRevenueOverview() {
7993
revenueByProduct[key] = (revenueByProduct[key] || 0) + Number(tx.amount)
8094
}
8195

82-
// Get product names
96+
// Get product names. The id list is as long as the transaction set is
97+
// varied, so it is chunked as well as paged (#548) — an over-long `.in()`
98+
// fails on the request side, before any of the response paging matters.
8399
const productIds = [...new Set(transactions.map(t => t.product_id).filter(Boolean))]
84-
const { data: products } = productIds.length > 0
85-
? await adminClient
86-
.from('products')
87-
.select('product_id, name')
88-
.in('product_id', productIds)
89-
: { data: [] }
90-
91-
const productMap = new Map((products || []).map(p => [p.product_id, p.name]))
100+
const products = await fetchAllRowsIn('products', productIds, (chunk, from, to) =>
101+
adminClient
102+
.from('products')
103+
.select('product_id, name', { count: 'exact' })
104+
.in('product_id', chunk)
105+
.order('product_id')
106+
.range(from, to)
107+
)
108+
109+
const productMap = new Map(products.map(p => [p.product_id, p.name]))
92110

93111
const revenueByCourse = Object.entries(revenueByProduct).map(([id, amount]) => ({
94112
id: Number(id),

app/actions/platform/billing-health.ts

Lines changed: 53 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import { createAdminClient } from '@/lib/supabase/admin'
44
import { isSuperAdmin } from '@/lib/supabase/get-user-role'
55
import { getCurrentUserId } from '@/lib/supabase/tenant'
6+
import { fetchAllRows } from '@/lib/supabase/fetch-all-rows'
7+
import { fetchAllRowsIn } from '@/lib/supabase/fetch-all-rows-in'
68
import {
79
computeBillingHealth,
810
mergeAtRiskTenants,
@@ -85,19 +87,42 @@ export async function getAtRiskTenants(): Promise<AtRiskTenant[]> {
8587
await verifySuperAdmin()
8688
const admin = createAdminClient()
8789

88-
const [
89-
{ data: pastDueTenants },
90-
{ data: pastDueSubscriptions },
91-
{ data: cutoffTenants },
92-
] = await Promise.all([
93-
admin.from('tenants').select(TENANT_SELECT).eq('billing_status', 'past_due'),
94-
admin.from('platform_subscriptions').select(SUBSCRIPTION_SELECT).eq('status', 'past_due'),
95-
admin.from('tenants').select(TENANT_SELECT).not('access_cutoff_at', 'is', null),
90+
// All five reads are paged and count-verified (#548). This dashboard exists
91+
// to make sure no at-risk school goes unnoticed, so a read silently capped
92+
// at the API row limit defeats its entire purpose: the missing school looks
93+
// exactly like a healthy one. Each is ordered by its primary key — `tenants`
94+
// and `platform_subscriptions` have no natural sort here, and an unordered
95+
// `.range()` window is not a stable page.
96+
const [pastDueTenants, pastDueSubscriptions, cutoffTenants] = await Promise.all([
97+
fetchAllRows('tenants:past_due', (from, to) =>
98+
admin
99+
.from('tenants')
100+
.select(TENANT_SELECT, { count: 'exact' })
101+
.eq('billing_status', 'past_due')
102+
.order('id')
103+
.range(from, to)
104+
),
105+
fetchAllRows('platform_subscriptions:past_due', (from, to) =>
106+
admin
107+
.from('platform_subscriptions')
108+
.select(SUBSCRIPTION_SELECT, { count: 'exact' })
109+
.eq('status', 'past_due')
110+
.order('subscription_id')
111+
.range(from, to)
112+
),
113+
fetchAllRows('tenants:access_cutoff', (from, to) =>
114+
admin
115+
.from('tenants')
116+
.select(TENANT_SELECT, { count: 'exact' })
117+
.not('access_cutoff_at', 'is', null)
118+
.order('id')
119+
.range(from, to)
120+
),
96121
])
97122

98-
const pastDueTenantRows = (pastDueTenants ?? []).map(toTenantRow)
99-
const cutoffTenantRows = (cutoffTenants ?? []).map(toTenantRow)
100-
const pastDueSubscriptionRows = (pastDueSubscriptions ?? []).map(toSubscriptionInput)
123+
const pastDueTenantRows = pastDueTenants.map(toTenantRow)
124+
const cutoffTenantRows = cutoffTenants.map(toTenantRow)
125+
const pastDueSubscriptionRows = pastDueSubscriptions.map(toSubscriptionInput)
101126

102127
const knownTenantIds = new Set([
103128
...pastDueTenantRows.map((t) => t.tenantId),
@@ -109,26 +134,31 @@ export async function getAtRiskTenants(): Promise<AtRiskTenant[]> {
109134
// Subscription-only past-due tenants appear in neither read above.
110135
const missingTenantIds = subscriptionPastDueTenantIds.filter((id) => !knownTenantIds.has(id))
111136

112-
const [{ data: extraTenants }, { data: knownSubscriptions }] = await Promise.all([
113-
missingTenantIds.length
114-
? admin.from('tenants').select(TENANT_SELECT).in('id', missingTenantIds)
115-
: Promise.resolve({ data: [] as never[] }),
116-
knownTenantIds.size
117-
? admin
118-
.from('platform_subscriptions')
119-
.select(SUBSCRIPTION_SELECT)
120-
.in('tenant_id', [...knownTenantIds])
121-
: Promise.resolve({ data: [] as never[] }),
137+
// Both follow-ups look up by id list, which grows with the reads above — so
138+
// they are chunked as well as paged: past a few hundred ids the `.in()` URL
139+
// is the thing that breaks, before any row cap is reached.
140+
const [extraTenants, knownSubscriptions] = await Promise.all([
141+
fetchAllRowsIn('tenants:subscription_only', missingTenantIds, (chunk, from, to) =>
142+
admin.from('tenants').select(TENANT_SELECT, { count: 'exact' }).in('id', chunk).order('id').range(from, to)
143+
),
144+
fetchAllRowsIn('platform_subscriptions:known', [...knownTenantIds], (chunk, from, to) =>
145+
admin
146+
.from('platform_subscriptions')
147+
.select(SUBSCRIPTION_SELECT, { count: 'exact' })
148+
.in('tenant_id', chunk)
149+
.order('subscription_id')
150+
.range(from, to)
151+
),
122152
])
123153

124154
return computeBillingHealth(
125155
mergeAtRiskTenants({
126156
pastDueTenants: pastDueTenantRows,
127157
cutoffTenants: cutoffTenantRows,
128158
subscriptionPastDueTenantIds,
129-
extraTenants: (extraTenants ?? []).map(toTenantRow),
159+
extraTenants: extraTenants.map(toTenantRow),
130160
}),
131-
[...pastDueSubscriptionRows, ...(knownSubscriptions ?? []).map(toSubscriptionInput)],
161+
[...pastDueSubscriptionRows, ...knownSubscriptions.map(toSubscriptionInput)],
132162
new Date(),
133163
)
134164
}

app/api/cron/solana-pull/route.ts

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { timingSafeEqual } from 'crypto'
2020
import { getSubscriptionState } from '@/lib/payments/solana-subscriptions'
2121
import { pullSplitForSubscription } from '@/lib/payments/solana-subscription-pull'
2222
import { decidePullAction } from '@/lib/payments/solana-pull-decision'
23+
import { fetchAllRows } from '@/lib/supabase/fetch-all-rows'
2324

2425
export const runtime = 'nodejs'
2526

@@ -43,6 +44,24 @@ interface SolanaSubMeta {
4344
mint: string
4445
}
4546

47+
/** One row of the crank's work queue, as selected below. */
48+
interface SolanaSubRow {
49+
subscription_id: string
50+
tenant_id: string
51+
plan_id: number | null
52+
provider_metadata: unknown
53+
subscription_status: string | null
54+
cancel_at_period_end: boolean | null
55+
cancel_at: string | null
56+
/**
57+
* The embedded `plans(price)`. Left `unknown`: the untyped service-role
58+
* client types every embed as an array, while a to-one embed arrives as an
59+
* object at runtime — the read site below narrows it with the same cast it
60+
* always used.
61+
*/
62+
plans: unknown
63+
}
64+
4665
export async function GET(req: NextRequest) {
4766
const cronSecret = process.env.CRON_SECRET
4867
const provided = req.headers.get('authorization')?.replace('Bearer ', '')
@@ -63,17 +82,32 @@ export async function GET(req: NextRequest) {
6382
// Active native-subscription rows, with their on-chain coordinates + amount.
6483
// We also pull the cancel fields so the pull/cancel decision (which must never
6584
// charge a canceled sub — issue #460) has the full DB state.
66-
const { data: subs, error } = await admin
67-
.from('subscriptions')
68-
.select('subscription_id, tenant_id, plan_id, provider_metadata, subscription_status, cancel_at_period_end, cancel_at, plans(price)')
69-
.eq('payment_provider', 'solana_subs')
70-
.eq('subscription_status', 'active')
71-
72-
if (error) {
73-
console.error('[solana-pull] query error', error)
85+
//
86+
// Paged and count-verified (#548). This read IS the crank's work queue, and
87+
// the crank is the only thing that ever charges a native Solana
88+
// subscription: a row silently dropped at the API cap is a period that never
89+
// gets billed, for a subscriber whose access we keep extending. Ordered by
90+
// primary key so the pages are stable. `fetchAllRows` throws on an
91+
// incomplete read, so a short queue can no longer look like a finished one.
92+
let subs: SolanaSubRow[]
93+
try {
94+
subs = await fetchAllRows<SolanaSubRow>('subscriptions', (from, to) =>
95+
admin
96+
.from('subscriptions')
97+
.select(
98+
'subscription_id, tenant_id, plan_id, provider_metadata, subscription_status, cancel_at_period_end, cancel_at, plans(price)',
99+
{ count: 'exact' },
100+
)
101+
.eq('payment_provider', 'solana_subs')
102+
.eq('subscription_status', 'active')
103+
.order('subscription_id')
104+
.range(from, to),
105+
)
106+
} catch (err) {
107+
console.error('[solana-pull] query error', err)
74108
return NextResponse.json({ error: 'Query failed' }, { status: 500 })
75109
}
76-
if (!subs?.length) return NextResponse.json({ pulled: 0 })
110+
if (!subs.length) return NextResponse.json({ pulled: 0 })
77111

78112
const nowSec = Math.floor(Date.now() / 1000)
79113
let pulled = 0

0 commit comments

Comments
 (0)