Skip to content

Commit 40c0fc2

Browse files
BrunoQuaresmaBrunoQuaresma
authored
refactor: Remove users redirect to active filter (coder#4056)
1 parent b78ab9e commit 40c0fc2

File tree

10 files changed

+205
-178
lines changed

10 files changed

+205
-178
lines changed

‎site/src/components/NavbarView/NavbarView.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,10 @@ const NavItems: React.FC<
4848
</NavLink>
4949
</ListItem>
5050
<ListItem button className={styles.item}>
51-
<NavLink className={styles.link} to="/users">
51+
<NavLink
52+
className={styles.link}
53+
to={`/users?filter=${encodeURIComponent("status:active")}`}
54+
>
5255
{Language.users}
5356
</NavLink>
5457
</ListItem>

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

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { fireEvent, screen } from "@testing-library/react"
1+
import { screen } from "@testing-library/react"
22
import userEvent from "@testing-library/user-event"
33
import { render } from "../../testHelpers/renderHelpers"
44
import { SearchBarWithFilter } from "./SearchBarWithFilter"
@@ -21,18 +21,6 @@ describe("SearchBarWithFilter", () => {
2121
await userEvent.type(searchInput, "workspace") // 9 characters
2222

2323
// Then
24-
expect(onFilter).toBeCalledTimes(10) // 9 characters + 1 on component mount
25-
})
26-
27-
it("calls the onFilter handler on submit", async () => {
28-
// When
29-
const onFilter = jest.fn()
30-
render(<SearchBarWithFilter onFilter={onFilter} />)
31-
32-
const searchInput = screen.getByRole("textbox")
33-
await fireEvent.keyDown(searchInput, { key: "Enter", code: "Enter", charCode: 13 })
34-
35-
// Then
36-
expect(onFilter).toBeCalledTimes(1)
24+
expect(onFilter).toBeCalledTimes(9) // 9 characters
3725
})
3826
})

‎site/src/components/SearchBarWithFilter/SearchBarWithFilter.tsx

Lines changed: 18 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,8 @@ import OutlinedInput from "@material-ui/core/OutlinedInput"
77
import { makeStyles } from "@material-ui/core/styles"
88
import { Theme } from "@material-ui/core/styles/createMuiTheme"
99
import SearchIcon from "@material-ui/icons/Search"
10-
import { FormikErrors, useFormik } from "formik"
1110
import debounce from "just-debounce-it"
12-
import { useCallback, useEffect, useState } from "react"
11+
import { useCallback, useRef, useState } from "react"
1312
import { getValidationErrorMessage } from "../../api/errors"
1413
import { CloseDropdown, OpenDropdown } from "../DropdownArrows/DropdownArrows"
1514
import { Stack } from "../Stack/Stack"
@@ -30,29 +29,14 @@ export interface PresetFilter {
3029
query: string
3130
}
3231

33-
interface FilterFormValues {
34-
query: string
35-
}
36-
37-
export type FilterFormErrors = FormikErrors<FilterFormValues>
38-
3932
export const SearchBarWithFilter: React.FC<React.PropsWithChildren<SearchBarWithFilterProps>> = ({
4033
filter,
4134
onFilter,
4235
presetFilters,
4336
error,
4437
}) => {
4538
const styles = useStyles({ error: Boolean(error) })
46-
47-
const form = useFormik<FilterFormValues>({
48-
enableReinitialize: true,
49-
initialValues: {
50-
query: filter ?? "",
51-
},
52-
onSubmit: ({ query }) => {
53-
onFilter(query)
54-
},
55-
})
39+
const searchInputRef = useRef<HTMLInputElement>(null)
5640

5741
// debounce query string entry by user
5842
// we want the dependency array empty here
@@ -65,12 +49,6 @@ export const SearchBarWithFilter: React.FC<React.PropsWithChildren<SearchBarWith
6549
[],
6650
)
6751

68-
// update the query params while typing
69-
useEffect(() => {
70-
debouncedOnFilter(form.values.query)
71-
return () => debouncedOnFilter.cancel()
72-
}, [debouncedOnFilter, form.values.query])
73-
7452
const [anchorEl, setAnchorEl] = useState<null | HTMLElement>(null)
7553

