Skip to content

Commit deae247

Browse files
BrunoQuaresmakylecarbs
BrunoQuaresma
authored and
kylecarbs
committed
fix: Display member role when user has no role (#1965)
1 parent b0b7a8d commit deae247

File tree

5 files changed

+86
-54
lines changed

5 files changed

+86
-54
lines changed

‎site/src/components/RoleSelect/RoleSelect.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export const RoleSelect: FC<RoleSelectProps> = ({ roles, selectedRoles, loading,
4141

4242
return (
4343
<MenuItem key={r.name} value={r.name} disabled={loading}>
44-
<Checkbox color="primary" checked={isChecked} /> {r.display_name}
44+
<Checkbox size="small" color="primary" checked={isChecked} /> {r.display_name}
4545
</MenuItem>
4646
)
4747
})}

‎site/src/components/UserDropdown/UserDropdown.test.tsx

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { screen } from "@testing-library/react"
2-
import { MockAdminRole, MockMemberRole, MockUser } from "../../testHelpers/entities"
2+
import { MockAdminRole, MockUser } from "../../testHelpers/entities"
33
import { render } from "../../testHelpers/renderHelpers"
44
import { Language, UserDropdown, UserDropdownProps } from "./UsersDropdown"
55

@@ -36,7 +36,6 @@ describe("UserDropdown", () => {
3636
await renderAndClick()
3737

3838
expect(screen.getByText(MockAdminRole.display_name)).toBeDefined()
39-
expect(screen.getByText(MockMemberRole.display_name)).toBeDefined()
4039
})
4140

