Skip to content

feat: Add filter on Users page #2653

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 28, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Handle filter form errors
  • Loading branch information
AbhineetJain committed Jun 28, 2022
commit be7eaacad2a228fea6af9b731c9af6327342c874
56 changes: 55 additions & 1 deletion site/src/api/errors.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isApiError, mapApiErrorToFieldErrors } from "./errors"
import { getValidationErrorMessage, isApiError, mapApiErrorToFieldErrors } from "./errors"

describe("isApiError", () => {
it("returns true when the object is an API Error", () => {
Expand Down Expand Up @@ -36,3 +36,57 @@ describe("mapApiErrorToFieldErrors", () => {
})
})
})

describe("getValidationErrorMessage", () => {
it("returns multiple validation messages", () => {
expect(
getValidationErrorMessage({
response: {
data: {
message: "Invalid user search query.",
validations: [
{
field: "status",
detail: `Query param "status" has invalid value: "inactive" is not a valid user status`,
},
{
field: "q",
detail: `Query element "role:a:e" can only contain 1 ':'`,
},
],
},
},
isAxiosError: true,
}),
).toEqual(
`Query param "status" has invalid value: "inactive" is not a valid user status\nQuery element "role:a:e" can only contain 1 ':'`,
)
})

it("non-API error returns empty validation message", () => {
expect(
getValidationErrorMessage({
response: {
data: {
error: "Invalid user search query.",
},
},
isAxiosError: true,
}),
).toEqual("")
})

it("no validations field returns empty validation message", () => {
expect(
getValidationErrorMessage({
response: {
data: {
message: "Invalid user search query.",
detail: `Query element "role:a:e" can only contain 1 ':'`,
},
},
isAxiosError: true,
}),
).toEqual("")
})
})
6 changes: 6 additions & 0 deletions site/src/api/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,9 @@ export const getErrorMessage = (
: error instanceof Error
? error.message
: defaultMessage

export const getValidationErrorMessage = (error: Error | ApiError | unknown): string => {
const validationErrors =
isApiError(error) && error.response.data.validations ? error.response.data.validations : []
return validationErrors.map((error) => error.detail).join("\n")
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,21 @@ WithPresetFilters.args = {
{ query: "random query", name: "Random query" },
],
}

export const WithError = Template.bind({})
WithError.args = {
presetFilters: [
{ query: workspaceFilterQuery.me, name: "Your workspaces" },
{ query: "random query", name: "Random query" },
],
error: {
response: {
data: {
validations: {
field: "status",
detail: `Query param "status" has invalid value: "inactive" is not a valid user status`,
},
},
},
},
}
121 changes: 67 additions & 54 deletions site/src/components/SearchBarWithFilter/SearchBarWithFilter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import TextField from "@material-ui/core/TextField"
import SearchIcon from "@material-ui/icons/Search"
import { FormikErrors, useFormik } from "formik"
import { useState } from "react"
import { getValidationErrorMessage } from "../../api/errors"
import { getFormHelpers, onChangeTrimmed } from "../../util/formUtils"
import { CloseDropdown, OpenDropdown } from "../DropdownArrows/DropdownArrows"
import { Stack } from "../Stack/Stack"
Expand All @@ -20,6 +21,7 @@ export interface SearchBarWithFilterProps {
filter?: string
onFilter: (query: string) => void
presetFilters?: PresetFilter[]
error?: unknown
}

export interface PresetFilter {
Expand All @@ -37,6 +39,7 @@ export const SearchBarWithFilter: React.FC<SearchBarWithFilterProps> = ({
filter,
onFilter,
presetFilters,
error,
}) => {
const styles = useStyles()

Expand Down Expand Up @@ -68,69 +71,76 @@ export const SearchBarWithFilter: React.FC<SearchBarWithFilterProps> = ({
handleClose()
}

const errorMessage = getValidationErrorMessage(error)

return (
<Stack direction="row" spacing={0} className={styles.filterContainer}>
{presetFilters && presetFilters.length > 0 && (
<Button
aria-controls="filter-menu"
aria-haspopup="true"
onClick={handleClick}
className={styles.buttonRoot}
>
{Language.filterName} {anchorEl ? <CloseDropdown /> : <OpenDropdown />}
</Button>
)}

<form onSubmit={form.handleSubmit} className={styles.filterForm}>
<TextField
{...getFieldHelpers("query")}
className={styles.textFieldRoot}
onChange={onChangeTrimmed(form)}
fullWidth
variant="outlined"
InputProps={{
startAdornment: (
<InputAdornment position="start">
<SearchIcon fontSize="small" />
</InputAdornment>
),
}}
/>
</form>

{presetFilters && presetFilters.length > 0 && (
<Menu
id="filter-menu"
anchorEl={anchorEl}
keepMounted
open={Boolean(anchorEl)}
onClose={handleClose}
TransitionComponent={Fade}
anchorOrigin={{
vertical: "bottom",
horizontal: "left",
}}
transformOrigin={{
vertical: "top",
horizontal: "left",
}}
>
{presetFilters.map((presetFilter) => (
<MenuItem key={presetFilter.name} onClick={setPresetFilter(presetFilter.query)}>
{presetFilter.name}
</MenuItem>
))}
</Menu>
)}
<Stack spacing={1} className={styles.root}>
<Stack direction="row" spacing={0} className={styles.filterContainer}>
{presetFilters && presetFilters.length > 0 && (
<Button
aria-controls="filter-menu"
aria-haspopup="true"
onClick={handleClick}
className={styles.buttonRoot}
>
{Language.filterName} {anchorEl ? <CloseDropdown /> : <OpenDropdown />}
</Button>
)}

<form onSubmit={form.handleSubmit} className={styles.filterForm}>
<TextField
{...getFieldHelpers("query")}
className={styles.textFieldRoot}
onChange={onChangeTrimmed(form)}
fullWidth
variant="outlined"
InputProps={{
startAdornment: (
<InputAdornment position="start">
<SearchIcon fontSize="small" />
</InputAdornment>
),
}}
/>
</form>

{presetFilters && presetFilters.length > 0 && (
<Menu
id="filter-menu"
anchorEl={anchorEl}
keepMounted
open={Boolean(anchorEl)}
onClose={handleClose}
TransitionComponent={Fade}
anchorOrigin={{
vertical: "bottom",
horizontal: "left",
}}
transformOrigin={{
vertical: "top",
horizontal: "left",
}}
>
{presetFilters.map((presetFilter) => (
<MenuItem key={presetFilter.name} onClick={setPresetFilter(presetFilter.query)}>
{presetFilter.name}
</MenuItem>
))}
</Menu>
)}
</Stack>
{errorMessage && <Stack className={styles.errorRoot}>{errorMessage}</Stack>}
</Stack>
)
}

const useStyles = makeStyles((theme) => ({
root: {
marginBottom: theme.spacing(2),
},
filterContainer: {
border: `1px solid ${theme.palette.divider}`,
borderRadius: theme.shape.borderRadius,
marginBottom: theme.spacing(2),
},
filterForm: {
width: "100%",
Expand All @@ -146,4 +156,7 @@ const useStyles = makeStyles((theme) => ({
border: "none",
},
},
errorRoot: {
color: theme.palette.error.dark,
},
}))
16 changes: 16 additions & 0 deletions site/src/components/UsersTable/UsersTable.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,19 @@ Empty.args = {
users: [],
roles: MockSiteRoles,
}

export const Error = Template.bind({})
Error.args = {
users: [MockUser, MockUser2],
roles: MockSiteRoles,
canEditUsers: true,
error: {
message: "Invalid user search query.",
validations: [
{
field: "status",
detail: `Query param "status" has invalid value: "inactive" is not a valid user status`,
},
],
},
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this story since it tests a particular behavior of the component, but I am not sure if it is a good UI snapshot since the loader is spinning and the snapshot may change across different builds. 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good thought. I haven't read through this in detail, but could we pause the animation for the loader? Or introduce a slight delay? https://www.chromatic.com/docs/animations

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing to this, I used

Loading.parameters = {
  chromatic: { pauseAnimationAtEnd: true },
}

and it seems to be constant for at least 3 builds.

26 changes: 16 additions & 10 deletions site/src/components/UsersTable/UsersTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export interface UsersTableProps {
onActivateUser: (user: TypesGen.User) => void
onResetUserPassword: (user: TypesGen.User) => void
onUpdateUserRoles: (user: TypesGen.User, roles: TypesGen.Role["name"][]) => void
error?: unknown
}

export const UsersTable: FC<UsersTableProps> = ({
Expand All @@ -48,6 +49,7 @@ export const UsersTable: FC<UsersTableProps> = ({
isUpdatingUserRoles,
canEditUsers,
isLoading,
error,
}) => {
const styles = useStyles()

Expand All @@ -63,8 +65,9 @@ export const UsersTable: FC<UsersTableProps> = ({
</TableRow>
</TableHead>
<TableBody>
{isLoading && <TableLoader />}
{isLoading && !error && <TableLoader />}
{!isLoading &&
!error &&
users &&
users.map((user) => {
// When the user has no role we want to show they are a Member
Expand Down Expand Up @@ -134,15 +137,18 @@ export const UsersTable: FC<UsersTableProps> = ({
)
})}

{users && users.length === 0 && (
<TableRow>
<TableCell colSpan={999}>
<Box p={4}>
<EmptyState message={Language.emptyMessage} />
</Box>
</TableCell>
</TableRow>
)}
{
// Default behavior for error state and empty list
(error || (users && users.length === 0)) && (
<TableRow>
<TableCell colSpan={999}>
<Box p={4}>
<EmptyState message={Language.emptyMessage} />
</Box>
</TableCell>
</TableRow>
)
}
</TableBody>
</Table>
)
Expand Down
35 changes: 18 additions & 17 deletions site/src/pages/UsersPage/UsersPageView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import Button from "@material-ui/core/Button"
import AddCircleOutline from "@material-ui/icons/AddCircleOutline"
import { FC } from "react"
import * as TypesGen from "../../api/typesGenerated"
import { ErrorSummary } from "../../components/ErrorSummary/ErrorSummary"
import { Margins } from "../../components/Margins/Margins"
import { PageHeader, PageHeaderTitle } from "../../components/PageHeader/PageHeader"
import { SearchBarWithFilter } from "../../components/SearchBarWithFilter/SearchBarWithFilter"
Expand Down Expand Up @@ -68,23 +67,25 @@ export const UsersPageView: FC<UsersPageViewProps> = ({
<PageHeaderTitle>Users</PageHeaderTitle>
</PageHeader>

<SearchBarWithFilter filter={filter} onFilter={onFilter} presetFilters={presetFilters} />
<SearchBarWithFilter
filter={filter}
onFilter={onFilter}
presetFilters={presetFilters}
error={error}
/>

{error ? (
<ErrorSummary error={error} />
) : (
<UsersTable
users={users}
roles={roles}
onSuspendUser={onSuspendUser}
onActivateUser={onActivateUser}
onResetUserPassword={onResetUserPassword}
onUpdateUserRoles={onUpdateUserRoles}
isUpdatingUserRoles={isUpdatingUserRoles}
canEditUsers={canEditUsers}
isLoading={isLoading}
/>
)}
<UsersTable
users={users}
roles={roles}
onSuspendUser={onSuspendUser}
onActivateUser={onActivateUser}
onResetUserPassword={onResetUserPassword}
onUpdateUserRoles={onUpdateUserRoles}
isUpdatingUserRoles={isUpdatingUserRoles}
canEditUsers={canEditUsers}
isLoading={isLoading}
error={error}
/>
</Margins>
)
}
Loading