Skip to content

Commit 61ba8e9

Browse files
atrakhConvex, Inc.
atrakh
authored and
Convex, Inc.
committed
dashboard: make sure DataRow is always aware of the row _id (#43504)
With FieldSelector allowing the user to hide any field, a bug was introduced where DataRow became unaware of a row's `_id` if the `_id` field was hidden. This fixes that GitOrigin-RevId: 63dbc57c7ef8725bb5e248781db017cf1def42e0
1 parent 4c81e5f commit 61ba8e9

File tree

2 files changed

+242
-40
lines changed

2 files changed

+242
-40
lines changed

‎npm-packages/dashboard-common/src/features/data/components/Table/DataRow.test.tsx‎

Lines changed: 231 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -126,45 +126,242 @@ describe("DataRow", () => {
126126
const X = true;
127127
const _ = false;
128128

129-
it("should select multiple rows at once", async () => {
130-
// 0 1 2 3 4 5 6 7 8 9
131-
createRows([_, _, _, X, _, _, _, _, _, _]);
132-
await shiftToggleRow(7);
133-
expectRows([_, _, _, X, X, X, X, X, _, _]);
134-
});
129+
describe("selection", () => {
130+
it("should select multiple rows at once", async () => {
131+
// 0 1 2 3 4 5 6 7 8 9
132+
createRows([_, _, _, X, _, _, _, _, _, _]);
133+
await shiftToggleRow(7);
134+
expectRows([_, _, _, X, X, X, X, X, _, _]);
135+
});
135136

136-
it("should unselect multiple rows", async () => {
137-
// 0 1 2 3 4 5 6 7 8 9
138-
createRows([_, _, _, X, X, X, X, X, _, _]);
139-
await shiftToggleRow(4);
140-
expectRows([_, _, _, X, _, _, _, _, _, _]);
141-
});
137+
it("should unselect multiple rows", async () => {
138+
// 0 1 2 3 4 5 6 7 8 9
139+
createRows([_, _, _, X, X, X, X, X, _, _]);
140+
await shiftToggleRow(4);
141+
expectRows([_, _, _, X, _, _, _, _, _, _]);
142+
});
142143

143-
it("should select from the start when nothing is selected", async () => {
144-
// 0 1 2 3 4 5 6 7 8 9
145-
createRows([_, _, _, _, _, _, _, _, _, _]);
146-
await shiftToggleRow(4);
147-
expectRows([X, X, X, X, X, _, _, _, _, _]);
148-
});
144+
it("should select from the start when nothing is selected", async () => {
145+
// 0 1 2 3 4 5 6 7 8 9
146+
createRows([_, _, _, _, _, _, _, _, _, _]);
147+
await shiftToggleRow(4);
148+
expectRows([X, X, X, X, X, _, _, _, _, _]);
149+
});
149150

150-
it("should select from the group above when it exists", async () => {
151-
// 0 1 2 3 4 5 6 7 8 9
152-
createRows([X, X, _, _, _, _, X, X, X, _]);
153-
await shiftToggleRow(4);
154-
expectRows([X, X, X, X, X, _, X, X, X, _]);
155-
});
151+
it("should select from the group above when it exists", async () => {
152+
// 0 1 2 3 4 5 6 7 8 9
153+
createRows([X, X, _, _, _, _, X, X, X, _]);
154+
await shiftToggleRow(4);
155+
expectRows([X, X, X, X, X, _, X, X, X, _]);
156+
});
156157

157-
it("should select from the group below when no group exists above", async () => {
158-
// 0 1 2 3 4 5 6 7 8 9
159-
createRows([_, _, _, _, _, _, X, X, X, _]);
160-
await shiftToggleRow(4);
161-
expectRows([_, _, _, _, X, X, X, X, X, _]);
158+
it("should select from the group below when no group exists above", async () => {
159+
// 0 1 2 3 4 5 6 7 8 9
160+
createRows([_, _, _, _, _, _, X, X, X, _]);
161+
await shiftToggleRow(4);
162+
expectRows([_, _, _, _, X, X, X, X, X, _]);
163+
});
164+
165+
it("should batch deselect correctly when multiple groups exist", async () => {
166+
// 0 1 2 3 4 5 6 7 8 9
167+
createRows([X, X, _, X, X, X, _, X, X, _]);
168+
await shiftToggleRow(4);
169+
expectRows([X, X, _, X, _, _, _, X, X, _]);
170+
});
162171
});
163172

164-
it("should batch deselect correctly when multiple groups exist", async () => {
165-
// 0 1 2 3 4 5 6 7 8 9
166-
createRows([X, X, _, X, X, X, _, X, X, _]);
167-
await shiftToggleRow(4);
168-
expectRows([X, X, _, X, _, _, _, X, X, _]);
173+
describe("hidden _id column", () => {
174+
// Helper component to wrap hooks properly
175+
function TestRowWithHiddenId({
176+
data,
177+
fields,
178+
patchDocument,
179+
onOpenContextMenu,
180+
isRowSelected,
181+
toggleIsRowSelected,
182+
}: {
183+
data: GenericDocument[];
184+
fields: string[];
185+
patchDocument: any;
186+
onOpenContextMenu: any;
187+
isRowSelected: (id: string) => boolean;
188+
toggleIsRowSelected: (id: string) => void;
189+
}) {
190+
// Only include specified fields in columns, potentially hiding _id
191+
const columns = useDataColumns({
192+
tableName: "test",
193+
localStorageKey: "_disabled_",
194+
fields,
195+
data,
196+
});
197+
198+
const { rows, prepareRow } = useTable(
199+
{ columns, data },
200+
useResizeColumns,
201+
);
202+
203+
const connectedDeployment = useMemo(
204+
() => ({ deployment, isDisconnected: false }),
205+
[],
206+
);
207+
208+
return (
209+
<ConnectedDeploymentContext.Provider value={connectedDeployment}>
210+
<DataRow
211+
index={0}
212+
style={{}}
213+
data={{
214+
areEditsAuthorized: true,
215+
isRowSelected,
216+
isSelectionAllNonExhaustive: false,
217+
resizingColumn: undefined,
218+
onAuthorizeEdits: () => {},
219+
patchDocument,
220+
prepareRow,
221+
rows,
222+
tableName: "test",
223+
toggleIsRowSelected,
224+
onOpenContextMenu,
225+
onCloseContextMenu: () => {},
226+
contextMenuColumn: null,
227+
contextMenuRow: null,
228+
canManageTable: true,
229+
activeSchema: null,
230+
onEditDocument: () => {},
231+
}}
232+
/>
233+
</ConnectedDeploymentContext.Provider>
234+
);
235+
}
236+
237+
it("should access _id from row.original when _id column is hidden", () => {
238+
const patchDocument = jest.fn();
239+
const onOpenContextMenu = jest.fn();
240+
241+
const data: GenericDocument[] = [
242+
{ _id: "test-id-1", name: "John", age: 30 },
243+
{ _id: "test-id-2", name: "Jane", age: 25 },
244+
];
245+
246+
const { container } = render(
247+
<DeploymentInfoContext.Provider value={mockDeploymentInfo}>
248+
<ConvexProvider client={mockClient}>
249+
<TestRowWithHiddenId
250+
data={data}
251+
fields={["name", "age"]}
252+
patchDocument={patchDocument}
253+
onOpenContextMenu={onOpenContextMenu}
254+
isRowSelected={() => false}
255+
toggleIsRowSelected={() => {}}
256+
/>
257+
</ConvexProvider>
258+
</DeploymentInfoContext.Provider>,
259+
);
260+
261+
// Verify the row renders without errors
262+
expect(container.querySelector(".DataRow")).toBeInTheDocument();
263+
264+
// Verify that we can see the name and age values
265+
expect(container).toHaveTextContent("John");
266+
expect(container).toHaveTextContent("30");
267+
});
268+
269+
it("should correctly identify rows by _id when _id column is hidden", () => {
270+
const isRowSelected = jest.fn(() => false);
271+
const toggleIsRowSelected = jest.fn();
272+
273+
const data: GenericDocument[] = [
274+
{ _id: "test-id-1", name: "John", age: 30 },
275+
{ _id: "test-id-2", name: "Jane", age: 25 },
276+
];
277+
278+
render(
279+
<DeploymentInfoContext.Provider value={mockDeploymentInfo}>
280+
<ConvexProvider client={mockClient}>
281+
<TestRowWithHiddenId
282+
data={data}
283+
fields={["name", "age"]}
284+
patchDocument={async () => undefined}
285+
onOpenContextMenu={() => {}}
286+
isRowSelected={isRowSelected}
287+
toggleIsRowSelected={toggleIsRowSelected}
288+
/>
289+
</ConvexProvider>
290+
</DeploymentInfoContext.Provider>,
291+
);
292+
293+
// Verify isRowSelected was called with the correct _id
294+
expect(isRowSelected).toHaveBeenCalledWith("test-id-1");
295+
});
296+
297+
it("should open context menu with correct _id when _id column is hidden", async () => {
298+
const onOpenContextMenu = jest.fn();
299+
300+
const data: GenericDocument[] = [
301+
{ _id: "test-id-1", name: "John", age: 30 },
302+
];
303+
304+
const { container } = render(
305+
<DeploymentInfoContext.Provider value={mockDeploymentInfo}>
306+
<ConvexProvider client={mockClient}>
307+
<TestRowWithHiddenId
308+
data={data}
309+
fields={["name", "age"]}
310+
patchDocument={async () => undefined}
311+
onOpenContextMenu={onOpenContextMenu}
312+
isRowSelected={() => false}
313+
toggleIsRowSelected={() => {}}
314+
/>
315+
</ConvexProvider>
316+
</DeploymentInfoContext.Provider>,
317+
);
318+
319+
// Find and right-click the checkbox
320+
const checkbox = container.querySelector('input[type="checkbox"]');
321+
expect(checkbox).toBeInTheDocument();
322+
323+
const user = userEvent.setup();
324+
await user.pointer({
325+
target: checkbox!.parentElement!,
326+
keys: "[MouseRight]",
327+
});
328+
329+
// Verify onOpenContextMenu was called with correct _id
330+
expect(onOpenContextMenu).toHaveBeenCalledWith(
331+
expect.any(Object),
332+
"test-id-1",
333+
null,
334+
);
335+
});
336+
337+
it("should handle recently created row highlighting when _id column is hidden", () => {
338+
const data: GenericDocument[] = [
339+
{
340+
_id: "test-id-1",
341+
_creationTime: Date.now() - 500, // Created 500ms ago
342+
name: "John",
343+
age: 30,
344+
},
345+
];
346+
347+
const { container } = render(
348+
<DeploymentInfoContext.Provider value={mockDeploymentInfo}>
349+
<ConvexProvider client={mockClient}>
350+
<TestRowWithHiddenId
351+
data={data}
352+
fields={["name", "age"]}
353+
patchDocument={async () => undefined}
354+
onOpenContextMenu={() => {}}
355+
isRowSelected={() => false}
356+
toggleIsRowSelected={() => {}}
357+
/>
358+
</ConvexProvider>
359+
</DeploymentInfoContext.Provider>,
360+
);
361+
362+
// Verify the row renders with the highlight animation
363+
const row = container.querySelector(".DataRow");
364+
expect(row).toHaveClass("animate-highlight");
365+
});
169366
});
170367
});

‎npm-packages/dashboard-common/src/features/data/components/Table/DataRow.tsx‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Value } from "convex/values";
1+
import { Value, GenericId } from "convex/values";
22
import { GenericDocument } from "convex/server";
33
import React, {
44
CSSProperties,
@@ -125,20 +125,25 @@ function DataRowLoaded({ index, style, data }: DataRowProps) {
125125
contextMenuRow,
126126
} = data;
127127

128-
const row: Row = rows[index];
128+
const row: Row<GenericDocument> = rows[index];
129129
const previousRow = usePrevious(row);
130130
const previousRows = usePrevious(rows);
131131

132132
const didNumberOfRowsChange = previousRows?.length !== rows.length;
133133

134-
const { _id } = row.values;
135-
const previousRowId = previousRow?.values._id;
134+
const _id = row.original._id as GenericId<string>;
135+
const previousRowId = previousRow?.original._id as
136+
| GenericId<string>
137+
| undefined;
136138

137139
const [didJustCreate, setDidJustCreate] = useState(false);
138140
useEffect(() => {
139141
// The entire row should be highlighted if the row was recently created and
140142
// not already rendered.
141-
if (!previousRowId && Date.now() - row.values._creationTime < 1000) {
143+
if (
144+
!previousRowId &&
145+
Date.now() - (row.original._creationTime as number) < 1000
146+
) {
142147
setDidJustCreate(true);
143148
// To reset the animatation, reset the state after one second.
144149
setTimeout(() => setDidJustCreate(false), 1000);
@@ -156,7 +161,7 @@ function DataRowLoaded({ index, style, data }: DataRowProps) {
156161
[onOpenContextMenu, _id],
157162
);
158163
useContextMenuTrigger(checkboxRef, contextMenuCallback, onCloseContextMenu);
159-
const document = useMemo(() => omit(row.values, "*select"), [row.values]);
164+
const document = useMemo(() => omit(row.original, "*select"), [row.original]);
160165

161166
const editDocument = useCallback(() => {
162167
canManageTable && onEditDocument(document);

0 commit comments

Comments
 (0)