4241
it("has the correct link for the documentation item", async () => {

‎site/src/components/UsersTable/UsersTable.tsx

+80-47
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import Box from "@material-ui/core/Box"
2+
import { makeStyles } from "@material-ui/core/styles"
23
import Table from "@material-ui/core/Table"
34
import TableBody from "@material-ui/core/TableBody"
45
import TableCell from "@material-ui/core/TableCell"
56
import TableHead from "@material-ui/core/TableHead"
67
import TableRow from "@material-ui/core/TableRow"
78
import { FC } from "react"
89
import * as TypesGen from "../../api/typesGenerated"
10+
import { combineClasses } from "../../util/combineClasses"
911
import { AvatarData } from "../AvatarData/AvatarData"
1012
import { EmptyState } from "../EmptyState/EmptyState"
1113
import { RoleSelect } from "../RoleSelect/RoleSelect"
@@ -45,6 +47,8 @@ export const UsersTable: FC<UsersTableProps> = ({
4547
canEditUsers,
4648
isLoading,
4749
}) => {
50+
const styles = useStyles()
51+
4852
return (
4953
<Table>
5054
<TableHead>
@@ -60,55 +64,75 @@ export const UsersTable: FC<UsersTableProps> = ({
6064
{isLoading && <TableLoader />}
6165
{!isLoading &&
6266
users &&
63-
users.map((u) => (
64-
<TableRow key={u.id}>
65-
<TableCell>
66-
<AvatarData title={u.username} subtitle={u.email} />
67-
</TableCell>
68-
<TableCell>{u.status}</TableCell>
69-
<TableCell>
70-
{canEditUsers ? (
71-
<RoleSelect
72-
roles={roles ?? []}
73-
selectedRoles={u.roles}
74-
loading={isUpdatingUserRoles}
75-
onChange={(roles) => onUpdateUserRoles(u, roles)}
76-
/>
77-
) : (
78-
<>{u.roles.map((r) => r.display_name).join(", ")}</>
79-
)}
80-
</TableCell>
81-
{canEditUsers && (
67+
users.map((user) => {
68+
// When the user has no role we want to show they are a Member
69+
const fallbackRole: TypesGen.Role = {
70+
name: "member",
71+
display_name: "Member",
72+
}
73+
const userRoles = user.roles.length === 0 ? [fallbackRole] : user.roles
74+
75+
return (
76+
<TableRow key={user.id}>
77+
<TableCell>
78+
<AvatarData title={user.username} subtitle={user.email} />
79+
</TableCell>
80+
<TableCell
81+
className={combineClasses([
82+
styles.status,
83+
user.status === "suspended" ? styles.suspended : undefined,
84+
])}
85+
>
86+
{user.status}
87+
</TableCell>
8288
<TableCell>
83-
<TableRowMenu
84-
data={u}
85-
menuItems={
86-
// Return either suspend or activate depending on status
87-
(u.status === "active"
88-
? [
89-
{
90-
label: Language.suspendMenuItem,
91-
onClick: onSuspendUser,
92-
},
93-
]
94-
: [
95-
// TODO: Uncomment this and add activate user functionality.
96-
// {
97-
// label: Language.activateMenuItem,
98-
// // eslint-disable-next-line @typescript-eslint/no-empty-function
99-
// onClick: function () {},
100-
// },
101-
]
102-
).concat({
103-
label: Language.resetPasswordMenuItem,
104-
onClick: onResetUserPassword,
105-
})
106-
}
107-
/>
89+
{canEditUsers ? (
90+
<RoleSelect
91+
roles={roles ?? []}
92+
selectedRoles={userRoles}
93+
loading={isUpdatingUserRoles}
94+
onChange={(roles) => {
95+
// Remove the fallback role because it is only for the UI
96+
roles = roles.filter((role) => role !== fallbackRole.name)
97+
onUpdateUserRoles(user, roles)
98+
}}
99+
/>
100+
) : (
101+
<>{userRoles.map((role) => role.display_name).join(", ")}</>
102+
)}
108103
</TableCell>
109-
)}
110-
</TableRow>
111-
))}
104+
{canEditUsers && (
105+
<TableCell>
106+
<TableRowMenu
107+
data={user}
108+
menuItems={
109+
// Return either suspend or activate depending on status
110+
(user.status === "active"
111+
? [
112+
{
113+
label: Language.suspendMenuItem,
114+
onClick: onSuspendUser,
115+
},
116+
]
117+
: [
118+
// TODO: Uncomment this and add activate user functionality.
119+
// {
120+
// label: Language.activateMenuItem,
121+
// // eslint-disable-next-line @typescript-eslint/no-empty-function
122+
// onClick: function () {},
123+
// },
124+
]
125+
).concat({
126+
label: Language.resetPasswordMenuItem,
127+
onClick: onResetUserPassword,
128+
})
129+
}
130+
/>
131+
</TableCell>
132+
)}
133+
</TableRow>
134+
)
135+
})}
112136

113137
{users && users.length === 0 && (
114138
<TableRow>
@@ -123,3 +147,12 @@ export const UsersTable: FC<UsersTableProps> = ({
123147
</Table>
124148
)
125149
}
150+
151+
const useStyles = makeStyles((theme) => ({
152+
status: {
153+
textTransform: "capitalize",
154+
},
155+
suspended: {
156+
color: theme.palette.text.secondary,
157+
},
158+
}))

‎site/src/pages/UsersPage/UsersPage.test.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ describe("Users Page", () => {
242242
}, MockAuditorRole)
243243

244244
// Check if the select text was updated with the Auditor role
245-
await waitFor(() => expect(rolesMenuTrigger).toHaveTextContent("Admin, Member, Auditor"))
245+
await waitFor(() => expect(rolesMenuTrigger).toHaveTextContent("Admin, Auditor"))
246246

247247
// Check if the API was called correctly
248248
const currentRoles = MockUser.roles.map((r) => r.name)

‎site/src/testHelpers/entities.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export const MockAuditorRole: TypesGen.Role = {
2929
display_name: "Auditor",
3030
}
3131

32-
export const MockSiteRoles = [MockAdminRole, MockAuditorRole, MockMemberRole]
32+
export const MockSiteRoles = [MockAdminRole, MockAuditorRole]
3333

3434
export const MockUser: TypesGen.User = {
3535
id: "test-user",
@@ -38,7 +38,7 @@ export const MockUser: TypesGen.User = {
3838
created_at: "",
3939
status: "active",
4040
organization_ids: ["fc0774ce-cc9e-48d4-80ae-88f7a4d4a8b0"],
41-
roles: [MockAdminRole, MockMemberRole],
41+
roles: [MockAdminRole],
4242
}
4343

4444
export const MockUser2: TypesGen.User = {
@@ -48,7 +48,7 @@ export const MockUser2: TypesGen.User = {
4848
created_at: "",
4949
status: "active",
5050
organization_ids: ["fc0774ce-cc9e-48d4-80ae-88f7a4d4a8b0"],
51-
roles: [MockMemberRole],
51+
roles: [],
5252
}
5353

5454
export const MockOrganization: TypesGen.Organization = {

0 commit comments

Comments
 (0)