diff --git a/apps/studio/components/grid/SupabaseGrid.tsx b/apps/studio/components/grid/SupabaseGrid.tsx index 4c7871006de..e1f3e5205f8 100644 --- a/apps/studio/components/grid/SupabaseGrid.tsx +++ b/apps/studio/components/grid/SupabaseGrid.tsx @@ -91,6 +91,7 @@ export const SupabaseGrid = ({ customHeader={customHeader} isRefetching={isRefetching} tableQueriesEnabled={tableQueriesEnabled} + rows={rows} /> {msSqlWarning.warning !== null && } diff --git a/apps/studio/components/grid/components/grid/Grid.tsx b/apps/studio/components/grid/components/grid/Grid.tsx index d5ba2b094b7..9544be6d7ef 100644 --- a/apps/studio/components/grid/components/grid/Grid.tsx +++ b/apps/studio/components/grid/components/grid/Grid.tsx @@ -21,6 +21,7 @@ import { useOnRowsChange } from './Grid.utils' import { GridError } from './GridError' import { useTableFilter } from '@/components/grid/hooks/useTableFilter' import { handleCellKeyDown } from '@/components/grid/SupabaseGrid.utils' +import { getStableRowIdentifiers } from '@/components/grid/utils/queueOperationUtils' import { formatForeignKeys } from '@/components/interfaces/TableGridEditor/SidePanelEditor/ForeignKeySelector/ForeignKeySelector.utils' import { useForeignKeyConstraintsQuery } from '@/data/database/foreign-key-constraints-query' import { ENTITY_TYPE } from '@/data/entity-types/entity-type-constants' @@ -166,7 +167,7 @@ export const Grid = memo( // Check if this cell has pending changes const isDirty = tableEditorSnap.hasPendingCellChange( snap.table.id, - rowIdentifiers, + getStableRowIdentifiers(row, rowIdentifiers), col.key ) return isDirty ? 'rdg-cell--dirty' : undefined diff --git a/apps/studio/components/grid/components/grid/Grid.utils.tsx b/apps/studio/components/grid/components/grid/Grid.utils.tsx index 8edb10cd8d6..ca19a3fea68 100644 --- a/apps/studio/components/grid/components/grid/Grid.utils.tsx +++ b/apps/studio/components/grid/components/grid/Grid.utils.tsx @@ -3,6 +3,7 @@ import { RowsChangeData } from 'react-data-grid' import { toast } from 'sonner' import { useTableRowOperations } from '../../hooks/useTableRowOperations' +import { getStableRowIdentifiers } from '../../utils/queueOperationUtils' import { SupaRow } from '@/components/grid/types' import { convertByteaToHex } from '@/components/interfaces/TableGridEditor/SidePanelEditor/RowEditor/RowEditor.utils' import { DocsButton } from '@/components/ui/DocsButton' @@ -33,7 +34,7 @@ export function useOnRowsChange(rows: SupaRow[]) { const identifiers = {} as Dictionary - isTableLike(snap.originalTable) && + if (isTableLike(snap.originalTable)) { snap.originalTable.primary_keys.forEach((column) => { const col = snap.originalTable.columns.find((c) => c.name === column.name) identifiers[column.name] = @@ -41,8 +42,10 @@ export function useOnRowsChange(rows: SupaRow[]) { ? convertByteaToHex(previousRow[column.name]) : previousRow[column.name] }) + } + const stableIdentifiers = getStableRowIdentifiers(previousRow, identifiers) - if (Object.keys(identifiers).length === 0) { + if (Object.keys(stableIdentifiers).length === 0) { return toast('Unable to update row as table has no primary keys', { description: (
@@ -62,7 +65,7 @@ export function useOnRowsChange(rows: SupaRow[]) { tableId: snap.table.id, table: snap.originalTable, row: previousRow, - rowIdentifiers: identifiers, + rowIdentifiers: stableIdentifiers, columnName: changedColumn, oldValue: previousRow[changedColumn], newValue: rowData[changedColumn], diff --git a/apps/studio/components/grid/components/header/Header.tsx b/apps/studio/components/grid/components/header/Header.tsx index e264ec62670..88e4e0ff15b 100644 --- a/apps/studio/components/grid/components/header/Header.tsx +++ b/apps/studio/components/grid/components/header/Header.tsx @@ -21,6 +21,7 @@ import { formatRowsForCSV, hydrateTruncatedRows } from './Header.utils' import { SortPopover } from './sort/SortPopover' import { useTableRowOperations } from '@/components/grid/hooks/useTableRowOperations' import { useTableSort } from '@/components/grid/hooks/useTableSort' +import type { SupaRow } from '@/components/grid/types' import { GridHeaderActions } from '@/components/interfaces/TableGridEditor/GridHeaderActions' import { isValueTruncated } from '@/components/interfaces/TableGridEditor/SidePanelEditor/RowEditor/RowEditor.utils' import { formatTableRowsToSQL } from '@/components/interfaces/TableGridEditor/TableEntity.utils' @@ -46,10 +47,16 @@ import { useTableEditorTableStateSnapshot } from '@/state/table-editor-table' export type HeaderProps = { customHeader: ReactNode isRefetching: boolean + rows?: SupaRow[] tableQueriesEnabled?: boolean } -export const Header = ({ customHeader, isRefetching, tableQueriesEnabled = true }: HeaderProps) => { +export const Header = ({ + customHeader, + isRefetching, + rows, + tableQueriesEnabled = true, +}: HeaderProps) => { useInitializeFiltersFromUrl() useSyncFiltersToUrl() @@ -80,7 +87,7 @@ export const Header = ({ customHeader, isRefetching, tableQueriesEnabled = true
{customHeader}
) : snap.selectedRows.size > 0 ? (
- +
) : (
{ +const RowHeader = ({ rows: visibleRows, tableQueriesEnabled = true }: RowHeaderProps) => { const { id: _id } = useParams() const tableId = _id ? Number(_id) : undefined @@ -194,7 +202,7 @@ const RowHeader = ({ tableQueriesEnabled = true }: RowHeaderProps) => { const onRowsDelete = () => { const rowIdxs = Array.from(snap.selectedRows) as number[] - const rows = allRows.filter((x) => rowIdxs.includes(x.idx)) + const rows = (visibleRows ?? allRows).filter((x) => rowIdxs.includes(x.idx)) deleteRows({ rows, diff --git a/apps/studio/components/grid/utils/queueConflictResolution.test.ts b/apps/studio/components/grid/utils/queueConflictResolution.test.ts index c8ffab9ac08..00b3bfa6438 100644 --- a/apps/studio/components/grid/utils/queueConflictResolution.test.ts +++ b/apps/studio/components/grid/utils/queueConflictResolution.test.ts @@ -539,6 +539,39 @@ describe('upsertOperation', () => { expect(result.operations[0]).toEqual(otherOp) }) + test('should remove primary key edit when reverted using original row identifiers', () => { + const existingOp: QueuedOperation = { + id: 'edit_cell_content:1:id:id:1', + type: QueuedOperationType.EDIT_CELL_CONTENT, + tableId: 1, + timestamp: Date.now() - 1000, + payload: { + rowIdentifiers: { id: 1 }, + columnName: 'id', + oldValue: 1, + newValue: 3, + table: mockTable, + }, + } + + const operations = [existingOp] + const newOperation: NewEditCellContentOperation = { + type: QueuedOperationType.EDIT_CELL_CONTENT, + tableId: 1, + payload: { + rowIdentifiers: { id: 1 }, + columnName: 'id', + oldValue: 3, + newValue: 1, + table: mockTable, + }, + } + + const result = upsertOperation(operations, newOperation) + + expect(result.operations).toHaveLength(0) + }) + test('should remove operation when number oldValue matches string newValue', () => { const existingOp: QueuedOperation = { id: 'edit_cell_content:1:age:id:1', diff --git a/apps/studio/components/grid/utils/queueOperationUtils.test.ts b/apps/studio/components/grid/utils/queueOperationUtils.test.ts index c250b566f86..325bd18a413 100644 --- a/apps/studio/components/grid/utils/queueOperationUtils.test.ts +++ b/apps/studio/components/grid/utils/queueOperationUtils.test.ts @@ -1,11 +1,14 @@ -import { describe, expect, test } from 'vitest' +import { describe, expect, test, vi } from 'vitest' import type { SupaRow } from '../types' import { formatGridDataWithOperationValues, generateTableChangeKey, + getStableRowIdentifiers, + queueRowDeletesWithOptimisticUpdate, rowMatchesIdentifiers, } from './queueOperationUtils' +import { ENTITY_TYPE } from '@/data/entity-types/entity-type-constants' import { QueuedOperationType, type NewAddRowOperation, @@ -152,6 +155,20 @@ describe('rowMatchesIdentifiers', () => { }) }) +describe('stable queued row identifiers', () => { + test('should use original row identifiers when present', () => { + const row = { idx: 0, id: 3, __originalRowIdentifiers: { id: 1 } } + + expect(getStableRowIdentifiers(row, { id: 3 })).toEqual({ id: 1 }) + }) + + test('should fall back to current identifiers when original row identifiers are not present', () => { + const row = { idx: 0, id: 3 } + + expect(getStableRowIdentifiers(row, { id: 3 })).toEqual({ id: 3 }) + }) +}) + describe('formatGridDataWithOperationValues', () => { const makeRow = (idx: number, data: Record = {}): SupaRow => ({ idx, @@ -217,7 +234,7 @@ describe('formatGridDataWithOperationValues', () => { }) const result = formatGridDataWithOperationValues({ operations: [op], rows }) - expect(result[0]).toEqual({ idx: 0, id: 1, name: 'Updated' }) + expect(result[0]).toMatchObject({ idx: 0, id: 1, name: 'Updated' }) expect(result[1]).toEqual(rows[1]) }) @@ -448,4 +465,106 @@ describe('formatGridDataWithOperationValues', () => { const result = formatGridDataWithOperationValues({ operations: [op], rows }) expect(result[0].name).toBe('Updated') }) + + test('should preserve row identity after a primary key edit', () => { + const rows = [makeRow(0, { id: 1, name: 'Alice' })] + const idEdit = makeEditOp({ + id: 'edit_cell_content:1:id:id:1', + payload: { + rowIdentifiers: { id: 1 }, + columnName: 'id', + oldValue: 1, + newValue: 3, + table: {} as any, + }, + }) + const nameEdit = makeEditOp({ + id: 'edit_cell_content:1:name:id:1', + payload: { + rowIdentifiers: { id: 1 }, + columnName: 'name', + oldValue: 'Alice', + newValue: 'Updated Alice', + table: {} as any, + }, + }) + + const result = formatGridDataWithOperationValues({ operations: [idEdit, nameEdit], rows }) + + expect(result[0]).toMatchObject({ idx: 0, id: 3, name: 'Updated Alice' }) + expect(getStableRowIdentifiers(result[0], { id: result[0].id })).toEqual({ id: 1 }) + }) + + test('should keep two rows distinct when one row takes another row primary key value', () => { + const rows = [makeRow(0, { id: 1, name: 'Alice' }), makeRow(1, { id: 2, name: 'Bob' })] + const aliceIdEdit = makeEditOp({ + id: 'edit_cell_content:1:id:id:1', + payload: { + rowIdentifiers: { id: 1 }, + columnName: 'id', + oldValue: 1, + newValue: 3, + table: {} as any, + }, + }) + const bobIdEdit = makeEditOp({ + id: 'edit_cell_content:1:id:id:2', + payload: { + rowIdentifiers: { id: 2 }, + columnName: 'id', + oldValue: 2, + newValue: 1, + table: {} as any, + }, + }) + const bobNameEdit = makeEditOp({ + id: 'edit_cell_content:1:name:id:2', + payload: { + rowIdentifiers: { id: 2 }, + columnName: 'name', + oldValue: 'Bob', + newValue: 'Updated Bob', + table: {} as any, + }, + }) + + const result = formatGridDataWithOperationValues({ + operations: [aliceIdEdit, bobIdEdit, bobNameEdit], + rows, + }) + + expect(result[0]).toMatchObject({ idx: 0, id: 3, name: 'Alice' }) + expect(getStableRowIdentifiers(result[0], { id: result[0].id })).toEqual({ id: 1 }) + expect(result[1]).toMatchObject({ idx: 1, id: 1, name: 'Updated Bob' }) + expect(getStableRowIdentifiers(result[1], { id: result[1].id })).toEqual({ id: 2 }) + }) +}) + +describe('queueRowDeletesWithOptimisticUpdate', () => { + test('should queue pending add row deletes with the temp row as original row', () => { + const queueOperation = vi.fn() + const row = { idx: -100, __tempId: '-100', name: 'New Row' } as SupaRow + + queueRowDeletesWithOptimisticUpdate({ + rows: [row], + table: { + id: 1, + schema: 'public', + name: 'users', + entity_type: ENTITY_TYPE.TABLE, + primary_keys: [{ name: 'id' }], + } as Parameters[0]['table'], + queueOperation, + projectRef: 'project-ref', + }) + + expect(queueOperation).toHaveBeenCalledWith({ + type: QueuedOperationType.DELETE_ROW, + tableId: 1, + payload: expect.objectContaining({ + rowIdentifiers: { id: undefined, __tempId: '-100' }, + originalRow: row, + }), + }) + }) }) diff --git a/apps/studio/components/grid/utils/queueOperationUtils.ts b/apps/studio/components/grid/utils/queueOperationUtils.ts index 3684886601c..9c19b4328b4 100644 --- a/apps/studio/components/grid/utils/queueOperationUtils.ts +++ b/apps/studio/components/grid/utils/queueOperationUtils.ts @@ -1,5 +1,6 @@ import { isPendingAddRow, PendingAddRow, SupaRow } from '../types' import { isTableLike, type Entity } from '@/data/table-editor/table-editor-types' +import { isObject } from '@/lib/helpers' import { EditCellContentOperation, NewQueuedOperation, @@ -8,6 +9,24 @@ import { } from '@/state/table-editor-operation-queue.types' import type { Dictionary } from '@/types' +// Client-only marker that preserves the original SQL WHERE identifiers after PK edits. +const ORIGINAL_ROW_IDENTIFIERS_KEY = '__originalRowIdentifiers' + +export function getStableRowIdentifiers( + row: Dictionary, + fallbackIdentifiers: Dictionary +): Dictionary { + const identifiers = row[ORIGINAL_ROW_IDENTIFIERS_KEY] + return { ...(isObject(identifiers) ? identifiers : fallbackIdentifiers) } +} + +function withOriginalRowIdentifiers( + row: T, + rowIdentifiers: Dictionary +): T { + return { ...row, [ORIGINAL_ROW_IDENTIFIERS_KEY]: { ...rowIdentifiers } } +} + interface EditCellKeyOperation extends Omit< EditCellContentOperation, 'payload' | 'id' | 'timestamp' @@ -59,6 +78,14 @@ export function rowMatchesIdentifiers( return identifierEntries.every(([key, value]) => row[key] === value) } +function rowMatchesOperationIdentifiers( + row: Dictionary, + rowIdentifiers: Dictionary +): boolean { + const identifiers = row[ORIGINAL_ROW_IDENTIFIERS_KEY] + return rowMatchesIdentifiers(isObject(identifiers) ? identifiers : row, rowIdentifiers) +} + export function removeRow(rows: SupaRow[], rowIdentifiers: Dictionary): SupaRow[] { return rows.filter((row) => !rowMatchesIdentifiers(row, rowIdentifiers)) } @@ -86,8 +113,8 @@ export function queueCellEditWithOptimisticUpdate({ newValue, enumArrayColumns, }: QueueCellEditParams) { - // Updated row identifiers to include __tempId for pending add rows so edits merge into ADD_ROW operation - const rowIdentifiers: Dictionary = { ...callerRowIdentifiers } + // Pending add rows use __tempId so edits merge into the ADD_ROW operation. + const rowIdentifiers = getStableRowIdentifiers(row, callerRowIdentifiers) if (isPendingAddRow(row)) { rowIdentifiers.__tempId = row.__tempId } @@ -151,9 +178,14 @@ export const formatGridDataWithOperationValues = ({ operations.forEach((op) => { if (op.type === QueuedOperationType.EDIT_CELL_CONTENT) { const { rowIdentifiers, columnName, newValue } = op.payload - const rowIdx = formattedRows.findIndex((row) => rowMatchesIdentifiers(row, rowIdentifiers)) + const rowIdx = formattedRows.findIndex((row) => + rowMatchesOperationIdentifiers(row, rowIdentifiers) + ) if (rowIdx !== -1) { - formattedRows[rowIdx] = { ...formattedRows[rowIdx], [columnName]: newValue } + formattedRows[rowIdx] = withOriginalRowIdentifiers( + { ...formattedRows[rowIdx], [columnName]: newValue }, + rowIdentifiers + ) } } else if (op.type === QueuedOperationType.ADD_ROW) { const { tempId, rowData } = op.payload @@ -176,9 +208,14 @@ export const formatGridDataWithOperationValues = ({ } } else if (op.type === QueuedOperationType.DELETE_ROW) { const { rowIdentifiers } = op.payload - const rowIdx = formattedRows.findIndex((row) => rowMatchesIdentifiers(row, rowIdentifiers)) + const rowIdx = formattedRows.findIndex((row) => + rowMatchesOperationIdentifiers(row, rowIdentifiers) + ) if (rowIdx !== -1) { - formattedRows[rowIdx] = { ...formattedRows[rowIdx], __isDeleted: true } + formattedRows[rowIdx] = withOriginalRowIdentifiers( + { ...formattedRows[rowIdx], __isDeleted: true }, + rowIdentifiers + ) } } }) @@ -225,12 +262,16 @@ export function queueRowDeletesWithOptimisticUpdate({ table.primary_keys.forEach((pk) => { rowIdentifiers[pk.name] = row[pk.name] }) + const stableRowIdentifiers = getStableRowIdentifiers(row, rowIdentifiers) + if (isPendingAddRow(row)) { + stableRowIdentifiers.__tempId = row.__tempId + } queueOperation({ type: QueuedOperationType.DELETE_ROW, tableId: table.id, payload: { - rowIdentifiers, + rowIdentifiers: stableRowIdentifiers, originalRow: row, table, }, diff --git a/apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/RowEditor/RowEditor.tsx b/apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/RowEditor/RowEditor.tsx index ced4a6bc70a..6c4b5a08f37 100644 --- a/apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/RowEditor/RowEditor.tsx +++ b/apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/RowEditor/RowEditor.tsx @@ -18,6 +18,7 @@ import { validateFields, } from './RowEditor.utils' import { TextEditor } from './TextEditor' +import { getStableRowIdentifiers } from '@/components/grid/utils/queueOperationUtils' import { useIsQueueOperationsEnabled } from '@/components/interfaces/Account/Preferences/useDashboardSettings' import { useForeignKeyConstraintsQuery } from '@/data/database/foreign-key-constraints-query' import { useSelectedProjectQuery } from '@/hooks/misc/useSelectedProject' @@ -144,7 +145,7 @@ export const RowEditor = ({ identifiers[column.name] = column.format === 'bytea' ? convertByteaToHex(row![column.name]) : row![column.name] }) - configuration.identifiers = identifiers + configuration.identifiers = getStableRowIdentifiers(row!, identifiers) configuration.rowIdx = row!.idx } diff --git a/apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/SidePanelEditor.tsx b/apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/SidePanelEditor.tsx index 42d782b88ec..9f7b919e671 100644 --- a/apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/SidePanelEditor.tsx +++ b/apps/studio/components/interfaces/TableGridEditor/SidePanelEditor/SidePanelEditor.tsx @@ -35,6 +35,7 @@ import { import { TableEditor } from './TableEditor/TableEditor' import type { ImportContent } from './TableEditor/TableEditor.types' import { useTableRowOperations } from '@/components/grid/hooks/useTableRowOperations' +import { getStableRowIdentifiers } from '@/components/grid/utils/queueOperationUtils' import { useIsQueueOperationsEnabled } from '@/components/interfaces/Account/Preferences/useDashboardSettings' import { acceptGeneratedPolicy, @@ -318,7 +319,7 @@ export const SidePanelEditor = ({ const { row, column } = selectedValueForJsonEdit payload = { [column]: value === null ? null : JSON.parse(value as any) } selectedTable.primary_keys.forEach((column) => (identifiers[column.name] = row![column.name])) - configuration = { identifiers, rowIdx: row.idx } + configuration = { identifiers: getStableRowIdentifiers(row!, identifiers), rowIdx: row.idx } } else if (snap.sidePanel?.type === 'cell') { const column = snap.sidePanel.value?.column const row = snap.sidePanel.value?.row @@ -326,7 +327,7 @@ export const SidePanelEditor = ({ if (!column || !row) return payload = { [column]: value === null ? null : value } selectedTable.primary_keys.forEach((column) => (identifiers[column.name] = row![column.name])) - configuration = { identifiers, rowIdx: row.idx } + configuration = { identifiers: getStableRowIdentifiers(row!, identifiers), rowIdx: row.idx } } if (payload !== undefined && configuration !== undefined) { @@ -354,7 +355,10 @@ export const SidePanelEditor = ({ }) const isNewRecord = false - const configuration = { identifiers, rowIdx: row.idx } + const configuration = { + identifiers: getStableRowIdentifiers(row, identifiers), + rowIdx: row.idx, + } await saveRow(value, isNewRecord, configuration, (error) => { if (error) { diff --git a/apps/studio/data/table-rows/operation-queue-save-mutation.test.ts b/apps/studio/data/table-rows/operation-queue-save-mutation.test.ts new file mode 100644 index 00000000000..e81a01e92fb --- /dev/null +++ b/apps/studio/data/table-rows/operation-queue-save-mutation.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from 'vitest' + +import { getOperationSqlStatements } from './operation-queue-save-mutation' +import type { Entity } from '@/data/table-editor/table-editor-types' +import { QueuedOperation, QueuedOperationType } from '@/state/table-editor-operation-queue.types' + +const usersTable = { id: 1, name: 'users', schema: 'public' } as Entity + +function createEditOperation( + columnName: string, + newValue: unknown, + rowIdentifiers: Record = { id: 1 } +): QueuedOperation { + return { + id: `${columnName}-${String(newValue)}`, + tableId: 1, + timestamp: 1, + type: QueuedOperationType.EDIT_CELL_CONTENT, + payload: { + rowIdentifiers, + columnName, + oldValue: undefined, + newValue, + table: usersTable, + enumArrayColumns: [], + }, + } +} + +describe('getOperationSqlStatements', () => { + it('merges edits for the same row into one update statement', () => { + const statements = getOperationSqlStatements([ + createEditOperation('id', '4'), + createEditOperation('name', 'Ram 1'), + ]) + + expect(statements).toHaveLength(1) + + const sql = String(statements[0]) + expect(sql).toContain('update public.users set (id,name)') + expect(sql).toContain('"id":"4"') + expect(sql).toContain('"name":"Ram 1"') + expect(sql.match(/where id = 1/g)).toHaveLength(1) + }) + + it('keeps edits for different rows as separate update statements', () => { + const statements = getOperationSqlStatements([ + createEditOperation('name', 'Ram 1', { id: 1 }), + createEditOperation('name', 'Shyam 1', { id: 2 }), + ]) + + expect(statements).toHaveLength(2) + expect(String(statements[0])).toContain('where id = 1') + expect(String(statements[1])).toContain('where id = 2') + }) + + it('does not merge rows whose identifier values would collide with delimiter keys', () => { + const statements = getOperationSqlStatements([ + createEditOperation('name', 'Row 1', { a: 'x', b: 'y|b:z' }), + createEditOperation('name', 'Row 2', { a: 'x|b:y', b: 'z' }), + ]) + + expect(statements).toHaveLength(2) + }) +}) diff --git a/apps/studio/data/table-rows/operation-queue-save-mutation.ts b/apps/studio/data/table-rows/operation-queue-save-mutation.ts index 67c554cd151..e31e920e8fc 100644 --- a/apps/studio/data/table-rows/operation-queue-save-mutation.ts +++ b/apps/studio/data/table-rows/operation-queue-save-mutation.ts @@ -11,9 +11,22 @@ import type { PendingAddRow } from '@/components/grid/types' import { executeSql } from '@/data/sql/execute-sql-mutation' import { RoleImpersonationState, wrapWithRoleImpersonation } from '@/lib/role-impersonation' import { isRoleImpersonationEnabled } from '@/state/role-impersonation-state' -import { QueuedOperation, QueuedOperationType } from '@/state/table-editor-operation-queue.types' +import { + isEditCellContentOperation, + QueuedOperation, + QueuedOperationType, + type EditCellContentOperation, +} from '@/state/table-editor-operation-queue.types' import type { ResponseError, UseCustomMutationOptions } from '@/types' +function getEditCellOperationRowKey(operation: EditCellContentOperation): string { + const rowIdentifiersKey = JSON.stringify( + Object.entries(operation.payload.rowIdentifiers).sort(([a], [b]) => a.localeCompare(b)) + ) + + return `${operation.tableId}:${rowIdentifiersKey}` +} + export type OperationQueueSaveVariables = { projectRef: string connectionString?: string | null @@ -81,6 +94,55 @@ function sortOperations(operations: readonly QueuedOperation[]): QueuedOperation }) } +function stripTrailingSemicolon(sql: SafeSqlFragment): SafeSqlFragment { + return (sql.endsWith(';') ? sql.slice(0, -1) : sql) as SafeSqlFragment +} + +export function getOperationSqlStatements( + operations: readonly QueuedOperation[] +): Array { + const statements: Array = [] + const editCellOperationsByRow = new Map() + + for (const operation of sortOperations(operations)) { + if (!isEditCellContentOperation(operation)) { + statements.push(stripTrailingSemicolon(getOperationSql(operation))) + continue + } + + const key = getEditCellOperationRowKey(operation) + const existing = editCellOperationsByRow.get(key) + if (existing) { + existing.push(operation) + continue + } + editCellOperationsByRow.set(key, [operation]) + } + + for (const editCellOperations of editCellOperationsByRow.values()) { + const firstOperation = editCellOperations[0] + const { table, rowIdentifiers } = firstOperation.payload + + statements.push( + stripTrailingSemicolon( + getTableRowUpdateSql({ + table: { id: table.id, name: table.name, schema: table.schema }, + configuration: { identifiers: rowIdentifiers }, + payload: Object.fromEntries( + editCellOperations.map(({ payload }) => [payload.columnName, payload.newValue]) + ), + enumArrayColumns: [ + ...new Set(editCellOperations.flatMap(({ payload }) => payload.enumArrayColumns ?? [])), + ], + returning: false, + }) + ) + ) + } + + return statements +} + /** * Saves all queued operations in a single database transaction. * If any operation fails, the entire transaction is rolled back. @@ -95,11 +157,7 @@ export async function saveOperationQueue({ return { result: [] } } - const sortedOperations = sortOperations(operations) - const statements: Array = sortedOperations.map((op) => { - const sql = getOperationSql(op) - return (sql.endsWith(';') ? sql.slice(0, -1) : sql) as SafeSqlFragment - }) + const statements = getOperationSqlStatements(operations) const transactionSql = wrapWithTransaction(safeSql`${joinSqlFragments(statements, ';\n')};`) diff --git a/e2e/studio/features/queue-table-operations.spec.ts b/e2e/studio/features/queue-table-operations.spec.ts index b4c55f9cd01..3ff1581a3a4 100644 --- a/e2e/studio/features/queue-table-operations.spec.ts +++ b/e2e/studio/features/queue-table-operations.spec.ts @@ -22,6 +22,24 @@ const openQueueDropdownAndClick = async (page: Page, itemName: string) => { const clickReview = async (page: Page) => openQueueDropdownAndClick(page, 'Review') const clickDiscard = async (page: Page) => openQueueDropdownAndClick(page, 'Discard') +const openTableWithQueueOperations = async (page: Page, ref: string, tableName: string) => { + await page.goto(toUrl(`/project/${ref}/editor?schema=public`)) + await enableQueueOperations(page) + const tableLoadWait = createApiResponseWaiter( + page, + 'pg-meta', + ref, + 'query?key=entity-types-public-' + ) + await page.reload() + await tableLoadWait + + const gridLoadWait = createApiResponseWaiter(page, 'pg-meta', ref, 'query?key=table-rows-') + await page.getByRole('button', { name: `View ${tableName}`, exact: true }).click() + await page.waitForURL(/\/editor\/\d+\?schema=public$/) + await gridLoadWait +} + test.describe('Queue Table Operations', () => { test.beforeEach(async ({ page, ref }) => { const loadPromise = waitForTableToLoad(page, ref) @@ -1028,3 +1046,347 @@ test.describe('Queue Table Operations', () => { await expect(page.getByRole('gridcell', { name: 'Smith' })).not.toBeVisible() }) }) + +test.describe('Queue Table Operations - queue identity fixes', () => { + test('primary-key and column edits to the same row are saved together', async ({ page, ref }) => { + const tableName = `${tableNamePrefix}_pk_col_edit` + + await using _ = await withSetupCleanup( + async () => { + await dropTable(tableName) + await query( + `CREATE TABLE ${tableName} ( + id bigint primary key, + name text + )` + ) + await query(`INSERT INTO ${tableName} (id, name) VALUES ($1, $2)`, [ + 13001, + 'original pk row', + ]) + }, + async () => { + await dropTable(tableName) + } + ) + + await openTableWithQueueOperations(page, ref, tableName) + + const originalNameCell = page.getByRole('gridcell', { name: 'original pk row' }) + await expect( + originalNameCell, + 'Original row should be visible before editing through the row editor' + ).toBeVisible() + + await originalNameCell.click({ button: 'right' }) + await page.getByRole('menuitem', { name: 'Edit row' }).click() + + const rowEditor = page.getByTestId('side-panel-row-editor') + await expect(rowEditor, 'Row editor should open for the selected row').toBeVisible() + + await rowEditor.getByTestId('id-input').fill('13002') + await rowEditor.getByTestId('name-input').fill('updated pk row') + await page.getByTestId('action-bar-save-row').click() + + await expect( + page.getByText('2 pending changes'), + 'Editing the primary key and name should queue two cell edits' + ).toBeVisible() + await expect( + page.getByRole('gridcell', { name: '13002' }), + 'Optimistic grid state should show the edited primary key' + ).toBeVisible() + await expect( + page.getByRole('gridcell', { name: 'updated pk row' }), + 'Optimistic grid state should show the edited name' + ).toBeVisible() + + await clickReview(page) + + const sidePanel = page.getByRole('dialog') + await expect(sidePanel.getByText('Pending changes')).toBeVisible() + await expect( + sidePanel.getByText('2 cell edits'), + 'Both queued edits should still be reviewed as cell edits' + ).toBeVisible() + + const saveWait = createApiResponseWaiter( + page, + 'pg-meta', + ref, + 'query?key=operation-queue-save', + { + method: 'POST', + } + ) + await sidePanel.getByRole('button', { name: /^Save/ }).click() + await saveWait + await expect( + page.getByText('Changes saved successfully'), + 'Queue save should complete successfully' + ).toBeVisible({ timeout: 15000 }) + + const rows = await query<{ id: string; name: string }>( + `SELECT id::text AS id, name FROM ${tableName} ORDER BY id` + ) + expect(rows).toEqual([{ id: '13002', name: 'updated pk row' }]) + + await expect(page.getByRole('gridcell', { name: 'updated pk row' })).toBeVisible() + await expect(page.getByRole('gridcell', { name: 'original pk row' })).not.toBeVisible() + }) + + test('primary-key edits can be reverted before saving', async ({ page, ref }) => { + const tableName = `${tableNamePrefix}_pk_revert` + + await using _ = await withSetupCleanup( + async () => { + await dropTable(tableName) + await query( + `CREATE TABLE ${tableName} ( + id bigint primary key, + name text + )` + ) + await query(`INSERT INTO ${tableName} (id, name) VALUES ($1, $2)`, [14001, 'pk revert row']) + }, + async () => { + await dropTable(tableName) + } + ) + + await openTableWithQueueOperations(page, ref, tableName) + + const originalIdCell = page.getByRole('gridcell', { name: '14001', exact: true }) + await expect( + originalIdCell, + 'Original primary key should be visible before editing' + ).toBeVisible() + + await originalIdCell.click({ button: 'right' }) + await page.getByRole('menuitem', { name: 'Edit row' }).click() + + const rowEditor = page.getByTestId('side-panel-row-editor') + await expect(rowEditor, 'Row editor should open for the selected row').toBeVisible() + await rowEditor.getByTestId('id-input').fill('14002') + await page.getByTestId('action-bar-save-row').click() + + await expect( + page.getByText('1 pending change'), + 'Changing a primary-key cell should queue one pending edit' + ).toBeVisible() + await expect( + page.getByRole('gridcell', { name: '14002', exact: true }), + 'Optimistic grid state should show the changed primary key' + ).toBeVisible() + + await page.getByRole('gridcell', { name: '14002', exact: true }).click({ button: 'right' }) + await page.getByRole('menuitem', { name: 'Edit row' }).click() + + const revertedRowEditor = page.getByTestId('side-panel-row-editor') + await expect( + revertedRowEditor, + 'Row editor should reopen after the primary key changed' + ).toBeVisible() + await revertedRowEditor.getByTestId('id-input').fill('14001') + await page.getByTestId('action-bar-save-row').click() + + await expect( + page.getByText('pending change'), + 'Reverting the primary key to its original value should clear the queued edit' + ).not.toBeVisible() + await expect(page.getByRole('gridcell', { name: '14001', exact: true })).toBeVisible() + }) + + test('primary-key edits keep rows distinct when another row takes the old key', async ({ + page, + ref, + }) => { + const tableName = `${tableNamePrefix}_pk_take_old` + + await using _ = await withSetupCleanup( + async () => { + await dropTable(tableName) + await query( + `CREATE TABLE ${tableName} ( + id bigint primary key, + name text + )` + ) + await query(`INSERT INTO ${tableName} (id, name) VALUES ($1, $2), ($3, $4)`, [ + 15001, + 'Alice keeps identity', + 15002, + 'Bob takes old key', + ]) + }, + async () => { + await dropTable(tableName) + } + ) + + await openTableWithQueueOperations(page, ref, tableName) + + const aliceCell = page.getByRole('gridcell', { name: 'Alice keeps identity' }) + await expect(aliceCell, 'Alice row should be visible before editing').toBeVisible() + await aliceCell.click({ button: 'right' }) + await page.getByRole('menuitem', { name: 'Edit row' }).click() + + const aliceEditor = page.getByTestId('side-panel-row-editor') + await expect(aliceEditor, 'Row editor should open for Alice').toBeVisible() + await aliceEditor.getByTestId('id-input').fill('15003') + await page.getByTestId('action-bar-save-row').click() + + await expect( + page.getByText('1 pending change'), + 'Alice primary-key edit should queue one pending change' + ).toBeVisible() + await expect( + page.getByRole('gridcell', { name: '15003', exact: true }), + 'Alice row should show the updated primary key in the grid' + ).toBeVisible() + + const bobCell = page.getByRole('gridcell', { name: 'Bob takes old key' }) + await expect( + bobCell, + 'Bob row should remain visible after Alice primary-key edit' + ).toBeVisible() + await bobCell.click({ button: 'right' }) + await page.getByRole('menuitem', { name: 'Edit row' }).click() + + const bobEditor = page.getByTestId('side-panel-row-editor') + await expect(bobEditor, 'Row editor should open for Bob').toBeVisible() + await bobEditor.getByTestId('id-input').fill('15001') + await page.getByTestId('action-bar-save-row').click() + + await expect( + page.getByText('2 pending changes'), + 'Both primary-key edits should stay queued against their original rows' + ).toBeVisible() + + await clickReview(page) + + const sidePanel = page.getByRole('dialog') + await expect( + sidePanel.getByText('Pending changes'), + 'Review panel should show pending changes before saving primary-key edits' + ).toBeVisible() + await expect( + sidePanel.getByText('2 cell edits'), + 'Review panel should show both primary-key edits as cell edits' + ).toBeVisible() + + const saveWait = createApiResponseWaiter( + page, + 'pg-meta', + ref, + 'query?key=operation-queue-save', + { + method: 'POST', + } + ) + await sidePanel.getByRole('button', { name: /^Save/ }).click() + await saveWait + await expect( + page.getByText('Changes saved successfully'), + 'Saving swapped primary-key edits should show a success toast' + ).toBeVisible({ timeout: 15000 }) + + const rows = await query<{ id: string; name: string }>( + `SELECT id::text AS id, name FROM ${tableName} ORDER BY id` + ) + expect(rows).toEqual([ + { id: '15001', name: 'Bob takes old key' }, + { id: '15003', name: 'Alice keeps identity' }, + ]) + }) + + test('deleting a newly inserted row removes it from pending changes', async ({ page, ref }) => { + const tableName = `${tableNamePrefix}_pending_del` + const columnName = 'name' + + await using _ = await withSetupCleanup( + async () => { + await createTable(tableName, columnName) + }, + async () => { + await dropTable(tableName) + } + ) + + await openTableWithQueueOperations(page, ref, tableName) + + await page.getByTestId('table-editor-insert-new-row').click() + await page.getByRole('menuitem', { name: 'Insert row' }).click() + await page.getByTestId(`${columnName}-input`).fill('keep pending row') + await page.getByTestId('action-bar-save-row').click() + + await page.getByTestId('table-editor-insert-new-row').click() + await page.getByRole('menuitem', { name: 'Insert row' }).click() + await page.getByTestId(`${columnName}-input`).fill('delete pending row') + await page.getByTestId('action-bar-save-row').click() + + await expect( + page.getByText('2 pending changes'), + 'Adding two pending rows should queue two pending changes' + ).toBeVisible() + await expect( + page.getByRole('gridcell', { name: 'keep pending row' }), + 'First pending row should be visible before deleting the second row' + ).toBeVisible() + await expect( + page.getByRole('gridcell', { name: 'delete pending row' }), + 'Second pending row should be visible before deletion' + ).toBeVisible() + + await page.getByRole('gridcell', { name: 'delete pending row' }).click({ button: 'right' }) + await page.getByRole('menuitem', { name: 'Delete row' }).click() + + await expect( + page.getByText('1 pending change'), + 'Deleting one pending insert should remove only that row from the queue' + ).toBeVisible() + await expect( + page.getByRole('gridcell', { name: 'keep pending row' }), + 'Remaining pending row should stay visible after deleting the other pending row' + ).toBeVisible() + await expect( + page.getByRole('gridcell', { name: 'delete pending row' }), + 'Deleted pending row should be removed from the grid' + ).not.toBeVisible() + + await clickReview(page) + + const sidePanel = page.getByRole('dialog') + await expect( + sidePanel.getByText('Pending changes'), + 'Review panel should show pending changes after deleting one pending row' + ).toBeVisible() + await expect( + sidePanel.getByText('1 row addition'), + 'Review panel should only show the remaining pending row addition' + ).toBeVisible() + await expect( + sidePanel.getByText('delete pending row'), + 'Review panel should not include the deleted pending row' + ).not.toBeVisible() + + const saveWait = createApiResponseWaiter( + page, + 'pg-meta', + ref, + 'query?key=operation-queue-save', + { + method: 'POST', + } + ) + await sidePanel.getByRole('button', { name: /^Save/ }).click() + await saveWait + await expect( + page.getByText('Changes saved successfully'), + 'Saving the remaining pending row should show a success toast' + ).toBeVisible({ timeout: 15000 }) + + const rows = await query<{ name: string }>(`SELECT name FROM ${tableName} ORDER BY name`) + expect(rows).toEqual([{ name: 'keep pending row' }]) + }) +})