fix(import): guard unparseable CSV dates and never wedge the button (L7)
A non-empty invalid transaction_date ('N/A') passed presence-only validation,
then new Date(str).toISOString() threw mid-import: nothing imported, no toast,
and setImporting(false) never ran, sticking the button on 'Importing...'.
Extracted parsePaymentRow to lib/import-logic.ts with a date guard, and wrapped
runImport so the importing flag always clears.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
Moheeb Zara
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
8274af7d5c
commit
9bd21b185d
@@ -0,0 +1,44 @@
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import { parsePaymentRow } from '@/lib/import-logic'
|
||||
|
||||
describe('parsePaymentRow', () => {
|
||||
it('parses a well-formed row into a normalized payment', () => {
|
||||
const r = parsePaymentRow(
|
||||
{ amount: '$1,200.50', from_identifier: 'Ada', from_note: 'dues', transaction_date: '2026-01-15', platform: 'venmo' },
|
||||
2,
|
||||
)
|
||||
expect(r.ok).toBe(true)
|
||||
if (r.ok) {
|
||||
expect(r.value.amount).toBe(1200.5)
|
||||
expect(r.value.platform).toBe('venmo')
|
||||
expect(r.value.from_identifier).toBe('Ada')
|
||||
expect(r.value.transaction_date).toBe(new Date('2026-01-15').toISOString())
|
||||
}
|
||||
})
|
||||
|
||||
it('defaults platform to csv when unmapped', () => {
|
||||
const r = parsePaymentRow({ amount: '10', from_identifier: 'Bo', transaction_date: '2026-02-01' }, 3)
|
||||
expect(r.ok).toBe(true)
|
||||
if (r.ok) expect(r.value.platform).toBe('csv')
|
||||
})
|
||||
|
||||
// The bug: a non-empty but unparseable date passed presence validation, then
|
||||
// new Date(str).toISOString() threw RangeError mid-map, wedging the import.
|
||||
it('reports an error (never throws) for a non-empty unparseable date', () => {
|
||||
expect(() =>
|
||||
parsePaymentRow({ amount: '10', from_identifier: 'Cy', transaction_date: 'N/A' }, 4),
|
||||
).not.toThrow()
|
||||
const r = parsePaymentRow({ amount: '10', from_identifier: 'Cy', transaction_date: 'N/A' }, 4)
|
||||
expect(r.ok).toBe(false)
|
||||
if (!r.ok) expect(r.error).toBe('Row 4: unparseable date "N/A"')
|
||||
})
|
||||
|
||||
it('reports missing required fields', () => {
|
||||
expect(parsePaymentRow({ amount: 'nope', from_identifier: 'Di', transaction_date: '2026-01-01' }, 5)).toEqual({
|
||||
ok: false,
|
||||
error: 'Row 5: missing required fields',
|
||||
})
|
||||
expect(parsePaymentRow({ amount: '10', from_identifier: '', transaction_date: '2026-01-01' }, 6).ok).toBe(false)
|
||||
expect(parsePaymentRow({ amount: '10', from_identifier: 'Di', transaction_date: '' }, 7).ok).toBe(false)
|
||||
})
|
||||
})
|
||||
@@ -4,6 +4,7 @@ import { useState, useRef, useCallback, useMemo } from 'react'
|
||||
import { Upload, CheckCircle, AlertCircle, X, ChevronDown, ArrowRight, FileText, RefreshCcw } from 'lucide-react'
|
||||
import { toast } from 'sonner'
|
||||
import { importPaymentsCsv, importMembers } from '@/lib/actions'
|
||||
import { parsePaymentRow, type ParsedPaymentRow } from '@/lib/import-logic'
|
||||
|
||||
// ─── Lightweight CSV parser (handles quoted fields, no external deps) ──────────
|
||||
function parseCSV(text: string): { headers: string[]; rows: string[][] } {
|
||||
@@ -196,85 +197,81 @@ export default function ImportClient({ role }: Props) {
|
||||
const errors: string[] = []
|
||||
let success = 0
|
||||
|
||||
if (mode === 'payments') {
|
||||
const paymentRows = rows.map((row, i) => {
|
||||
const obj: Record<string, string> = {}
|
||||
headers.forEach((h, j) => {
|
||||
if (columnMap[h] && columnMap[h] !== 'skip') obj[columnMap[h]] = row[j] ?? ''
|
||||
// try/finally so a thrown error can never leave the button wedged on
|
||||
// 'Importing...' — setImporting(false) always runs.
|
||||
try {
|
||||
if (mode === 'payments') {
|
||||
const paymentRows: ParsedPaymentRow[] = []
|
||||
rows.forEach((row, i) => {
|
||||
const obj: Record<string, string> = {}
|
||||
headers.forEach((h, j) => {
|
||||
if (columnMap[h] && columnMap[h] !== 'skip') obj[columnMap[h]] = row[j] ?? ''
|
||||
})
|
||||
// Row 1 is the header, so rows[i] is spreadsheet row i + 2. The parse
|
||||
// guards the date: 'N/A' and friends become a skipped row, not a throw.
|
||||
const parsed = parsePaymentRow(obj, i + 2)
|
||||
if (parsed.ok) paymentRows.push(parsed.value)
|
||||
else errors.push(parsed.error)
|
||||
})
|
||||
const amount = parseFloat((obj.amount ?? '').replace(/[$,\s]/g, ''))
|
||||
if (isNaN(amount) || !obj.from_identifier || !obj.transaction_date) {
|
||||
errors.push(`Row ${i + 2}: missing required fields`)
|
||||
return null
|
||||
}
|
||||
return {
|
||||
platform: (obj.platform || 'csv') as any,
|
||||
amount,
|
||||
from_identifier: obj.from_identifier,
|
||||
from_note: obj.from_note,
|
||||
transaction_date: new Date(obj.transaction_date).toISOString(),
|
||||
}
|
||||
}).filter(Boolean) as any[]
|
||||
|
||||
const result = await importPaymentsCsv(paymentRows)
|
||||
if ('error' in result && result.error) {
|
||||
toast.error(result.error)
|
||||
setImporting(false)
|
||||
return
|
||||
}
|
||||
success = (result as any).count ?? paymentRows.length
|
||||
|
||||
} else {
|
||||
// Members import goes through the validated, admin-gated server action
|
||||
// (Zod per row, batched upsert on (space_id,email), user_id left NULL
|
||||
// for offline members). No direct client DB writes.
|
||||
const tierMap: Record<string, 'plus' | 'basic' | 'associate'> = {
|
||||
plus: 'plus', premium: 'plus', full: 'plus',
|
||||
basic: 'basic', member: 'basic', standard: 'basic',
|
||||
associate: 'associate', visitor: 'associate', guest: 'associate',
|
||||
}
|
||||
const memberRows = rows.map((row, i) => {
|
||||
const obj: Record<string, string> = {}
|
||||
headers.forEach((h, j) => {
|
||||
if (columnMap[h] && columnMap[h] !== 'skip') obj[columnMap[h]] = row[j] ?? ''
|
||||
})
|
||||
if (!obj.display_name || !obj.email) {
|
||||
errors.push(`Row ${i + 2}: missing name or email`)
|
||||
return null
|
||||
const result = await importPaymentsCsv(paymentRows)
|
||||
if ('error' in result && result.error) {
|
||||
toast.error(result.error)
|
||||
return
|
||||
}
|
||||
return {
|
||||
display_name: obj.display_name,
|
||||
email: obj.email,
|
||||
phone: obj.phone || null,
|
||||
tier: tierMap[(obj.tier || '').toLowerCase()] || 'basic',
|
||||
joined_at: obj.joined_at || undefined,
|
||||
last_paid_at: obj.last_paid_at || undefined,
|
||||
has_card_access: /yes|true|1|y/i.test(obj.has_card_access ?? ''),
|
||||
}
|
||||
}).filter(Boolean) as Array<Record<string, unknown>>
|
||||
success = result.count ?? paymentRows.length
|
||||
|
||||
if (memberRows.length === 0) {
|
||||
toast.error('No valid rows to import')
|
||||
setImporting(false)
|
||||
return
|
||||
} else {
|
||||
// Members import goes through the validated, admin-gated server action
|
||||
// (Zod per row, batched upsert on (space_id,email), user_id left NULL
|
||||
// for offline members). No direct client DB writes.
|
||||
const tierMap: Record<string, 'plus' | 'basic' | 'associate'> = {
|
||||
plus: 'plus', premium: 'plus', full: 'plus',
|
||||
basic: 'basic', member: 'basic', standard: 'basic',
|
||||
associate: 'associate', visitor: 'associate', guest: 'associate',
|
||||
}
|
||||
const memberRows = rows.map((row, i) => {
|
||||
const obj: Record<string, string> = {}
|
||||
headers.forEach((h, j) => {
|
||||
if (columnMap[h] && columnMap[h] !== 'skip') obj[columnMap[h]] = row[j] ?? ''
|
||||
})
|
||||
if (!obj.display_name || !obj.email) {
|
||||
errors.push(`Row ${i + 2}: missing name or email`)
|
||||
return null
|
||||
}
|
||||
return {
|
||||
display_name: obj.display_name,
|
||||
email: obj.email,
|
||||
phone: obj.phone || null,
|
||||
tier: tierMap[(obj.tier || '').toLowerCase()] || 'basic',
|
||||
joined_at: obj.joined_at || undefined,
|
||||
last_paid_at: obj.last_paid_at || undefined,
|
||||
has_card_access: /yes|true|1|y/i.test(obj.has_card_access ?? ''),
|
||||
}
|
||||
}).filter(Boolean) as Array<Record<string, unknown>>
|
||||
|
||||
if (memberRows.length === 0) {
|
||||
toast.error('No valid rows to import')
|
||||
return
|
||||
}
|
||||
|
||||
const result = await importMembers(memberRows)
|
||||
if ('error' in result && result.error) {
|
||||
toast.error(result.error)
|
||||
return
|
||||
}
|
||||
const r = result as { count: number; skipped: number }
|
||||
success = r.count
|
||||
if (r.skipped > 0) errors.push(`${r.skipped} row(s) skipped (invalid email, name, tier, or date)`)
|
||||
}
|
||||
|
||||
const result = await importMembers(memberRows)
|
||||
if ('error' in result && result.error) {
|
||||
toast.error(result.error)
|
||||
setImporting(false)
|
||||
return
|
||||
}
|
||||
const r = result as { count: number; skipped: number }
|
||||
success = r.count
|
||||
if (r.skipped > 0) errors.push(`${r.skipped} row(s) skipped (invalid email, name, tier, or date)`)
|
||||
setResult({ success, failed: errors.length, errors })
|
||||
setStep(3)
|
||||
if (success > 0) toast.success(`Imported ${success} ${mode === 'members' ? 'members' : 'payments'}`)
|
||||
if (errors.length > 0) toast.error(`${errors.length} rows failed`)
|
||||
} finally {
|
||||
setImporting(false)
|
||||
}
|
||||
|
||||
setImporting(false)
|
||||
setResult({ success, failed: errors.length, errors })
|
||||
setStep(3)
|
||||
if (success > 0) toast.success(`Imported ${success} ${mode === 'members' ? 'members' : 'payments'}`)
|
||||
if (errors.length > 0) toast.error(`${errors.length} rows failed`)
|
||||
}
|
||||
|
||||
function reset() {
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
// Pure parse/validate for one column-mapped CSV payment row, shared by the
|
||||
// import wizard's payments path. Extracted so the date guard is unit-testable:
|
||||
// a non-empty but unparseable transaction_date (e.g. 'N/A') must surface as a
|
||||
// per-row error, never throw. A thrown RangeError from `new Date(str).toISOString()`
|
||||
// mid-map aborts the entire import — nothing gets sent, no toast fires, and the
|
||||
// wizard's 'Importing...' button stays wedged. Validation here mirrors the
|
||||
// server's Zod row schema loosely (presence + a real date); the server remains
|
||||
// the source of truth and re-validates every row.
|
||||
|
||||
export interface ParsedPaymentRow {
|
||||
platform: string
|
||||
amount: number
|
||||
from_identifier: string
|
||||
from_note: string | undefined
|
||||
transaction_date: string
|
||||
}
|
||||
|
||||
export type PaymentRowResult =
|
||||
| { ok: true; value: ParsedPaymentRow }
|
||||
| { ok: false; error: string }
|
||||
|
||||
// `mapped` is the app-field-keyed row (field key -> raw cell string). `rowNumber`
|
||||
// is the 1-based spreadsheet row (header = row 1) used only for error messages.
|
||||
export function parsePaymentRow(
|
||||
mapped: Record<string, string>,
|
||||
rowNumber: number,
|
||||
): PaymentRowResult {
|
||||
const amount = parseFloat((mapped.amount ?? '').replace(/[$,\s]/g, ''))
|
||||
if (isNaN(amount) || !mapped.from_identifier || !mapped.transaction_date) {
|
||||
return { ok: false, error: `Row ${rowNumber}: missing required fields` }
|
||||
}
|
||||
const parsed = new Date(mapped.transaction_date)
|
||||
if (isNaN(parsed.getTime())) {
|
||||
return { ok: false, error: `Row ${rowNumber}: unparseable date "${mapped.transaction_date}"` }
|
||||
}
|
||||
return {
|
||||
ok: true,
|
||||
value: {
|
||||
platform: mapped.platform || 'csv',
|
||||
amount,
|
||||
from_identifier: mapped.from_identifier,
|
||||
from_note: mapped.from_note,
|
||||
transaction_date: parsed.toISOString(),
|
||||
},
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user