diff --git a/site/src/components/NavbarView/NavbarView.tsx b/site/src/components/NavbarView/NavbarView.tsx index 57305b577a..6eeeda814e 100644 --- a/site/src/components/NavbarView/NavbarView.tsx +++ b/site/src/components/NavbarView/NavbarView.tsx @@ -48,7 +48,10 @@ const NavItems: React.FC< - + {Language.users} diff --git a/site/src/components/SearchBarWithFilter/SearchBarWithFilter.test.tsx b/site/src/components/SearchBarWithFilter/SearchBarWithFilter.test.tsx index 48061e1d9b..540f634701 100644 --- a/site/src/components/SearchBarWithFilter/SearchBarWithFilter.test.tsx +++ b/site/src/components/SearchBarWithFilter/SearchBarWithFilter.test.tsx @@ -1,4 +1,4 @@ -import { fireEvent, screen } from "@testing-library/react" +import { screen } from "@testing-library/react" import userEvent from "@testing-library/user-event" import { render } from "../../testHelpers/renderHelpers" import { SearchBarWithFilter } from "./SearchBarWithFilter" @@ -21,18 +21,6 @@ describe("SearchBarWithFilter", () => { await userEvent.type(searchInput, "workspace") // 9 characters // Then - expect(onFilter).toBeCalledTimes(10) // 9 characters + 1 on component mount - }) - - it("calls the onFilter handler on submit", async () => { - // When - const onFilter = jest.fn() - render() - - const searchInput = screen.getByRole("textbox") - await fireEvent.keyDown(searchInput, { key: "Enter", code: "Enter", charCode: 13 }) - - // Then - expect(onFilter).toBeCalledTimes(1) + expect(onFilter).toBeCalledTimes(9) // 9 characters }) }) diff --git a/site/src/components/SearchBarWithFilter/SearchBarWithFilter.tsx b/site/src/components/SearchBarWithFilter/SearchBarWithFilter.tsx index 927eb495c8..7e1fe9f14b 100644 --- a/site/src/components/SearchBarWithFilter/SearchBarWithFilter.tsx +++ b/site/src/components/SearchBarWithFilter/SearchBarWithFilter.tsx @@ -7,9 +7,8 @@ import OutlinedInput from "@material-ui/core/OutlinedInput" import { makeStyles } from "@material-ui/core/styles" import { Theme } from "@material-ui/core/styles/createMuiTheme" import SearchIcon from "@material-ui/icons/Search" -import { FormikErrors, useFormik } from "formik" import debounce from "just-debounce-it" -import { useCallback, useEffect, useState } from "react" +import { useCallback, useRef, useState } from "react" import { getValidationErrorMessage } from "../../api/errors" import { CloseDropdown, OpenDropdown } from "../DropdownArrows/DropdownArrows" import { Stack } from "../Stack/Stack" @@ -30,12 +29,6 @@ export interface PresetFilter { query: string } -interface FilterFormValues { - query: string -} - -export type FilterFormErrors = FormikErrors - export const SearchBarWithFilter: React.FC> = ({ filter, onFilter, @@ -43,16 +36,7 @@ export const SearchBarWithFilter: React.FC { const styles = useStyles({ error: Boolean(error) }) - - const form = useFormik({ - enableReinitialize: true, - initialValues: { - query: filter ?? "", - }, - onSubmit: ({ query }) => { - onFilter(query) - }, - }) + const searchInputRef = useRef(null) // debounce query string entry by user // we want the dependency array empty here @@ -65,12 +49,6 @@ export const SearchBarWithFilter: React.FC { - debouncedOnFilter(form.values.query) - return () => debouncedOnFilter.cancel() - }, [debouncedOnFilter, form.values.query]) - const [anchorEl, setAnchorEl] = useState(null) const handleClick = (event: React.MouseEvent) => { @@ -82,8 +60,15 @@ export const SearchBarWithFilter: React.FC () => { - void form.setFieldValue("query", query) - void form.submitForm() + if (!searchInputRef.current) { + throw new Error("Search input not found.") + } + + onFilter(query) + // Update this to the input directly instead of create a new state and + // re-render the component since the onFilter is already calling the + // filtering process + searchInputRef.current.value = query handleClose() } @@ -103,21 +88,24 @@ export const SearchBarWithFilter: React.FC )} -
+
{ + debouncedOnFilter(event.currentTarget.value) + }} + inputRef={searchInputRef} startAdornment={ } /> - +
{presetFilters && presetFilters.length && ( { + renderWithAuth() + await waitForLoaderToBeRemoved() +} + const fillForm = async ({ username = "someuser", email = "someone@coder.com", @@ -34,7 +43,7 @@ describe("Create User Page", () => { }) it("shows validation error message", async () => { - render() + await renderCreateUserPage() await fillForm({ email: "test" }) const errorMessage = await screen.findByText(FormLanguage.emailInvalid) expect(errorMessage).toBeDefined() @@ -44,9 +53,9 @@ describe("Create User Page", () => { jest.spyOn(API, "createUser").mockRejectedValueOnce({ data: "unknown error", }) - render() + await renderCreateUserPage() await fillForm({}) - const errorMessage = await screen.findByText(UserLanguage.createUserError) + const errorMessage = await screen.findByText(CreateUserLanguage.createUserError) expect(errorMessage).toBeDefined() }) @@ -68,30 +77,16 @@ describe("Create User Page", () => { ) }), ) - render() + await renderCreateUserPage() await fillForm({}) const errorMessage = await screen.findByText(fieldErrorMessage) expect(errorMessage).toBeDefined() }) it("shows success notification and redirects to users page", async () => { - render() + await renderCreateUserPage() await fillForm({}) - const successMessage = screen.findByText(UserLanguage.createUserSuccess) + const successMessage = screen.findByText(CreateUserLanguage.createUserSuccess) expect(successMessage).toBeDefined() }) - - it("redirects to users page on cancel", async () => { - render() - const cancelButton = await screen.findByText(FooterLanguage.cancelLabel) - cancelButton.click() - expect(history.location.pathname).toEqual("/users") - }) - - it("redirects to users page on close", async () => { - render() - const closeButton = await screen.findByText("ESC") - closeButton.click() - expect(history.location.pathname).toEqual("/users") - }) }) diff --git a/site/src/pages/UsersPage/CreateUserPage/CreateUserPage.tsx b/site/src/pages/UsersPage/CreateUserPage/CreateUserPage.tsx index a4b75bc12b..7cc8f2db40 100644 --- a/site/src/pages/UsersPage/CreateUserPage/CreateUserPage.tsx +++ b/site/src/pages/UsersPage/CreateUserPage/CreateUserPage.tsx @@ -1,24 +1,29 @@ -import { useActor, useSelector } from "@xstate/react" -import React, { useContext } from "react" +import { useMachine } from "@xstate/react" +import { useOrganizationId } from "hooks/useOrganizationId" +import React from "react" import { Helmet } from "react-helmet-async" import { useNavigate } from "react-router" +import { createUserMachine } from "xServices/users/createUserXService" import * as TypesGen from "../../../api/typesGenerated" import { CreateUserForm } from "../../../components/CreateUserForm/CreateUserForm" import { Margins } from "../../../components/Margins/Margins" import { pageTitle } from "../../../util/page" -import { selectOrgId } from "../../../xServices/auth/authSelectors" -import { XServiceContext } from "../../../xServices/StateContext" export const Language = { unknownError: "Oops, an unknown error occurred.", } export const CreateUserPage: React.FC = () => { - const xServices = useContext(XServiceContext) - const myOrgId = useSelector(xServices.authXService, selectOrgId) - const [usersState, usersSend] = useActor(xServices.usersXService) - const { createUserErrorMessage, createUserFormErrors } = usersState.context + const myOrgId = useOrganizationId() const navigate = useNavigate() + const [createUserState, createUserSend] = useMachine(createUserMachine, { + actions: { + redirectToUsersPage: () => { + navigate("/users") + }, + }, + }) + const { createUserErrorMessage, createUserFormErrors } = createUserState.context // There is no field for organization id in Community Edition, so handle its field error like a generic error const genericError = createUserErrorMessage || @@ -32,14 +37,14 @@ export const CreateUserPage: React.FC = () => { usersSend({ type: "CREATE", user })} + onSubmit={(user: TypesGen.CreateUserRequest) => createUserSend({ type: "CREATE", user })} onCancel={() => { - usersSend("CANCEL_CREATE_USER") + createUserSend("CANCEL_CREATE_USER") navigate("/users") }} - isLoading={usersState.hasTag("loading")} + isLoading={createUserState.hasTag("loading")} error={genericError} - myOrgId={myOrgId ?? ""} + myOrgId={myOrgId} /> ) diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index 5853347ee2..6a6b022050 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -1,11 +1,11 @@ -import { useActor } from "@xstate/react" +import { useActor, useMachine } from "@xstate/react" import { FC, ReactNode, useContext, useEffect } from "react" import { Helmet } from "react-helmet-async" import { useNavigate } from "react-router" import { useSearchParams } from "react-router-dom" +import { usersMachine } from "xServices/users/usersXService" import { ConfirmDialog } from "../../components/Dialogs/ConfirmDialog/ConfirmDialog" import { ResetPasswordDialog } from "../../components/Dialogs/ResetPasswordDialog/ResetPasswordDialog" -import { userFilterQuery } from "../../util/filters" import { pageTitle } from "../../util/page" import { XServiceContext } from "../../xServices/StateContext" import { UsersPageView } from "./UsersPageView" @@ -24,7 +24,14 @@ export const Language = { export const UsersPage: FC<{ children?: ReactNode }> = () => { const xServices = useContext(XServiceContext) - const [usersState, usersSend] = useActor(xServices.usersXService) + const navigate = useNavigate() + const [searchParams, setSearchParams] = useSearchParams() + const filter = searchParams.get("filter") ?? undefined + const [usersState, usersSend] = useMachine(usersMachine, { + context: { + filter, + }, + }) const { users, getUsersError, @@ -34,8 +41,7 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { userIdToResetPassword, newUserPassword, } = usersState.context - const navigate = useNavigate() - const [searchParams, setSearchParams] = useSearchParams() + const userToBeSuspended = users?.find((u) => u.id === userIdToSuspend) const userToBeDeleted = users?.find((u) => u.id === userIdToDelete) const userToBeActivated = users?.find((u) => u.id === userIdToActivate) @@ -60,13 +66,11 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { // Fetch users on component mount useEffect(() => { - const filter = searchParams.get("filter") - const query = filter ?? userFilterQuery.active usersSend({ type: "GET_USERS", - query, + query: filter, }) - }, [searchParams, usersSend]) + }, [filter, usersSend]) // Fetch roles on component mount useEffect(() => { diff --git a/site/src/testHelpers/renderHelpers.tsx b/site/src/testHelpers/renderHelpers.tsx index b0e7a91bb7..4f7bd34028 100644 --- a/site/src/testHelpers/renderHelpers.tsx +++ b/site/src/testHelpers/renderHelpers.tsx @@ -1,5 +1,10 @@ import ThemeProvider from "@material-ui/styles/ThemeProvider" -import { render as wrappedRender, RenderResult } from "@testing-library/react" +import { + render as wrappedRender, + RenderResult, + screen, + waitForElementToBeRemoved, +} from "@testing-library/react" import { createMemoryHistory } from "history" import { i18n } from "i18n" import { FC, ReactElement } from "react" @@ -68,4 +73,7 @@ export function renderWithAuth( } } +export const waitForLoaderToBeRemoved = (): Promise => + waitForElementToBeRemoved(() => screen.getByRole("progressbar")) + export * from "./entities" diff --git a/site/src/xServices/StateContext.tsx b/site/src/xServices/StateContext.tsx index 93c9291d37..a795690e35 100644 --- a/site/src/xServices/StateContext.tsx +++ b/site/src/xServices/StateContext.tsx @@ -6,13 +6,11 @@ import { authMachine } from "./auth/authXService" import { buildInfoMachine } from "./buildInfo/buildInfoXService" import { entitlementsMachine } from "./entitlements/entitlementsXService" import { siteRolesMachine } from "./roles/siteRolesXService" -import { usersMachine } from "./users/usersXService" interface XServiceContextType { authXService: ActorRefFrom buildInfoXService: ActorRefFrom entitlementsXService: ActorRefFrom - usersXService: ActorRefFrom siteRolesXService: ActorRefFrom } @@ -28,9 +26,6 @@ export const XServiceContext = createContext({} as XServiceContextType) export const XServiceProvider: FC<{ children: ReactNode }> = ({ children }) => { const navigate = useNavigate() - const redirectToUsersPage = () => { - navigate("users") - } const redirectToSetupPage = () => { navigate("setup") } @@ -43,9 +38,6 @@ export const XServiceProvider: FC<{ children: ReactNode }> = ({ children }) => { ), buildInfoXService: useInterpret(buildInfoMachine), entitlementsXService: useInterpret(entitlementsMachine), - usersXService: useInterpret(() => - usersMachine.withConfig({ actions: { redirectToUsersPage } }), - ), siteRolesXService: useInterpret(siteRolesMachine), }} > diff --git a/site/src/xServices/users/createUserXService.ts b/site/src/xServices/users/createUserXService.ts new file mode 100644 index 0000000000..4f7190e165 --- /dev/null +++ b/site/src/xServices/users/createUserXService.ts @@ -0,0 +1,101 @@ +import { assign, createMachine } from "xstate" +import * as API from "../../api/api" +import { + ApiError, + FieldErrors, + getErrorMessage, + hasApiFieldErrors, + isApiError, + mapApiErrorToFieldErrors, +} from "../../api/errors" +import * as TypesGen from "../../api/typesGenerated" +import { displaySuccess } from "../../components/GlobalSnackbar/utils" + +export const Language = { + createUserSuccess: "Successfully created user.", + createUserError: "Error on creating the user.", +} + +export interface CreateUserContext { + createUserErrorMessage?: string + createUserFormErrors?: FieldErrors +} + +export type CreateUserEvent = + | { type: "CREATE"; user: TypesGen.CreateUserRequest } + | { type: "CANCEL_CREATE_USER" } + +export const createUserMachine = createMachine( + { + id: "usersState", + predictableActionArguments: true, + tsTypes: {} as import("./createUserXService.typegen").Typegen0, + schema: { + context: {} as CreateUserContext, + events: {} as CreateUserEvent, + services: {} as { + createUser: { + data: TypesGen.User + } + }, + }, + initial: "idle", + states: { + idle: { + on: { + CREATE: "creatingUser", + CANCEL_CREATE_USER: { actions: ["clearCreateUserError"] }, + }, + }, + creatingUser: { + entry: "clearCreateUserError", + invoke: { + src: "createUser", + id: "createUser", + onDone: { + target: "idle", + actions: ["displayCreateUserSuccess", "redirectToUsersPage"], + }, + onError: [ + { + target: "idle", + cond: "hasFieldErrors", + actions: ["assignCreateUserFormErrors"], + }, + { + target: "idle", + actions: ["assignCreateUserError"], + }, + ], + }, + tags: "loading", + }, + }, + }, + { + services: { + createUser: (_, event) => API.createUser(event.user), + }, + guards: { + hasFieldErrors: (_, event) => isApiError(event.data) && hasApiFieldErrors(event.data), + }, + actions: { + assignCreateUserError: assign({ + createUserErrorMessage: (_, event) => getErrorMessage(event.data, Language.createUserError), + }), + assignCreateUserFormErrors: assign({ + // the guard ensures it is ApiError + createUserFormErrors: (_, event) => + mapApiErrorToFieldErrors((event.data as ApiError).response.data), + }), + clearCreateUserError: assign((context: CreateUserContext) => ({ + ...context, + createUserErrorMessage: undefined, + createUserFormErrors: undefined, + })), + displayCreateUserSuccess: () => { + displaySuccess(Language.createUserSuccess) + }, + }, + }, +) diff --git a/site/src/xServices/users/usersXService.ts b/site/src/xServices/users/usersXService.ts index 9a64224242..1e8ed7eeed 100644 --- a/site/src/xServices/users/usersXService.ts +++ b/site/src/xServices/users/usersXService.ts @@ -1,13 +1,6 @@ import { assign, createMachine } from "xstate" import * as API from "../../api/api" -import { - ApiError, - FieldErrors, - getErrorMessage, - hasApiFieldErrors, - isApiError, - mapApiErrorToFieldErrors, -} from "../../api/errors" +import { getErrorMessage } from "../../api/errors" import * as TypesGen from "../../api/typesGenerated" import { displayError, displaySuccess } from "../../components/GlobalSnackbar/utils" import { queryToFilter } from "../../util/filters" @@ -15,8 +8,6 @@ import { generateRandomString } from "../../util/random" export const Language = { getUsersError: "Error getting users.", - createUserSuccess: "Successfully created user.", - createUserError: "Error on creating the user.", suspendUserSuccess: "Successfully suspended the user.", suspendUserError: "Error suspending user.", deleteUserSuccess: "Successfully deleted the user.", @@ -34,8 +25,6 @@ export interface UsersContext { users?: TypesGen.User[] filter?: string getUsersError?: Error | unknown - createUserErrorMessage?: string - createUserFormErrors?: FieldErrors // Suspend user userIdToSuspend?: TypesGen.User["id"] suspendUserError?: Error | unknown @@ -55,9 +44,7 @@ export interface UsersContext { } export type UsersEvent = - | { type: "GET_USERS"; query: string } - | { type: "CREATE"; user: TypesGen.CreateUserRequest } - | { type: "CANCEL_CREATE_USER" } + | { type: "GET_USERS"; query?: string } // Suspend events | { type: "SUSPEND_USER"; userId: TypesGen.User["id"] } | { type: "CONFIRM_USER_SUSPENSION" } @@ -109,16 +96,34 @@ export const usersMachine = createMachine( } }, }, - initial: "idle", + initial: "gettingUsers", states: { + gettingUsers: { + entry: "clearGetUsersError", + invoke: { + src: "getUsers", + id: "getUsers", + onDone: [ + { + target: "#usersState.idle", + actions: "assignUsers", + }, + ], + onError: [ + { + actions: ["clearUsers", "assignGetUsersError", "displayGetUsersErrorMessage"], + target: "#usersState.error", + }, + ], + }, + tags: "loading", + }, idle: { on: { GET_USERS: { actions: "assignFilter", target: "gettingUsers", }, - CREATE: "creatingUser", - CANCEL_CREATE_USER: { actions: ["clearCreateUserError"] }, SUSPEND_USER: { target: "confirmUserSuspension", actions: ["assignUserIdToSuspend"], @@ -141,49 +146,6 @@ export const usersMachine = createMachine( }, }, }, - gettingUsers: { - entry: "clearGetUsersError", - invoke: { - src: "getUsers", - id: "getUsers", - onDone: [ - { - target: "#usersState.idle", - actions: "assignUsers", - }, - ], - onError: [ - { - actions: ["clearUsers", "assignGetUsersError", "displayGetUsersErrorMessage"], - target: "#usersState.error", - }, - ], - }, - tags: "loading", - }, - creatingUser: { - entry: "clearCreateUserError", - invoke: { - src: "createUser", - id: "createUser", - onDone: { - target: "idle", - actions: ["displayCreateUserSuccess", "redirectToUsersPage"], - }, - onError: [ - { - target: "idle", - cond: "hasFieldErrors", - actions: ["assignCreateUserFormErrors"], - }, - { - target: "idle", - actions: ["assignCreateUserError"], - }, - ], - }, - tags: "loading", - }, confirmUserSuspension: { on: { CONFIRM_USER_SUSPENSION: "suspendingUser", @@ -301,7 +263,6 @@ export const usersMachine = createMachine( // when it is mocked. This happen in the UsersPage tests inside of the // "shows a success message and refresh the page" test case. getUsers: (context) => API.getUsers(queryToFilter(context.filter)), - createUser: (_, event) => API.createUser(event.user), suspendUser: (context) => { if (!context.userIdToSuspend) { throw new Error("userIdToSuspend is undefined") @@ -344,9 +305,7 @@ export const usersMachine = createMachine( return API.updateUserRoles(event.roles, context.userIdToUpdateRoles) }, }, - guards: { - hasFieldErrors: (_, event) => isApiError(event.data) && hasApiFieldErrors(event.data), - }, + actions: { assignUsers: assign({ users: (_, event) => event.data, @@ -376,14 +335,6 @@ export const usersMachine = createMachine( ...context, getUsersError: undefined, })), - assignCreateUserError: assign({ - createUserErrorMessage: (_, event) => getErrorMessage(event.data, Language.createUserError), - }), - assignCreateUserFormErrors: assign({ - // the guard ensures it is ApiError - createUserFormErrors: (_, event) => - mapApiErrorToFieldErrors((event.data as ApiError).response.data), - }), assignSuspendUserError: assign({ suspendUserError: (_, event) => event.data, }), @@ -403,11 +354,6 @@ export const usersMachine = createMachine( ...context, users: undefined, })), - clearCreateUserError: assign((context: UsersContext) => ({ - ...context, - createUserErrorMessage: undefined, - createUserFormErrors: undefined, - })), clearSuspendUserError: assign({ suspendUserError: (_) => undefined, }), @@ -427,9 +373,6 @@ export const usersMachine = createMachine( const message = getErrorMessage(context.getUsersError, Language.getUsersError) displayError(message) }, - displayCreateUserSuccess: () => { - displaySuccess(Language.createUserSuccess) - }, displaySuspendSuccess: () => { displaySuccess(Language.suspendUserSuccess) },