From 8e13dd5746b99f311bb44deb1425ae31f3679184 Mon Sep 17 00:00:00 2001
From: Wang Sijie <wangsijie@silverhand.io>
Date: Thu, 21 Apr 2022 12:38:54 +0800
Subject: [PATCH] feat(console): user role select (#589)

---
 .../UserDetails/components/RoleSelect.tsx     | 44 ++++++++++++++++
 .../console/src/pages/UserDetails/index.tsx   | 15 ++++--
 packages/core/src/routes/admin-user.test.ts   | 52 ++++++-------------
 packages/core/src/routes/admin-user.ts        | 51 ++++++------------
 packages/phrases/src/locales/en.ts            |  4 ++
 packages/phrases/src/locales/zh-cn.ts         |  4 ++
 6 files changed, 94 insertions(+), 76 deletions(-)
 create mode 100644 packages/console/src/pages/UserDetails/components/RoleSelect.tsx

diff --git a/packages/console/src/pages/UserDetails/components/RoleSelect.tsx b/packages/console/src/pages/UserDetails/components/RoleSelect.tsx
new file mode 100644
index 000000000..333b2a9ef
--- /dev/null
+++ b/packages/console/src/pages/UserDetails/components/RoleSelect.tsx
@@ -0,0 +1,44 @@
+import React, { useMemo } from 'react';
+import { useTranslation } from 'react-i18next';
+
+import Select from '@/components/Select';
+
+type Props = {
+  value?: string[];
+  onChange?: (value: string[]) => void;
+};
+
+const roleDefault = 'default';
+const roleAdmin = 'admin';
+
+const RoleSelect = ({ value, onChange }: Props) => {
+  const { t } = useTranslation(undefined, { keyPrefix: 'admin_console' });
+
+  const options = useMemo(
+    () => [
+      { value: roleDefault, title: t('user_details.roles.default') },
+      { value: roleAdmin, title: t('user_details.roles.admin') },
+    ],
+    [t]
+  );
+
+  const selectValue = useMemo(() => {
+    if (!value?.length) {
+      return roleDefault;
+    }
+
+    if (value.length === 1 && value[0] === 'admin') {
+      return roleAdmin;
+    }
+
+    throw new Error('Unsupported user role value');
+  }, [value]);
+
+  const handleChange = (value: string) => {
+    onChange?.(value === roleAdmin ? ['admin'] : []);
+  };
+
+  return <Select options={options} value={selectValue} onChange={handleChange} />;
+};
+
+export default RoleSelect;
diff --git a/packages/console/src/pages/UserDetails/index.tsx b/packages/console/src/pages/UserDetails/index.tsx
index a4dc82787..44ea36294 100644
--- a/packages/console/src/pages/UserDetails/index.tsx
+++ b/packages/console/src/pages/UserDetails/index.tsx
@@ -1,7 +1,7 @@
 import { User } from '@logto/schemas';
 import { Nullable } from '@silverhand/essentials';
 import React, { useEffect, useState } from 'react';
-import { useController, useForm } from 'react-hook-form';
+import { Controller, useController, useForm } from 'react-hook-form';
 import { toast } from 'react-hot-toast';
 import { useTranslation } from 'react-i18next';
 import ReactModal from 'react-modal';
@@ -30,6 +30,7 @@ import { safeParseJson } from '@/utilities/json';
 import CreateSuccess from './components/CreateSuccess';
 import DeleteForm from './components/DeleteForm';
 import ResetPasswordForm from './components/ResetPasswordForm';
+import RoleSelect from './components/RoleSelect';
 import UserConnectors from './components/UserConnectors';
 import * as styles from './index.module.scss';
 
@@ -39,7 +40,7 @@ type FormData = {
   username: Nullable<string>;
   name: Nullable<string>;
   avatar: Nullable<string>;
-  roles: Nullable<string>;
+  roleNames: string[];
   customData: string;
 };
 
@@ -72,7 +73,6 @@ const UserDetails = () => {
     }
     reset({
       ...data,
-      roles: data.roleNames.join(','),
       customData: JSON.stringify(data.customData, null, 2),
     });
   }, [data, reset]);
