feat(api): add CSV bulk importer for project work items - #307
Conversation
Add a bulk-import framework and a CSV importer so a project can be seeded from an existing spreadsheet instead of hand-entering issues. Backend: adapt the pre-existing (unused) importers table via migration 000012 (relax the token-based NOT NULL, add progress columns), and add the model/store/service/handler. Upload parses the CSV (name/title/summary required; description/priority/state mapped when present, unknown priorities normalized to none, unknown states left to the project default), persists the rows, and enqueues an import_run task on a new RabbitMQ queue. The worker creates one issue per row, tracking processed/error counts and status; when no queue is configured it runs inline so the feature degrades gracefully. router.New now also returns the ImporterService so cmd/api can register the worker. Frontend: an "Import CSV" action on the project work-item list opens a modal that uploads the file and polls progress to completion, then refreshes the list. Jira and GitHub bulk import are planned follow-ups on top of this framework. Closes Devlaner#207 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@martian56 this is green, CI passing and CodeRabbit had no comments. It's the framework + CSV slice of #207 (Jira/GitHub bulk import noted as follow-ups). Good to merge whenever you're happy with it. |
# Conflicts: # apps/api/cmd/api/main.go # apps/web/src/api/types.ts
|
Rebased on main and resolved the conflicts from the webhooks merge (#306) in main.go and types.ts. Both features coexist and CI is re-running. |
|
Warning Review limit reached
Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds CSV bulk import support across the API and web application, including importer persistence, CSV parsing, synchronous or RabbitMQ-backed execution, authenticated project routes, progress tracking, and an upload modal with polling. ChangesCSV Import
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@martian56 conflicts are resolved and build/lint are green. CodeRabbit's check is stuck showing in-progress but it has no open comments. Good to merge whenever you're ready. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
apps/api/migrations/000012_importer_progress.up.sql (1)
15-15: 🩺 Stability & Availability | 🔵 TrivialConsider
CREATE INDEX CONCURRENTLYfor production deployments.A standard
CREATE INDEXacquires aSHARElock that blocks writes for the duration of index creation.CONCURRENTLYavoids this but cannot be used inside a transaction block. If your migration tool wraps each file in a transaction (e.g., golang-migrate), you may need to split this into a separate non-transactional migration or accept the brief write lock on a small-to-medium table.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/migrations/000012_importer_progress.up.sql` at line 15, Update the index creation statement for idx_importers_project to use concurrent index creation when the migration environment supports non-transactional migrations. If the migration runner wraps this file in a transaction, move the index creation to a separate non-transactional migration or retain the current statement explicitly to preserve compatibility.apps/api/internal/service/importer.go (3)
192-212: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCheck
ctx.Err()in the processing loop for graceful cancellation.Without a context check, a cancelled context (e.g., worker shutdown or inline request timeout) causes every remaining
s.issues.Createcall to fail, inflatingErrorCountand marking the import asfailedorpartialrather than allowing a clean retry.♻️ Add context cancellation check
for _, row := range im.Data.Rows { + if err := ctx.Err(); err != nil { + im.ErrorMessage = fmt.Sprintf("import cancelled: %v", err) + break + } var stateID *uuid.UUID🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/internal/service/importer.go` around lines 192 - 212, Add a ctx.Err() check at the start of the processing loop over im.Data.Rows and stop processing immediately when the context is canceled. Avoid calling s.issues.Create or updating progress for remaining rows after cancellation, while preserving existing success and error counting for rows already processed.
211-211: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPer-row progress updates cause excessive DB writes for large imports.
For a 5000-row import, this loop issues 5000
UPDATEqueries just for progress tracking. Consider batching (e.g., every 100 rows or at the end) to reduce I/O load while still providing reasonable polling granularity.♻️ Batch progress updates
+ const progressBatchSize = 100 for i, row := range im.Data.Rows { // ... create issue ... if cerr != nil { im.ErrorCount++ } else { im.ProcessedCount++ } - _ = s.importers.UpdateProgress(ctx, im) + if (i+1)%progressBatchSize == 0 { + _ = s.importers.UpdateProgress(ctx, im) + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/internal/service/importer.go` at line 211, Batch the progress update calls in the import loop around importers.UpdateProgress so they run at a reasonable interval, such as every 100 processed rows, and always run once after the loop completes. Preserve the existing progress state while reducing per-row database writes.
81-90: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
parseCSVreads the entire file into memory before the row-limit check.A maliciously large upload (e.g., millions of rows) would be fully parsed into a
[]ImportRowslice beforeCreateCSVcheckslen(rows) > maxImportRows. Moving the limit into the parser loop (or using aLimitReader) would bound memory usage.♻️ Enforce row limit during parsing
var rows []model.ImportRow for { + if len(rows) >= maxImportRows { + return nil, fmt.Errorf("%w: at most %d rows are supported per import", ErrImportBadFile, maxImportRows) + } rec, err := cr.Read()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/internal/service/importer.go` around lines 81 - 90, Update parseCSV to enforce maxImportRows while reading rows, stopping and returning ErrImportBadFile once the limit is exceeded instead of building an unbounded []ImportRow. Preserve the existing empty-input handling and CreateCSV validation behavior, including the maxImportRows error context.apps/api/migrations/000012_importer_progress.down.sql (1)
12-12: 🩺 Stability & Availability | 🔵 Trivial
SET NOT NULLon rollback blocks reads while the table is scanned.This is expected for a down migration and the comment correctly notes it only succeeds on a clean rollback. If the
importerstable could be large at rollback time, consider validating with aCHECKconstraint first, then promoting toNOT NULLin a second step. For a typical rollback scenario this is acceptable as-is.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/migrations/000012_importer_progress.down.sql` at line 12, No code change is required: the review confirms ALTER TABLE importers ALTER COLUMN token_id SET NOT NULL is acceptable for the expected clean rollback scenario. Leave the down migration unchanged.apps/web/src/components/work-item/ImportCSVModal.tsx (1)
100-112: 🚀 Performance & Scalability | 🔵 TrivialConsider announcing progress updates for screen readers.
The status text/percentage update visually only; adding
role="status"/aria-live="polite"on the progress container would announce updates as the async import progresses.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/work-item/ImportCSVModal.tsx` around lines 100 - 112, Update the progress container in ImportCSVModal around the status text and percentage to include an accessible polite live-region configuration, such as role="status" with aria-live="polite", so screen readers announce asynchronous import progress and status changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/internal/service/importer.go`:
- Around line 161-163: Update Run’s import idempotency flow around the existing
status check and processing-status update: add a processing guard or stale-lease
check so active imports are not reprocessed, while allowing crashed imports to
be reclaimed; preserve completed and partial-completed skips. Do not reset prior
progress for an active/reclaimed import, and handle or propagate failures from
the processing status update instead of ignoring them.
- Around line 157-160: Update the importer lookup handling in Run so a nil
importer with a nil error is converted into an explicit not-found error before
returning. Preserve propagation of non-nil errors from s.importers.Get, ensuring
the queue worker does not acknowledge missing-importer jobs as successful.
- Around line 109-115: Update the queue-publish failure branch in the importer
flow so that after Run executes inline, it reloads the importer record before
returning it. Match the fresh-record behavior of the no-queue path, ensuring the
returned im reflects the status persisted by Run rather than the stale queued
value.
In `@apps/web/src/components/work-item/ImportCSVModal.tsx`:
- Around line 47-69: Update the polling useEffect in ImportCSVModal to avoid
depending on the changing onImported callback identity: store the latest
callback in a ref and invoke that ref when the job reaches a terminal state.
Remove onImported from the polling effect’s dependency array while preserving
the existing notification guard and polling behavior.
---
Nitpick comments:
In `@apps/api/internal/service/importer.go`:
- Around line 192-212: Add a ctx.Err() check at the start of the processing loop
over im.Data.Rows and stop processing immediately when the context is canceled.
Avoid calling s.issues.Create or updating progress for remaining rows after
cancellation, while preserving existing success and error counting for rows
already processed.
- Line 211: Batch the progress update calls in the import loop around
importers.UpdateProgress so they run at a reasonable interval, such as every 100
processed rows, and always run once after the loop completes. Preserve the
existing progress state while reducing per-row database writes.
- Around line 81-90: Update parseCSV to enforce maxImportRows while reading
rows, stopping and returning ErrImportBadFile once the limit is exceeded instead
of building an unbounded []ImportRow. Preserve the existing empty-input handling
and CreateCSV validation behavior, including the maxImportRows error context.
In `@apps/api/migrations/000012_importer_progress.down.sql`:
- Line 12: No code change is required: the review confirms ALTER TABLE importers
ALTER COLUMN token_id SET NOT NULL is acceptable for the expected clean rollback
scenario. Leave the down migration unchanged.
In `@apps/api/migrations/000012_importer_progress.up.sql`:
- Line 15: Update the index creation statement for idx_importers_project to use
concurrent index creation when the migration environment supports
non-transactional migrations. If the migration runner wraps this file in a
transaction, move the index creation to a separate non-transactional migration
or retain the current statement explicitly to preserve compatibility.
In `@apps/web/src/components/work-item/ImportCSVModal.tsx`:
- Around line 100-112: Update the progress container in ImportCSVModal around
the status text and percentage to include an accessible polite live-region
configuration, such as role="status" with aria-live="polite", so screen readers
announce asynchronous import progress and status changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bba4acf6-dbfe-484f-832c-5ce22b01264d
📒 Files selected for processing (18)
apps/api/cmd/api/main.goapps/api/internal/handler/importer.goapps/api/internal/handler/importer_test.goapps/api/internal/model/importer.goapps/api/internal/queue/consumer.goapps/api/internal/queue/queue.goapps/api/internal/router/router.goapps/api/internal/service/importer.goapps/api/internal/service/importer_internal_test.goapps/api/internal/store/importer.goapps/api/internal/testutil/http.goapps/api/internal/testutil/router.goapps/api/migrations/000012_importer_progress.down.sqlapps/api/migrations/000012_importer_progress.up.sqlapps/web/src/api/types.tsapps/web/src/components/work-item/ImportCSVModal.tsxapps/web/src/pages/IssueListPage.tsxapps/web/src/services/importerService.ts
Address review findings on the importer: - Run() returned nil (acking the queue message) when the importer row was missing, silently dropping the job. Now it logs and returns nil explicitly only for a genuinely absent row, and propagates real lookup errors. - A crash mid-import left the row in "processing"; on redelivery Run replayed every row from zero, creating duplicate issues. "processing" now joins the skip set so a redelivered in-flight import isn't reprocessed. - The inline fallback (used when enqueue fails) returned the stale "queued" snapshot; it now re-reads the finished job like the no-queue path. - ImportCSVModal restarted its 1s poll timer on every parent re-render because the effect depended on the non-memoized onImported prop; it's now held in a ref so polling isn't interrupted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@martian56 all CodeRabbit comments addressed (worker not-found handling, no duplicate reprocessing of in-flight imports, inline fallback returns the real status, and the poll timer no longer restarts). Green across the board — good to merge. |
3 participants
Feature summary
You can now seed a project from a spreadsheet: upload a CSV of work items and Devlane creates one issue per row, showing live import progress.
Linked issues / discussion
Closes #207
User-facing behavior
On a project's work-item list there's a new Import CSV action. It opens a modal to pick a
.csvfile (needs aname/title/summarycolumn; optionaldescription,priority, andstate/statuscolumns are mapped when present). On upload the import is queued and the modal polls progress (total / created / errors) to completion, then the list refreshes to show the new issues.What changed
API (
apps/api/)POST/api/workspaces/:slug/projects/:projectId/importers/(multipartfile)GET/api/workspaces/:slug/projects/:projectId/importers/GET/api/workspaces/:slug/projects/:projectId/importers/:importerId/dataJSONB), and enqueues animport_runtask on a newdevlane.importsRabbitMQ queue. The worker creates one issue per row via the existingIssueService, trackingprocessed_count/error_count/status. Unknown priorities normalize tonone; astatecolumn is matched to a project state by name, otherwise the project default applies.router.Newnow returns theImporterServicealongside the engine socmd/apican register the background worker (its only two callers, main + testutil, are updated).UI (
apps/web/)ImportCSVModal+importerService.ts+ types; an Import CSV button on the project work-item list (IssueListPage).Database
000012adapts the pre-existing (unused)importerstable: relaxes the token-basedtoken_id NOT NULL(a UI import has no API token) and addstotal_count,processed_count,error_count,error_message,source_filename+ a(project_id, created_at)index. No new table.Why this design
The
importerstable already existed (scaffolded, unmapped in Go), so this reuses it rather than adding one. Rows are parsed once at upload and carried on the job so the worker needs only the id, keeping the queue payload tiny and avoiding a MinIO round-trip. Import runs off the request via the same RabbitMQ pattern as emails/webhooks so a large file never blocks the uploader, with an inline fallback so optional infra stays optional. This is deliberately the framework + CSV slice; Jira/GitHub bulk import are follow-ups that plug into the same model + queue.Test plan
npm run validategreen (typecheck + lint + prettier + go vet + go test)Out of scope (follow-ups)
AI assistance
Claude Code (Claude Opus 4.8)— and AI-assisted commits include aCo-Authored-By:trailerChecklist
internal/config/config.go(none added)Summary by CodeRabbit