fix: Display member role when user has no role (#1965)

coder · Jun 2, 2022 · 3fd4dcd · 3fd4dcd
1 parent dcf03d8
commit 3fd4dcd
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 54 deletions.
diff --git a/site/src/components/RoleSelect/RoleSelect.tsx b/site/src/components/RoleSelect/RoleSelect.tsx
@@ -41,7 +41,7 @@ export const RoleSelect: FC<RoleSelectProps> = ({ roles, selectedRoles, loading,
 
         return (
           <MenuItem key={r.name} value={r.name} disabled={loading}>
-            <Checkbox color="primary" checked={isChecked} /> {r.display_name}
+            <Checkbox size="small" color="primary" checked={isChecked} /> {r.display_name}
           </MenuItem>
         )
       })}

diff --git a/site/src/components/UserDropdown/UserDropdown.test.tsx b/site/src/components/UserDropdown/UserDropdown.test.tsx
@@ -1,5 +1,5 @@
 import { screen } from "@testing-library/react"
-import { MockAdminRole, MockMemberRole, MockUser } from "../../testHelpers/entities"
+import { MockAdminRole, MockUser } from "../../testHelpers/entities"
 import { render } from "../../testHelpers/renderHelpers"
 import { Language, UserDropdown, UserDropdownProps } from "./UsersDropdown"
 
