fix: update redirects for /organizations routes

coder · Parkreiner · Oct 29, 2024 · Oct 24, 2024 · Oct 24, 2024 · Oct 24, 2024
commit a8fd9e640d58d910df88ce04c57fc958392b4ee3
diff --git a/site/src/modules/management/ManagementSettingsLayout.tsx b/site/src/modules/management/ManagementSettingsLayout.tsx
@@ -12,6 +12,7 @@ import { useQuery } from "react-query";
 import { Outlet, useLocation } from "react-router-dom";
 import { Sidebar } from "./Sidebar";
 import type { Permissions } from "contexts/auth/permissions";
+import { useDashboard } from "modules/dashboard/useDashboard";
 
 type ManagementSettingsValue = Readonly<{
 	deploymentValues: DeploymentConfig | undefined;
@@ -47,14 +48,25 @@ export const canEditOrganization = (
 };
 
 const isManagementRoutePermitted = (
-	href: string,
+	locationPath: string,
 	permissions: Permissions,
+	showOrganizations: boolean,
 ): boolean => {
-	console.log(href);
+	if (locationPath.startsWith("/organizations")) {
+		return showOrganizations;
+	}
+
+	if (!locationPath.startsWith("/deployment")) {
+		return false;
+	}
 
-	// Switch logic should mirror the conditions used to display the sidebar
-	// tabs from SidebarView.tsx
+	// Switch logic for deployment routes should mirror the conditions used to
+	// display the sidebar tabs from SidebarView.tsx
+	const href = locationPath.replace(/^\/deployment/, "");
 	switch (href) {
+		case "/": {
+			return true;
+		}
 		case "/general": {
 			return permissions.viewDeploymentValues;
 		}
@@ -102,6 +114,7 @@ const isManagementRoutePermitted = (
  */
 export const ManagementSettingsLayout: FC = () => {
 	const location = useLocation();
+	const { showOrganizations } = useDashboard();
 	const { permissions } = useAuthenticated();
 	const deploymentConfigQuery = useQuery({
 		...deploymentConfig(),
@@ -115,7 +128,6 @@ export const ManagementSettingsLayout: FC = () => {
 		return <Loader />;
 	}
 
-	const href = location.pathname.replace(/^\/?deployment/, "");
 	const canViewAtLeastOneTab =
 		permissions.viewDeploymentValues ||
 		permissions.viewAllUsers ||
@@ -125,23 +137,16 @@ export const ManagementSettingsLayout: FC = () => {
 	return (
 		<RequirePermission
 			permitted={
-				canViewAtLeastOneTab && isManagementRoutePermitted(href, permissions)
+				canViewAtLeastOneTab &&
+				isManagementRoutePermitted(
+					location.pathname,
+					permissions,
+					showOrganizations,
+				)
 			}
 			unpermittedRedirect={
-				/**
-				 * @todo 2024-10-14 - MES - The router logic is set up so that
-				 * if the user goes to the /deployment page, they get redirected
-				 * to the general page, purely for UX reasons (the /deployment
-				 * page is empty). But it's not guaranteed that the user can see
-				 * that page.
-				 *
-				 * Might be good to remove that router logic, and add a basic
-				 * "landing" page to the /deployment route so that we can safely
-				 * redirect the user there. But that will require coordination
-				 * with the design team.
-				 */
-				canViewAtLeastOneTab && href !== "/general"
-					? "/deployment/general"
+				canViewAtLeastOneTab && !location.pathname.startsWith("/deployment")
+					? "/deployment"
 					: "/workspaces"
 			}
 		>

diff --git a/site/src/router.tsx b/site/src/router.tsx
@@ -426,16 +426,24 @@ export const router = createBrowserRouter(
 						</Route>
 					</Route>
 
+					{/**
+					 * @todo 2024-10-24 - MES - The current deployment page is
+					 * almost empty, which isn't great for two reasons:
+					 *
+					 * 1. Even though none of our links lead to the page, the
+					 *    user is still able to navigate straight there by
+					 *    accident.
+					 * 2. If the user has access to some deployment pages, but
+					 *    tries accessing one that they shouldn't be able to
+					 *    access, we have to reroute them somewhere. The base
+					 *    deployment route is the only safe option, because not
+					 *    even the general page is accessible by everyone.
+					 *
+					 * Should update the base /deployment page so that there's
+					 * something there, but that will require coordination with
+					 * the design team.
+					 */}
 					<Route path="/deployment" element={<ManagementSettingsLayout />}>
-						{/*
-						  None of the UI elements link directly to the base
-						  /deployment route, but if you navigate there directly,
-						  you would just see a mostly empty screen. Redirect to
-						  the general page for better UX. */}
-						<Route
-							path=""
-							element={<Navigate to="/deployment/general" replace />}
-						/>
 						<Route path="general" element={<GeneralSettingsPage />} />
 						<Route path="licenses" element={<LicensesSettingsPage />} />
 						<Route path="licenses/add" element={<AddNewLicensePage />} />