mirror of
https://github.com/supabase/supabase.git
synced 2026-05-06 22:18:00 +08:00
fix: revert changes when the value is the same as old (#44196)
## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? Revert changes when the value is the same as the old value --------- Co-authored-by: Joshen Lim <joshenlimek@gmail.com>
This commit is contained in:
@@ -491,6 +491,120 @@ describe('upsertOperation', () => {
|
||||
expect((updated.payload as any).newValue).toBe('final value')
|
||||
})
|
||||
|
||||
test('should remove operation when value is reverted to original', () => {
|
||||
const existingOp: QueuedOperation = {
|
||||
id: 'edit_cell_content:1:name:id:1',
|
||||
type: QueuedOperationType.EDIT_CELL_CONTENT,
|
||||
tableId: 1,
|
||||
timestamp: Date.now() - 1000,
|
||||
payload: {
|
||||
rowIdentifiers: { id: 1 },
|
||||
columnName: 'name',
|
||||
oldValue: 'original',
|
||||
newValue: 'changed',
|
||||
table: mockTable,
|
||||
},
|
||||
}
|
||||
|
||||
const otherOp: QueuedOperation = {
|
||||
id: 'edit_cell_content:1:email:id:1',
|
||||
type: QueuedOperationType.EDIT_CELL_CONTENT,
|
||||
tableId: 1,
|
||||
timestamp: Date.now(),
|
||||
payload: {
|
||||
rowIdentifiers: { id: 1 },
|
||||
columnName: 'email',
|
||||
oldValue: 'old@test.com',
|
||||
newValue: 'new@test.com',
|
||||
table: mockTable,
|
||||
},
|
||||
}
|
||||
|
||||
const operations = [existingOp, otherOp]
|
||||
const newOperation: NewEditCellContentOperation = {
|
||||
type: QueuedOperationType.EDIT_CELL_CONTENT,
|
||||
tableId: 1,
|
||||
payload: {
|
||||
rowIdentifiers: { id: 1 },
|
||||
columnName: 'name',
|
||||
oldValue: 'changed',
|
||||
newValue: 'original',
|
||||
table: mockTable,
|
||||
},
|
||||
}
|
||||
|
||||
const result = upsertOperation(operations, newOperation)
|
||||
|
||||
expect(result.operations).toHaveLength(1)
|
||||
expect(result.operations[0]).toEqual(otherOp)
|
||||
})
|
||||
|
||||
test('should remove operation when number oldValue matches string newValue', () => {
|
||||
const existingOp: QueuedOperation = {
|
||||
id: 'edit_cell_content:1:age:id:1',
|
||||
type: QueuedOperationType.EDIT_CELL_CONTENT,
|
||||
tableId: 1,
|
||||
timestamp: Date.now() - 1000,
|
||||
payload: {
|
||||
rowIdentifiers: { id: 1 },
|
||||
columnName: 'age',
|
||||
oldValue: 42,
|
||||
newValue: 'changed',
|
||||
table: mockTable,
|
||||
},
|
||||
}
|
||||
|
||||
const operations = [existingOp]
|
||||
const newOperation: NewEditCellContentOperation = {
|
||||
type: QueuedOperationType.EDIT_CELL_CONTENT,
|
||||
tableId: 1,
|
||||
payload: {
|
||||
rowIdentifiers: { id: 1 },
|
||||
columnName: 'age',
|
||||
oldValue: 'changed',
|
||||
newValue: '42',
|
||||
table: mockTable,
|
||||
},
|
||||
}
|
||||
|
||||
const result = upsertOperation(operations, newOperation)
|
||||
|
||||
expect(result.operations).toHaveLength(0)
|
||||
})
|
||||
|
||||
test('should remove operation when string oldValue matches JSON object newValue', () => {
|
||||
const existingOp: QueuedOperation = {
|
||||
id: 'edit_cell_content:1:metadata:id:1',
|
||||
type: QueuedOperationType.EDIT_CELL_CONTENT,
|
||||
tableId: 1,
|
||||
timestamp: Date.now() - 1000,
|
||||
payload: {
|
||||
rowIdentifiers: { id: 1 },
|
||||
columnName: 'metadata',
|
||||
oldValue: '{"key":"value","count":3}',
|
||||
newValue: 'changed',
|
||||
table: mockTable,
|
||||
},
|
||||
}
|
||||
|
||||
const operations = [existingOp]
|
||||
const newOperation: NewEditCellContentOperation = {
|
||||
type: QueuedOperationType.EDIT_CELL_CONTENT,
|
||||
tableId: 1,
|
||||
payload: {
|
||||
rowIdentifiers: { id: 1 },
|
||||
columnName: 'metadata',
|
||||
oldValue: 'changed',
|
||||
newValue: { key: 'value', count: 3 },
|
||||
table: mockTable,
|
||||
},
|
||||
}
|
||||
|
||||
const result = upsertOperation(operations, newOperation)
|
||||
|
||||
expect(result.operations).toHaveLength(0)
|
||||
})
|
||||
|
||||
test('should update existing DELETE_ROW operation', () => {
|
||||
const existingOp: QueuedOperation = {
|
||||
id: 'delete_row:1:id:1',
|
||||
|
||||
@@ -1,13 +1,16 @@
|
||||
import { isEqual } from 'lodash'
|
||||
|
||||
import { isPendingAddRow } from '../types'
|
||||
import { generateTableChangeKey, rowMatchesIdentifiers } from './queueOperationUtils'
|
||||
import { tryParseJson } from '@/lib/helpers'
|
||||
import {
|
||||
type NewDeleteRowOperation,
|
||||
type NewEditCellContentOperation,
|
||||
isDeleteRowOperation,
|
||||
isEditCellContentOperation,
|
||||
NewQueuedOperation,
|
||||
QueuedOperation,
|
||||
QueuedOperationType,
|
||||
isDeleteRowOperation,
|
||||
isEditCellContentOperation,
|
||||
type NewDeleteRowOperation,
|
||||
type NewEditCellContentOperation,
|
||||
} from '@/state/table-editor-operation-queue.types'
|
||||
|
||||
export type DeleteConflictResult =
|
||||
@@ -165,6 +168,17 @@ export function upsertOperation(
|
||||
existingOp.type === QueuedOperationType.EDIT_CELL_CONTENT
|
||||
) {
|
||||
queuedOperation.payload.oldValue = existingOp.payload.oldValue
|
||||
|
||||
const { oldValue, newValue } = queuedOperation.payload
|
||||
// [Joshen] These comparisons by data type are because of how the table editor renders the values
|
||||
if (
|
||||
(typeof oldValue === 'number' && Number(oldValue) === Number(newValue)) ||
|
||||
(typeof newValue === 'object' && isEqual(tryParseJson(oldValue), newValue)) ||
|
||||
oldValue === newValue
|
||||
) {
|
||||
updatedOperations.splice(existingOpIndex, 1)
|
||||
return { operations: updatedOperations }
|
||||
}
|
||||
}
|
||||
|
||||
updatedOperations[existingOpIndex] = queuedOperation
|
||||
|
||||
@@ -786,6 +786,53 @@ test.describe('Queue Table Operations', () => {
|
||||
await expect(page.getByRole('gridcell', { name: 'only in table 2' })).not.toBeVisible()
|
||||
})
|
||||
|
||||
test('reverted cell edit clears the pending change', async ({ page, ref }) => {
|
||||
const tableName = `${tableNamePrefix}_revert_edit`
|
||||
const columnName = 'name'
|
||||
|
||||
await using _ = await withSetupCleanup(
|
||||
async () => {
|
||||
await createTable(tableName, columnName, [{ name: 'original value' }])
|
||||
},
|
||||
async () => {
|
||||
await dropTable(tableName)
|
||||
}
|
||||
)
|
||||
|
||||
await page.goto(toUrl(`/project/${ref}/editor?schema=public`))
|
||||
await enableQueueOperations(page)
|
||||
await page.reload()
|
||||
await waitForTableToLoad(page, ref)
|
||||
|
||||
await page.getByRole('button', { name: `View ${tableName}`, exact: true }).click()
|
||||
await page.waitForURL(/\/editor\/\d+\?schema=public$/)
|
||||
|
||||
await expect(page.getByRole('gridcell', { name: 'original value' })).toBeVisible()
|
||||
|
||||
// Edit the cell to a different value
|
||||
const cell = page.getByRole('gridcell', { name: 'original value' })
|
||||
await cell.dblclick()
|
||||
|
||||
const editor = page.getByRole('textbox', { name: /Editor content/ })
|
||||
await expect(editor).toBeVisible()
|
||||
await editor.fill('changed value')
|
||||
await page.keyboard.press('Enter')
|
||||
|
||||
await expect(page.getByText('1 pending change')).toBeVisible()
|
||||
|
||||
// Edit the cell back to the original value
|
||||
const changedCell = page.getByRole('gridcell', { name: 'changed value' })
|
||||
await changedCell.dblclick()
|
||||
|
||||
const editor2 = page.getByRole('textbox', { name: /Editor content/ })
|
||||
await expect(editor2).toBeVisible()
|
||||
await editor2.fill('original value')
|
||||
await page.keyboard.press('Enter')
|
||||
|
||||
// The pending change should be cleared since the value was reverted
|
||||
await expect(page.getByText('pending change')).not.toBeVisible()
|
||||
})
|
||||
|
||||
test('editing multiple columns via side panel queues all changes', async ({ page, ref }) => {
|
||||
const tableName = `${tableNamePrefix}_multi_col`
|
||||
|
||||
|
||||
Reference in New Issue
Block a user