7654
const handleClick = (event: React.MouseEvent<HTMLButtonElement>) => {
@@ -82,8 +60,15 @@ export const SearchBarWithFilter: React.FC<React.PropsWithChildren<SearchBarWith
8260
}
8361

8462
const setPresetFilter = (query: string) => () => {
85-
void form.setFieldValue("query", query)
86-
void form.submitForm()
63+
if (!searchInputRef.current) {
64+
throw new Error("Search input not found.")
65+
}
66+
67+
onFilter(query)
68+
// Update this to the input directly instead of create a new state and
69+
// re-render the component since the onFilter is already calling the
70+
// filtering process
71+
searchInputRef.current.value = query
8772
handleClose()
8873
}
8974

@@ -103,21 +88,24 @@ export const SearchBarWithFilter: React.FC<React.PropsWithChildren<SearchBarWith
10388
</Button>
10489
)}
10590

106-
<form onSubmit={form.handleSubmit} className={styles.filterForm}>
91+
<div role="form" className={styles.filterForm}>
10792
<OutlinedInput
10893
id="query"
10994
name="query"
110-
value={form.values.query}
95+
defaultValue={filter}
11196
error={Boolean(error)}
11297
className={styles.inputStyles}
113-
onChange={form.handleChange}
98+
onChange={(event) => {
99+
debouncedOnFilter(event.currentTarget.value)
100+
}}
101+
inputRef={searchInputRef}
114102
startAdornment={
115103
<InputAdornment position="start" className={styles.searchIcon}>
116104
<SearchIcon fontSize="small" />
117105
</InputAdornment>
118106
}
119107
/>
120-
</form>
108+
</div>
121109

122110
{presetFilters && presetFilters.length && (
123111
<Menu

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

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,20 @@ import { rest } from "msw"
44
import * as API from "../../../api/api"
55
import { Language as FormLanguage } from "../../../components/CreateUserForm/CreateUserForm"
66
import { Language as FooterLanguage } from "../../../components/FormFooter/FormFooter"
7-
import { history, render } from "../../../testHelpers/renderHelpers"
7+
import {
8+
history,
9+
renderWithAuth,
10+
waitForLoaderToBeRemoved,
11+
} from "../../../testHelpers/renderHelpers"
812
import { server } from "../../../testHelpers/server"
9-
import { Language as UserLanguage } from "../../../xServices/users/usersXService"
13+
import { Language as CreateUserLanguage } from "../../../xServices/users/createUserXService"
1014
import { CreateUserPage } from "./CreateUserPage"
1115

16+
const renderCreateUserPage = async () => {
17+
renderWithAuth(<CreateUserPage />)
18+
await waitForLoaderToBeRemoved()
19+
}
20+
1221
const fillForm = async ({
1322
username = "someuser",
1423
email = "someone@coder.com",
@@ -34,7 +43,7 @@ describe("Create User Page", () => {
3443
})
3544

3645
it("shows validation error message", async () => {
37-
render(<CreateUserPage />)
46+
await renderCreateUserPage()
3847
await fillForm({ email: "test" })
3948
const errorMessage = await screen.findByText(FormLanguage.emailInvalid)
4049
expect(errorMessage).toBeDefined()
@@ -44,9 +53,9 @@ describe("Create User Page", () => {
4453
jest.spyOn(API, "createUser").mockRejectedValueOnce({
4554
data: "unknown error",
4655
})
47-
render(<CreateUserPage />)
56+
await renderCreateUserPage()
4857
await fillForm({})
49-
const errorMessage = await screen.findByText(UserLanguage.createUserError)
58+
const errorMessage = await screen.findByText(CreateUserLanguage.createUserError)
5059
expect(errorMessage).toBeDefined()
5160
})
5261

@@ -68,30 +77,16 @@ describe("Create User Page", () => {
6877
)
6978
}),
7079
)
71-
render(<CreateUserPage />)
80+
await renderCreateUserPage()
7281
await fillForm({})
7382
const errorMessage = await screen.findByText(fieldErrorMessage)
7483
expect(errorMessage).toBeDefined()
7584
})
7685

7786
it("shows success notification and redirects to users page", async () => {
78-
render(<CreateUserPage />)
87+
await renderCreateUserPage()
7988
await fillForm({})
80-
const successMessage = screen.findByText(UserLanguage.createUserSuccess)
89+
const successMessage = screen.findByText(CreateUserLanguage.createUserSuccess)
8190
expect(successMessage).toBeDefined()
8291
})
83-
84-
it("redirects to users page on cancel", async () => {
85-
render(<CreateUserPage />)
86-
const cancelButton = await screen.findByText(FooterLanguage.cancelLabel)
87-
cancelButton.click()
88-
expect(history.location.pathname).toEqual("/users")
89-
})
90-
91-
it("redirects to users page on close", async () => {
92-
render(<CreateUserPage />)
93-
const closeButton = await screen.findByText("ESC")
94-
closeButton.click()
95-
expect(history.location.pathname).toEqual("/users")
96-
})
9792
})

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

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,29 @@
1-
import { useActor, useSelector } from "@xstate/react"
2-
import React, { useContext } from "react"
1+
import { useMachine } from "@xstate/react"
2+
import { useOrganizationId } from "hooks/useOrganizationId"
3+
import React from "react"
34
import { Helmet } from "react-helmet-async"
45
import { useNavigate } from "react-router"
6+
import { createUserMachine } from "xServices/users/createUserXService"
57
import * as TypesGen from "../../../api/typesGenerated"
68
import { CreateUserForm } from "../../../components/CreateUserForm/CreateUserForm"
79
import { Margins } from "../../../components/Margins/Margins"
810
import { pageTitle } from "../../../util/page"
9-
import { selectOrgId } from "../../../xServices/auth/authSelectors"
10-
import { XServiceContext } from "../../../xServices/StateContext"
1111

