Skip to content

Commit d88c8ec

Browse files
os-zhuangclaude
andauthored
fix(data-table): surface inline-edit save failures instead of swallowing them (#2106)
A rejected inline-edit save (e.g. a 400 validation failure — an invalid status transition) was caught with only `console.error`. Symptom: the save bar stayed stuck, the cell kept the unsaved value, and the author got no feedback at all (reproduced live on the showcase Studio preview: done→in_review was rejected server-side, silently). - saveRow / saveBatch: extract the server's reason (the ObjectStack adapter decorates thrown errors with the parsed body on `details`) and show it in the toolbar with an alert icon, instead of failing silently. The pending edit is kept so the author can fix and retry. - Tint failed row(s) destructive (`erroredRows`) so it's unambiguous which rows didn't persist — no phantom "looks saved" state. - Clear the error on a successful save (per-row or batch) and on cancel. - Add `table.saveFailed` across all 10 locales. - Tests: a failed save surfaces the reason and keeps the pending change; a successful save shows no failure message. Applies to both the runtime list grid and the Studio design preview (one code path). Found while verifying the editInline work end-to-end. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 998c5b3 commit d88c8ec

13 files changed

Lines changed: 140 additions & 2 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
"@object-ui/components": patch
3+
"@object-ui/i18n": patch
4+
---
5+
6+
fix(data-table): surface inline-edit save failures instead of swallowing them
7+
8+
A rejected inline-edit save (e.g. a 400 validation failure like an invalid
9+
status transition) was caught with only `console.error` — the toolbar stayed
10+
stuck, the cell kept the unsaved value, and the author got no feedback. Now the
11+
data-table shows the server's reason in the toolbar (with an alert icon) and
12+
tints the affected row(s) destructive so it's clear which rows didn't persist.
13+
The pending edit is kept for retry; the error clears on a successful save or on
14+
cancel. Adds the `table.saveFailed` string across all locales.

packages/components/src/__tests__/data-table-inline-edit.test.tsx

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
* render a native date picker (<input type="date">).
2020
*/
2121
import { describe, it, expect, vi, beforeAll } from 'vitest';
22-
import { fireEvent } from '@testing-library/react';
22+
import { fireEvent, waitFor } from '@testing-library/react';
2323
import '@testing-library/jest-dom';
2424
import React from 'react';
2525
import { renderComponent } from './test-utils';
@@ -131,4 +131,52 @@ describe('data-table — inline edit is per-row usable', () => {
131131

132132
expect(onCellChange).not.toHaveBeenCalled();
133133
});
134+
135+
it('D) a failed save surfaces the server error and keeps the pending change', async () => {
136+
// Regression: a rejected save (e.g. a 400 validation failure) was swallowed
137+
// with only console.error — the toolbar stayed stuck, the cell kept the
138+
// unsaved value, and the author got no feedback. The reason must surface.
139+
const onBatchSave = vi.fn().mockRejectedValue({
140+
// Shape the ObjectStack adapter decorates onto thrown errors.
141+
details: { message: 'Invalid task status transition.' },
142+
});
143+
const { container, getByText } = renderComponent({ ...editableSchema, onBatchSave });
144+
145+
// Stage an edit.
146+
const cells = container.querySelectorAll('tbody td');
147+
const qtyCell = cells[2] as HTMLElement; // 报工数量
148+
fireEvent.click(qtyCell);
149+
const input = qtyCell.querySelector('input') as HTMLInputElement;
150+
fireEvent.change(input, { target: { value: '42' } });
151+
fireEvent.blur(input);
152+
153+
// Save All → saveBatch → onBatchSave rejects.
154+
fireEvent.click(getByText(/Save All/i));
155+
expect(onBatchSave).toHaveBeenCalledTimes(1);
156+
157+
// The server's reason is surfaced, not swallowed.
158+
await waitFor(() =>
159+
expect(container.textContent).toContain('Invalid task status transition'),
160+
);
161+
// And the pending change is kept (Save All still offered) — no silent drop
162+
// and no phantom "saved" state.
163+
expect(getByText(/Save All/i)).toBeInTheDocument();
164+
});
165+
166+
it('D2) a successful save does NOT show a save-failure message', async () => {
167+
const onBatchSave = vi.fn().mockResolvedValue(undefined);
168+
const { container, getByText } = renderComponent({ ...editableSchema, onBatchSave });
169+
170+
const cells = container.querySelectorAll('tbody td');
171+
const qtyCell = cells[2] as HTMLElement;
172+
fireEvent.click(qtyCell);
173+
const input = qtyCell.querySelector('input') as HTMLInputElement;
174+
fireEvent.change(input, { target: { value: '7' } });
175+
fireEvent.blur(input);
176+
177+
fireEvent.click(getByText(/Save All/i));
178+
await waitFor(() => expect(onBatchSave).toHaveBeenCalledTimes(1));
179+
180+
expect(container.textContent).not.toContain('Save failed');
181+
});
134182
});

