feat(import): import multiple CSV files at once - #481
Merged
Conversation
The CSV import path was single-file end to end: the file input had no `multiple`, the change handler read `files[0]`, and the service imported exactly one file per call. Selecting several CSVs now creates one Grid page per file under the same parent. Files import one at a time on purpose — the server caps pending import tasks per user (MAXIMUM_IMPORT_PENDING_TASK, 3 by default), so a parallel fan-out would fail every file past the cap. One bad file no longer sinks the batch: its error is reported and the rest still import. Also: - the close button cancels a running batch (a batch can run for minutes), and the abort signal now reaches the in-flight upload so cancelling stops it immediately; backdrop click and Escape still never interrupt an import - per-file progress on the CSV button, and toasts that say which files failed - failure text is interpolated unescaped, so `Q1&Q2.csv` is not shown as `Q1&Q2.csv` Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reviewer's GuideAdds client-side support for importing multiple CSV files in a single batch, executing them sequentially with improved cancellation, progress reporting, and per-file error handling, while wiring abort signals through to uploads and expanding test coverage for the new behavior. Sequence diagram for multi-file CSV import with cancellationsequenceDiagram
actor User
participant ImportDialog
participant importCsvFilesAsDatabases
participant importCsvAsDatabase
participant uploadDatabaseCsvImportFile
participant axios
User->>ImportDialog: click csvInputRef (multiple files)
ImportDialog->>ImportDialog: onCsvPicked
ImportDialog->>ImportDialog: handleCsv(files)
ImportDialog->>ImportDialog: AbortController()
ImportDialog->>importCsvFilesAsDatabases: importCsvFilesAsDatabases({ workspaceId, parentViewId, files, onFileStart, signal })
loop for each file
importCsvFilesAsDatabases->>ImportDialog: onFileStart(index, total)
importCsvFilesAsDatabases->>importCsvAsDatabase: importCsvAsDatabase({ workspaceId, parentViewId, file, signal })
importCsvAsDatabase->>uploadDatabaseCsvImportFile: uploadDatabaseCsvImportFile(presignedUrl, file, onProgress, signal)
uploadDatabaseCsvImportFile->>axios: put(presignedUrl, file, { onUploadProgress, signal })
axios-->>uploadDatabaseCsvImportFile: response 200/204
uploadDatabaseCsvImportFile-->>importCsvAsDatabase: void
importCsvAsDatabase-->>importCsvFilesAsDatabases: { viewId }
importCsvFilesAsDatabases-->>importCsvFilesAsDatabases: items.push({ fileName, viewId })
end
importCsvFilesAsDatabases-->>ImportDialog: { items, aborted: false }
ImportDialog->>ImportDialog: toast.success / toast.error
ImportDialog->>ImportDialog: close()
ImportDialog->>ImportDialog: toView(importedViewIds[0])
alt user cancels mid-batch
User->>ImportDialog: click close button
ImportDialog->>ImportDialog: handleCloseClick
ImportDialog->>ImportDialog: abortRef.current.abort()
importCsvFilesAsDatabases-->>ImportDialog: { items, aborted: true }
ImportDialog->>ImportDialog: toast.success (partial)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
🥷 Ninja i18n – 🛎️ Translations need to be updatedProject
|
| lint rule | new reports | level | link |
|---|---|---|---|
| Missing translation | 308 | warning | contribute (via Fink 🐦) |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The dialog close/cancel flow now has three separate handlers (
close,handleDismiss,handleCloseClick) that partially duplicate logic; consider consolidating around a single helper that consistently resets state, aborts as needed, and callsonOpenChange(false)to reduce the chance of future divergence. - In
handleCsv,files.lengthis used as the denominator for success/partial-success toasts even when some files may never be attempted due to an aborted signal; you might consider distinguishing betweenfiles.lengthand the number of attempted files so the messaging more precisely reflects what actually ran.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The dialog close/cancel flow now has three separate handlers (`close`, `handleDismiss`, `handleCloseClick`) that partially duplicate logic; consider consolidating around a single helper that consistently resets state, aborts as needed, and calls `onOpenChange(false)` to reduce the chance of future divergence.
- In `handleCsv`, `files.length` is used as the denominator for success/partial-success toasts even when some files may never be attempted due to an aborted signal; you might consider distinguishing between `files.length` and the number of attempted files so the messaging more precisely reflects what actually ran.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Four issues from a pass with the Vercel React rules.
The count strings hand-rolled their pluralisation: `successCount` baked in
"Imported {{total}} files" and the component branched on `=== 1` around it.
English is the only language that works for — `en.json` already carries 39
`_one`/`_other` keys, and the repo ships `ar-SA`, `cs-CZ` and friends, where
2-4 and 5+ take different forms. Moved the three counted strings onto
i18next plurals keyed on `count`, which also collapses the ternaries. Added
`import-i18n.test.ts`, since the component tests stub `t` and would not
notice `t()` handing back a raw key after a rename.
The branch plumbed the abort signal into the CSV upload but left the same
gap on the Notion path, which handles the largest files: `importNotionZipToView`
took a `signal`, checked it before uploading, then dropped it. Cancelling did
nothing until a multi-hundred-MB zip finished, and with the dialog refusing
backdrop and Escape there was no way out at all. Signal now reaches both the
single-part and multipart uploads, the multipart workers stop claiming parts
once it fires, and an incomplete upload is never finalised. A cancelled upload
is normalised to `ImportAbortError` so it reads as a cancel, not a failure.
`TooManyImportTask` describes the account, not the file — the server counts
pending tasks per user in `ensure_import_task_capacity`. Retrying it per file
spent a round trip each and then blamed all N files for a queue the user only
had to wait out. The batch now stops on it, and the dialog derives its
denominator from the files actually attempted, so a batch that stopped early
never reports "Imported 1 of 20". Those runs keep the dialog open, since the
untried files are still worth retrying.
The progress counter lives inside a disabled button, which assistive tech
skips, so batch progress is also announced from a live region — as words
rather than the bare "3/7" ratio a screen reader would read out.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Opening a relation cell painted every row as "Untitled" until its title arrived. The row list lands as soon as the target view doc resolves, but the titles come from a separate pass that fetches one row doc at a time, and `rowContents.get(id) || ''` collapsed "not fetched yet" and "fetched, and the primary cell really is empty" into the same falsy value — so a picker mid-load was indistinguishable from a database full of unnamed rows. The distinction was already available as `rowContents.has(id)`; it was just being discarded. Rows still waiting on a title now render a pulsing placeholder instead, and deleted rows are excluded: their wording is final and no doc is ever coming. That made error handling load-bearing rather than optional. Presence in `rowContents` is what ends a placeholder, and the fetch loop had none at all, so one rejected `createRow` would leave that row — and every row queued behind it — pulsing forever. A row that cannot be read now falls back to the "Untitled" wording and the batch continues. Also stop announcing "no result" off the back of a row list that has not arrived. An empty list mid-load reads as a terminal answer; it now waits for the load to finish before concluding anything. The fetch loop is still sequential, unlike the cell renderer in `RelationItems`, which already fans out with Promise.all. The placeholders make that visible rather than hiding it behind text, but parallelising it wants a concurrency cap and is left for its own change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Importing CSVs on web was single-file end to end, so bringing in a folder of exports meant repeating the whole dialog flow once per file. The block was entirely client-side — the file input had no
multiple, the change handler readfiles[0], andimportCsvAsDatabasehandled exactly one file per call. The server endpoint is one task per file by design, so the client loops.Selecting several CSVs now creates one Grid page per file under the same parent.
Files import one at a time on purpose. The server caps pending import tasks per user (
MAXIMUM_IMPORT_PENDING_TASK, 3 by default), so a parallel fan-out would fail every file past the cap withTooManyImportTask. A unit test asserts max-in-flight is 1.One bad file no longer sinks the batch — its error is reported and the remaining files still import. Behaviour for a single selected file is unchanged, including the existing toast wording and navigating to the new grid.
Along the way:
axios.putgets the signal), so cancelling stops the transfer immediately rather than at the next file boundary. Backdrop click and Esc still never interrupt an import — only the button, which is labelled "Cancel import" while a batch runs. Pages already imported are kept and reported.2/7), and toasts that name which files failed rather than a single generic error.escapeValue: false, so a file namedQ1&Q2.csvis not shown asQ1&Q2.csv(i18next escapes interpolated values by default and toasts render plain text).Notes for reviewers
en.jsongains 6importPanelkeys; other locales fall back to English until the translation sync runs.ImportCsvInput.onProgresshad no caller before this change either, so a large single CSV shows a spinner without a percentage. Out of scope here.', '. Locale-aware joining needsIntl.ListFormatkeyed offi18n.language, which risks aRangeErroron tags in this repo's locale set (e.g.ckb-KU), so it was left alone deliberately.Checklist
General
Testing
22 new unit tests across two suites:
import-csv-batch.test.ts— selection order, one-at-a-time execution, progress callbacks, per-file failure continuing the batch, server-sideFailedstatus plus task cancellation, mid-batch abort returning partial results, pre-aborted signal, empty error message left for the caller to translate, and the signal reaching the upload.ImportDialog.multiCsv.test.tsx— multi-select attribute, file forwarding, toast wording per case (single / all / partial / cancelled), failure naming and translated overflow, all-fail keeps the dialog open, progress display, cancel-on-close aborts the signal, and backdrop/Escape not aborting.Full suite: 309 suites / 2959 tests passing.
pnpm type-checkand eslint clean.Feature-Specific
Summary by Sourcery
Enable resilient multi-file CSV imports with sequential processing, cancellation, progress feedback, and clearer failure reporting.
New Features:
Bug Fixes:
Enhancements:
Tests: