From 70affe96d0f0102230036713022ef231ad36c8b7 Mon Sep 17 00:00:00 2001 From: Ian Chen Date: Thu, 12 Dec 2024 19:05:21 +0800 Subject: [PATCH] fix: use .data() to automically update nested objects --- .../__tests__/actions/update-row.itest.ts | 30 +++++++- .../apps/tiles/actions/update-row/index.ts | 77 +++++++++---------- .../__tests__/table-row/functions.itest.ts | 60 +++++++++++++++ .../models/dynamodb/table-row/functions.ts | 32 ++++++++ 4 files changed, 159 insertions(+), 40 deletions(-) diff --git a/packages/backend/src/apps/tiles/__tests__/actions/update-row.itest.ts b/packages/backend/src/apps/tiles/__tests__/actions/update-row.itest.ts index e3aa3ab71..86df80561 100644 --- a/packages/backend/src/apps/tiles/__tests__/actions/update-row.itest.ts +++ b/packages/backend/src/apps/tiles/__tests__/actions/update-row.itest.ts @@ -18,7 +18,7 @@ import Context from '@/types/express/context' import tiles from '../..' import updateRowAction from '../../actions/update-row' -describe('tiles create row action', () => { +describe('tiles update row action', () => { let context: Context let dummyTable: TableMetadata let dummyColumnIds: string[] @@ -113,4 +113,32 @@ describe('tiles create row action', () => { .where({ table_id: $.step.parameters.tableId }) await expect(updateRowAction.run($)).rejects.toThrow(StepError) }) + + it('should not fail if row does not exist', async () => { + $.step.parameters.rowId = '123' + await expect(updateRowAction.run($)).resolves.not.toThrow(StepError) + }) + + it('should not update columns that are not in the table', async () => { + const data = generateMockTableRowData({ + columnIds: dummyColumnIds, + }) + const row = await createTableRow({ tableId: dummyTable.id, data }) + $.step.parameters.rowId = row.rowId + $.step.parameters.rowData = [ + { columnId: 'invalid_column', cellValue: '123' }, + { columnId: dummyColumnIds[0], cellValue: '123' }, + ] + await updateRowAction.run($) + expect($.setActionItem).toBeCalledWith({ + raw: { + rowId: row.rowId, + updated: true, + row: { + ...row.data, + [dummyColumnIds[0]]: 123, + }, + }, + }) + }) }) diff --git a/packages/backend/src/apps/tiles/actions/update-row/index.ts b/packages/backend/src/apps/tiles/actions/update-row/index.ts index 9096caacd..dab2400c7 100644 --- a/packages/backend/src/apps/tiles/actions/update-row/index.ts +++ b/packages/backend/src/apps/tiles/actions/update-row/index.ts @@ -2,7 +2,7 @@ import { IRawAction } from '@plumber/types' import StepError from '@/errors/step' import { stripInvalidKeys } from '@/models/dynamodb/helpers' -import { getRawRowById, updateTableRow } from '@/models/dynamodb/table-row' +import { patchTableRow } from '@/models/dynamodb/table-row' import TableCollaborator from '@/models/table-collaborators' import TableColumnMetadata from '@/models/table-column-metadata' @@ -129,50 +129,49 @@ const action: IRawAction = { return } - const row = await getRawRowById({ - tableId, - rowId, - columnIds, - }) - - /** - * Row ID does not correspond to a row, we short circuit the action but do not fail it. - */ - if (!row) { - $.setActionItem({ - raw: { - updated: false, - } satisfies UpdateRowOutput, - }) - return - } - - const updatedData = { - ...(row.data ?? {}), + const patchData = { ...rowData.reduce((acc, { columnId, cellValue }) => { - acc[columnId] = cellValue + // Check that the column still exists + if (columnIds.includes(columnId)) { + acc[columnId] = cellValue + } return acc }, {} as Record), } + try { + const updatedRow = await patchTableRow({ + tableId, + rowId, + data: patchData, + }) - const strippedUpdatedData = stripInvalidKeys({ - columnIds, - data: updatedData, - }) - - await updateTableRow({ - tableId, - rowId, - data: strippedUpdatedData, - }) + const updatedRowData = stripInvalidKeys({ + columnIds, + data: updatedRow.data, + }) - $.setActionItem({ - raw: { - row: strippedUpdatedData, - rowId, - updated: true, - } satisfies UpdateRowOutput, - }) + $.setActionItem({ + raw: { + row: updatedRowData, + rowId, + updated: true, + } satisfies UpdateRowOutput, + }) + } catch (e) { + if ( + e instanceof Error && + e.message.includes('The conditional request failed') + ) { + // This means the corresponding row does not exist + $.setActionItem({ + raw: { + updated: false, + } satisfies UpdateRowOutput, + }) + return + } + throw e + } }, } diff --git a/packages/backend/src/models/dynamodb/__tests__/table-row/functions.itest.ts b/packages/backend/src/models/dynamodb/__tests__/table-row/functions.itest.ts index a8323b1f1..83d01bdce 100644 --- a/packages/backend/src/models/dynamodb/__tests__/table-row/functions.itest.ts +++ b/packages/backend/src/models/dynamodb/__tests__/table-row/functions.itest.ts @@ -16,6 +16,7 @@ import { getRawRowById, getTableRowCount, getTableRows, + patchTableRow, updateTableRow, } from '../../table-row/functions' @@ -333,4 +334,63 @@ describe('dynamodb table row functions', () => { expect(updatedRow.data).toEqual(newData) }) }) + + describe('patchTableRow', () => { + it('should patch the data map for the row, leaving other columns unchanged', async () => { + const data = generateMockTableRowData({ + columnIds: dummyColumnIds, + }) + const row = await createTableRow({ tableId: dummyTable.id, data }) + const newData = generateMockTableRowData({ + columnIds: [dummyColumnIds[0]], + }) + const updatedRow = await patchTableRow({ + tableId: dummyTable.id, + rowId: row.rowId, + data: newData, + }) + + expect(updatedRow.data).toEqual({ ...data, ...newData }) + }) + + it('should not change if no columns are provided', async () => { + const data = generateMockTableRowData({ + columnIds: dummyColumnIds, + }) + const row = await createTableRow({ tableId: dummyTable.id, data }) + const updatedRow = await patchTableRow({ + tableId: dummyTable.id, + rowId: row.rowId, + data: {}, + }) + + expect(updatedRow.data).toEqual(data) + }) + + it('should auto-marshall the data', async () => { + const data = generateMockTableRowData({ + columnIds: dummyColumnIds, + }) + const row = await createTableRow({ tableId: dummyTable.id, data }) + const updatedRow = await patchTableRow({ + tableId: dummyTable.id, + rowId: row.rowId, + data: { [dummyColumnIds[0]]: '123' }, + }) + expect(updatedRow.data).toEqual({ ...data, [dummyColumnIds[0]]: 123 }) + }) + + it('should update the updatedAt value', async () => { + const data = generateMockTableRowData({ + columnIds: dummyColumnIds, + }) + const row = await createTableRow({ tableId: dummyTable.id, data }) + const updatedRow = await patchTableRow({ + tableId: dummyTable.id, + rowId: row.rowId, + data: { [dummyColumnIds[0]]: '123' }, + }) + expect(updatedRow.updatedAt).toBeGreaterThan(row.updatedAt) + }) + }) }) diff --git a/packages/backend/src/models/dynamodb/table-row/functions.ts b/packages/backend/src/models/dynamodb/table-row/functions.ts index 6105e7bfd..b2f2e2a70 100644 --- a/packages/backend/src/models/dynamodb/table-row/functions.ts +++ b/packages/backend/src/models/dynamodb/table-row/functions.ts @@ -221,6 +221,38 @@ export const updateTableRow = async ({ } } +/** + * This atomically updates the data object for keys that are changed + */ +export const patchTableRow = async ({ + rowId, + tableId, + data: patchData, +}: UpdateRowInput): Promise => { + try { + const res = await TableRow.patch({ + tableId, + rowId, + }) + .data(({ data }, { set }) => { + for (const key in patchData) { + set( + data[key], + patchData[key] ? autoMarshallNumberStrings(patchData[key]) : '', + ) + } + }) + .go({ + ignoreOwnership: true, + // Return the new row data + response: 'all_new', + }) + return res.data + } catch (e: unknown) { + handleDynamoDBError(e) + } +} + export const deleteTableRows = async ({ rowIds, tableId,