From a25dd55fe27524f7ab646d8fbc3f8b0b8e1627fc Mon Sep 17 00:00:00 2001 From: Bruno Date: Wed, 1 Jun 2022 19:16:30 +0000 Subject: [PATCH 1/4] refactor: Display member role when user has no role --- site/src/components/RoleSelect/RoleSelect.tsx | 2 +- site/src/components/UsersTable/UsersTable.tsx | 125 +++++++++++------- 2 files changed, 79 insertions(+), 48 deletions(-) diff --git a/site/src/components/RoleSelect/RoleSelect.tsx b/site/src/components/RoleSelect/RoleSelect.tsx index 63b19d23d41cd..2c96fd4375bc9 100644 --- a/site/src/components/RoleSelect/RoleSelect.tsx +++ b/site/src/components/RoleSelect/RoleSelect.tsx @@ -41,7 +41,7 @@ export const RoleSelect: FC = ({ roles, selectedRoles, loading, return ( - {r.display_name} + {r.display_name} ) })} diff --git a/site/src/components/UsersTable/UsersTable.tsx b/site/src/components/UsersTable/UsersTable.tsx index d0e507eda690f..59f5132928fee 100644 --- a/site/src/components/UsersTable/UsersTable.tsx +++ b/site/src/components/UsersTable/UsersTable.tsx @@ -1,4 +1,5 @@ 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" @@ -6,6 +7,7 @@ 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 = ({ canEditUsers, isLoading, }) => { + const styles = useStyles() + return ( @@ -60,55 +64,73 @@ export const UsersTable: FC = ({ {isLoading && } {!isLoading && users && - users.map((u) => ( - - - - - {u.status} - - {canEditUsers ? ( - onUpdateUserRoles(u, roles)} - /> - ) : ( - <>{u.roles.map((r) => r.display_name).join(", ")} - )} - - {canEditUsers && ( + users.map((user) => { + // When the user has no role, it is because they are a member + const fallbackRoles: TypesGen.Role[] = [ + { + name: "member", + display_name: "Member", + }, + ] + const userRoles = user.roles.length === 0 ? fallbackRoles : user.roles + + return ( + + + + + + {user.status} + - + {canEditUsers ? ( + onUpdateUserRoles(user, roles)} + /> + ) : ( + <>{userRoles.map((role) => role.display_name).join(", ")} + )} - )} - - ))} + {canEditUsers && ( + + + + )} + + ) + })} {users && users.length === 0 && ( @@ -123,3 +145,12 @@ export const UsersTable: FC = ({
) } + +const useStyles = makeStyles((theme) => ({ + status: { + textTransform: "capitalize", + }, + suspended: { + color: theme.palette.text.secondary, + }, +})) From fcc7b91a824e905ff48ba309573bfe1a89f3c434 Mon Sep 17 00:00:00 2001 From: Bruno Date: Wed, 1 Jun 2022 19:46:53 +0000 Subject: [PATCH 2/4] Remove fallback role from the change event --- site/src/components/UsersTable/UsersTable.tsx | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/site/src/components/UsersTable/UsersTable.tsx b/site/src/components/UsersTable/UsersTable.tsx index 59f5132928fee..bd746c65c0265 100644 --- a/site/src/components/UsersTable/UsersTable.tsx +++ b/site/src/components/UsersTable/UsersTable.tsx @@ -65,14 +65,12 @@ export const UsersTable: FC = ({ {!isLoading && users && users.map((user) => { - // When the user has no role, it is because they are a member - const fallbackRoles: TypesGen.Role[] = [ - { - name: "member", - display_name: "Member", - }, - ] - const userRoles = user.roles.length === 0 ? fallbackRoles : user.roles + // 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 ( @@ -93,7 +91,11 @@ export const UsersTable: FC = ({ roles={roles ?? []} selectedRoles={userRoles} loading={isUpdatingUserRoles} - onChange={(roles) => onUpdateUserRoles(user, roles)} + 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(", ")} From 8c116dfeeb9ac91ef70276370d2d7dd2fee63769 Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 2 Jun 2022 12:58:19 +0000 Subject: [PATCH 3/4] Fix tests --- site/src/pages/UsersPage/UsersPage.test.tsx | 2 +- site/src/testHelpers/entities.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/site/src/pages/UsersPage/UsersPage.test.tsx b/site/src/pages/UsersPage/UsersPage.test.tsx index 086990a920d2e..85ca87b7aced2 100644 --- 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 index 0c17805ba9bb7..749ef631ac0bc 100644 --- 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 = { From 884f3541f9bfecfbf3b2c0977c1f45b2500d5565 Mon Sep 17 00:00:00 2001 From: Bruno Date: Thu, 2 Jun 2022 13:04:43 +0000 Subject: [PATCH 4/4] Fix test --- site/src/components/UserDropdown/UserDropdown.test.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/site/src/components/UserDropdown/UserDropdown.test.tsx b/site/src/components/UserDropdown/UserDropdown.test.tsx index 400c006af4982..6ef615182cff8 100644 --- 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 () => {