From 8f3de6aa0b19e1edf3843f6534b2a1ddae00bfb0 Mon Sep 17 00:00:00 2001 From: Charles Zhao Date: Thu, 10 Nov 2022 19:27:53 +0800 Subject: [PATCH] fix(core): fix field updating issues in patch user management api --- packages/core/src/queries/user.ts | 9 ++++--- packages/core/src/routes/admin-user.test.ts | 27 +++++++++++++++------ packages/core/src/routes/admin-user.ts | 10 ++++---- packages/core/src/routes/session/utils.ts | 25 ++++++++++--------- 4 files changed, 45 insertions(+), 26 deletions(-) diff --git a/packages/core/src/queries/user.ts b/packages/core/src/queries/user.ts index 6a8d43cc8..65ec70c1c 100644 --- a/packages/core/src/queries/user.ts +++ b/packages/core/src/queries/user.ts @@ -47,11 +47,12 @@ export const findUserByIdentity = async (target: string, userId: string) => ` ); -export const hasUser = async (username: string) => +export const hasUser = async (username: string, excludeUserId?: string) => envSet.pool.exists(sql` select ${fields.id} from ${table} where ${fields.username}=${username} + ${conditionalSql(excludeUserId, (id) => sql`and ${fields.id}<>${id}`)} `); export const hasUserWithId = async (id: string) => @@ -61,18 +62,20 @@ export const hasUserWithId = async (id: string) => where ${fields.id}=${id} `); -export const hasUserWithEmail = async (email: string) => +export const hasUserWithEmail = async (email: string, excludeUserId?: string) => envSet.pool.exists(sql` select ${fields.primaryEmail} from ${table} where lower(${fields.primaryEmail})=lower(${email}) + ${conditionalSql(excludeUserId, (id) => sql`and ${fields.id}<>${id}`)} `); -export const hasUserWithPhone = async (phone: string) => +export const hasUserWithPhone = async (phone: string, excludeUserId?: string) => envSet.pool.exists(sql` select ${fields.primaryPhone} from ${table} where ${fields.primaryPhone}=${phone} + ${conditionalSql(excludeUserId, (id) => sql`and ${fields.id}<>${id}`)} `); export const hasUserWithIdentity = async (target: string, userId: string) => diff --git a/packages/core/src/routes/admin-user.test.ts b/packages/core/src/routes/admin-user.test.ts index 66ea3daa3..f61288cff 100644 --- a/packages/core/src/routes/admin-user.test.ts +++ b/packages/core/src/routes/admin-user.test.ts @@ -177,16 +177,29 @@ describe('adminUserRoutes', () => { }); }); - it('PATCH /users/:userId should allow empty avatar URL', async () => { - const name = 'Michael'; - const avatar = ''; - - const response = await userRequest.patch('/users/foo').send({ name, avatar }); + it('PATCH /users/:userId should allow empty string for clearable fields', async () => { + const response = await userRequest + .patch('/users/foo') + .send({ name: '', avatar: '', primaryEmail: '' }); expect(response.status).toEqual(200); expect(response.body).toEqual({ ...mockUserResponse, - name, - avatar, + name: '', + avatar: '', + primaryEmail: '', + }); + }); + + it('PATCH /users/:userId should allow null values for clearable fields', async () => { + const response = await userRequest + .patch('/users/foo') + .send({ name: null, username: null, primaryPhone: null }); + expect(response.status).toEqual(200); + expect(response.body).toEqual({ + ...mockUserResponse, + name: null, + username: null, + primaryPhone: null, }); }); diff --git a/packages/core/src/routes/admin-user.ts b/packages/core/src/routes/admin-user.ts index 796baef37..e57ed0a27 100644 --- a/packages/core/src/routes/admin-user.ts +++ b/packages/core/src/routes/admin-user.ts @@ -170,10 +170,10 @@ export default function adminUserRoutes(router: T) { koaGuard({ params: object({ userId: string() }), body: object({ - username: string().regex(usernameRegEx).optional(), - primaryEmail: string().regex(emailRegEx).optional(), - primaryPhone: string().regex(phoneRegEx).optional(), - name: string().nullable().optional(), + username: string().regex(usernameRegEx).or(literal('')).nullable().optional(), + primaryEmail: string().regex(emailRegEx).or(literal('')).nullable().optional(), + primaryPhone: string().regex(phoneRegEx).or(literal('')).nullable().optional(), + name: string().or(literal('')).nullable().optional(), avatar: string().url().or(literal('')).nullable().optional(), customData: arbitraryObjectGuard.optional(), roleNames: string().array().optional(), @@ -186,7 +186,7 @@ export default function adminUserRoutes(router: T) { } = ctx.guard; await findUserById(userId); - await checkExistingSignUpIdentifiers(body); + await checkExistingSignUpIdentifiers(body, userId); // Temp solution to validate the existence of input roleNames if (body.roleNames?.length) { diff --git a/packages/core/src/routes/session/utils.ts b/packages/core/src/routes/session/utils.ts index a4d0d4f0e..be880b0d7 100644 --- a/packages/core/src/routes/session/utils.ts +++ b/packages/core/src/routes/session/utils.ts @@ -191,9 +191,9 @@ export const checkRequiredProfile = async ( }; export const checkRequiredSignUpIdentifiers = async (identifiers: { - username?: string; - primaryEmail?: string; - primaryPhone?: string; + username?: Nullable; + primaryEmail?: Nullable; + primaryPhone?: Nullable; }) => { const { username, primaryEmail, primaryPhone } = identifiers; @@ -217,22 +217,25 @@ export const checkRequiredSignUpIdentifiers = async (identifiers: { }; /* eslint-enable complexity */ -export const checkExistingSignUpIdentifiers = async (identifiers: { - username?: string; - primaryEmail?: string; - primaryPhone?: string; -}) => { +export const checkExistingSignUpIdentifiers = async ( + identifiers: { + username?: Nullable; + primaryEmail?: Nullable; + primaryPhone?: Nullable; + }, + excludeUserId: string +) => { const { username, primaryEmail, primaryPhone } = identifiers; - if (username && (await hasUser(username))) { + if (username && (await hasUser(username, excludeUserId))) { throw new RequestError({ code: 'user.username_exists', status: 422 }); } - if (primaryEmail && (await hasUserWithEmail(primaryEmail))) { + if (primaryEmail && (await hasUserWithEmail(primaryEmail, excludeUserId))) { throw new RequestError({ code: 'user.email_exists', status: 422 }); } - if (primaryPhone && (await hasUserWithPhone(primaryPhone))) { + if (primaryPhone && (await hasUserWithPhone(primaryPhone, excludeUserId))) { throw new RequestError({ code: 'user.sms_exists', status: 422 }); } };