|
| 1 | +import { readdirSync, readFileSync } from 'node:fs' |
| 2 | +import { join } from 'node:path' |
| 3 | +import { describe, expect, it } from 'vitest' |
| 4 | + |
| 5 | +/** |
| 6 | + * Review guard against the next phantom column (issue #547 §2). |
| 7 | + * |
| 8 | + * `transactions` has `transaction_date`. Two shipped revenue screens asked for |
| 9 | + * `created_at`, which does not exist on it. PostgREST rejects the whole request |
| 10 | + * with 42703, and neither call site checked the error — so |
| 11 | + * `/dashboard/admin/analytics` showed **$0.00 revenue and a count of 0 on every |
| 12 | + * render, for every school, permanently, with nothing logged**, and |
| 13 | + * `/dashboard/teacher/revenue` 500'd once #548 moved it onto `fetchAllRows`. |
| 14 | + * |
| 15 | + * Nothing caught it. TypeScript could not: the rows come back from PostgREST as |
| 16 | + * `any`-ish data and the column name lives inside a string. The unit suite could |
| 17 | + * not: no test rendered those pages. It survived a code review and a whole |
| 18 | + * payout-accuracy epic sitting in plain sight. |
| 19 | + * |
| 20 | + * So, like `unbounded-read-guard.test.ts`, the check has to be static. This |
| 21 | + * walks the source, pulls every column name a PostgREST chain mentions for the |
| 22 | + * money tables, and asserts each one actually exists in `lib/database.types.ts` |
| 23 | + * — the generated schema. A column that is not there cannot be queried, and now |
| 24 | + * cannot be committed either. |
| 25 | + * |
| 26 | + * Deliberately narrow: only the tables where a silently-wrong number is money, |
| 27 | + * only plain column tokens (embeds, aliases and FK hints are skipped rather |
| 28 | + * than half-parsed). A guard that cries wolf gets deleted. |
| 29 | + */ |
| 30 | + |
| 31 | +const ROOTS = ['app', 'lib', 'components'] |
| 32 | +const SOURCE_EXTENSIONS = ['.ts', '.tsx'] |
| 33 | + |
| 34 | +/** |
| 35 | + * Tables whose columns feed a currency figure. A typo in any of these reads as |
| 36 | + * $0.00 rather than as an error, which is exactly the failure #547 §2 was. |
| 37 | + */ |
| 38 | +const MONEY_TABLES = ['transactions', 'payouts', 'revenue_splits'] |
| 39 | + |
| 40 | +/** Filter/order methods that take a column name as their first argument. */ |
| 41 | +const COLUMN_ARG_METHODS = [ |
| 42 | + 'eq', 'neq', 'gt', 'gte', 'lt', 'lte', 'like', 'ilike', 'is', 'in', 'contains', 'order', |
| 43 | +] |
| 44 | + |
| 45 | +function sourceFiles(dir: string, acc: string[] = []): string[] { |
| 46 | + let entries |
| 47 | + try { |
| 48 | + entries = readdirSync(dir, { withFileTypes: true }) |
| 49 | + } catch { |
| 50 | + return acc |
| 51 | + } |
| 52 | + for (const entry of entries) { |
| 53 | + const full = join(dir, entry.name) |
| 54 | + if (entry.isDirectory()) { |
| 55 | + if (entry.name === 'node_modules' || entry.name === '.next') continue |
| 56 | + sourceFiles(full, acc) |
| 57 | + } else if (SOURCE_EXTENSIONS.some((ext) => entry.name.endsWith(ext))) { |
| 58 | + acc.push(full) |
| 59 | + } |
| 60 | + } |
| 61 | + return acc |
| 62 | +} |
| 63 | + |
| 64 | +/** |
| 65 | + * Columns of one table, read out of the generated types. Parsed rather than |
| 66 | + * imported so the guard reads the same artifact a reviewer would check by hand. |
| 67 | + */ |
| 68 | +function columnsOf(table: string): Set<string> { |
| 69 | + const types = readFileSync('lib/database.types.ts', 'utf8') |
| 70 | + const start = types.indexOf(` ${table}: {\n Row: {\n`) |
| 71 | + if (start < 0) throw new Error(`table ${table} not found in database.types.ts`) |
| 72 | + const rowStart = types.indexOf('Row: {', start) + 'Row: {'.length |
| 73 | + const rowEnd = types.indexOf('\n }', rowStart) |
| 74 | + const columns = new Set<string>() |
| 75 | + for (const line of types.slice(rowStart, rowEnd).split('\n')) { |
| 76 | + const match = line.match(/^\s*([a-z_][a-z0-9_]*)\??:/i) |
| 77 | + if (match) columns.add(match[1]) |
| 78 | + } |
| 79 | + return columns |
| 80 | +} |
| 81 | + |
| 82 | +interface Reference { |
| 83 | + file: string |
| 84 | + table: string |
| 85 | + column: string |
| 86 | +} |
| 87 | + |
| 88 | +/** |
| 89 | + * Every column name a chain mentions for a money table. |
| 90 | + * |
| 91 | + * A "chain" is taken as the text from `.from('<table>')` up to the next |
| 92 | + * `.from(` or the end of the statement block — long enough to cover a |
| 93 | + * multi-line builder, short enough not to swallow the next query. |
| 94 | + */ |
| 95 | +function collectReferences(): Reference[] { |
| 96 | + const found: Reference[] = [] |
| 97 | + for (const root of ROOTS) { |
| 98 | + for (const file of sourceFiles(root)) { |
| 99 | + const source = readFileSync(file, 'utf8') |
| 100 | + for (const table of MONEY_TABLES) { |
| 101 | + const fromPattern = new RegExp(`\\.from\\(\\s*['"\`]${table}['"\`]\\s*\\)`, 'g') |
| 102 | + let match: RegExpExecArray | null |
| 103 | + while ((match = fromPattern.exec(source)) !== null) { |
| 104 | + const rest = source.slice(match.index + match[0].length) |
| 105 | + const nextFrom = rest.indexOf('.from(') |
| 106 | + const chain = nextFrom === -1 ? rest.slice(0, 1200) : rest.slice(0, nextFrom) |
| 107 | + |
| 108 | + // .select('a, b, c') |
| 109 | + const select = chain.match(/\.select\(\s*'([^']*)'/) |
| 110 | + if (select) { |
| 111 | + for (const raw of select[1].split(',')) { |
| 112 | + const column = raw.trim() |
| 113 | + // Skip anything that is not a plain column: '*', embedded |
| 114 | + // relations `course:courses(...)`, FK hints `profiles!fk(...)`, |
| 115 | + // and the `count` aggregate. |
| 116 | + if (!column || column.includes('(') || column.includes(':') || column.includes('!')) continue |
| 117 | + if (column === '*' || column === 'count') continue |
| 118 | + found.push({ file, table, column }) |
| 119 | + } |
| 120 | + } |
| 121 | + |
| 122 | + // .eq('col', …) / .order('col', …) / .gte('col', …) |
| 123 | + const argPattern = new RegExp(`\\.(${COLUMN_ARG_METHODS.join('|')})\\(\\s*'([a-z_][a-z0-9_]*)'`, 'g') |
| 124 | + let argMatch: RegExpExecArray | null |
| 125 | + while ((argMatch = argPattern.exec(chain)) !== null) { |
| 126 | + found.push({ file, table, column: argMatch[2] }) |
| 127 | + } |
| 128 | + } |
| 129 | + } |
| 130 | + } |
| 131 | + } |
| 132 | + return found |
| 133 | +} |
| 134 | + |
| 135 | +describe('phantom-column guard (#547 §2)', () => { |
| 136 | + it('every column these queries name actually exists on its table', () => { |
| 137 | + const schema = new Map(MONEY_TABLES.map((table) => [table, columnsOf(table)])) |
| 138 | + const violations = collectReferences() |
| 139 | + .filter((ref) => !schema.get(ref.table)!.has(ref.column)) |
| 140 | + .map((ref) => `${ref.file}: ${ref.table}.${ref.column} does not exist`) |
| 141 | + |
| 142 | + // #547 §2 named four references across two pages. Run against the pre-fix |
| 143 | + // tree this guard reports SEVEN, across five: the admin dashboard's recent- |
| 144 | + // transactions list, the admin user-detail page and the platform tenant- |
| 145 | + // detail page were broken the same way and nobody had noticed, because each |
| 146 | + // renders its ordinary "no transactions yet" empty state on the rejected |
| 147 | + // query. That is the entire argument for a static guard over a reviewer. |
| 148 | + expect([...new Set(violations)]).toEqual([]) |
| 149 | + }) |
| 150 | + |
| 151 | + it('finds the columns it is supposed to be checking (the guard is not vacuous)', () => { |
| 152 | + // A guard that silently matches nothing passes forever. Pin that the |
| 153 | + // scan actually reaches the money tables and the real column names. |
| 154 | + const refs = collectReferences() |
| 155 | + expect(refs.length).toBeGreaterThan(20) |
| 156 | + expect(refs.some((r) => r.table === 'transactions' && r.column === 'transaction_date')).toBe(true) |
| 157 | + expect(refs.some((r) => r.table === 'transactions' && r.column === 'amount')).toBe(true) |
| 158 | + expect(refs.some((r) => r.table === 'payouts')).toBe(true) |
| 159 | + }) |
| 160 | + |
| 161 | + it('would catch the exact regression: transactions has no created_at', () => { |
| 162 | + // The bug in one line. `payouts.created_at` DOES exist, which is part of why |
| 163 | + // the mistake looked plausible in review — the same identifier is correct |
| 164 | + // one table over. |
| 165 | + expect(columnsOf('transactions').has('created_at')).toBe(false) |
| 166 | + expect(columnsOf('transactions').has('transaction_date')).toBe(true) |
| 167 | + expect(columnsOf('payouts').has('created_at')).toBe(true) |
| 168 | + }) |
| 169 | +}) |
| 170 | + |
| 171 | +describe('applies_to_providers stays retired (#547 §3)', () => { |
| 172 | + it('no reader has reintroduced the stale fee predicate', () => { |
| 173 | + // It stored the labels 'stripe'/'manual' rather than provider slugs, so a |
| 174 | + // PayPal sale matched nothing and bore a 0% platform fee on the school's |
| 175 | + // revenue screens while `getPayoutsOwed` applied the full split to the same |
| 176 | + // row — two authoritative screens an entire platform fee apart. Whether a |
| 177 | + // fee is taken is now a provider capability (`bearsPlatformFee`), in one |
| 178 | + // place. Reading the column again would silently reopen the divergence. |
| 179 | + const offenders: string[] = [] |
| 180 | + for (const root of ROOTS) { |
| 181 | + for (const file of sourceFiles(root)) { |
| 182 | + // `database.types.ts` still declares the column — it exists in the |
| 183 | + // schema, it is just no longer read. Prose mentioning it (this fix's own |
| 184 | + // explanatory comments) is fine too; only a READ reopens the divergence, |
| 185 | + // so comments are stripped before matching. |
| 186 | + if (file.endsWith('database.types.ts')) continue |
| 187 | + const source = readFileSync(file, 'utf8') |
| 188 | + .replace(/\/\*[\s\S]*?\*\//g, '') |
| 189 | + .replace(/(^|[^:])\/\/[^\n]*/g, '$1') |
| 190 | + if (source.includes('applies_to_providers')) offenders.push(file) |
| 191 | + } |
| 192 | + } |
| 193 | + expect(offenders).toEqual([]) |
| 194 | + }) |
| 195 | +}) |
0 commit comments