From c16f1057274349bc4cec61c98b21332e1386d6dd Mon Sep 17 00:00:00 2001 From: Presley Pizzo <1290996+presleyp@users.noreply.github.com> Date: Thu, 28 Apr 2022 16:32:23 -0400 Subject: [PATCH] feat: Create user page (#1197) * Add button and route * Hook up api * Lint * Add basic form * Get users on page mount * Make cancel work * Creating -> idle bc users page refetches * Import as TypesGen * Handle api errors * Lint * Add handler * Add FormFooter * Add FullPageForm * Lint * Better form, error, stories bug in formErrors story * Make detail optional * Use Language * Remove detail prop * Add back autoFocus * Remove displayError, use displaySuccess * Lint, export Language * Tests - wip * Fix cancel tests * Switch back to mock * Add navigate to xservice Doesn't work in test * Move error type predicate to xservice * Lint * Switch to using creation mode in XState still problems in tests * Lint * Lint * Lint * Revert "Switch to using creation mode in XState" This reverts commit cf8442fa4b150a3d61389a7e71c45e1072040a44. * Give XService a navigate action * Add missing validation messages * Fix XState warning * Fix tests IRL is broken bc I need to send org id * Pretend user has org id and make it work * Format * Lint * Switch to org ids array * Skip lines between tests Co-authored-by: G r e y * Punctuate notification messages Co-authored-by: G r e y --- site/src/AppRouter.tsx | 27 +++-- site/src/api/errors.ts | 2 +- site/src/api/index.ts | 5 + site/src/api/types.ts | 9 ++ .../CreateUserForm/CreateUserForm.stories.tsx | 43 ++++++++ .../CreateUserForm/CreateUserForm.tsx | 92 +++++++++++++++++ site/src/components/FormFooter/FormFooter.tsx | 2 +- .../AccountPage/AccountPage.test.tsx | 4 +- .../CreateUserPage/CreateUserPage.test.tsx | 98 +++++++++++++++++++ .../CreateUserPage/CreateUserPage.tsx | 33 +++++++ site/src/pages/UsersPage/UsersPage.tsx | 23 ++++- site/src/pages/UsersPage/UsersPageView.tsx | 11 ++- site/src/testHelpers/entities.ts | 4 + site/src/testHelpers/handlers.ts | 3 + site/src/xServices/StateContext.tsx | 8 +- site/src/xServices/auth/authSelectors.ts | 6 ++ site/src/xServices/users/usersXService.ts | 65 ++++++++++-- 17 files changed, 412 insertions(+), 23 deletions(-) create mode 100644 site/src/components/CreateUserForm/CreateUserForm.stories.tsx create mode 100644 site/src/components/CreateUserForm/CreateUserForm.tsx create mode 100644 site/src/pages/UsersPage/CreateUserPage/CreateUserPage.test.tsx create mode 100644 site/src/pages/UsersPage/CreateUserPage/CreateUserPage.tsx create mode 100644 site/src/xServices/auth/authSelectors.ts diff --git a/site/src/AppRouter.tsx b/site/src/AppRouter.tsx index 4016b010d2..691f46bf9a 100644 --- a/site/src/AppRouter.tsx +++ b/site/src/AppRouter.tsx @@ -17,6 +17,7 @@ import { SettingsPage } from "./pages/SettingsPage/SettingsPage" import { CreateWorkspacePage } from "./pages/TemplatesPages/OrganizationPage/TemplatePage/CreateWorkspacePage" import { TemplatePage } from "./pages/TemplatesPages/OrganizationPage/TemplatePage/TemplatePage" import { TemplatesPage } from "./pages/TemplatesPages/TemplatesPage" +import { CreateUserPage } from "./pages/UsersPage/CreateUserPage/CreateUserPage" import { UsersPage } from "./pages/UsersPage/UsersPage" import { WorkspacePage } from "./pages/WorkspacesPage/WorkspacesPage" @@ -83,14 +84,24 @@ export const AppRouter: React.FC = () => ( /> - - - - } - /> + + + + + } + /> + + + + } + /> + +export type FieldErrors = Record export interface ApiErrorResponse { message: string diff --git a/site/src/api/index.ts b/site/src/api/index.ts index cd6a387f6d..aff8d5235c 100644 --- a/site/src/api/index.ts +++ b/site/src/api/index.ts @@ -85,6 +85,11 @@ export const getUsers = async (): Promise => { }) } +export const createUser = async (user: Types.CreateUserRequest): Promise => { + const response = await axios.post("/api/v2/users", user) + return response.data +} + export const getBuildInfo = async (): Promise => { const response = await axios.get("/api/v2/buildinfo") return response.data diff --git a/site/src/api/types.ts b/site/src/api/types.ts index c303ffc9d4..24467756cb 100644 --- a/site/src/api/types.ts +++ b/site/src/api/types.ts @@ -10,11 +10,20 @@ export interface LoginResponse { session_token: string } +export interface CreateUserRequest { + username: string + email: string + password: string + organization_id: string +} + export interface UserResponse { readonly id: string readonly username: string readonly email: string readonly created_at: string + readonly status: "active" | "suspended" + readonly organization_ids: string[] } /** diff --git a/site/src/components/CreateUserForm/CreateUserForm.stories.tsx b/site/src/components/CreateUserForm/CreateUserForm.stories.tsx new file mode 100644 index 0000000000..1ae6255955 --- /dev/null +++ b/site/src/components/CreateUserForm/CreateUserForm.stories.tsx @@ -0,0 +1,43 @@ +import { action } from "@storybook/addon-actions" +import { Story } from "@storybook/react" +import React from "react" +import { CreateUserForm, CreateUserFormProps } from "./CreateUserForm" + +export default { + title: "components/CreateUserForm", + component: CreateUserForm, +} + +const Template: Story = (args: CreateUserFormProps) => + +export const Ready = Template.bind({}) +Ready.args = { + onCancel: action("cancel"), + onSubmit: action("submit"), + isLoading: false, +} + +export const UnknownError = Template.bind({}) +UnknownError.args = { + onCancel: action("cancel"), + onSubmit: action("submit"), + isLoading: false, + error: "Something went wrong", +} + +export const FormError = Template.bind({}) +FormError.args = { + onCancel: action("cancel"), + onSubmit: action("submit"), + isLoading: false, + formErrors: { + username: "Username taken", + }, +} + +export const Loading = Template.bind({}) +Loading.args = { + onCancel: action("cancel"), + onSubmit: action("submit"), + isLoading: true, +} diff --git a/site/src/components/CreateUserForm/CreateUserForm.tsx b/site/src/components/CreateUserForm/CreateUserForm.tsx new file mode 100644 index 0000000000..f95e7fd0c8 --- /dev/null +++ b/site/src/components/CreateUserForm/CreateUserForm.tsx @@ -0,0 +1,92 @@ +import FormHelperText from "@material-ui/core/FormHelperText" +import TextField from "@material-ui/core/TextField" +import { FormikContextType, FormikErrors, useFormik } from "formik" +import React from "react" +import * as Yup from "yup" +import { CreateUserRequest } from "../../api/types" +import { getFormHelpers, onChangeTrimmed } from "../../util/formUtils" +import { FormFooter } from "../FormFooter/FormFooter" +import { FullPageForm } from "../FullPageForm/FullPageForm" + +export const Language = { + emailLabel: "Email", + passwordLabel: "Password", + usernameLabel: "Username", + emailInvalid: "Please enter a valid email address.", + emailRequired: "Please enter an email address.", + passwordRequired: "Please enter a password.", + usernameRequired: "Please enter a username.", + createUser: "Create", + cancel: "Cancel", +} + +export interface CreateUserFormProps { + onSubmit: (user: CreateUserRequest) => void + onCancel: () => void + formErrors?: FormikErrors + isLoading: boolean + error?: string + myOrgId: string +} + +const validationSchema = Yup.object({ + email: Yup.string().trim().email(Language.emailInvalid).required(Language.emailRequired), + password: Yup.string().required(Language.passwordRequired), + username: Yup.string().required(Language.usernameRequired), +}) + +export const CreateUserForm: React.FC = ({ + onSubmit, + onCancel, + formErrors, + isLoading, + error, + myOrgId, +}) => { + const form: FormikContextType = useFormik({ + initialValues: { + email: "", + password: "", + username: "", + organization_id: myOrgId, + }, + validationSchema, + onSubmit, + }) + const getFieldHelpers = getFormHelpers(form, formErrors) + + return ( + +
+ + + + {error && {error}} + + +
+ ) +} diff --git a/site/src/components/FormFooter/FormFooter.tsx b/site/src/components/FormFooter/FormFooter.tsx index a7b5c48ba1..03233f42b7 100644 --- a/site/src/components/FormFooter/FormFooter.tsx +++ b/site/src/components/FormFooter/FormFooter.tsx @@ -3,7 +3,7 @@ import { makeStyles } from "@material-ui/core/styles" import React from "react" import { LoadingButton } from "../LoadingButton/LoadingButton" -const Language = { +export const Language = { cancelLabel: "Cancel", defaultSubmitLabel: "Submit", } diff --git a/site/src/pages/PreferencesPages/AccountPage/AccountPage.test.tsx b/site/src/pages/PreferencesPages/AccountPage/AccountPage.test.tsx index 038a12f7b3..180b0ac69d 100644 --- a/site/src/pages/PreferencesPages/AccountPage/AccountPage.test.tsx +++ b/site/src/pages/PreferencesPages/AccountPage/AccountPage.test.tsx @@ -34,8 +34,10 @@ describe("AccountPage", () => { jest.spyOn(API, "updateProfile").mockImplementationOnce((userId, data) => Promise.resolve({ id: userId, - ...data, created_at: new Date().toString(), + status: "active", + organization_ids: ["123"], + ...data, }), ) const { user } = renderPage() diff --git a/site/src/pages/UsersPage/CreateUserPage/CreateUserPage.test.tsx b/site/src/pages/UsersPage/CreateUserPage/CreateUserPage.test.tsx new file mode 100644 index 0000000000..80c86367dd --- /dev/null +++ b/site/src/pages/UsersPage/CreateUserPage/CreateUserPage.test.tsx @@ -0,0 +1,98 @@ +import { screen } from "@testing-library/react" +import userEvent from "@testing-library/user-event" +import { rest } from "msw" +import React from "react" +import * as API from "../../../api" +import { Language as FormLanguage } from "../../../components/CreateUserForm/CreateUserForm" +import { Language as FooterLanguage } from "../../../components/FormFooter/FormFooter" +import { history, render } from "../../../testHelpers" +import { server } from "../../../testHelpers/server" +import { Language as UserLanguage } from "../../../xServices/users/usersXService" +import { CreateUserPage, Language } from "./CreateUserPage" + +const fillForm = async ({ + username = "someuser", + email = "someone@coder.com", + password = "password", +}: { + username?: string + email?: string + password?: string +}) => { + const usernameField = screen.getByLabelText(FormLanguage.usernameLabel) + const emailField = screen.getByLabelText(FormLanguage.emailLabel) + const passwordField = screen.getByLabelText(FormLanguage.passwordLabel) + await userEvent.type(usernameField, username) + await userEvent.type(emailField, email) + await userEvent.type(passwordField, password) + const submitButton = await screen.findByText(FooterLanguage.defaultSubmitLabel) + submitButton.click() +} + +describe("Create User Page", () => { + beforeEach(() => { + history.replace("/users/create") + }) + + it("shows validation error message", async () => { + render() + await fillForm({ email: "test" }) + const errorMessage = await screen.findByText(FormLanguage.emailInvalid) + expect(errorMessage).toBeDefined() + }) + + it("shows generic error message", async () => { + jest.spyOn(API, "createUser").mockRejectedValueOnce({ + data: "unknown error", + }) + render() + await fillForm({}) + const errorMessage = await screen.findByText(Language.unknownError) + expect(errorMessage).toBeDefined() + }) + + it("shows API error message", async () => { + const fieldErrorMessage = "username already in use" + server.use( + rest.post("/api/v2/users", async (req, res, ctx) => { + return res( + ctx.status(400), + ctx.json({ + message: "invalid field", + errors: [ + { + detail: fieldErrorMessage, + field: "username", + }, + ], + }), + ) + }), + ) + render() + await fillForm({}) + const errorMessage = await screen.findByText(fieldErrorMessage) + expect(errorMessage).toBeDefined() + }) + + it("shows success notification and redirects to users page", async () => { + render() + await fillForm({}) + const successMessage = screen.findByText(UserLanguage.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 new file mode 100644 index 0000000000..c8f36498c3 --- /dev/null +++ b/site/src/pages/UsersPage/CreateUserPage/CreateUserPage.tsx @@ -0,0 +1,33 @@ +import { useActor, useSelector } from "@xstate/react" +import React, { useContext } from "react" +import { useNavigate } from "react-router" +import { CreateUserRequest } from "../../../api/types" +import { CreateUserForm } from "../../../components/CreateUserForm/CreateUserForm" +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 { createUserError, createUserFormErrors } = usersState.context + const navigate = useNavigate() + // There is no field for organization id in Community Edition, so handle its field error like a generic error + const genericError = + createUserError || createUserFormErrors?.organization_id || !myOrgId ? Language.unknownError : undefined + + return ( + usersSend({ type: "CREATE", user })} + onCancel={() => navigate("/users")} + isLoading={usersState.hasTag("loading")} + error={genericError} + myOrgId={myOrgId ?? ""} + /> + ) +} diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index fd4f073097..b54422d5bf 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -1,17 +1,34 @@ import { useActor } from "@xstate/react" -import React, { useContext } from "react" +import React, { useContext, useEffect } from "react" +import { useNavigate } from "react-router" import { ErrorSummary } from "../../components/ErrorSummary/ErrorSummary" import { XServiceContext } from "../../xServices/StateContext" import { UsersPageView } from "./UsersPageView" export const UsersPage: React.FC = () => { const xServices = useContext(XServiceContext) - const [usersState] = useActor(xServices.usersXService) + const [usersState, usersSend] = useActor(xServices.usersXService) const { users, pager, getUsersError } = usersState.context + const navigate = useNavigate() + + /** + * Fetch users on component mount + */ + useEffect(() => { + usersSend("GET_USERS") + }, [usersSend]) if (usersState.matches("error")) { return } else { - return + return ( + { + navigate("/users/create") + }} + /> + ) } } diff --git a/site/src/pages/UsersPage/UsersPageView.tsx b/site/src/pages/UsersPage/UsersPageView.tsx index 812cc4e318..a919ac9f5f 100644 --- a/site/src/pages/UsersPage/UsersPageView.tsx +++ b/site/src/pages/UsersPage/UsersPageView.tsx @@ -7,18 +7,25 @@ import { UsersTable } from "../../components/UsersTable/UsersTable" export const Language = { pageTitle: "Users", pageSubtitle: (pager: Pager | undefined): string => (pager ? `${pager.total} total` : ""), + newUserButton: "New User", } export interface UsersPageViewProps { users: UserResponse[] pager?: Pager + openUserCreationDialog: () => void } -export const UsersPageView: React.FC = ({ users, pager }) => { +export const UsersPageView: React.FC = ({ users, pager, openUserCreationDialog }) => { const styles = useStyles() + return (
-
+
) diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index ffdd7418f2..3163ed24c0 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -25,6 +25,8 @@ export const MockUser: UserResponse = { username: "TestUser", email: "test@coder.com", created_at: "", + status: "active", + organization_ids: ["fc0774ce-cc9e-48d4-80ae-88f7a4d4a8b0"], } export const MockUser2: UserResponse = { @@ -32,6 +34,8 @@ export const MockUser2: UserResponse = { username: "TestUser2", email: "test2@coder.com", created_at: "", + status: "active", + organization_ids: ["fc0774ce-cc9e-48d4-80ae-88f7a4d4a8b0"], } export const MockPager: Pager = { diff --git a/site/src/testHelpers/handlers.ts b/site/src/testHelpers/handlers.ts index 22e4818085..a6b53714b9 100644 --- a/site/src/testHelpers/handlers.ts +++ b/site/src/testHelpers/handlers.ts @@ -24,6 +24,9 @@ export const handlers = [ rest.get("/api/v2/users", async (req, res, ctx) => { return res(ctx.status(200), ctx.json({ page: [M.MockUser, M.MockUser2], pager: M.MockPager })) }), + rest.post("/api/v2/users", async (req, res, ctx) => { + return res(ctx.status(200), ctx.json(M.MockUser)) + }), rest.post("/api/v2/users/me/workspaces", async (req, res, ctx) => { return res(ctx.status(200), ctx.json(M.MockWorkspace)) }), diff --git a/site/src/xServices/StateContext.tsx b/site/src/xServices/StateContext.tsx index ecd1673922..d35573ef23 100644 --- a/site/src/xServices/StateContext.tsx +++ b/site/src/xServices/StateContext.tsx @@ -1,5 +1,6 @@ import { useInterpret } from "@xstate/react" import React, { createContext } from "react" +import { useNavigate } from "react-router" import { ActorRefFrom } from "xstate" import { authMachine } from "./auth/authXService" import { buildInfoMachine } from "./buildInfo/buildInfoXService" @@ -22,12 +23,17 @@ interface XServiceContextType { export const XServiceContext = createContext({} as XServiceContextType) export const XServiceProvider: React.FC = ({ children }) => { + const navigate = useNavigate() + const redirectToUsersPage = () => { + navigate("users") + } + return ( usersMachine.withConfig({ actions: { redirectToUsersPage } })), }} > {children} diff --git a/site/src/xServices/auth/authSelectors.ts b/site/src/xServices/auth/authSelectors.ts new file mode 100644 index 0000000000..b57ac2ebfb --- /dev/null +++ b/site/src/xServices/auth/authSelectors.ts @@ -0,0 +1,6 @@ +import { State } from "xstate" +import { AuthContext, AuthEvent } from "./authXService" + +export const selectOrgId = (state: State): string | undefined => { + return state.context.me?.organization_ids[0] +} diff --git a/site/src/xServices/users/usersXService.ts b/site/src/xServices/users/usersXService.ts index d02c98b84b..a2763ff76c 100644 --- a/site/src/xServices/users/usersXService.ts +++ b/site/src/xServices/users/usersXService.ts @@ -1,14 +1,23 @@ import { assign, createMachine } from "xstate" import * as API from "../../api" +import { ApiError, FieldErrors, isApiError, mapApiErrorToFieldErrors } from "../../api/errors" import * as Types from "../../api/types" +import * as TypesGen from "../../api/typesGenerated" +import { displaySuccess } from "../../components/GlobalSnackbar/utils" + +export const Language = { + createUserSuccess: "Successfully created user.", +} export interface UsersContext { users: Types.UserResponse[] pager?: Types.Pager getUsersError?: Error | unknown + createUserError?: Error | unknown + createUserFormErrors?: FieldErrors } -export type UsersEvent = { type: "GET_USERS" } +export type UsersEvent = { type: "GET_USERS" } | { type: "CREATE"; user: Types.CreateUserRequest } export const usersMachine = createMachine( { @@ -20,21 +29,30 @@ export const usersMachine = createMachine( getUsers: { data: Types.PagedUsers } + createUser: { + data: TypesGen.User + } }, }, id: "usersState", context: { users: [], }, - initial: "gettingUsers", + initial: "idle", states: { + idle: { + on: { + GET_USERS: "gettingUsers", + CREATE: "creatingUser", + }, + }, gettingUsers: { invoke: { src: "getUsers", id: "getUsers", onDone: [ { - target: "#usersState.ready", + target: "#usersState.idle", actions: ["assignUsers", "clearGetUsersError"], }, ], @@ -47,10 +65,27 @@ export const usersMachine = createMachine( }, tags: "loading", }, - ready: { - on: { - GET_USERS: "gettingUsers", + creatingUser: { + invoke: { + src: "createUser", + id: "createUser", + onDone: { + target: "idle", + actions: ["displayCreateUserSuccess", "redirectToUsersPage", "clearCreateUserError"], + }, + onError: [ + { + target: "idle", + cond: "isFormError", + actions: ["assignCreateUserFormErrors"], + }, + { + target: "idle", + actions: ["assignCreateUserError"], + }, + ], }, + tags: "loading", }, error: { on: { @@ -62,6 +97,10 @@ export const usersMachine = createMachine( { services: { getUsers: API.getUsers, + createUser: (_, event) => API.createUser(event.user), + }, + guards: { + isFormError: (_, event) => isApiError(event.data), }, actions: { assignUsers: assign({ @@ -75,6 +114,20 @@ export const usersMachine = createMachine( ...context, getUsersError: undefined, })), + assignCreateUserError: assign({ + createUserError: (_, event) => event.data, + }), + assignCreateUserFormErrors: assign({ + // the guard ensures it is ApiError + createUserFormErrors: (_, event) => mapApiErrorToFieldErrors((event.data as ApiError).response.data), + }), + clearCreateUserError: assign((context: UsersContext) => ({ + ...context, + createUserError: undefined, + })), + displayCreateUserSuccess: () => { + displaySuccess(Language.createUserSuccess) + }, }, }, )