Skip to content

Commit 948ac2d

Browse files
committed
fix(upload): address CodeRabbit findings — NaN guard, API status counts, XLSX key trim
- Guard parseInt against NaN for page/pageSize query params - Add statusCounts aggregate query to history API and return in response - Replace page-level statusCounts reduce with API-sourced state - Add res.ok check in fetchHistory before parsing JSON - Remove .xls from drop-zone accept attribute - Switch column-mapper to index-based identification in handleRemap - Catch JSON.parse error in commit route and return 400 - Trim XLSX row keys to match trimmed headers - Use rowCount ?? 0 for accurate insert count (avoid inflating on conflict)
1 parent e1497ec commit 948ac2d

6 files changed

Lines changed: 81 additions & 61 deletions

File tree

codebenders-dashboard/app/admin/upload/history/page.tsx

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ export default function UploadHistoryPage() {
4545
const [total, setTotal] = useState(0)
4646
const [page, setPage] = useState(1)
4747
const [loading, setLoading] = useState(true)
48+
const [statusCounts, setStatusCounts] = useState<Record<string, number>>({})
4849
const pageSize = 20
4950

5051
const fetchHistory = useCallback(async (p: number) => {
@@ -53,11 +54,14 @@ export default function UploadHistoryPage() {
5354
const res = await fetch(
5455
`/api/admin/upload/history?page=${p}&pageSize=${pageSize}`
5556
)
57+
if (!res.ok) throw new Error(`HTTP ${res.status}`)
5658
const data = await res.json()
5759
setEntries(data.data ?? [])
5860
setTotal(data.total ?? 0)
61+
setStatusCounts(data.statusCounts ?? {})
5962
} catch {
6063
setEntries([])
64+
setTotal(0)
6165
} finally {
6266
setLoading(false)
6367
}
@@ -69,14 +73,6 @@ export default function UploadHistoryPage() {
6973

7074
const pageCount = Math.ceil(total / pageSize)
7175

72-
const statusCounts = entries.reduce(
73-
(acc, e) => {
74-
acc[e.status] = (acc[e.status] ?? 0) + 1
75-
return acc
76-
},
77-
{} as Record<string, number>
78-
)
79-
8076
return (
8177
<div className="container mx-auto px-4 py-6 max-w-5xl">
8278
<div className="flex items-center justify-between mb-6">

codebenders-dashboard/app/api/admin/upload/commit/route.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,12 @@ export async function POST(request: NextRequest) {
3131
return NextResponse.json({ error: `Unknown schema: ${schemaId}` }, { status: 400 })
3232
}
3333

34-
const columnMapping: ColumnMapping[] = JSON.parse(mappingJson)
34+
let columnMapping: ColumnMapping[]
35+
try {
36+
columnMapping = JSON.parse(mappingJson)
37+
} catch {
38+
return NextResponse.json({ error: "Invalid column mapping JSON" }, { status: 400 })
39+
}
3540

3641
const fileType = getFileType(file.name)
3742
if (!fileType) {
@@ -170,7 +175,7 @@ async function upsertRows(
170175
ON CONFLICT (${conflictClause}) DO NOTHING`
171176

172177
const result = await pool.query(batchSql, batchParams)
173-
inserted += result.rowCount ?? batchValues.length
178+
inserted += result.rowCount ?? 0
174179
} catch (err) {
175180
// If batch fails, fall back to per-row to identify the bad row(s)
176181
for (let j = 0; j < batch.length; j++) {

codebenders-dashboard/app/api/admin/upload/history/route.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,13 @@ import { getPool } from "@/lib/db"
44
export async function GET(request: NextRequest) {
55
try {
66
const { searchParams } = new URL(request.url)
7-
const page = Math.max(1, parseInt(searchParams.get("page") ?? "1"))
8-
const pageSize = Math.min(50, Math.max(1, parseInt(searchParams.get("pageSize") ?? "20")))
7+
const page = Math.max(1, parseInt(searchParams.get("page") ?? "1") || 1)
8+
const pageSize = Math.min(50, Math.max(1, parseInt(searchParams.get("pageSize") ?? "20") || 20))
99
const offset = (page - 1) * pageSize
1010

1111
const pool = getPool()
1212

13-
const [dataResult, countResult] = await Promise.all([
13+
const [dataResult, countResult, statusResult] = await Promise.all([
1414
pool.query(
1515
`SELECT id, user_email, filename, file_type, rows_inserted, rows_skipped,
1616
error_count, status, uploaded_at
@@ -20,9 +20,16 @@ export async function GET(request: NextRequest) {
2020
[pageSize, offset]
2121
),
2222
pool.query(`SELECT COUNT(*)::int AS total FROM upload_history`),
23+
pool.query(
24+
`SELECT status, COUNT(*)::int AS count FROM upload_history GROUP BY status`
25+
),
2326
])
2427

2528
const total = countResult.rows[0].total
29+
const statusCounts: Record<string, number> = {}
30+
for (const row of statusResult.rows) {
31+
statusCounts[row.status] = row.count
32+
}
2633

2734
return NextResponse.json({
2835
data: dataResult.rows.map((row) => ({
@@ -39,6 +46,7 @@ export async function GET(request: NextRequest) {
3946
total,
4047
page,
4148
pageSize,
49+
statusCounts,
4250
})
4351
} catch (err) {
4452
console.error("Upload history error:", err)

codebenders-dashboard/components/upload/column-mapper.tsx

Lines changed: 50 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,18 @@ interface ColumnMapperProps {
1212
export function ColumnMapper({ columns, schema, onMappingChange }: ColumnMapperProps) {
1313
const [showAll, setShowAll] = useState(false)
1414

15-
const matched = columns.filter((c) => c.status === "matched")
16-
const unmapped = columns.filter((c) => c.status === "unmapped")
15+
const matchedCount = columns.filter((c) => c.status === "matched").length
16+
const unmappedCount = columns.filter((c) => c.status === "unmapped").length
1717

1818
const availableTargets = schema
1919
? schema.columns
2020
.map((c) => c.name)
2121
.filter((name) => !columns.some((col) => col.mappedTo === name))
2222
: []
2323

24-
function handleRemap(header: string, newTarget: string | null) {
25-
const updated = columns.map((col) =>
26-
col.header === header
24+
function handleRemap(index: number, newTarget: string | null) {
25+
const updated = columns.map((col, i) =>
26+
i === index
2727
? { ...col, mappedTo: newTarget, status: (newTarget ? "matched" : "unmapped") as "matched" | "unmapped" }
2828
: col
2929
)
@@ -39,58 +39,62 @@ export function ColumnMapper({ columns, schema, onMappingChange }: ColumnMapperP
3939
<span>Status</span>
4040
</div>
4141

42-
{unmapped.map((col) => (
43-
<div
44-
key={col.header}
45-
className="grid grid-cols-[1fr_24px_1fr_80px] gap-2 px-4 py-2 border-b bg-amber-50/50 items-center"
46-
>
47-
<code className="text-xs bg-muted px-1.5 py-0.5 rounded truncate">
48-
{col.header}
49-
</code>
50-
<span className="text-muted-foreground"></span>
51-
<select
52-
className="text-xs border border-amber-400 rounded px-2 py-1 bg-white"
53-
value={col.mappedTo ?? ""}
54-
onChange={(e) => handleRemap(col.header, e.target.value || null)}
42+
{columns.map((col, idx) =>
43+
col.status === "unmapped" ? (
44+
<div
45+
key={idx}
46+
className="grid grid-cols-[1fr_24px_1fr_80px] gap-2 px-4 py-2 border-b bg-amber-50/50 items-center"
5547
>
56-
<option value="">— select or skip —</option>
57-
{availableTargets.map((t) => (
58-
<option key={t} value={t}>{t}</option>
59-
))}
60-
</select>
61-
<span className="text-xs bg-amber-100 text-amber-700 px-2 py-0.5 rounded text-center">
62-
unmapped
63-
</span>
64-
</div>
65-
))}
48+
<code className="text-xs bg-muted px-1.5 py-0.5 rounded truncate">
49+
{col.header}
50+
</code>
51+
<span className="text-muted-foreground"></span>
52+
<select
53+
className="text-xs border border-amber-400 rounded px-2 py-1 bg-white"
54+
value={col.mappedTo ?? ""}
55+
onChange={(e) => handleRemap(idx, e.target.value || null)}
56+
>
57+
<option value="">— select or skip —</option>
58+
{availableTargets.map((t) => (
59+
<option key={t} value={t}>{t}</option>
60+
))}
61+
</select>
62+
<span className="text-xs bg-amber-100 text-amber-700 px-2 py-0.5 rounded text-center">
63+
unmapped
64+
</span>
65+
</div>
66+
) : null
67+
)}
6668

67-
{matched.length > 0 && !showAll && (
69+
{matchedCount > 0 && !showAll && (
6870
<button
6971
className="w-full px-4 py-2 text-xs text-muted-foreground hover:bg-muted/50 text-center"
7072
onClick={() => setShowAll(true)}
7173
>
72-
+ {matched.length} matched columns (click to expand)
74+
+ {matchedCount} matched columns (click to expand)
7375
</button>
7476
)}
7577

7678
{showAll &&
77-
matched.map((col) => (
78-
<div
79-
key={col.header}
80-
className="grid grid-cols-[1fr_24px_1fr_80px] gap-2 px-4 py-2 border-b items-center"
81-
>
82-
<code className="text-xs bg-muted px-1.5 py-0.5 rounded truncate">
83-
{col.header}
84-
</code>
85-
<span className="text-muted-foreground"></span>
86-
<span className="text-xs text-green-700">{col.mappedTo}</span>
87-
<span className="text-xs bg-green-100 text-green-700 px-2 py-0.5 rounded text-center">
88-
matched
89-
</span>
90-
</div>
91-
))}
79+
columns.map((col, idx) =>
80+
col.status === "matched" ? (
81+
<div
82+
key={idx}
83+
className="grid grid-cols-[1fr_24px_1fr_80px] gap-2 px-4 py-2 border-b items-center"
84+
>
85+
<code className="text-xs bg-muted px-1.5 py-0.5 rounded truncate">
86+
{col.header}
87+
</code>
88+
<span className="text-muted-foreground"></span>
89+
<span className="text-xs text-green-700">{col.mappedTo}</span>
90+
<span className="text-xs bg-green-100 text-green-700 px-2 py-0.5 rounded text-center">
91+
matched
92+
</span>
93+
</div>
94+
) : null
95+
)}
9296

93-
{showAll && matched.length > 0 && (
97+
{showAll && matchedCount > 0 && (
9498
<button
9599
className="w-full px-4 py-2 text-xs text-muted-foreground hover:bg-muted/50 text-center"
96100
onClick={() => setShowAll(false)}

codebenders-dashboard/components/upload/drop-zone.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ export function DropZone({ onFile, disabled }: DropZoneProps) {
7272
<input
7373
ref={inputRef}
7474
type="file"
75-
accept=".csv,.xlsx,.xls"
75+
accept=".csv,.xlsx"
7676
className="hidden"
7777
onChange={handleInputChange}
7878
/>

codebenders-dashboard/lib/upload-parser.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,14 @@ function parseXlsx(buffer: Buffer, maxRows?: number): Promise<ParseResult> {
7676
}
7777

7878
const headers = Object.keys(jsonData[0]).map((h) => h.trim())
79-
const rows = maxRows ? jsonData.slice(0, maxRows) : jsonData
79+
const trimmedRows = jsonData.map((row) => {
80+
const trimmed: Record<string, string> = {}
81+
for (const [key, value] of Object.entries(row)) {
82+
trimmed[key.trim()] = String(value)
83+
}
84+
return trimmed
85+
})
86+
const rows = maxRows ? trimmedRows.slice(0, maxRows) : trimmedRows
8087

8188
resolve({ headers, rows, totalRows: jsonData.length })
8289
} catch (err) {

0 commit comments

Comments
 (0)