@@ -36,7 +36,6 @@ describe("UserDropdown", () => {
       await renderAndClick()
 
       expect(screen.getByText(MockAdminRole.display_name)).toBeDefined()
-      expect(screen.getByText(MockMemberRole.display_name)).toBeDefined()
     })
 
     it("has the correct link for the documentation item", async () => {

diff --git a/site/src/components/UsersTable/UsersTable.tsx b/site/src/components/UsersTable/UsersTable.tsx
@@ -1,11 +1,13 @@
 import Box from "@material-ui/core/Box"
+import { makeStyles } from "@material-ui/core/styles"
 import Table from "@material-ui/core/Table"
 import TableBody from "@material-ui/core/TableBody"
 import TableCell from "@material-ui/core/TableCell"
 import TableHead from "@material-ui/core/TableHead"
 import TableRow from "@material-ui/core/TableRow"
 import { FC } from "react"
 import * as TypesGen from "../../api/typesGenerated"
+import { combineClasses } from "../../util/combineClasses"
 import { AvatarData } from "../AvatarData/AvatarData"
 import { EmptyState } from "../EmptyState/EmptyState"
 import { RoleSelect } from "../RoleSelect/RoleSelect"
@@ -45,6 +47,8 @@ export const UsersTable: FC<UsersTableProps> = ({
   canEditUsers,
   isLoading,
 }) => {
+  const styles = useStyles()
+
   return (
     <Table>
       <TableHead>
@@ -60,55 +64,75 @@ export const UsersTable: FC<UsersTableProps> = ({
         {isLoading && <TableLoader />}
         {!isLoading &&
           users &&
-          users.map((u) => (
-            <TableRow key={u.id}>
-              <TableCell>
-                <AvatarData title={u.username} subtitle={u.email} />
-              </TableCell>
-              <TableCell>{u.status}</TableCell>
-              <TableCell>
-                {canEditUsers ? (
-                  <RoleSelect
-                    roles={roles ?? []}
-                    selectedRoles={u.roles}
-                    loading={isUpdatingUserRoles}
-                    onChange={(roles) => onUpdateUserRoles(u, roles)}
-                  />
-                ) : (
-                  <>{u.roles.map((r) => r.display_name).join(", ")}</>
-                )}
-              </TableCell>
-              {canEditUsers && (
+          users.map((user) => {
+            // When the user has no role we want to show they are a Member
+            const fallbackRole: TypesGen.Role = {
+              name: "member",
+              display_name: "Member",
+            }
+            const userRoles = user.roles.length === 0 ? [fallbackRole] : user.roles
+
+            return (
+              <TableRow key={user.id}>
+                <TableCell>
+                  <AvatarData title={user.username} subtitle={user.email} />
+                </TableCell>
+                <TableCell
+                  className={combineClasses([
+                    styles.status,
+                    user.status === "suspended" ? styles.suspended : undefined,
+                  ])}
+                >
+                  {user.status}
+                </TableCell>
                 <TableCell>
-                  <TableRowMenu
-                    data={u}
-                    menuItems={
-                      // Return either suspend or activate depending on status
-                      (u.status === "active"
-                        ? [
-                            {
-                              label: Language.suspendMenuItem,
-                              onClick: onSuspendUser,
-                            },
-                          ]
-                        : [
-                            // TODO: Uncomment this and add activate user functionality.
-                            // {
-                            //   label: Language.activateMenuItem,
-                            //   // eslint-disable-next-line @typescript-eslint/no-empty-function
-                            //   onClick: function () {},
-                            // },
-                          ]
-                      ).concat({
-                        label: Language.resetPasswordMenuItem,
-                        onClick: onResetUserPassword,
-                      })
-                    }
-                  />
+                  {canEditUsers ? (
+                    <RoleSelect
+                      roles={roles ?? []}
+                      selectedRoles={userRoles}
+                      loading={isUpdatingUserRoles}
+                      onChange={(roles) => {
+                        // Remove the fallback role because it is only for the UI
+                        roles = roles.filter((role) => role !== fallbackRole.name)
+                        onUpdateUserRoles(user, roles)
+                      }}
+                    />
+                  ) : (
+                    <>{userRoles.map((role) => role.display_name).join(", ")}</>
+                  )}
                 </TableCell>
-              )}
-            </TableRow>
-          ))}
+                {canEditUsers && (
+                  <TableCell>
+                    <TableRowMenu
+                      data={user}
+                      menuItems={
+                        // Return either suspend or activate depending on status
+                        (user.status === "active"
+                          ? [
+                              {
+                                label: Language.suspendMenuItem,
+                                onClick: onSuspendUser,
+                              },
+                            ]
+                          : [
+                              // TODO: Uncomment this and add activate user functionality.
+                              // {
+                              //   label: Language.activateMenuItem,
+                              //   // eslint-disable-next-line @typescript-eslint/no-empty-function
+                              //   onClick: function () {},
+                              // },
+                            ]
+                        ).concat({
+                          label: Language.resetPasswordMenuItem,
+                          onClick: onResetUserPassword,
+                        })
+                      }
+                    />
+                  </TableCell>
+                )}
+              </TableRow>
+            )
+          })}
 
         {users && users.length === 0 && (
           <TableRow>
@@ -123,3 +147,12 @@ export const UsersTable: FC<UsersTableProps> = ({
     </Table>
   )
 }
+
+const useStyles = makeStyles((theme) => ({
+  status: {
+    textTransform: "capitalize",
+  },
+  suspended: {
+    color: theme.palette.text.secondary,
+  },
+}))
diff --git a/site/src/pages/UsersPage/UsersPage.test.tsx b/site/src/pages/UsersPage/UsersPage.test.tsx
@@ -242,7 +242,7 @@ describe("Users Page", () => {
         }, MockAuditorRole)
 
         // Check if the select text was updated with the Auditor role
-        await waitFor(() => expect(rolesMenuTrigger).toHaveTextContent("Admin, Member, Auditor"))
+        await waitFor(() => expect(rolesMenuTrigger).toHaveTextContent("Admin, Auditor"))
 
         // Check if the API was called correctly
         const currentRoles = MockUser.roles.map((r) => r.name)

diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts
@@ -29,7 +29,7 @@ export const MockAuditorRole: TypesGen.Role = {
   display_name: "Auditor",
 }
 
-export const MockSiteRoles = [MockAdminRole, MockAuditorRole, MockMemberRole]
+export const MockSiteRoles = [MockAdminRole, MockAuditorRole]
 
 export const MockUser: TypesGen.User = {
   id: "test-user",
@@ -38,7 +38,7 @@ export const MockUser: TypesGen.User = {
   created_at: "",
   status: "active",
   organization_ids: ["fc0774ce-cc9e-48d4-80ae-88f7a4d4a8b0"],
-  roles: [MockAdminRole, MockMemberRole],
+  roles: [MockAdminRole],
 }
 
 export const MockUser2: TypesGen.User = {
@@ -48,7 +48,7 @@ export const MockUser2: TypesGen.User = {
   created_at: "",
   status: "active",
   organization_ids: ["fc0774ce-cc9e-48d4-80ae-88f7a4d4a8b0"],
-  roles: [MockMemberRole],
+  roles: [],
 }
 
 export const MockOrganization: TypesGen.Organization = {