Feb 21, 2025 · Feb 20, 2025 · Feb 20, 2025 · Feb 20, 2025 · Feb 21, 2025 · Feb 21, 2025
diff --git a/coderd/httpapi/httpapi.go b/coderd/httpapi/httpapi.go
 func Forbidden(rw http.ResponseWriter) {
 Write(context.Background(), rw, http.StatusForbidden, codersdk.Response{
 Message: "Forbidden.",
 Detail:  "You don't have permission to view this page. If you believe this is a mistake, please contact your administrator or try signing in with different credentials.",
 })
 }

diff --git a/site/e2e/setup/addUsersAndLicense.spec.ts b/site/e2e/setup/addUsersAndLicense.spec.ts
 import { expect, test } from "@playwright/test";
 import { API } from "api/api";
 import { Language } from "pages/CreateUserPage/CreateUserForm";
 import { Language } from "pages/CreateUserPage/Language";
 import { coderPort, license, premiumTestsRequired, users } from "../constants";
 import { expectUrl } from "../expectUrl";
 import { createUser } from "../helpers";
diff --git a/site/src/api/errors.ts b/site/src/api/errors.ts
 return undefined;
 };

 export const getErrorStatus = (error: unknown): number | undefined => {
 if (isApiError(error)) {
 return error.status;
 }

 return undefined;
 };

 export class DetailedError extends Error {
 constructor(
 message: string,
diff --git a/site/src/components/Alert/ErrorAlert.tsx b/site/src/components/Alert/ErrorAlert.tsx
 import AlertTitle from "@mui/material/AlertTitle";
 import { getErrorDetail, getErrorMessage } from "api/errors";
 import { getErrorDetail, getErrorMessage, getErrorStatus } from "api/errors";
 import type { FC } from "react";
 import { Link } from "../Link/Link";
 import { Alert, AlertDetail, type AlertProps } from "./Alert";

 export const ErrorAlert: FC<
 Omit<AlertProps, "severity" | "children"> & { error: unknown }
 > = ({ error, ...alertProps }) => {
 const message = getErrorMessage(error, "Something went wrong.");
 const detail = getErrorDetail(error);
 const status = getErrorStatus(error);

 // For some reason, the message and detail can be the same on the BE, but does
 // not make sense in the FE to showing them duplicated
 const shouldDisplayDetail = message !== detail;

 return (
 <Alert severity="error" {...alertProps}>
 {detail ? (
 const body = () => {
 // When the error is a Forbidden response we include a link for the user to
 // go back to a known viewable page.
 // Additionally since the error messages and details from the server can be
 // missing or confusing for an end user we render a friendlier message
 // regardless of the response from the server.
 if (status === 403) {
 return (
 <>
 <AlertTitle>{message}</AlertTitle>
 <AlertDetail>
 {detail}{" "}
 <Link href="/workspaces" className="w-fit">
 Go to workspaces
 </Link>
 </AlertDetail>
 </>
 );
 }

 if (detail) {
 return (
 <>
 <AlertTitle>{message}</AlertTitle>
 {shouldDisplayDetail && <AlertDetail>{detail}</AlertDetail>}
 </>
 ) : (
 message
 )}
 );
 }

 return message;
 };

 return (
 <Alert severity="error" {...alertProps}>
 {body()}
 </Alert>
 );
 };
diff --git a/site/src/pages/CreateUserPage/CreateUserForm.tsx b/site/src/pages/CreateUserPage/CreateUserForm.tsx
 onChangeTrimmed,
 } from "utils/formUtils";
 import * as Yup from "yup";

 export const Language = {
 emailLabel: "Email",
 passwordLabel: "Password",
 usernameLabel: "Username",
 nameLabel: "Full name",
 emailInvalid: "Please enter a valid email address.",
 emailRequired: "Please enter an email address.",
 passwordRequired: "Please enter a password.",
 createUser: "Create",
 cancel: "Cancel",
 };
 import { Language } from "./Language";

 export const authMethodLanguage = {
 password: {
diff --git a/site/src/pages/CreateUserPage/CreateUserPage.test.tsx b/site/src/pages/CreateUserPage/CreateUserPage.test.tsx
 renderWithAuth,
 waitForLoaderToBeRemoved,
 } from "testHelpers/renderHelpers";
 import { Language as FormLanguage } from "./CreateUserForm";
 import { CreateUserPage } from "./CreateUserPage";
 import { Language as FormLanguage } from "./Language";

 const renderCreateUserPage = async () => {
 renderWithAuth(<CreateUserPage />, {
diff --git a/site/src/pages/CreateUserPage/Language.ts b/site/src/pages/CreateUserPage/Language.ts
 export const Language = {
 emailLabel: "Email",
 passwordLabel: "Password",
 usernameLabel: "Username",
 nameLabel: "Full name",
 emailInvalid: "Please enter a valid email address.",
 emailRequired: "Please enter an email address.",
 passwordRequired: "Please enter a password.",
 createUser: "Create",
 cancel: "Cancel",
 };
Original file line number	Diff line number	Diff line change
		@@ -1,6 +1,6 @@
		import { expect, test } from "@playwright/test";
		import { API } from "api/api";
		import { Language } from "pages/CreateUserPage/CreateUserForm";
		import { Language } from "pages/CreateUserPage/Language";
		import { coderPort, license, premiumTestsRequired, users } from "../constants";
		import { expectUrl } from "../expectUrl";
		import { createUser } from "../helpers";
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -133,6 +133,14 @@ export const getErrorDetail = (error: unknown): string \| undefined => {
		return undefined;
		};

		export const getErrorStatus = (error: unknown): number \| undefined => {
		if (isApiError(error)) {
		return error.status;
		}

		return undefined;
		};

		export class DetailedError extends Error {
		constructor(
		message: string,
Expand Down
Original file line number	Diff line number	Diff line change
		@@ -1,28 +1,55 @@
		import AlertTitle from "@mui/material/AlertTitle";
		import { getErrorDetail, getErrorMessage } from "api/errors";
		import { getErrorDetail, getErrorMessage, getErrorStatus } from "api/errors";
		import type { FC } from "react";
		import { Link } from "../Link/Link";
		import { Alert, AlertDetail, type AlertProps } from "./Alert";

		export const ErrorAlert: FC<
		Omit<AlertProps, "severity" \| "children"> & { error: unknown }
		> = ({ error, ...alertProps }) => {
		const message = getErrorMessage(error, "Something went wrong.");
		const detail = getErrorDetail(error);
		const status = getErrorStatus(error);

		// For some reason, the message and detail can be the same on the BE, but does
		// not make sense in the FE to showing them duplicated
		const shouldDisplayDetail = message !== detail;

		return (
		<Alert severity="error" {...alertProps}>
		{detail ? (
		const body = () => {
		// When the error is a Forbidden response we include a link for the user to
		// go back to a known viewable page.
		// Additionally since the error messages and details from the server can be
		// missing or confusing for an end user we render a friendlier message
		// regardless of the response from the server.
		if (status === 403) {
Copy link Member ParkreinerFeb 21, 2025 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I'm okay with this as a baseline solution, but do you know if there are ever cases where it makes sense to redirect the user to a page more relevant to what they were trying to access? Copy link ContributorAuthor brettkolodnyFeb 21, 2025 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Yeah I agree, and I'm actually working on a PR right now that will make`RequirePermissions` a bit more user friendly in that way. But this change is still relevant in the cases where we don't wrap a portion of the page in`RequirePermissions`
		return (
		<>
		<AlertTitle>{message}</AlertTitle>
		<AlertDetail>
		{detail}{" "}
Copy link Member aslilacFeb 21, 2025 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I know theoretically we're now always setting`Detail` for 403 responses, but what if it comes back undefined from some endpoint that doesn't use`httpapi.Forbidden` when it should? do we care about handling that? maybe we don't and it's fine 🤷‍♀️
		<Link href="/workspaces" className="w-fit">
		Go to workspaces
		</Link>
		</AlertDetail>
		</>
		);
		}

		if (detail) {
		return (
		<>
		<AlertTitle>{message}</AlertTitle>
		{shouldDisplayDetail && <AlertDetail>{detail}</AlertDetail>}
		</>
		) : (
		message
		)}
		);
		}

		return message;
		};

		return (
		<Alert severity="error" {...alertProps}>
		{body()}
Copy link Member ParkreinerFeb 21, 2025• edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Nit: I don't know your opinion on ternaries, but I feel like the typical React way of doing this would be to inline everything into a single return statement: return(<Alertseverity="error"{...alterProps}>{status===403 ?(<--JSX-->) :detail ?(<--JSX-->) :(message)</Alert>); Prettier/Biome should be able to format the ternaries to make them more readable than this, but you get the idea Copy link ContributorAuthor brettkolodnyFeb 21, 2025 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I'm usually pro ternary but I've been trying to match other general vibes of the code base. Generally though I think I've started to learn towards if the logic is more than an "if/else" it's more readable being broken out
		</Alert>
		);
		};
Original file line number	Diff line number	Diff line change
Expand Up		@@ -19,18 +19,7 @@ import {
		onChangeTrimmed,
		} from "utils/formUtils";
		import * as Yup from "yup";

		export const Language = {
		emailLabel: "Email",
		passwordLabel: "Password",
		usernameLabel: "Username",
		nameLabel: "Full name",
		emailInvalid: "Please enter a valid email address.",
		emailRequired: "Please enter an email address.",
		passwordRequired: "Please enter a password.",
		createUser: "Create",
		cancel: "Cancel",
		};
		import { Language } from "./Language";

		export const authMethodLanguage = {
		password: {
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -4,8 +4,8 @@ import {
		renderWithAuth,
		waitForLoaderToBeRemoved,
		} from "testHelpers/renderHelpers";
		import { Language as FormLanguage } from "./CreateUserForm";
		import { CreateUserPage } from "./CreateUserPage";
		import { Language as FormLanguage } from "./Language";

		const renderCreateUserPage = async () => {
		renderWithAuth(<CreateUserPage />, {
Expand Down
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,11 @@
		export const Language = {
		emailLabel: "Email",
		passwordLabel: "Password",
		usernameLabel: "Username",
		nameLabel: "Full name",
		emailInvalid: "Please enter a valid email address.",
		emailRequired: "Please enter an email address.",
		passwordRequired: "Please enter a password.",
		createUser: "Create",
		cancel: "Cancel",
		};