From 3cf34b59112a2d20cdc1f1dfc0d2802a27c886c2 Mon Sep 17 00:00:00 2001 From: Wang Sijie Date: Fri, 17 Jun 2022 14:45:39 +0800 Subject: [PATCH] fix(core): align jsonb replace mode (#1138) --- packages/core/src/queries/application.ts | 5 +-- packages/core/src/queries/setting.ts | 7 +--- .../src/queries/sign-in-experience.test.ts | 4 +- .../core/src/queries/sign-in-experience.ts | 6 +-- packages/core/src/queries/user.test.ts | 37 ------------------- packages/core/src/queries/user.ts | 14 +------ packages/core/src/routes/admin-user.test.ts | 11 ------ packages/core/src/routes/admin-user.ts | 16 ++++---- 8 files changed, 15 insertions(+), 85 deletions(-) diff --git a/packages/core/src/queries/application.ts b/packages/core/src/queries/application.ts index c29254790..a04023202 100644 --- a/packages/core/src/queries/application.ts +++ b/packages/core/src/queries/application.ts @@ -43,9 +43,8 @@ const updateApplication = buildUpdateWhere(Appli export const updateApplicationById = async ( id: string, - set: Partial>, - jsonbMode: 'replace' | 'merge' = 'merge' -) => updateApplication({ set, where: { id }, jsonbMode }); + set: Partial> +) => updateApplication({ set, where: { id }, jsonbMode: 'replace' }); export const deleteApplicationById = async (id: string) => { const { rowCount } = await envSet.pool.query(sql` diff --git a/packages/core/src/queries/setting.ts b/packages/core/src/queries/setting.ts index a8b902d0b..fc17bcd39 100644 --- a/packages/core/src/queries/setting.ts +++ b/packages/core/src/queries/setting.ts @@ -16,12 +16,9 @@ export const getSetting = async () => where ${fields.id}=${defaultSettingId} `); -export const updateSetting = async ( - setting: Partial>, - jsonbMode: 'replace' | 'merge' = 'merge' -) => { +export const updateSetting = async (setting: Partial>) => { return buildUpdateWhere( Settings, true - )({ set: setting, where: { id: defaultSettingId }, jsonbMode }); + )({ set: setting, where: { id: defaultSettingId }, jsonbMode: 'merge' }); }; diff --git a/packages/core/src/queries/sign-in-experience.test.ts b/packages/core/src/queries/sign-in-experience.test.ts index a7ff6cf75..09f8f123b 100644 --- a/packages/core/src/queries/sign-in-experience.test.ts +++ b/packages/core/src/queries/sign-in-experience.test.ts @@ -55,9 +55,7 @@ describe('sign-in-experience query', () => { /* eslint-disable sql/no-unsafe-query */ const expectSql = ` update "sign_in_experiences" - set - "terms_of_use"= - coalesce("terms_of_use",'{}'::jsonb)|| $1 + set "terms_of_use"=$1 where "id"=$2 returning * `; diff --git a/packages/core/src/queries/sign-in-experience.ts b/packages/core/src/queries/sign-in-experience.ts index 40a58a390..792c1733c 100644 --- a/packages/core/src/queries/sign-in-experience.ts +++ b/packages/core/src/queries/sign-in-experience.ts @@ -14,10 +14,8 @@ const updateSignInExperience = buildUpdateWhere, - jsonbMode: 'replace' | 'merge' = 'merge' -) => updateSignInExperience({ set, where: { id }, jsonbMode }); +export const updateDefaultSignInExperience = async (set: Partial) => + updateSignInExperience({ set, where: { id }, jsonbMode: 'replace' }); export const findDefaultSignInExperience = async () => envSet.pool.one(sql` diff --git a/packages/core/src/queries/user.test.ts b/packages/core/src/queries/user.test.ts index 9bcb013ab..04c8926fd 100644 --- a/packages/core/src/queries/user.test.ts +++ b/packages/core/src/queries/user.test.ts @@ -23,7 +23,6 @@ import { findUsers, updateUserById, deleteUserById, - clearUserCustomDataById, deleteUserIdentity, } from './user'; @@ -366,42 +365,6 @@ describe('user query', () => { await expect(deleteUserById(id)).rejects.toMatchError(new DeletionError(Users.table, id)); }); - it('clearUserCustomDataById', async () => { - const id = 'foo'; - const expectSql = sql` - update ${table} - set ${fields.customData}='{}'::jsonb - where ${fields.id}=$1 - `; - - mockQuery.mockImplementationOnce(async (sql, values) => { - expectSqlAssert(sql, expectSql.sql); - expect(values).toEqual([id]); - - return createMockQueryResult([dbvalue]); - }); - - await clearUserCustomDataById(id); - }); - - it('clearUserCustomDataById should throw when user can not be found by id', async () => { - const id = 'foo'; - const expectSql = sql` - update ${table} - set ${fields.customData}='{}'::jsonb - where ${fields.id}=$1 - `; - - mockQuery.mockImplementationOnce(async (sql, values) => { - expectSqlAssert(sql, expectSql.sql); - expect(values).toEqual([id]); - - return createMockQueryResult([]); - }); - - await expect(clearUserCustomDataById(id)).rejects.toThrowError(); - }); - it('deleteUserIdentity', async () => { const userId = 'foo'; const target = 'connector1'; diff --git a/packages/core/src/queries/user.ts b/packages/core/src/queries/user.ts index 82dc2c789..19d3a48fc 100644 --- a/packages/core/src/queries/user.ts +++ b/packages/core/src/queries/user.ts @@ -5,7 +5,7 @@ import { buildInsertInto } from '@/database/insert-into'; import { buildUpdateWhere } from '@/database/update-where'; import { conditionalSql, convertToIdentifiers, OmitAutoSetFields } from '@/database/utils'; import envSet from '@/env-set'; -import { DeletionError, UpdateError } from '@/errors/SlonikError'; +import { DeletionError } from '@/errors/SlonikError'; const { table, fields } = convertToIdentifiers(Users); @@ -131,18 +131,6 @@ export const deleteUserById = async (id: string) => { } }; -export const clearUserCustomDataById = async (id: string) => { - const { rowCount } = await envSet.pool.query(sql` - update ${table} - set ${fields.customData}='{}'::jsonb - where ${fields.id}=${id} - `); - - if (rowCount < 1) { - throw new UpdateError(Users, { set: { customData: {} }, where: { id }, jsonbMode: 'replace' }); - } -}; - export const deleteUserIdentity = async (userId: string, target: string) => envSet.pool.one(sql` update ${table} diff --git a/packages/core/src/routes/admin-user.test.ts b/packages/core/src/routes/admin-user.test.ts index ccce8c46a..e321070b4 100644 --- a/packages/core/src/routes/admin-user.test.ts +++ b/packages/core/src/routes/admin-user.test.ts @@ -11,7 +11,6 @@ import { updateUserById, deleteUserIdentity, deleteUserById, - clearUserCustomDataById, } from '@/queries/user'; import { createRequester } from '@/utils/test-utils'; @@ -51,7 +50,6 @@ jest.mock('@/queries/user', () => ({ ...user, }) ), - clearUserCustomDataById: jest.fn(), deleteUserIdentity: jest.fn(), })); @@ -169,15 +167,6 @@ describe('adminUserRoutes', () => { }); }); - it('PATCH /users/:userId should call clearUserCustomDataById if customData is present', async () => { - const updateNameResponse = await userRequest.patch('/users/foo').send({ customData: {} }); - expect(updateNameResponse.status).toEqual(200); - expect(updateNameResponse.body).toEqual({ - ...mockUserResponse, - }); - expect(clearUserCustomDataById).toHaveBeenCalledTimes(1); - }); - it('PATCH /users/:userId should updated with one field if the other is undefined', async () => { const name = 'Micheal'; diff --git a/packages/core/src/routes/admin-user.ts b/packages/core/src/routes/admin-user.ts index 93533789c..9e02df7e1 100644 --- a/packages/core/src/routes/admin-user.ts +++ b/packages/core/src/routes/admin-user.ts @@ -12,7 +12,6 @@ import koaGuard from '@/middleware/koa-guard'; import koaPagination from '@/middleware/koa-pagination'; import { findRolesByRoleNames } from '@/queries/roles'; import { - clearUserCustomDataById, deleteUserById, deleteUserIdentity, findUsers, @@ -124,11 +123,6 @@ export default function adminUserRoutes(router: T) { await findUserById(userId); - // Clear customData to achieve full replacement - if (body.customData) { - await clearUserCustomDataById(userId); - } - // Temp solution to validate the existence of input roleNames if (body.roleNames?.length) { const { roleNames } = body; @@ -143,9 +137,13 @@ export default function adminUserRoutes(router: T) { } } - const user = await updateUserById(userId, { - ...body, - }); + const user = await updateUserById( + userId, + { + ...body, + }, + 'replace' + ); ctx.body = pick(user, ...userInfoSelectFields);