1212
export const Language = {
1313
unknownError: "Oops, an unknown error occurred.",
1414
}
1515

1616
export const CreateUserPage: React.FC = () => {
17-
const xServices = useContext(XServiceContext)
18-
const myOrgId = useSelector(xServices.authXService, selectOrgId)
19-
const [usersState, usersSend] = useActor(xServices.usersXService)
20-
const { createUserErrorMessage, createUserFormErrors } = usersState.context
17+
const myOrgId = useOrganizationId()
2118
const navigate = useNavigate()
19+
const [createUserState, createUserSend] = useMachine(createUserMachine, {
20+
actions: {
21+
redirectToUsersPage: () => {
22+
navigate("/users")
23+
},
24+
},
25+
})
26+
const { createUserErrorMessage, createUserFormErrors } = createUserState.context
2227
// There is no field for organization id in Community Edition, so handle its field error like a generic error
2328
const genericError =
2429
createUserErrorMessage ||
@@ -32,14 +37,14 @@ export const CreateUserPage: React.FC = () => {
3237
</Helmet>
3338
<CreateUserForm
3439
formErrors={createUserFormErrors}
35-
onSubmit={(user: TypesGen.CreateUserRequest) => usersSend({ type: "CREATE", user })}
40+
onSubmit={(user: TypesGen.CreateUserRequest) => createUserSend({ type: "CREATE", user })}
3641
onCancel={() => {
37-
usersSend("CANCEL_CREATE_USER")
42+
createUserSend("CANCEL_CREATE_USER")
3843
navigate("/users")
3944
}}
40-
isLoading={usersState.hasTag("loading")}
45+
isLoading={createUserState.hasTag("loading")}
4146
error={genericError}
42-
myOrgId={myOrgId ?? ""}
47+
myOrgId={myOrgId}
4348
/>
4449
</Margins>
4550
)

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

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
import { useActor } from "@xstate/react"
1+
import { useActor, useMachine } from "@xstate/react"
22
import { FC, ReactNode, useContext, useEffect } from "react"
33
import { Helmet } from "react-helmet-async"
44
import { useNavigate } from "react-router"
55
import { useSearchParams } from "react-router-dom"
6+
import { usersMachine } from "xServices/users/usersXService"
67
import { ConfirmDialog } from "../../components/Dialogs/ConfirmDialog/ConfirmDialog"
78
import { ResetPasswordDialog } from "../../components/Dialogs/ResetPasswordDialog/ResetPasswordDialog"
8-
import { userFilterQuery } from "../../util/filters"
99
import { pageTitle } from "../../util/page"
1010
import { XServiceContext } from "../../xServices/StateContext"
1111
import { UsersPageView } from "./UsersPageView"
@@ -24,7 +24,14 @@ export const Language = {
2424

2525
export const UsersPage: FC<{ children?: ReactNode }> = () => {
2626
const xServices = useContext(XServiceContext)
27-
const [usersState, usersSend] = useActor(xServices.usersXService)
27+
const navigate = useNavigate()
28+
const [searchParams, setSearchParams] = useSearchParams()
29+
const filter = searchParams.get("filter") ?? undefined
30+
const [usersState, usersSend] = useMachine(usersMachine, {
31+
context: {
32+
filter,
33+
},
34+
})
2835
const {
2936
users,
3037
getUsersError,
@@ -34,8 +41,7 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => {
3441
userIdToResetPassword,
3542
newUserPassword,
3643
} = usersState.context
37-
const navigate = useNavigate()
38-
const [searchParams, setSearchParams] = useSearchParams()
44+
3945
const userToBeSuspended = users?.find((u) => u.id === userIdToSuspend)
4046
const userToBeDeleted = users?.find((u) => u.id === userIdToDelete)
4147
const userToBeActivated = users?.find((u) => u.id === userIdToActivate)
@@ -60,13 +66,11 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => {
6066

6167
// Fetch users on component mount
6268
useEffect(() => {
63-
const filter = searchParams.get("filter")
64-
const query = filter ?? userFilterQuery.active
6569
usersSend({
6670
type: "GET_USERS",
67-
query,
71+
query: filter,
6872
})
69-
}, [searchParams, usersSend])
73+
}, [filter, usersSend])
7074

7175
// Fetch roles on component mount
7276
useEffect(() => {

‎site/src/testHelpers/renderHelpers.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import ThemeProvider from "@material-ui/styles/ThemeProvider"
2-
import { render as wrappedRender, RenderResult } from "@testing-library/react"
2+
import {
3+
render as wrappedRender,
4+
RenderResult,
5+
screen,
6+
waitForElementToBeRemoved,
7+
} from "@testing-library/react"
38
import { createMemoryHistory } from "history"
49
import { i18n } from "i18n"
510
import { FC, ReactElement } from "react"
@@ -68,4 +73,7 @@ export function renderWithAuth(
6873
}
6974
}
7075

76+
export const waitForLoaderToBeRemoved = (): Promise<void> =>
77+
waitForElementToBeRemoved(() => screen.getByRole("progressbar"))
78+
7179
export * from "./entities"

‎site/src/xServices/StateContext.tsx

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,11 @@ import { authMachine } from "./auth/authXService"
66
import { buildInfoMachine } from "./buildInfo/buildInfoXService"
77
import { entitlementsMachine } from "./entitlements/entitlementsXService"
88
import { siteRolesMachine } from "./roles/siteRolesXService"
9-
import { usersMachine } from "./users/usersXService"
109

1110
interface XServiceContextType {
1211
authXService: ActorRefFrom<typeof authMachine>
1312
buildInfoXService: ActorRefFrom<typeof buildInfoMachine>
1413
entitlementsXService: ActorRefFrom<typeof entitlementsMachine>
15-
usersXService: ActorRefFrom<typeof usersMachine>
1614
siteRolesXService: ActorRefFrom<typeof siteRolesMachine>
1715
}
1816

@@ -28,9 +26,6 @@ export const XServiceContext = createContext({} as XServiceContextType)
2826

2927
export const XServiceProvider: FC<{ children: ReactNode }> = ({ children }) => {
3028
const navigate = useNavigate()
31-
const redirectToUsersPage = () => {
32-
navigate("users")
33-
}
3429
const redirectToSetupPage = () => {
3530
navigate("setup")
3631
}
@@ -43,9 +38,6 @@ export const XServiceProvider: FC<{ children: ReactNode }> = ({ children }) => {
4338
),
4439
buildInfoXService: useInterpret(buildInfoMachine),
4540
entitlementsXService: useInterpret(entitlementsMachine),
46-
usersXService: useInterpret(() =>
47-
usersMachine.withConfig({ actions: { redirectToUsersPage } }),
48-
),
4941
siteRolesXService: useInterpret(siteRolesMachine),
5042
}}
5143
>

0 commit comments

Comments
 (0)