-
Notifications
You must be signed in to change notification settings - Fork 937
feat: add the preferences/account page #999
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
Changes from all commits
Commits
Show all changes
35 commits
Select commit
Hold shift + click to select a range
19579eb
feat: Add account form
BrunoQuaresma 9ab1f96
chore: Merge branch 'main' of github.com:coder/coder into bq/755/acco…
BrunoQuaresma a23cc88
Merge branch 'main' of github.com:coder/coder into bq/755/account-page
BrunoQuaresma f9f1c5a
feat: Add account form
BrunoQuaresma 1bbaf8a
chore: merge branch 'main' of github.com:coder/coder into bq/755/acco…
BrunoQuaresma fa1d0e6
feat: show notification when preferences are updated
BrunoQuaresma 1278ed6
test: account form submission with success
BrunoQuaresma 7ccf811
chore: remove unecessary timeout
BrunoQuaresma 2ae0987
test: add tests
BrunoQuaresma 4595186
style: fix message copy
BrunoQuaresma fa91276
style: improve success message
BrunoQuaresma fc01ff8
refactor: name is not optional
BrunoQuaresma f3fedd0
chore: move renderWithAuth to test_hepers/index.tsx
BrunoQuaresma 807d4e9
chore: move error types and utils to api/errors.ts
BrunoQuaresma fa580c7
test: use userEvent
BrunoQuaresma 3d76331
fix: remove async from onSubmit
BrunoQuaresma 37bc235
refactor: improve error types
BrunoQuaresma 17a0b16
chore: merge branch 'main' of github.com:coder/coder into bq/755/acco…
BrunoQuaresma 0e8ac63
chore: merge branch 'bq/755/account-page' of github.com:coder/coder i…
BrunoQuaresma 12058f8
refactor: api errors
BrunoQuaresma e489210
refactor: move UPDATE_PROFILE to idle state
BrunoQuaresma 8098628
refactor: change FormStack to Stack and add storybook
BrunoQuaresma 1f23e30
fix: error handling and tests
BrunoQuaresma a0588d1
feat: handle unknown error
BrunoQuaresma b3159d0
fix: make the eslint-disable inline
BrunoQuaresma e07d717
chore: rename story
BrunoQuaresma 4d7da77
chore: merge branch 'bq/755/account-page' of github.com:coder/coder i…
BrunoQuaresma 8d63848
Update site/src/xServices/auth/authXService.ts
BrunoQuaresma a11ff10
Update site/src/pages/preferences/account.tsx
BrunoQuaresma bde7c15
Fix errors
BrunoQuaresma 684b902
chore: merge branch 'main' of github.com:coder/coder into bq/755/acco…
BrunoQuaresma bbf2152
Fix type
BrunoQuaresma 7f32600
Fix forms
BrunoQuaresma eb65490
Normalize machine
BrunoQuaresma 59bac76
Fix: tests
BrunoQuaresma File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
import { isApiError, mapApiErrorToFieldErrors } from "./errors" | ||
|
||
describe("isApiError", () => { | ||
it("returns true when the object is an API Error", () => { | ||
expect( | ||
isApiError({ | ||
isAxiosError: true, | ||
response: { | ||
data: { | ||
message: "Invalid entry", | ||
errors: [{ detail: "Username is already in use", field: "username" }], | ||
}, | ||
}, | ||
}), | ||
).toBe(true) | ||
}) | ||
|
||
it("returns false when the object is Error", () => { | ||
expect(isApiError(new Error())).toBe(false) | ||
}) | ||
|
||
it("returns false when the object is undefined", () => { | ||
expect(isApiError(undefined)).toBe(false) | ||
}) | ||
}) | ||
|
||
describe("mapApiErrorToFieldErrors", () => { | ||
it("returns correct field errors", () => { | ||
expect( | ||
mapApiErrorToFieldErrors({ | ||
message: "Invalid entry", | ||
errors: [{ detail: "Username is already in use", field: "username" }], | ||
}), | ||
).toEqual({ | ||
username: "Username is already in use", | ||
}) | ||
}) | ||
}) | ||
BrunoQuaresma marked this conversation as resolved.
Show resolved
Hide resolved
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
import axios, { AxiosError, AxiosResponse } from "axios" | ||
|
||
export const Language = { | ||
errorsByCode: { | ||
defaultErrorCode: "Invalid value", | ||
}, | ||
} | ||
|
||
interface FieldError { | ||
field: string | ||
detail: string | ||
} | ||
|
||
type FieldErrors = Record<FieldError["field"], FieldError["detail"]> | ||
|
||
export interface ApiErrorResponse { | ||
message: string | ||
errors?: FieldError[] | ||
} | ||
|
||
export type ApiError = AxiosError<ApiErrorResponse> & { response: AxiosResponse<ApiErrorResponse> } | ||
BrunoQuaresma marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types, @typescript-eslint/no-explicit-any | ||
export const isApiError = (err: any): err is ApiError => { | ||
if (axios.isAxiosError(err)) { | ||
const response = err.response?.data | ||
|
||
return ( | ||
typeof response.message === "string" && (typeof response.errors === "undefined" || Array.isArray(response.errors)) | ||
) | ||
} | ||
|
||
return false | ||
} | ||
|
||
export const mapApiErrorToFieldErrors = (apiErrorResponse: ApiErrorResponse): FieldErrors => { | ||
BrunoQuaresma marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const result: FieldErrors = {} | ||
|
||
if (apiErrorResponse.errors) { | ||
for (const error of apiErrorResponse.errors) { | ||
result[error.field] = error.detail || Language.errorsByCode.defaultErrorCode | ||
} | ||
} | ||
|
||
return result | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,93 @@ | ||
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 { getFormHelpers, onChangeTrimmed } from "../Form" | ||
import { Stack } from "../Stack/Stack" | ||
import { LoadingButton } from "./../Button" | ||
|
||
interface AccountFormValues { | ||
name: string | ||
email: string | ||
username: string | ||
} | ||
|
||
export const Language = { | ||
nameLabel: "Name", | ||
usernameLabel: "Username", | ||
emailLabel: "Email", | ||
emailInvalid: "Please enter a valid email address.", | ||
emailRequired: "Please enter an email address.", | ||
updatePreferences: "Update preferences", | ||
} | ||
|
||
const validationSchema = Yup.object({ | ||
email: Yup.string().trim().email(Language.emailInvalid).required(Language.emailRequired), | ||
name: Yup.string().optional(), | ||
username: Yup.string().trim(), | ||
}) | ||
|
||
export type AccountFormErrors = FormikErrors<AccountFormValues> | ||
export interface AccountFormProps { | ||
isLoading: boolean | ||
initialValues: AccountFormValues | ||
onSubmit: (values: AccountFormValues) => void | ||
formErrors?: AccountFormErrors | ||
error?: string | ||
} | ||
|
||
export const AccountForm: React.FC<AccountFormProps> = ({ | ||
isLoading, | ||
onSubmit, | ||
initialValues, | ||
formErrors = {}, | ||
error, | ||
}) => { | ||
const form: FormikContextType<AccountFormValues> = useFormik<AccountFormValues>({ | ||
initialValues, | ||
validationSchema, | ||
onSubmit, | ||
}) | ||
|
||
return ( | ||
<> | ||
<form onSubmit={form.handleSubmit}> | ||
<Stack> | ||
<TextField | ||
{...getFormHelpers<AccountFormValues>(form, "name")} | ||
autoFocus | ||
autoComplete="name" | ||
fullWidth | ||
label={Language.nameLabel} | ||
variant="outlined" | ||
/> | ||
<TextField | ||
{...getFormHelpers<AccountFormValues>(form, "email", formErrors.email)} | ||
onChange={onChangeTrimmed(form)} | ||
autoComplete="email" | ||
fullWidth | ||
label={Language.emailLabel} | ||
variant="outlined" | ||
/> | ||
<TextField | ||
{...getFormHelpers<AccountFormValues>(form, "username", formErrors.username)} | ||
onChange={onChangeTrimmed(form)} | ||
autoComplete="username" | ||
fullWidth | ||
label={Language.usernameLabel} | ||
variant="outlined" | ||
/> | ||
|
||
{error && <FormHelperText error>{error}</FormHelperText>} | ||
|
||
<div> | ||
<LoadingButton color="primary" loading={isLoading} type="submit" variant="contained"> | ||
{isLoading ? "" : Language.updatePreferences} | ||
</LoadingButton> | ||
</div> | ||
</Stack> | ||
</form> | ||
</> | ||
) | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
import TextField from "@material-ui/core/TextField" | ||
import { Story } from "@storybook/react" | ||
import React from "react" | ||
import { Stack, StackProps } from "./Stack" | ||
|
||
export default { | ||
title: "Components/Stack", | ||
component: Stack, | ||
} | ||
|
||
const Template: Story<StackProps> = (args: StackProps) => ( | ||
<Stack {...args}> | ||
<TextField autoFocus autoComplete="name" fullWidth label="Name" variant="outlined" /> | ||
<TextField autoComplete="email" fullWidth label="Email" variant="outlined" /> | ||
<TextField autoComplete="username" fullWidth label="Username" variant="outlined" /> | ||
</Stack> | ||
) | ||
|
||
export const Example = Template.bind({}) | ||
Example.args = { | ||
spacing: 2, | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import { makeStyles } from "@material-ui/core/styles" | ||
import React from "react" | ||
|
||
export interface StackProps { | ||
spacing?: number | ||
} | ||
|
||
const useStyles = makeStyles((theme) => ({ | ||
stack: { | ||
display: "flex", | ||
flexDirection: "column", | ||
gap: ({ spacing }: { spacing: number }) => theme.spacing(spacing), | ||
}, | ||
})) | ||
|
||
export const Stack: React.FC<StackProps> = ({ children, spacing = 2 }) => { | ||
const styles = useStyles({ spacing }) | ||
return <div className={styles.stack}>{children}</div> | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,106 @@ | ||
import { fireEvent, screen, waitFor } from "@testing-library/react" | ||
import React from "react" | ||
import * as API from "../../api" | ||
import * as AccountForm from "../../components/Preferences/AccountForm" | ||
import { GlobalSnackbar } from "../../components/Snackbar/GlobalSnackbar" | ||
import { renderWithAuth } from "../../test_helpers" | ||
import * as AuthXService from "../../xServices/auth/authXService" | ||
import { Language, PreferencesAccountPage } from "./account" | ||
|
||
const renderPage = () => { | ||
return renderWithAuth( | ||
<> | ||
<PreferencesAccountPage /> | ||
<GlobalSnackbar /> | ||
</>, | ||
) | ||
} | ||
|
||
const newData = { | ||
name: "User", | ||
email: "user@coder.com", | ||
username: "user", | ||
} | ||
|
||
const fillAndSubmitForm = async () => { | ||
await waitFor(() => screen.findByLabelText("Name")) | ||
fireEvent.change(screen.getByLabelText("Name"), { target: { value: newData.name } }) | ||
fireEvent.change(screen.getByLabelText("Email"), { target: { value: newData.email } }) | ||
fireEvent.change(screen.getByLabelText("Username"), { target: { value: newData.username } }) | ||
BrunoQuaresma marked this conversation as resolved.
Show resolved
Hide resolved
|
||
fireEvent.click(screen.getByText(AccountForm.Language.updatePreferences)) | ||
} | ||
|
||
describe("PreferencesAccountPage", () => { | ||
describe("when it is a success", () => { | ||
it("shows the success message", async () => { | ||
jest.spyOn(API, "updateProfile").mockImplementationOnce((userId, data) => | ||
Promise.resolve({ | ||
id: userId, | ||
...data, | ||
created_at: new Date().toString(), | ||
}), | ||
) | ||
BrunoQuaresma marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const { user } = renderPage() | ||
await fillAndSubmitForm() | ||
|
||
const successMessage = await screen.findByText(AuthXService.Language.successProfileUpdate) | ||
expect(successMessage).toBeDefined() | ||
expect(API.updateProfile).toBeCalledTimes(1) | ||
expect(API.updateProfile).toBeCalledWith(user.id, newData) | ||
}) | ||
}) | ||
|
||
describe("when the email is already taken", () => { | ||
it("shows an error", async () => { | ||
jest.spyOn(API, "updateProfile").mockRejectedValueOnce({ | ||
isAxiosError: true, | ||
response: { | ||
data: { message: "Invalid profile", errors: [{ detail: "Email is already in use", field: "email" }] }, | ||
}, | ||
}) | ||
|
||
const { user } = renderPage() | ||
await fillAndSubmitForm() | ||
|
||
const errorMessage = await screen.findByText("Email is already in use") | ||
expect(errorMessage).toBeDefined() | ||
expect(API.updateProfile).toBeCalledTimes(1) | ||
expect(API.updateProfile).toBeCalledWith(user.id, newData) | ||
}) | ||
}) | ||
|
||
describe("when the username is already taken", () => { | ||
it("shows an error", async () => { | ||
jest.spyOn(API, "updateProfile").mockRejectedValueOnce({ | ||
isAxiosError: true, | ||
response: { | ||
data: { message: "Invalid profile", errors: [{ detail: "Username is already in use", field: "username" }] }, | ||
}, | ||
}) | ||
|
||
const { user } = renderPage() | ||
await fillAndSubmitForm() | ||
|
||
const errorMessage = await screen.findByText("Username is already in use") | ||
expect(errorMessage).toBeDefined() | ||
expect(API.updateProfile).toBeCalledTimes(1) | ||
expect(API.updateProfile).toBeCalledWith(user.id, newData) | ||
}) | ||
}) | ||
|
||
describe("when it is an unknown error", () => { | ||
it("shows a generic error message", async () => { | ||
jest.spyOn(API, "updateProfile").mockRejectedValueOnce({ | ||
data: "unknown error", | ||
}) | ||
|
||
const { user } = renderPage() | ||
await fillAndSubmitForm() | ||
|
||
const errorMessage = await screen.findByText(Language.unknownError) | ||
expect(errorMessage).toBeDefined() | ||
expect(API.updateProfile).toBeCalledTimes(1) | ||
expect(API.updateProfile).toBeCalledWith(user.id, newData) | ||
}) | ||
}) | ||
}) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love having these tests because it helps me see what the data structure looks like :)