packages/components/src/renderers/complex/data-table.tsx

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import {
5151
Plus,
5252
Expand,
5353
MoreHorizontal,
54+
AlertCircle,
5455
} from 'lucide-react';
5556
import {
5657
DropdownMenu,
@@ -118,6 +119,7 @@ const TABLE_DEFAULT_TRANSLATIONS: Record<string, string> = {
118119
'table.open': 'Open',
119120
'table.search': 'Search...',
120121
'table.modified': '{{count}} row modified',
122+
'table.saveFailed': 'Save failed',
121123
'table.selected': '{{count}} selected',
122124
'table.edit': 'Edit',
123125
'table.delete': 'Delete',
@@ -163,6 +165,23 @@ function useTableTranslation() {
163165
}
164166
}
165167

168+
/**
169+
* Pull the most useful human-readable message out of whatever the save path
170+
* threw. The ObjectStack adapter decorates thrown errors with the parsed
171+
* response body on `details` (e.g. a `{ message, error }` from a validation
172+
* failure), so prefer that; fall back to `error.message`, then a raw string.
173+
* Never returns empty — callers render it as the save-failure reason.
174+
*/
175+
function extractSaveErrorMessage(error: unknown): string {
176+
if (error && typeof error === 'object') {
177+
const e = error as { message?: unknown; details?: { message?: unknown; error?: unknown } };
178+
const detail = e.details && (e.details.message ?? e.details.error);
179+
if (typeof detail === 'string' && detail.trim()) return detail.trim();
180+
if (typeof e.message === 'string' && e.message.trim()) return e.message.trim();
181+
}
182+
return typeof error === 'string' && error.trim() ? error.trim() : 'Unknown error';
183+
}
184+
166185
/**
167186
* Enterprise-level data table component with Airtable-like features.
168187
*
@@ -322,6 +341,12 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
322341
// Track pending changes for multi-cell editing: rowIndex -> { columnKey -> newValue }
323342
const [pendingChanges, setPendingChanges] = useState<Map<number, Record<string, any>>>(new Map());
324343
const [isSaving, setIsSaving] = useState(false);
344+
// Last save failure message (server validation text, etc.) shown in the
345+
// toolbar; null when the last save attempt succeeded or nothing's been saved.
346+
const [saveError, setSaveError] = useState<string | null>(null);
347+
// Row indices whose last save attempt failed — tinted destructive so the
348+
// author sees exactly which rows didn't persist (no silent "phantom save").
349+
const [erroredRows, setErroredRows] = useState<Set<number>>(new Set());
325350
// Column header context menu state
326351
const [contextMenu, setContextMenu] = useState<{ x: number; y: number; columnKey: string } | null>(null);
327352

@@ -706,8 +731,21 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
706731
const newPendingChanges = new Map(pendingChanges);
707732
newPendingChanges.delete(rowIndex);
708733
setPendingChanges(newPendingChanges);
734+
// Saved — drop any prior error for this row, and clear the banner once
735+
// no errored rows remain.
736+
setErroredRows((prev) => {
737+
if (!prev.has(rowIndex)) return prev;
738+
const next = new Set(prev);
739+
next.delete(rowIndex);
740+
if (next.size === 0) setSaveError(null);
741+
return next;
742+
});
709743
} catch (error) {
744+
// Keep the pending change so the author can fix and retry; surface the
745+
// reason instead of failing silently, and flag the row.
710746
console.error('Failed to save row:', error);
747+
setSaveError(extractSaveErrorMessage(error));
748+
setErroredRows((prev) => new Set(prev).add(rowIndex));
711749
} finally {
712750
setIsSaving(false);
713751
}
@@ -717,6 +755,13 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
717755
const newPendingChanges = new Map(pendingChanges);
718756
newPendingChanges.delete(rowIndex);
719757
setPendingChanges(newPendingChanges);
758+
setErroredRows((prev) => {
759+
if (!prev.has(rowIndex)) return prev;
760+
const next = new Set(prev);
761+
next.delete(rowIndex);
762+
if (next.size === 0) setSaveError(null);
763+
return next;
764+
});
720765
};
721766

722767
const saveBatch = async () => {
@@ -736,15 +781,24 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
736781

737782
// Clear all pending changes
738783
setPendingChanges(new Map());
784+
// Saved — clear any prior errors.
785+
setErroredRows(new Set());
786+
setSaveError(null);
739787
} catch (error) {
788+
// Batch is all-or-nothing here: keep every pending row, flag them all,
789+
// and surface the reason instead of failing silently.
740790
console.error('Failed to save batch:', error);
791+
setSaveError(extractSaveErrorMessage(error));
792+
setErroredRows(new Set(pendingChanges.keys()));
741793
} finally {
742794
setIsSaving(false);
743795
}
744796
};
745797

746798
const cancelAllChanges = () => {
747799
setPendingChanges(new Map());
800+
setErroredRows(new Set());
801+
setSaveError(null);
748802
};
749803

750804
const handleCellKeyDown = (e: React.KeyboardEvent, rowIndex: number, columnKey: string) => {
@@ -843,6 +897,17 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
843897
<div className="flex flex-wrap items-center gap-2">
844898
{hasPendingChanges && (
845899
<>
900+
{saveError && (
901+
<div
902+
role="alert"
903+
className="flex items-center gap-1.5 text-sm text-destructive max-w-[16rem] sm:max-w-sm"
904+
>
905+
<AlertCircle className="h-4 w-4 flex-none" />
906+
<span className="truncate" title={`${t('table.saveFailed')}: ${saveError}`}>
907+
{t('table.saveFailed')}: {saveError}
908+
</span>
909+
</div>
910+
)}
846911
<div className="text-sm text-muted-foreground">
847912
{t('table.modified', { count: pendingChanges.size })}
848913
</div>
@@ -1071,7 +1136,8 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
10711136
"data-[state=selected]:bg-primary/5 data-[state=selected]:hover:bg-primary/10",
10721137
"focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring focus-visible:ring-inset",
10731138
schema.onRowClick && "cursor-pointer",
1074-
rowHasChanges && "bg-amber-50 dark:bg-amber-950/20",
1139+
rowHasChanges && !erroredRows.has(rowIndex) && "bg-amber-50 dark:bg-amber-950/20",
1140+
erroredRows.has(rowIndex) && "bg-destructive/10 dark:bg-destructive/15 ring-1 ring-inset ring-destructive/40",
10751141
rowClassName && rowClassName(row, rowIndex)
10761142
)}
10771143
style={rowStyle ? rowStyle(row, rowIndex) : undefined}

packages/i18n/src/locales/ar.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ const ar = {
137137
open: "فتح",
138138
search: "بحث...",
139139
modified: "{{count}} صف(صفوف) معدّل(ة)",
140+
saveFailed: 'فشل الحفظ',
140141
selected: "{{count}} محدد(ة)",
141142
edit: "تعديل",
142143
delete: "حذف",

packages/i18n/src/locales/de.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ const de = {
137137
open: "Öffnen",
138138
search: "Suchen...",
139139
modified: "{{count}} Zeile geändert",
140+
saveFailed: 'Speichern fehlgeschlagen',
140141
selected: "{{count}} ausgewählt",
141142
edit: "Bearbeiten",
142143
delete: "Löschen",

packages/i18n/src/locales/en.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ const en = {
144144
open: 'Open',
145145
search: 'Search...',
146146
modified: '{{count}} row modified',
147+
saveFailed: 'Save failed',
147148
selected: '{{count}} selected',
148149
edit: 'Edit',
149150
delete: 'Delete',

packages/i18n/src/locales/es.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ const es = {
137137
open: "Abrir",
138138
search: "Buscar...",
139139
modified: "{{count}} filas modificadas",
140+
saveFailed: 'Error al guardar',
140141
selected: "{{count}} seleccionados",
141142
edit: "Editar",
142143
delete: "Eliminar",

packages/i18n/src/locales/fr.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ const fr = {
137137
open: "Ouvrir",
138138
search: "Rechercher...",
139139
modified: "{{count}} ligne(s) modifiée(s)",
140+
saveFailed: 'Échec de l’enregistrement',
140141
selected: "{{count}} sélectionné(s)",
141142
edit: "Modifier",
142143
delete: "Supprimer",

packages/i18n/src/locales/ja.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ const ja = {
137137
open: "開く",
138138
search: "検索...",
139139
modified: "{{count}} 行が変更されました",
140+
saveFailed: '保存に失敗しました',
140141
selected: "{{count}} 件選択中",
141142
edit: "編集",
142143
delete: "削除",

packages/i18n/src/locales/ko.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ const ko = {
137137
open: "열기",
138138
search: "검색...",
139139
modified: "{{count}}개 행 수정됨",
140+
saveFailed: '저장 실패',
140141
selected: "{{count}}개 선택됨",
141142
edit: "편집",
142143
delete: "삭제",

0 commit comments

Comments
 (0)