@@ -93,6 +93,7 @@ const UserDetails = () => {
     const payload: Partial<User> = {
       name: formData.name,
       avatar: formData.avatar,
+      roleNames: formData.roleNames,
       customData,
     };
 
@@ -217,7 +218,13 @@ const UserDetails = () => {
                   title="admin_console.user_details.field_roles"
                   className={styles.textField}
                 >
-                  <TextInput readOnly {...register('roles')} />
+                  <Controller
+                    name="roleNames"
+                    control={control}
+                    render={({ field: { value, onChange } }) => (
+                      <RoleSelect value={value} onChange={onChange} />
+                    )}
+                  />
                 </FormField>
                 <FormField
                   title="admin_console.user_details.field_connectors"
diff --git a/packages/core/src/routes/admin-user.test.ts b/packages/core/src/routes/admin-user.test.ts
index d50b9e028..39037d4b1 100644
--- a/packages/core/src/routes/admin-user.test.ts
+++ b/packages/core/src/routes/admin-user.test.ts
@@ -223,6 +223,21 @@ describe('adminUserRoutes', () => {
     expect(updateUserById).not.toBeCalled();
   });
 
+  it('PATCH /users/:userId should throw if role names are invalid', async () => {
+    const mockedFindRolesByRoleNames = findRolesByRoleNames as jest.Mock;
+    mockedFindRolesByRoleNames.mockImplementationOnce(
+      async (): Promise<Role[]> => [
+        { name: 'worker', description: 'none' },
+        { name: 'cleaner', description: 'none' },
+      ]
+    );
+    await expect(
+      userRequest.patch('/users/foo').send({ roleNames: ['admin'] })
+    ).resolves.toHaveProperty('status', 500);
+    expect(findUserById).toHaveBeenCalledTimes(1);
+    expect(updateUserById).not.toHaveBeenCalled();
+  });
+
   it('PATCH /users/:userId/password', async () => {
     const mockedUserId = 'foo';
     const password = '123456';
@@ -273,43 +288,6 @@ describe('adminUserRoutes', () => {
     expect(deleteUserById).not.toHaveBeenCalled();
   });
 
-  it('PATCH /users/:userId/roleNames', async () => {
-    const response = await userRequest.patch('/users/foo/roleNames').send({ roleNames: ['admin'] });
-    expect(findUserById).toHaveBeenCalledTimes(1);
-    expect(updateUserById).toHaveBeenCalledTimes(1);
-    expect(response.status).toEqual(200);
-  });
-
-  it('PATCH /users/:userId/roleNames should throw if user not found', async () => {
-    const notExistedUserId = 'notExisitedUserId';
-    const mockedFindUserById = findUserById as jest.Mock;
-    mockedFindUserById.mockImplementationOnce((userId) => {
-      if (userId === notExistedUserId) {
-        throw new Error(' ');
-      }
-    });
-    await expect(
-      userRequest.patch(`/users/${notExistedUserId}/roleNames`).send({ roleNames: ['admin'] })
-    ).resolves.toHaveProperty('status', 500);
-    expect(findRolesByRoleNames).not.toHaveBeenCalled();
-    expect(updateUserById).not.toHaveBeenCalled();
-  });
-
-  it('PATCH /users/:userId/roleNames should throw if role names are invalid', async () => {
-    const mockedFindRolesByRoleNames = findRolesByRoleNames as jest.Mock;
-    mockedFindRolesByRoleNames.mockImplementationOnce(
-      async (): Promise<Role[]> => [
-        { name: 'worker', description: 'none' },
-        { name: 'cleaner', description: 'none' },
-      ]
-    );
-    await expect(
-      userRequest.patch('/users/foo/roleNames').send({ roleNames: ['admin'] })
-    ).resolves.toHaveProperty('status', 500);
-    expect(findUserById).toHaveBeenCalledTimes(1);
-    expect(updateUserById).not.toHaveBeenCalled();
-  });
-
   it('PATCH /users/:userId/custom-data', async () => {
     const customData = { level: 1 };
     const response = await userRequest.patch('/users/foo/custom-data').send({ customData });
diff --git a/packages/core/src/routes/admin-user.ts b/packages/core/src/routes/admin-user.ts
index a9a0adbe1..cf62d2df4 100644
--- a/packages/core/src/routes/admin-user.ts
+++ b/packages/core/src/routes/admin-user.ts
@@ -112,8 +112,9 @@ export default function adminUserRoutes<T extends AuthedRouter>(router: T) {
       params: object({ userId: string() }),
       body: object({
         name: string().regex(nameRegEx).optional(),
-        avatar: string().url().optional(),
+        avatar: string().url().nullable().optional(),
         customData: arbitraryObjectGuard.optional(),
+        roleNames: string().array().optional(),
       }),
     }),
     async (ctx, next) => {
@@ -130,6 +131,20 @@ export default function adminUserRoutes<T extends AuthedRouter>(router: T) {
         await clearUserCustomDataById(userId);
       }
 
+      // Temp solution to validate the existence of input roleNames
+      if (body.roleNames) {
+        const { roleNames } = body;
+        const roles = await findRolesByRoleNames(roleNames);
+
+        if (roles.length !== roleNames.length) {
+          const resourcesNotFound = roleNames.filter(
+            (roleName) => !roles.some(({ name }) => roleName === name)
+          );
+          // TODO: Should be cached by the error handler and return request error
+          throw new InvalidInputError(`role names (${resourcesNotFound.join(',')}) are not valid`);
+        }
+      }
+
       const user = await updateUserById(userId, {
         ...body,
       });
@@ -189,40 +204,6 @@ export default function adminUserRoutes<T extends AuthedRouter>(router: T) {
     }
   );
 
-  router.patch(
-    '/users/:userId/roleNames',
-    koaGuard({
-      params: object({ userId: string() }),
-      body: object({ roleNames: string().array() }),
-    }),
-    async (ctx, next) => {
-      const {
-        params: { userId },
-        body: { roleNames },
-      } = ctx.guard;
-
-      await findUserById(userId);
-
-      // Temp solution to validate the existence of input roleNames
-      if (roleNames.length > 0) {
-        const roles = await findRolesByRoleNames(roleNames);
-
-        if (roles.length !== roleNames.length) {
-          const resourcesNotFound = roleNames.filter(
-            (roleName) => !roles.some(({ name }) => roleName === name)
-          );
-          // TODO: Should be cached by the error handler and return request error
-          throw new InvalidInputError(`role names (${resourcesNotFound.join(',')}) are not valid`);
-        }
-      }
-
-      const user = await updateUserById(userId, { roleNames });
-      ctx.body = pick(user, ...userInfoSelectFields);
-
-      return next();
-    }
-  );
-
   router.patch(
     '/users/:userId/custom-data',
     koaGuard({
diff --git a/packages/phrases/src/locales/en.ts b/packages/phrases/src/locales/en.ts
index 17907820f..8ab54f25e 100644
--- a/packages/phrases/src/locales/en.ts
+++ b/packages/phrases/src/locales/en.ts
@@ -301,6 +301,10 @@ const translation = {
         remove: 'remove',
         not_connected: 'The user is not connected to any social connector.',
       },
+      roles: {
+        default: 'Default',
+        admin: 'Admin',
+      },
     },
     contact: {
       title: 'Contact us',
diff --git a/packages/phrases/src/locales/zh-cn.ts b/packages/phrases/src/locales/zh-cn.ts
index 3c998b137..880d4ac25 100644
--- a/packages/phrases/src/locales/zh-cn.ts
+++ b/packages/phrases/src/locales/zh-cn.ts
@@ -297,6 +297,10 @@ const translation = {
         remove: '删除',
         not_connected: '该用户还没有绑定社交账号。',
       },
+      roles: {
+        default: '默认',
+        admin: '管理员',
+      },
     },
     contact: {
       title: 'Contact us',