fix: apply changes from previous PR

coder · Parkreiner · Oct 29, 2024 · Oct 24, 2024 · Oct 24, 2024 · Oct 24, 2024
commit 234c6060035f9532788991502673b6f1f8c4d2b3
diff --git a/site/e2e/global.setup.ts b/site/e2e/global.setup.ts
@@ -35,6 +35,7 @@ test("setup deployment", async ({ page }) => {
 		expect(constants.license.split(".").length).toBe(3); // otherwise it's invalid
 
 		await page.goto("/deployment/licenses", { waitUntil: "domcontentloaded" });
+		await expect(page).toHaveTitle("License Settings - Coder");
 
 		await page.getByText("Add a license").click();
 		await page.getByRole("textbox").fill(constants.license);

diff --git a/site/jest.setup.ts b/site/jest.setup.ts
@@ -1,7 +1,7 @@
 import "@testing-library/jest-dom";
 import "jest-location-mock";
 import { cleanup } from "@testing-library/react";
-import crypto from "crypto";
+import crypto from "node:crypto";
 import { useMemo } from "react";
 import type { Region } from "api/typesGenerated";
 import type { ProxyLatencyReport } from "contexts/useProxyLatency";
@@ -48,9 +48,7 @@ global.ResizeObserver = require("resize-observer-polyfill");
 // Polyfill the getRandomValues that is used on utils/random.ts
 Object.defineProperty(global.self, "crypto", {
 	value: {
-		getRandomValues: function (buffer: Buffer) {
-			return crypto.randomFillSync(buffer);
-		},
+		getRandomValues: crypto.randomFillSync,
 	},
 });
 
@@ -72,5 +70,5 @@ afterEach(() => {
 // Clean up after the tests are finished.
 afterAll(() => server.close());
 
-// This is needed because we are compiling under `--isolatedModules`
+// biome-ignore lint/complexity/noUselessEmptyExport: This is needed because we are compiling under `--isolatedModules`
 export {};
diff --git a/site/src/contexts/auth/permissions.tsx b/site/src/contexts/auth/permissions.tsx
@@ -1,3 +1,5 @@
+import type { AuthorizationCheck } from "api/typesGenerated";
+
 export const checks = {
 	viewAllUsers: "viewAllUsers",
 	updateUsers: "updateUsers",
@@ -11,13 +13,20 @@ export const checks = {
 	viewUpdateCheck: "viewUpdateCheck",
 	viewExternalAuthConfig: "viewExternalAuthConfig",
 	viewDeploymentStats: "viewDeploymentStats",
+	readWorkspaceProxies: "readWorkspaceProxies",
 	editWorkspaceProxies: "editWorkspaceProxies",
 	createOrganization: "createOrganization",
 	editAnyOrganization: "editAnyOrganization",
 	viewAnyGroup: "viewAnyGroup",
 	createGroup: "createGroup",
 	viewAllLicenses: "viewAllLicenses",
-} as const;
+	viewNotificationTemplate: "viewNotificationTemplate",
+} as const satisfies Record<string, string>;
+
+// Type expression seems a little redundant (`keyof typeof checks` has the same
+// result), just because each key-value pair is currently symmetrical; this may
+// change down the line
+type PermissionValue = (typeof checks)[keyof typeof checks];
 
 export const permissionsToCheck = {
 	[checks.viewAllUsers]: {
@@ -94,6 +103,12 @@ export const permissionsToCheck = {
 		},
 		action: "read",
 	},
+	[checks.readWorkspaceProxies]: {
+		object: {
+			resource_type: "workspace_proxy",
+		},
+		action: "read",
+	},
 	[checks.editWorkspaceProxies]: {
 		object: {
 			resource_type: "workspace_proxy",
@@ -116,7 +131,6 @@ export const permissionsToCheck = {
 	[checks.viewAnyGroup]: {
 		object: {
 			resource_type: "group",
-			org_id: "any",
 		},
 		action: "read",
 	},
@@ -132,6 +146,12 @@ export const permissionsToCheck = {
 		},
 		action: "read",
 	},
-} as const;
+	[checks.viewNotificationTemplate]: {
+		object: {
+			resource_type: "notification_template",
+		},
+		action: "read",
+	},
+} as const satisfies Record<PermissionValue, AuthorizationCheck>;
 
-export type Permissions = Record<keyof typeof permissionsToCheck, boolean>;
+export type Permissions = Record<PermissionValue, boolean>;
diff --git a/site/src/modules/management/DeploymentSettingsProvider.tsx b/site/src/modules/management/DeploymentSettingsProvider.tsx
@@ -20,7 +20,7 @@ export const useDeploymentSettings = (): DeploymentSettingsValue => {
 	const context = useContext(DeploymentSettingsContext);
 	if (!context) {
 		throw new Error(
-			"useDeploymentSettings should be used inside of DeploymentSettingsLayout",
+			`${useDeploymentSettings.name} should be used inside of ${DeploymentSettingsProvider.name}`,
 		);
 	}
 
@@ -39,6 +39,9 @@ const DeploymentSettingsProvider: FC = () => {
 		permissions.editAnyOrganization ||
 		permissions.viewAnyAuditLog;
 
+	// Not a huge problem to unload the content in the event of an error,
+	// because the sidebar rendering isn't tied to this. Even if the user hits
+	// a 403 error, they'll still have navigation options
 	if (deploymentConfigQuery.error) {
 		return <ErrorAlert error={deploymentConfigQuery.error} />;
 	}

diff --git a/site/src/modules/management/ManagementSettingsLayout.tsx b/site/src/modules/management/ManagementSettingsLayout.tsx
@@ -74,7 +74,7 @@ const ManagementSettingsLayout: FC = () => {
 				<Margins>
 					<Stack css={{ padding: "48px 0" }} direction="row" spacing={6}>
 						<Sidebar />
-						<main css={{ width: "100%" }}>
+						<main css={{ flexGrow: 1 }}>
 							<Suspense fallback={<Loader />}>
 								<Outlet />
 							</Suspense>

diff --git a/site/src/modules/management/SidebarView.stories.tsx b/site/src/modules/management/SidebarView.stories.tsx
@@ -1,5 +1,6 @@
 import type { Meta, StoryObj } from "@storybook/react";
 import {
+	MockNoPermissions,
 	MockOrganization,
 	MockOrganization2,
 	MockPermissions,
@@ -96,7 +97,7 @@ export const NoDeploymentValues: Story = {
 
 export const NoPermissions: Story = {
 	args: {
-		permissions: {},
+		permissions: MockNoPermissions,
 	},
 };
 

diff --git a/site/src/modules/management/SidebarView.tsx b/site/src/modules/management/SidebarView.tsx
@@ -12,9 +12,9 @@ import { Loader } from "components/Loader/Loader";
 import { Sidebar as BaseSidebar } from "components/Sidebar/Sidebar";
 import { Stack } from "components/Stack/Stack";
 import { UserAvatar } from "components/UserAvatar/UserAvatar";
+import type { Permissions } from "contexts/auth/permissions";
 import { type ClassName, useClassName } from "hooks/useClassName";
 import { useDashboard } from "modules/dashboard/useDashboard";
-import { linkToUsers } from "modules/navigation";
 import type { FC, ReactNode } from "react";
 import { Link, NavLink } from "react-router-dom";
 
@@ -30,7 +30,7 @@ interface SidebarProps {
 	/** Organizations and their permissions or undefined if still fetching. */
 	organizations: OrganizationWithPermissions[] | undefined;
 	/** Site-wide permissions. */
-	permissions: AuthorizationResponse;
+	permissions: Permissions;
 }
 
 /**
@@ -72,7 +72,7 @@ interface DeploymentSettingsNavigationProps {
 	/** Whether a deployment setting page is being viewed. */
 	active: boolean;
 	/** Site-wide permissions. */
-	permissions: AuthorizationResponse;
+	permissions: Permissions;
 }
 
 /**
@@ -130,10 +130,11 @@ const DeploymentSettingsNavigation: FC<DeploymentSettingsNavigationProps> = ({
 					{permissions.viewDeploymentValues && (
 						<SidebarNavSubItem href="network">Network</SidebarNavSubItem>
 					)}
-					{/* All users can view workspace regions.  */}
-					<SidebarNavSubItem href="workspace-proxies">
-						Workspace Proxies
-					</SidebarNavSubItem>
+					{permissions.readWorkspaceProxies && (
+						<SidebarNavSubItem href="workspace-proxies">
+							Workspace Proxies
+						</SidebarNavSubItem>
+					)}
 					{permissions.viewDeploymentValues && (
 						<SidebarNavSubItem href="security">Security</SidebarNavSubItem>
 					)}
@@ -145,12 +146,14 @@ const DeploymentSettingsNavigation: FC<DeploymentSettingsNavigationProps> = ({
 					{permissions.viewAllUsers && (
 						<SidebarNavSubItem href="users">Users</SidebarNavSubItem>
 					)}
-					<SidebarNavSubItem href="notifications">
-						<Stack direction="row" alignItems="center" spacing={1}>
-							<span>Notifications</span>
-							<FeatureStageBadge contentType="beta" size="sm" />
-						</Stack>
-					</SidebarNavSubItem>
+					{permissions.viewNotificationTemplate && (
+						<SidebarNavSubItem href="notifications">
+							<Stack direction="row" alignItems="center" spacing={1}>
+								<span>Notifications</span>
+								<FeatureStageBadge contentType="beta" size="sm" />
+							</Stack>
+						</SidebarNavSubItem>
+					)}
 				</Stack>
 			)}
 		</div>
@@ -167,7 +170,7 @@ interface OrganizationsSettingsNavigationProps {
 	/** Organizations and their permissions or undefined if still fetching. */
 	organizations: OrganizationWithPermissions[] | undefined;
 	/** Site-wide permissions. */
-	permissions: AuthorizationResponse;
+	permissions: Permissions;
 }
 
 /**
@@ -241,8 +244,6 @@ interface OrganizationSettingsNavigationProps {
 const OrganizationSettingsNavigation: FC<
 	OrganizationSettingsNavigationProps
 > = ({ active, organization }) => {
-	const { experiments } = useDashboard();
-
 	return (
 		<>
 			<SidebarNavItem

diff --git a/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPage.tsx
@@ -13,12 +13,7 @@ const ExternalAuthSettingsPage: FC = () => {
 			<Helmet>
 				<title>{pageTitle("External Authentication Settings")}</title>
 			</Helmet>
-
-			{deploymentConfig ? (
-				<ExternalAuthSettingsPageView config={deploymentConfig.config} />
-			) : (
-				<Loader />
-			)}
+			<ExternalAuthSettingsPageView config={deploymentConfig.config} />
 		</>
 	);
 };

diff --git a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx
@@ -1,7 +1,6 @@
 import { deploymentDAUs } from "api/queries/deployment";
 import { entitlements } from "api/queries/entitlements";
 import { availableExperiments, experiments } from "api/queries/experiments";
-import { Loader } from "components/Loader/Loader";
 import { useEmbeddedMetadata } from "hooks/useEmbeddedMetadata";
 import type { FC } from "react";
 import { Helmet } from "react-helmet-async";
@@ -30,18 +29,14 @@ const GeneralSettingsPage: FC = () => {
 			<Helmet>
 				<title>{pageTitle("General Settings")}</title>
 			</Helmet>
-			{deploymentConfig ? (
-				<GeneralSettingsPageView
-					deploymentOptions={deploymentConfig.options}
-					deploymentDAUs={deploymentDAUsQuery.data}
-					deploymentDAUsError={deploymentDAUsQuery.error}
-					entitlements={entitlementsQuery.data}
-					invalidExperiments={invalidExperiments}
-					safeExperiments={safeExperiments}
-				/>
-			) : (
-				<Loader />
-			)}
+			<GeneralSettingsPageView
+				deploymentOptions={deploymentConfig.options}
+				deploymentDAUs={deploymentDAUsQuery.data}
+				deploymentDAUsError={deploymentDAUsQuery.error}
+				entitlements={entitlementsQuery.data}
+				invalidExperiments={invalidExperiments}
+				safeExperiments={safeExperiments}
+			/>
 		</>
 	);
 };

diff --git a/site/src/pages/DeploymentSettingsPage/NetworkSettingsPage/NetworkSettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/NetworkSettingsPage/NetworkSettingsPage.tsx
@@ -13,12 +13,7 @@ const NetworkSettingsPage: FC = () => {
 			<Helmet>
 				<title>{pageTitle("Network Settings")}</title>
 			</Helmet>
-
-			{deploymentConfig ? (
-				<NetworkSettingsPageView options={deploymentConfig.options} />
-			) : (
-				<Loader />
-			)}
+			<NetworkSettingsPageView options={deploymentConfig.options} />
 		</>
 	);
 };

diff --git a/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationsPage.tsx
@@ -11,15 +11,14 @@ import { Section } from "pages/UserSettingsPage/Section";
 import type { FC } from "react";
 import { Helmet } from "react-helmet-async";
 import { useQueries } from "react-query";
-import { useSearchParams } from "react-router-dom";
 import { deploymentGroupHasParent } from "utils/deployOptions";
 import { pageTitle } from "utils/page";
 import OptionsTable from "../OptionsTable";
 import { NotificationEvents } from "./NotificationEvents";
 import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider";
+import { useSearchParamsKey } from "hooks/useSearchParamsKey";
 
 export const NotificationsPage: FC = () => {
-	const [searchParams] = useSearchParams();
 	const { deploymentConfig } = useDeploymentSettings();
 	const [templatesByGroup, dispatchMethods] = useQueries({
 		queries: [
@@ -30,10 +29,12 @@ export const NotificationsPage: FC = () => {
 			notificationDispatchMethods(),
 		],
 	});
-	const ready =
-		templatesByGroup.data && dispatchMethods.data && deploymentConfig;
-	const tab = searchParams.get("tab") || "events";
+	const tabState = useSearchParamsKey({
+		key: "tab",
+		defaultValue: "events",
+	});
 
+	const ready = !!(templatesByGroup.data && dispatchMethods.data);
 	return (
 		<>
 			<Helmet>
@@ -45,7 +46,7 @@ export const NotificationsPage: FC = () => {
 				layout="fluid"
 				featureStage={"beta"}
 			>
-				<Tabs active={tab}>
+				<Tabs active={tabState.value}>
 					<TabsList>
 						<TabLink to="?tab=events" value="events">
 							Events
@@ -58,7 +59,7 @@ export const NotificationsPage: FC = () => {
 
 				<div css={styles.content}>
 					{ready ? (
-						tab === "events" ? (
+						tabState.value === "events" ? (
 							<NotificationEvents
 								templatesByGroup={templatesByGroup.data}
 								deploymentConfig={deploymentConfig.config}
@@ -71,7 +72,7 @@ export const NotificationsPage: FC = () => {
 							/>
 						) : (
 							<OptionsTable
-								options={deploymentConfig?.options.filter((o) =>
+								options={deploymentConfig.options.filter((o) =>
 									deploymentGroupHasParent(o.group, "Notifications"),
 								)}
 							/>

diff --git a/.../src/pages/DeploymentSettingsPage/ObservabilitySettingsPage/ObservabilitySettingsPage.tsx b/.../src/pages/DeploymentSettingsPage/ObservabilitySettingsPage/ObservabilitySettingsPage.tsx
@@ -1,4 +1,3 @@
-import { Loader } from "components/Loader/Loader";
 import { useDashboard } from "modules/dashboard/useDashboard";
 import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility";
 import type { FC } from "react";
@@ -17,16 +16,11 @@ const ObservabilitySettingsPage: FC = () => {
 			<Helmet>
 				<title>{pageTitle("Observability Settings")}</title>
 			</Helmet>
-
-			{deploymentConfig ? (
-				<ObservabilitySettingsPageView
-					options={deploymentConfig.options}
-					featureAuditLogEnabled={entitlements.features.audit_log.enabled}
-					isPremium={hasPremiumLicense}
-				/>
-			) : (
-				<Loader />
-			)}
+			<ObservabilitySettingsPageView
+				options={deploymentConfig.options}
+				featureAuditLogEnabled={entitlements.features.audit_log.enabled}
+				isPremium={hasPremiumLicense}
+			/>
 		</>
 	);
 };

diff --git a/site/src/pages/DeploymentSettingsPage/SecuritySettingsPage/SecuritySettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/SecuritySettingsPage/SecuritySettingsPage.tsx
@@ -15,15 +15,10 @@ const SecuritySettingsPage: FC = () => {
 			<Helmet>
 				<title>{pageTitle("Security Settings")}</title>
 			</Helmet>
-
-			{deploymentConfig ? (
-				<SecuritySettingsPageView
-					options={deploymentConfig.options}
-					featureBrowserOnlyEnabled={entitlements.features.browser_only.enabled}
-				/>
-			) : (
-				<Loader />
-			)}
+			<SecuritySettingsPageView
+				options={deploymentConfig.options}
+				featureBrowserOnlyEnabled={entitlements.features.browser_only.enabled}
+			/>
 		</>
 	);
 };