Skip to content

Commit 372cbbe

Browse files
committed
fix(tables): address reference review findings
1 parent 6b7da53 commit 372cbbe

8 files changed

Lines changed: 353 additions & 21 deletions

File tree

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-config-sidebar/column-config-sidebar.test.tsx

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ interface ComboboxProps {
1818
searchable?: boolean
1919
searchPlaceholder?: string
2020
disabled?: boolean
21+
isLoading?: boolean
2122
onChange?: (value: string) => void
2223
}
2324

@@ -124,6 +125,7 @@ beforeEach(() => {
124125
{ id: 'table-current', name: 'Current table' },
125126
{ id: 'table-customers', name: 'Customers' },
126127
],
128+
isPending: false,
127129
})
128130
mockAddColumn.mockResolvedValue({ data: { columns: [] } })
129131
mockUpdateColumn.mockResolvedValue({ data: { columns: [] } })
@@ -233,6 +235,8 @@ describe('ColumnConfigSidebar', () => {
233235
})
234236

235237
it('keeps an existing Reference column visible but not retargetable when disabled', async () => {
238+
mockUseTablesList.mockReturnValue({ data: [], isPending: false })
239+
236240
await act(async () => {
237241
root.render(
238242
<ColumnConfigSidebar
@@ -251,13 +255,36 @@ describe('ColumnConfigSidebar', () => {
251255
)
252256
})
253257

254-
expect(mockUseTablesList).toHaveBeenCalledWith('workspace-1', 'active', { enabled: false })
255-
expect(findCombobox('Select table')?.disabled).toBe(true)
258+
expect(mockUseTablesList).toHaveBeenCalledWith('workspace-1', 'active', { enabled: true })
259+
expect(findCombobox('Select table')).toMatchObject({
260+
disabled: true,
261+
options: [{ label: 'table-current', value: 'table-current' }],
262+
value: 'table-current',
263+
})
256264
expect(findCombobox('Select type')?.options).toContainEqual(
257265
expect.objectContaining({ value: 'reference', disabled: true })
258266
)
259267
})
260268

269+
it('shows the Reference table selector as loading while tables are fetched', async () => {
270+
mockUseTablesList.mockReturnValue({ data: [], isPending: true })
271+
272+
await act(async () => {
273+
root.render(
274+
<ColumnConfigSidebar
275+
config={{ mode: 'create', proposedName: 'Related row', type: 'reference' }}
276+
onClose={vi.fn()}
277+
existingColumn={null}
278+
workspaceId='workspace-1'
279+
tableId='table-current'
280+
referenceColumnsEnabled
281+
/>
282+
)
283+
})
284+
285+
expect(findCombobox('Select table')?.isLoading).toBe(true)
286+
})
287+
261288
it('keeps Select options in the edit sidebar', async () => {
262289
await act(async () => {
263290
root.render(

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-config-sidebar/column-config-sidebar.tsx

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -177,10 +177,19 @@ function ColumnConfigBody({
177177
existingColumn.referenceTableId !== referenceTableInput)
178178
const saveDisabled = updateColumn.isPending || addColumn.isPending || referenceMutationBlocked
179179
const supportsUnique = columnTypeById(typeInput).supportsUnique
180-
const { data: workspaceTables = [] } = useTablesList(workspaceId, 'active', {
181-
enabled: wantsReference && referenceColumnsEnabled,
182-
})
183-
const tableOptions = workspaceTables.map((table) => ({ value: table.id, label: table.name }))
180+
const shouldLoadReferenceTables =
181+
wantsReference && (referenceColumnsEnabled || existingColumn?.type === 'reference')
182+
const { data: workspaceTables = [], isPending: workspaceTablesPending } = useTablesList(
183+
workspaceId,
184+
'active',
185+
{ enabled: shouldLoadReferenceTables }
186+
)
187+
const tableOptions = [
188+
...workspaceTables.map((table) => ({ value: table.id, label: table.name })),
189+
...(referenceTableInput && !workspaceTables.some((table) => table.id === referenceTableInput)
190+
? [{ value: referenceTableInput, label: referenceTableInput }]
191+
: []),
192+
]
184193
const trimmedOptions = optionsInput.map((o) => ({ ...o, name: o.name.trim() }))
185194

186195
/** Client-side option validation mirroring the server rules; returns an error message or null. */
@@ -423,6 +432,7 @@ function ColumnConfigBody({
423432
placeholder='Select table'
424433
searchable
425434
searchPlaceholder='Search tables'
435+
isLoading={shouldLoadReferenceTables && workspaceTablesPending}
426436
maxHeight={260}
427437
/>
428438
{referenceTableError && <FieldError message={referenceTableError} />}

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/column-dropdown/column-dropdown.test.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,11 @@ describe('ColumnDropdown', () => {
114114
const labels = [...document.body.querySelectorAll<HTMLElement>('[role="menuitem"]')].map(
115115
(item) => item.textContent
116116
)
117-
expect(labels).not.toContain('Reference')
117+
expect(labels).toEqual([
118+
...COLUMN_TYPE_OPTIONS.filter((option) => option.type !== 'reference').map(
119+
(option) => option.label
120+
),
121+
'Enrichments',
122+
])
118123
})
119124
})

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1745,7 +1745,10 @@ export function TableGrid({
17451745
function handleCopyRowId() {
17461746
const rowId = contextMenu.row?.id
17471747
if (!rowId) return
1748-
void navigator.clipboard.writeText(rowId).catch(() => {})
1748+
void navigator.clipboard.writeText(rowId).catch((error) => {
1749+
logger.error('Failed to copy row ID', { error })
1750+
toast.error('Failed to copy row ID')
1751+
})
17491752
}
17501753

17511754
const handleGoToReferenceTable = useCallback(

apps/sim/lib/copilot/generated/tool-catalog-v1.ts

Lines changed: 141 additions & 6 deletions
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)