Skip to content

Commit 59c6d99

Browse files
committed
fix(tables): address timezone and delete review findings
1 parent 5551be8 commit 59c6d99

11 files changed

Lines changed: 512 additions & 77 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/settings/components/general/timezone-picker.test.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,17 @@ describe('getTimezonePickerPresentation', () => {
2929
).toBe('America/Los_Angeles')
3030
})
3131

32+
it('adds a valid saved timezone that is absent from the curated options', () => {
33+
expect(getTimezonePickerPresentation('Etc/GMT+5', 'UTC', timezoneOptions)).toEqual({
34+
value: 'Etc/GMT+5',
35+
options: [
36+
{ label: 'Auto: UTC', value: AUTO_TIMEZONE_OPTION_VALUE },
37+
{ label: 'Etc/GMT+5', value: 'Etc/GMT+5' },
38+
...timezoneOptions,
39+
],
40+
})
41+
})
42+
3243
it('surfaces an invalid saved timezone without making it selectable', () => {
3344
expect(getTimezonePickerPresentation('Mars/Olympus', 'UTC', timezoneOptions)).toEqual({
3445
value: INVALID_TIMEZONE_OPTION_VALUE,

‎apps/sim/app/workspace/[workspaceId]/settings/components/general/timezone-picker.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@ export function getTimezonePickerPresentation(
1616
timezoneOptions: readonly ComboboxOption[]
1717
): TimezonePickerPresentation {
1818
const hasInvalidTimezone = savedTimezone !== null && !isValidTimezone(savedTimezone)
19+
const unlistedTimezone =
20+
savedTimezone !== null &&
21+
!hasInvalidTimezone &&
22+
!timezoneOptions.some((option) => option.value === savedTimezone)
23+
? savedTimezone
24+
: null
1925
const safeInvalidTimezone =
2026
savedTimezone === null ? '' : sanitizeTimezoneForDisplay(savedTimezone)
2127

@@ -34,6 +40,14 @@ export function getTimezonePickerPresentation(
3440
},
3541
]
3642
: []),
43+
...(unlistedTimezone
44+
? [
45+
{
46+
label: sanitizeTimezoneForDisplay(unlistedTimezone),
47+
value: unlistedTimezone,
48+
},
49+
]
50+
: []),
3751
...timezoneOptions,
3852
],
3953
}

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.test.tsx‎

Lines changed: 88 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ vi.mock('@sim/emcn', () => {
3131
const passthrough = ({ children }: { children?: ReactNode }) => children ?? null
3232
return {
3333
Checkbox: () => null,
34+
Chip: ({ children, ...props }: { children?: ReactNode }) =>
35+
createElement('button', { type: 'button', ...props }, children),
3436
ChipConfirmModal: passthrough,
3537
ChipDatePicker: ({ value, onChange }: { value?: string; onChange: (value: string) => void }) =>
3638
createElement(
@@ -41,7 +43,27 @@ vi.mock('@sim/emcn', () => {
4143
ChipModal: passthrough,
4244
ChipModalBody: passthrough,
4345
ChipModalError: passthrough,
44-
ChipModalField: passthrough,
46+
ChipModalField: ({
47+
type,
48+
value,
49+
onChange,
50+
children,
51+
}: {
52+
type?: string
53+
value?: string
54+
onChange?: (value: string) => void
55+
children?: ReactNode | ((aria: Record<string, string>) => ReactNode)
56+
}) =>
57+
type === 'input'
58+
? createElement('input', {
59+
'data-testid': 'modal-input',
60+
value: value ?? '',
61+
onChange: (event: { currentTarget: { value: string } }) =>
62+
onChange?.(event.currentTarget.value),
63+
})
64+
: typeof children === 'function'
65+
? children({ 'aria-describedby': 'field-hint' })
66+
: (children ?? null),
4567
ChipModalFooter: ({
4668
primaryAction,
4769
}: {
@@ -114,7 +136,9 @@ describe('RowModal expiration editing', () => {
114136
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
115137
act(() => root.render(createElement(RowModal, props)))
116138

117-
expect(container.querySelector('[role="status"]')?.textContent).toBe('Loading timezone…')
139+
expect(container.querySelector('[aria-label="Edit expires_at"]')?.textContent).toBe(
140+
'Loading timezone…'
141+
)
118142
expect(container.querySelector<HTMLInputElement>('[data-testid="time"]')).toBeNull()
119143
expect(container.querySelector<HTMLButtonElement>('[data-testid="submit"]')?.disabled).toBe(
120144
true
@@ -169,7 +193,9 @@ describe('RowModal expiration editing', () => {
169193

170194
act(() => root.render(createElement(RowModal, props)))
171195

172-
expect(container.querySelector('[role="status"]')?.textContent).toBe('Loading timezone…')
196+
expect(container.querySelector('[aria-label="Edit starts_at"]')?.textContent).toBe(
197+
'Loading timezone…'
198+
)
173199
expect(container.querySelector<HTMLInputElement>('[data-testid="time"]')).toBeNull()
174200

175201
mockUseTimezoneState.mockReturnValue({
@@ -203,14 +229,72 @@ describe('RowModal expiration editing', () => {
203229

204230
act(() => root.render(createElement(RowModal, props)))
205231

206-
expect(container.querySelector('[role="status"]')?.textContent).toBe('Invalid timezone')
232+
const blockedField = container.querySelector<HTMLButtonElement>(
233+
'[aria-label="Edit expires_at"]'
234+
)
235+
expect(blockedField?.textContent).toBe(String(row.data.expires_at))
207236
expect(container.querySelector<HTMLButtonElement>('[data-testid="submit"]')?.disabled).toBe(
208237
true
209238
)
239+
expect(mockToastError).not.toHaveBeenCalled()
240+
act(() => blockedField?.click())
210241
expect(mockToastError).toHaveBeenCalledWith(
211242
'Your saved timezone “Mars/Olympus” is invalid. Update it in Settings → General before editing Date or Expiration cells.'
212243
)
213244
act(() => root.unmount())
214245
container.remove()
215246
})
247+
248+
it('keeps unrelated fields editable and omits blocked date values from the update', async () => {
249+
mockUseTimezoneState.mockReturnValue({
250+
timezone: 'America/Los_Angeles',
251+
savedTimezone: 'Mars/Olympus',
252+
status: 'invalid',
253+
})
254+
const container = document.createElement('div')
255+
document.body.appendChild(container)
256+
const root = createRoot(container)
257+
const mixedTable: TableInfo = {
258+
...table,
259+
schema: {
260+
columns: [
261+
{ name: 'name', type: 'string' },
262+
{ name: 'expires_at', type: 'ttl' },
263+
],
264+
},
265+
}
266+
const mixedRow = { ...row, data: { name: 'Ada', expires_at: row.data.expires_at } }
267+
const props = {
268+
mode: 'edit' as const,
269+
isOpen: true,
270+
onClose: vi.fn(),
271+
table: mixedTable,
272+
row: mixedRow,
273+
onSuccess: vi.fn(),
274+
}
275+
276+
act(() => root.render(createElement(RowModal, props)))
277+
278+
const nameInput = container.querySelector<HTMLInputElement>('[data-testid="modal-input"]')
279+
const blockedField = container.querySelector<HTMLButtonElement>(
280+
'[aria-label="Edit expires_at"]'
281+
)
282+
const submit = container.querySelector<HTMLButtonElement>('[data-testid="submit"]')
283+
expect(nameInput?.value).toBe('Ada')
284+
expect(blockedField?.textContent).toBe(String(row.data.expires_at))
285+
expect(submit?.disabled).toBe(false)
286+
287+
act(() => changeInput(nameInput as HTMLInputElement, 'Grace'))
288+
await act(async () => submit?.click())
289+
290+
expect(mockUpdateRow).toHaveBeenCalledWith({
291+
rowId: 'row-1',
292+
data: { name: 'Grace' },
293+
})
294+
expect(props.onSuccess).toHaveBeenCalledTimes(1)
295+
expect(mockToastError).not.toHaveBeenCalled()
296+
297+
act(() => root.unmount())
298+
container.remove()
299+
})
216300
})

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.tsx‎

Lines changed: 84 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
'use client'
22

3-
import { useEffect, useId, useRef, useState } from 'react'
3+
import { useId, useRef, useState } from 'react'
44
import {
55
Checkbox,
6+
Chip,
67
ChipConfirmModal,
78
ChipDatePicker,
89
ChipModal,
@@ -22,7 +23,7 @@ import type { ColumnDefinition, TableInfo, TableRow } from '@/lib/table'
2223
import { columnTypeOf } from '@/lib/table/column-types'
2324
import { resolveCurrencyCode } from '@/lib/table/currency'
2425
import { getTimezoneEditBlockedMessage } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/timezone-editing'
25-
import { useTimezoneState } from '@/hooks/queries/general-settings'
26+
import { type TimezoneState, useTimezoneState } from '@/hooks/queries/general-settings'
2627
import { useDeleteTableRow, useDeleteTableRows, useUpdateTableRow } from '@/hooks/queries/tables'
2728
import {
2829
cleanCellValue,
@@ -48,12 +49,16 @@ export interface RowModalProps {
4849
function cleanRowData(
4950
columns: ColumnDefinition[],
5051
rowData: Record<string, unknown>,
51-
timeZone: string
52+
timeZone: string,
53+
dateEditorsReady: boolean
5254
): Record<string, unknown> {
5355
const cleanData: Record<string, unknown> = {}
5456

5557
columns.forEach((col) => {
5658
const value = rowData[col.name]
59+
if (columnTypeOf(col).editor === 'date' && !dateEditorsReady) {
60+
return
61+
}
5762
try {
5863
cleanData[col.name] = cleanCellValue(value, col, timeZone)
5964
} catch {
@@ -85,9 +90,7 @@ export function RowModal({ mode, isOpen, onClose, table, row, rowIds, onSuccess
8590
if (timezoneState.status === 'ready' && editTimeZoneRef.current === null) {
8691
editTimeZoneRef.current = timezoneState.timezone
8792
}
88-
const hasDateEditorColumn =
89-
mode === 'edit' && columns.some((column) => columnTypeOf(column).editor === 'date')
90-
const timezoneUnavailable = hasDateEditorColumn && editTimeZoneRef.current === null
93+
const dateEditorsReady = editTimeZoneRef.current !== null
9194
const timeZone = editTimeZoneRef.current ?? timezoneState.timezone
9295
const [rowData, setRowData] = useState<Record<string, unknown>>(() =>
9396
mode === 'edit' && row ? row.data : {}
@@ -100,25 +103,17 @@ export function RowModal({ mode, isOpen, onClose, table, row, rowIds, onSuccess
100103
updateRowMutation.isPending || deleteRowMutation.isPending || deleteRowsMutation.isPending
101104

102105
const timezoneBlockedMessage = getTimezoneEditBlockedMessage(timezoneState)
103-
useEffect(() => {
104-
if (
105-
!isOpen ||
106-
!timezoneUnavailable ||
107-
(timezoneState.status !== 'invalid' && timezoneState.status !== 'error') ||
108-
!timezoneBlockedMessage
109-
) {
110-
return
111-
}
112-
toast.error(timezoneBlockedMessage)
113-
}, [isOpen, timezoneBlockedMessage, timezoneState.status, timezoneUnavailable])
106+
const hasEditableColumn = columns.some(
107+
(column) => columnTypeOf(column).editor !== 'date' || dateEditorsReady
108+
)
114109

115110
const handleFormSubmit = async (e?: React.FormEvent) => {
116111
e?.preventDefault()
117112
setError(null)
118-
if (timezoneUnavailable) return
113+
if (!hasEditableColumn) return
119114

120115
try {
121-
const cleanData = cleanRowData(columns, rowData, timeZone)
116+
const cleanData = cleanRowData(columns, rowData, timeZone, dateEditorsReady)
122117

123118
if (row) {
124119
await updateRowMutation.mutateAsync({ rowId: row.id, data: cleanData })
@@ -193,25 +188,27 @@ export function RowModal({ mode, isOpen, onClose, table, row, rowIds, onSuccess
193188
Update values for {table?.name ?? 'table'}
194189
</p>
195190
<form onSubmit={handleFormSubmit} className='contents'>
196-
<button type='submit' hidden disabled={isSubmitting || timezoneUnavailable} />
197-
{timezoneUnavailable ? (
198-
<p role='status' className='px-2 text-[var(--text-muted)] text-small'>
199-
{timezoneState.status === 'loading'
200-
? 'Loading timezone…'
201-
: timezoneState.status === 'invalid'
202-
? 'Invalid timezone'
203-
: 'Timezone unavailable'}
204-
</p>
205-
) : (
206-
columns.map((column) => (
191+
<button type='submit' hidden disabled={isSubmitting || !hasEditableColumn} />
192+
{columns.map((column) =>
193+
columnTypeOf(column).editor === 'date' && !dateEditorsReady ? (
194+
<TimezoneBlockedColumnField
195+
key={column.name}
196+
column={column}
197+
value={rowData[column.name]}
198+
status={timezoneState.status}
199+
onAttemptEdit={() => {
200+
if (timezoneBlockedMessage) toast.error(timezoneBlockedMessage)
201+
}}
202+
/>
203+
) : (
207204
<ColumnField
208205
key={column.name}
209206
column={column}
210207
value={rowData[column.name]}
211208
timeZone={timeZone}
212209
onChange={(value) => setRowData((prev) => ({ ...prev, [column.name]: value }))}
213210
/>
214-
))
211+
)
215212
)}
216213
</form>
217214
<ChipModalError>{error}</ChipModalError>
@@ -222,7 +219,7 @@ export function RowModal({ mode, isOpen, onClose, table, row, rowIds, onSuccess
222219
primaryAction={{
223220
label: isSubmitting ? 'Updating...' : 'Update Row',
224221
onClick: () => handleFormSubmit(),
225-
disabled: isSubmitting || timezoneUnavailable,
222+
disabled: isSubmitting || !hasEditableColumn,
226223
}}
227224
/>
228225
</ChipModal>
@@ -236,23 +233,72 @@ interface ColumnFieldProps {
236233
onChange: (value: unknown) => void
237234
}
238235

239-
function ColumnField({ column, value, timeZone, onChange }: ColumnFieldProps) {
240-
const checkboxId = useId()
241-
const title = (
236+
function ColumnTitle({ column }: { column: ColumnDefinition }) {
237+
return (
242238
<>
243239
{column.name}
244240
{column.unique && (
245241
<span className='ml-1.5 font-normal text-[var(--text-tertiary)] text-xs'>(unique)</span>
246242
)}
247243
</>
248244
)
249-
// Currency names its code — the modal edits the bare amount, so without it
250-
// there is nothing on screen saying which currency the number is in.
245+
}
246+
247+
function columnFieldHint(column: ColumnDefinition): string {
251248
const typeLabel =
252249
column.type === 'currency'
253250
? `currency (${resolveCurrencyCode(column.currencyCode)})`
254251
: column.type
255-
const hint = `Type: ${typeLabel}${column.required ? '' : ' (optional)'}`
252+
return `Type: ${typeLabel}${column.required ? '' : ' (optional)'}`
253+
}
254+
255+
interface TimezoneBlockedColumnFieldProps {
256+
column: ColumnDefinition
257+
value: unknown
258+
status: TimezoneState['status']
259+
onAttemptEdit: () => void
260+
}
261+
262+
function TimezoneBlockedColumnField({
263+
column,
264+
value,
265+
status,
266+
onAttemptEdit,
267+
}: TimezoneBlockedColumnFieldProps) {
268+
const rawValue =
269+
typeof value === 'string'
270+
? value
271+
: value === null || value === undefined
272+
? ''
273+
: JSON.stringify(value)
274+
const displayValue = status === 'loading' ? 'Loading timezone…' : rawValue || 'No value'
275+
276+
return (
277+
<ChipModalField
278+
type='custom'
279+
title={<ColumnTitle column={column} />}
280+
required={column.required}
281+
hint={columnFieldHint(column)}
282+
>
283+
{(aria) => (
284+
<Chip
285+
{...aria}
286+
variant='border'
287+
fullWidth
288+
onClick={onAttemptEdit}
289+
aria-label={`Edit ${column.name}`}
290+
>
291+
{displayValue}
292+
</Chip>
293+
)}
294+
</ChipModalField>
295+
)
296+
}
297+
298+
function ColumnField({ column, value, timeZone, onChange }: ColumnFieldProps) {
299+
const checkboxId = useId()
300+
const title = <ColumnTitle column={column} />
301+
const hint = columnFieldHint(column)
256302
const definition = columnTypeOf(column)
257303

258304
if (definition.editor === 'toggle') {

0 commit comments

Comments
 (0)