fix(platform): fix import dialogs getting stuck and upload failing#699
Merged
Conversation
The import button became permanently disabled because setValue calls did not propagate dirty state, and mode:'onChange' combined with isValid gating blocked submission. Fix by adding shouldDirty:true to all programmatic setValue calls in import forms, removing mode:'onChange', and dropping the isValid gate from import dialogs. Also add errorCode to bulk create results so the UI can show specific error messages (duplicate email, duplicate external ID) instead of generic failures. Simplify action menus by removing unused integration navigation shortcuts. Closes #688
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThis PR addresses the customer import issue by improving form state tracking, error handling, and streamlining integrations. The changes add Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes span multiple files with consistent patterns (form dirtyness additions, error code integration, import simplifications), but require verification across several components and backend types. The error handling logic additions and form state changes are straightforward but need careful validation across each import flow. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/platform/app/features/documents/components/documents-action-menu.tsx (1)
1-1:⚠️ Potential issue | 🟡 MinorCI lint is currently failing due to formatting in this file.
GitHub Actions reports an
oxfmtstyle error; please runbunx oxfmt -c ../../.oxfmtrc.jsonon this file before merge.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/features/documents/components/documents-action-menu.tsx` at line 1, CI lint fails due to formatting in documents-action-menu.tsx; run the formatter with the repo config (bunx oxfmt -c ../../.oxfmtrc.json services/platform/app/features/documents/components/documents-action-menu.tsx) to reformat the file, then stage the changes so the oxFmt style error is resolved before merging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@services/platform/app/features/customers/components/customer-import-form.tsx`:
- Line 71: The onValueChange callback formatting is failing the oxfmt linter;
reformat the arrow function passed to onValueChange so it meets project style
(avoid inline statement that mixes expressions), e.g. use a properly spaced
concise arrow or a block body, and ensure the call to setValue('dataSource',
value, { shouldDirty: true }) is a single clear expression; locate the
onValueChange prop on the component and adjust the handler formatting for
setValue, keeping the same arguments and behavior.
In
`@services/platform/app/features/customers/components/customers-import-dialog.tsx`:
- Around line 181-194: The code unsafely casts firstError.errorCode and can
leave toast.description undefined; add a runtime type guard (e.g.,
isValidErrorCode) that checks if firstError?.errorCode is one of the keys of
errorCodeKeys before using it, compute errorKey =
isValidErrorCode(firstError.errorCode) ? errorCodeKeys[firstError.errorCode] :
errorCodeKeys.unknown, and always pass a description to toast by using
tCustomers(errorKey) (so description never becomes undefined); reference the
existing variables result, firstError, errorCodeKeys, errorKey, toast, and
tCustomers when applying this change.
In
`@services/platform/app/features/documents/components/documents-action-menu.tsx`:
- Around line 67-85: The DocumentUploadDialog is duplicated across branches;
keep a single render to avoid drift by moving the DocumentUploadDialog element
(using props open={isUploadDialogOpen} onOpenChange={setIsUploadDialogOpen}
organizationId={organizationId}) out of the hasMicrosoftAccount conditional and
only conditionally render the action control (DataTableActionMenu with label
tDocuments('upload.importDocuments'), icon Plus, and
onClick={handleDeviceUpload}) inside the branch that currently lacks Microsoft
account support; ensure state vars isUploadDialogOpen and setIsUploadDialogOpen
remain in scope and remove the duplicated dialog render in the other branch.
In `@services/platform/app/features/vendors/components/vendor-import-form.tsx`:
- Line 69: The onValueChange callback on the VendorImportForm is failing
formatting; reformat the arrow function call to comply with oxfmt by adjusting
spacing and line breaking for the expression using onValueChange and setValue
(specifically the onValueChange handler that calls setValue('dataSource', value,
{ shouldDirty: true })). Ensure the arrow function is formatted consistently
with surrounding JSX props (e.g., single-line or properly indented multi-line)
so onValueChange, setValue, and the options object conform to the project's
formatter.
In `@services/platform/app/features/vendors/components/vendors-import-dialog.tsx`:
- Around line 169-179: The frontend is using an unsafe type assertion on
firstError.errorCode; replace that with a type guard that checks whether
firstError?.errorCode is one of the keys of errorCodeKeys (e.g., use
Object.prototype.hasOwnProperty.call(errorCodeKeys, firstError.errorCode) or an
explicit set/array check) and only map to errorCodeKeys[...] when the guard
passes, otherwise fall back to errorCodeKeys.unknown; additionally, update the
backend bulkCreateVendors mutation
(services/platform/convex/vendors/mutations.ts) so each pushed error object
includes an explicit errorCode string like "duplicate_email" or
"duplicate_external_id" (set error.errorCode before pushing to result.errors) so
the frontend can rely on that field.
---
Outside diff comments:
In
`@services/platform/app/features/documents/components/documents-action-menu.tsx`:
- Line 1: CI lint fails due to formatting in documents-action-menu.tsx; run the
formatter with the repo config (bunx oxfmt -c ../../.oxfmtrc.json
services/platform/app/features/documents/components/documents-action-menu.tsx)
to reformat the file, then stage the changes so the oxFmt style error is
resolved before merging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3adde6e1-5919-4ed4-8ef1-6781bb55e7ff
📒 Files selected for processing (13)
services/platform/app/features/customers/components/customer-import-form.tsxservices/platform/app/features/customers/components/customers-action-menu.tsxservices/platform/app/features/customers/components/customers-import-dialog.tsxservices/platform/app/features/documents/components/documents-action-menu.tsxservices/platform/app/features/products/components/product-import-form.tsxservices/platform/app/features/products/components/products-action-menu.tsxservices/platform/app/features/products/components/products-import-dialog.tsxservices/platform/app/features/vendors/components/vendor-import-form.tsxservices/platform/app/features/vendors/components/vendors-import-dialog.tsxservices/platform/convex/customers/mutations.tsservices/platform/convex/customers/types.tsservices/platform/convex/vendors/validators.tsservices/platform/messages/en.json
💤 Files with no reviewable changes (1)
- services/platform/app/features/customers/components/customers-action-menu.tsx
yannickmonney
pushed a commit
that referenced
this pull request
Apr 8, 2026
This was referenced Apr 10, 2026
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
1 participant
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.
Summary
shouldDirty: trueto all programmaticsetValuecalls in customer, product, and vendor import formsmode: 'onChange'andisValidgating from import dialogs that caused premature validation blockingerrorCodefield to bulk create results (customers & vendors) so the UI can display specific error messages instead of generic failuresCloses #688
Test plan
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor