Skip to content

Commit 80977d0

Browse files
authored
fix(grid): explain the import wizard's disabled Next and silent downgrade (#2640, #2639) (#2646)
#2640: the mapping step renders an inline hint listing every unmapped required field as `label (name)`, so a disabled Next button explains itself; the hint updates live and clears once the columns are supplied. #2639: the scalar-only legacy per-row fallback is no longer silent — a new optional `ImportResult.degraded` flag drives a "compatibility fallback" notice on the completion screen. The pre-existing guard that refuses the fallback when relation columns are mapped (preventing raw natural keys in FK columns) is retained. Component tests cover both paths. Closes #2640 Closes #2639
1 parent 9d4a429 commit 80977d0

4 files changed

Lines changed: 136 additions & 2 deletions

File tree

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
"@object-ui/plugin-grid": patch
3+
---
4+
5+
Import wizard: stop leaving the user at two silent dead-ends (surfaced during framework batch-write testing).
6+
7+
- #2640 — the mapping step now renders an inline hint listing every required field that has no column mapped (as `label (name)`), so a disabled **Next** button always explains itself. The hint updates live with the mapping and clears once the columns are supplied; the disable logic itself is unchanged.
8+
- #2639 — when the server `/import` route is unavailable and the wizard downgrades to the legacy per-row `create` loop, the completion screen now shows a "compatibility fallback" notice (values written as-is, without server-side coercion) via a new optional `ImportResult.degraded` flag — the downgrade is no longer silent. The pre-existing guard that refuses the fallback when relation columns are mapped (which would otherwise write raw natural keys into FK columns) is retained.

packages/plugin-grid/src/ImportWizard.tsx

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,12 @@ const IMPORT_DEFAULT_TRANSLATIONS: Record<string, string> = {
166166
'grid.import.required': 'Required',
167167
'grid.import.invalidType': 'Invalid {{type}}',
168168
'grid.import.legacyReferenceBlocked': 'Import blocked: {{fields}} are relation fields that need the server import route to resolve names into record IDs, and this connection doesn’t support it. Importing them as plain text would corrupt the data. Upgrade the backend/client, or unmap these columns and import them separately.',
169+
// Shown in the mapping step when a required field has no column mapped — the
170+
// reason the Next button is disabled. Field names are listed as `label (name)`.
171+
'grid.import.missingRequiredHint': 'Can’t continue — required field(s) not mapped: {{fields}}. Add a matching column to your file, or go back and upload one that includes it.',
172+
// Shown on the completion screen when the import ran via the legacy per-row
173+
// fallback (server `/import` route unavailable) — no server-side coercion.
174+
'grid.import.legacyFallbackNotice': 'Imported via a compatibility fallback: this connection doesn’t support the server import route, so values were saved as text without server-side type coercion. Upgrade the backend/client for full import support (type coercion and relation lookups).',
169175
};
170176

171177
/** Apply `{{var}}` interpolation to a translation template. */
@@ -303,6 +309,11 @@ export interface ImportResult {
303309
resultsTruncated?: boolean;
304310
/** True when the user cancelled an in-flight async import job. */
305311
cancelled?: boolean;
312+
/** True when the import ran via the legacy per-row `create` fallback because
313+
* the server `/import` route was unavailable — values are written as-is,
314+
* with no server-side type coercion or reference resolution. Surfaced on the
315+
* completion screen so a silent downgrade never passes for a full import. */
316+
degraded?: boolean;
306317
}
307318

308319
type WizardStep = 'upload' | 'mapping' | 'preview';
@@ -960,7 +971,10 @@ const StepMapping: React.FC<{
960971
activeMapping: SavedMapping | null;
961972
onSelectSavedMapping: (name: string) => void;
962973
onClearSavedMapping: () => void;
963-
}> = ({ headers, fields, mapping, onMappingChange, inferredTypes, suggestions, templates, selectedTemplateId, onSelectTemplate, onSaveTemplate, onDeleteTemplate, savedMappings, activeMapping, onSelectSavedMapping, onClearSavedMapping }) => {
974+
/** Required fields with no column mapped — surfaced as an inline hint so the
975+
* disabled Next button always explains itself. */
976+
missingRequired: ImportWizardProps['fields'];
977+
}> = ({ headers, fields, mapping, onMappingChange, inferredTypes, suggestions, templates, selectedTemplateId, onSelectTemplate, onSaveTemplate, onDeleteTemplate, savedMappings, activeMapping, onSelectSavedMapping, onClearSavedMapping, missingRequired }) => {
964978
const { t } = useImportTranslation();
965979
const usedFields = useMemo(() => new Set(Object.values(mapping)), [mapping]);
966980
const suggestionByCol = useMemo(() => {
@@ -1070,6 +1084,21 @@ const StepMapping: React.FC<{
10701084
</TableBody>
10711085
</Table>
10721086
</div>
1087+
{missingRequired.length > 0 && (
1088+
<div
1089+
className="mt-3 flex items-start gap-2 rounded-md border border-amber-400/40 bg-amber-50 p-3 text-xs text-amber-900 dark:bg-amber-950/30 dark:text-amber-200"
1090+
data-testid="import-missing-required"
1091+
>
1092+
<AlertCircle className="mt-0.5 h-4 w-4 shrink-0" />
1093+
<span>
1094+
{t('grid.import.missingRequiredHint', {
1095+
fields: missingRequired
1096+
.map((f) => (f.label && f.label !== f.name ? `${f.label} (${f.name})` : f.name))
1097+
.join(', '),
1098+
})}
1099+
</span>
1100+
</div>
1101+
)}
10731102
</>
10741103
)}
10751104
</div>
@@ -1756,7 +1785,10 @@ export const ImportWizard: React.FC<ImportWizardProps> = ({
17561785
}
17571786
setProgress(Math.round(((i + 1) / rows.length) * 100));
17581787
}
1759-
const importResult: ImportResult = { totalRows: rows.length, importedRows, skippedRows, errors };
1788+
// `degraded`: this per-row path ran because the server `/import` route was
1789+
// unavailable — flag it so the completion screen tells the user the import
1790+
// was downgraded (no server-side coercion) instead of reporting a clean win.
1791+
const importResult: ImportResult = { totalRows: rows.length, importedRows, skippedRows, errors, degraded: true };
17601792
setResult(importResult); setImporting(false); onComplete?.(importResult);
17611793
}, [rows, mapping, fields, dataSource, objectName, onComplete, onErrorMode, corrections, t]);
17621794

@@ -2092,6 +2124,7 @@ export const ImportWizard: React.FC<ImportWizardProps> = ({
20922124
activeMapping={activeMapping}
20932125
onSelectSavedMapping={handleSelectSavedMapping}
20942126
onClearSavedMapping={handleClearSavedMapping}
2127+
missingRequired={missingRequired}
20952128
/>
20962129
)}
20972130
{step === 'preview' && (
@@ -2234,6 +2267,15 @@ export const ImportWizard: React.FC<ImportWizardProps> = ({
22342267
)}
22352268
{result.skippedRows > 0 && <Badge variant="destructive">{t('grid.import.skippedCount', { count: result.skippedRows })}</Badge>}
22362269
</div>
2270+
{result.degraded && !result.cancelled && (
2271+
<div
2272+
className="flex w-full items-start gap-2 rounded-md border border-amber-400/40 bg-amber-50 p-3 text-xs text-amber-900 dark:bg-amber-950/30 dark:text-amber-200"
2273+
data-testid="import-degraded-notice"
2274+
>
2275+
<AlertCircle className="mt-0.5 h-4 w-4 shrink-0" />
2276+
<span>{t('grid.import.legacyFallbackNotice')}</span>
2277+
</div>
2278+
)}
22372279
{renderResultExtra && (
22382280
<div className="w-full" data-testid="import-result-extra">{renderResultExtra(result)}</div>
22392281
)}

packages/plugin-grid/src/__tests__/importLegacyReferenceGuard.test.tsx

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,21 @@ describe('ImportWizard legacy fallback: relation columns', () => {
7777
expect(onComplete.mock.calls[0][0].importedRows).toBe(1);
7878
});
7979
});
80+
81+
describe('ImportWizard legacy fallback: downgrade is not silent (issue #2639)', () => {
82+
it('flags the result degraded and shows a downgrade notice on the scalar path', async () => {
83+
const create = vi.fn().mockResolvedValue({ id: '1' });
84+
const onComplete = vi.fn();
85+
// Scalar-only columns → the fallback runs (no importRecords) and succeeds,
86+
// but the user must be told the server import route was unavailable.
87+
await runWizardImport('Name\nAcme', { create }, onComplete);
88+
89+
await waitFor(() => expect(onComplete).toHaveBeenCalled());
90+
expect(create).toHaveBeenCalledTimes(1);
91+
// The result carries the downgrade flag...
92+
expect(onComplete.mock.calls[0][0].degraded).toBe(true);
93+
// ...and the completion screen surfaces it rather than reporting a clean win.
94+
expect(screen.getByTestId('import-degraded-notice')).toBeInTheDocument();
95+
expect(screen.getByText(/compatibility fallback/i)).toBeInTheDocument();
96+
});
97+
});
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
/**
2+
* Missing-required-field hint — mapping step (issue #2640).
3+
*
4+
* When a required object field has no column mapped, the Next button is
5+
* disabled. Without a hint the user faces a greyed-out button with no reason
6+
* and no way to know which field is missing. The mapping step must list every
7+
* unmapped required field (as `label (name)`) and clear the hint the moment the
8+
* column is supplied — the disable logic itself stays put.
9+
*/
10+
import { describe, it, expect } from 'vitest';
11+
import { render, screen, fireEvent, waitFor, act } from '@testing-library/react';
12+
import '@testing-library/jest-dom';
13+
import React from 'react';
14+
15+
import { ImportWizard } from '../ImportWizard';
16+
17+
// `status` is required but its label ("Lifecycle") differs from its api-name,
18+
// so the hint must show both — that's exactly the value a user needs to fix it.
19+
const FIELDS = [
20+
{ name: 'name', label: 'Name', type: 'text' },
21+
{ name: 'status', label: 'Lifecycle', type: 'text', required: true },
22+
];
23+
24+
/** Paste TSV into the upload step via the wizard's window-level handler. */
25+
function pasteRows(text: string) {
26+
const evt = new Event('paste', { bubbles: true, cancelable: true }) as Event & {
27+
clipboardData: { getData: (type: string) => string };
28+
};
29+
evt.clipboardData = { getData: (type: string) => (type === 'text/plain' ? text : '') };
30+
act(() => { window.dispatchEvent(evt); });
31+
}
32+
33+
function renderWizard() {
34+
render(
35+
<ImportWizard objectName="account" fields={FIELDS} open onOpenChange={() => {}} />,
36+
);
37+
}
38+
39+
describe('ImportWizard mapping step: missing required-field hint', () => {
40+
it('names each unmapped required field as `label (name)` and disables Next', async () => {
41+
renderWizard();
42+
// File omits the required `status` column — only `name` auto-maps.
43+
pasteRows('Name\nAcme');
44+
45+
const hint = await screen.findByTestId('import-missing-required');
46+
expect(hint).toHaveTextContent('Lifecycle (status)');
47+
expect(screen.getByTestId('import-next-btn')).toBeDisabled();
48+
});
49+
50+
it('clears the hint and enables Next once the required column is supplied', async () => {
51+
renderWizard();
52+
pasteRows('Name\nAcme');
53+
await screen.findByTestId('import-missing-required');
54+
expect(screen.getByTestId('import-next-btn')).toBeDisabled();
55+
56+
// Go back and re-upload a file that now carries the required column; the
57+
// hint must react live to the new mapping, not require a remount.
58+
fireEvent.click(screen.getByRole('button', { name: 'Back' }));
59+
pasteRows('Name\tstatus\nAcme\tactive');
60+
61+
await waitFor(() => {
62+
expect(screen.queryByTestId('import-missing-required')).not.toBeInTheDocument();
63+
});
64+
expect(screen.getByTestId('import-next-btn')).toBeEnabled();
65+
});
66+
});

0 commit comments

Comments
 (0)