Skip to content

chore: consolidate ManageSettingsLayout code #14885

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 17 commits into from
Oct 3, 2024
8 changes: 6 additions & 2 deletions site/e2e/tests/users/createUserWithPassword.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import { beforeCoderTest } from "../../hooks";
test.beforeEach(async ({ page }) => await beforeCoderTest(page));

test("create user with password", async ({ page, baseURL }) => {
await page.goto(`${baseURL}/users`, { waitUntil: "domcontentloaded" });
await page.goto(`${baseURL}/deployment/users`, {
waitUntil: "domcontentloaded",
});
await expect(page).toHaveTitle("Users - Coder");

await page.getByRole("button", { name: "Create user" }).click();
Expand Down Expand Up @@ -37,7 +39,9 @@ test("create user with password", async ({ page, baseURL }) => {
});

test("create user without full name is optional", async ({ page, baseURL }) => {
await page.goto(`${baseURL}/users`, { waitUntil: "domcontentloaded" });
await page.goto(`${baseURL}/deployment/users`, {
waitUntil: "domcontentloaded",
});
await expect(page).toHaveTitle("Users - Coder");

await page.getByRole("button", { name: "Create user" }).click();
Expand Down
2 changes: 1 addition & 1 deletion site/src/modules/dashboard/Navbar/DeploymentDropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ const DeploymentDropdownContent: FC<DeploymentDropdownProps> = ({
{canViewAllUsers && (
<MenuItem
component={NavLink}
to={canViewOrganizations ? `/deployment${linkToUsers}` : linkToUsers}
to={linkToUsers}
css={styles.menuItem}
onClick={onPopoverClose}
>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,24 +1,37 @@
import type { DeploymentConfig } from "api/api";
import { deploymentConfig } from "api/queries/deployment";
import type { AuthorizationResponse, Organization } from "api/typesGenerated";
import { ErrorAlert } from "components/Alert/ErrorAlert";
import { Loader } from "components/Loader/Loader";
import { Margins } from "components/Margins/Margins";
import { Stack } from "components/Stack/Stack";
import { useAuthenticated } from "contexts/auth/RequireAuth";
import { RequirePermission } from "contexts/auth/RequirePermission";
import { useDashboard } from "modules/dashboard/useDashboard";
import { type FC, Suspense } from "react";
import { type FC, Suspense, createContext, useContext } from "react";
import { useQuery } from "react-query";
import { Outlet } from "react-router-dom";
import { DeploySettingsContext } from "../DeploySettingsPage/DeploySettingsLayout";
import { Outlet, useParams } from "react-router-dom";
import { Sidebar } from "./Sidebar";

type OrganizationSettingsValue = Readonly<{
export const ManagementSettingsContext = createContext<
ManagementSettingsValue | undefined
>(undefined);

type ManagementSettingsValue = Readonly<{
deploymentValues: DeploymentConfig;
organizations: readonly Organization[];
organization?: Organization;
}>;

export const useOrganizationSettings = (): OrganizationSettingsValue => {
const { organizations } = useDashboard();
return { organizations };
export const useManagementSettings = (): ManagementSettingsValue => {
const context = useContext(ManagementSettingsContext);
if (!context) {
throw new Error(
"useManagementSettings should be used inside of ManagementSettingsLayout",
);
}

return context;
};

/**
Expand All @@ -43,13 +56,11 @@ export const canEditOrganization = (
*/
export const ManagementSettingsLayout: FC = () => {
const { permissions } = useAuthenticated();
const deploymentConfigQuery = useQuery(
// TODO: This is probably normally fine because we will not show links to
// pages that need this data, but if you manually visit the page you
// will see an endless loader when maybe we should show a "permission
// denied" error or at least a 404 instead.
permissions.viewDeploymentValues ? deploymentConfig() : { enabled: false },
);
const deploymentConfigQuery = useQuery(deploymentConfig());
const { organizations } = useDashboard();
const { organization: orgName } = useParams() as {
organization?: string;
};

// The deployment settings page also contains users, audit logs, groups and
// organizations, so this page must be visible if you can see any of these.
Expand All @@ -59,24 +70,39 @@ export const ManagementSettingsLayout: FC = () => {
permissions.editAnyOrganization ||
permissions.viewAnyAuditLog;

if (deploymentConfigQuery.error) {
return <ErrorAlert error={deploymentConfigQuery.error} />;
}

if (!deploymentConfigQuery.data) {
return <Loader />;
}

const organization =
organizations && orgName
? organizations.find((org) => org.name === orgName)
: undefined;

return (
<RequirePermission isFeatureVisible={canViewDeploymentSettingsPage}>
<Margins>
<Stack css={{ padding: "48px 0" }} direction="row" spacing={6}>
<Sidebar />
<main css={{ width: "100%" }}>
<DeploySettingsContext.Provider
value={{
deploymentValues: deploymentConfigQuery.data,
}}
>
<ManagementSettingsContext.Provider
value={{
deploymentValues: deploymentConfigQuery.data,
organizations,
organization,
}}
>
<Margins>
<Stack css={{ padding: "48px 0" }} direction="row" spacing={6}>
<Sidebar />
<main css={{ width: "100%" }}>
<Suspense fallback={<Loader />}>
<Outlet />
</Suspense>
</DeploySettingsContext.Provider>
</main>
</Stack>
</Margins>
</main>
</Stack>
</Margins>
</ManagementSettingsContext.Provider>
</RequirePermission>
);
};
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { organizationsPermissions } from "api/queries/organizations";
import { useAuthenticated } from "contexts/auth/RequireAuth";
import { useDashboard } from "modules/dashboard/useDashboard";
import {
canEditOrganization,
useManagementSettings,
} from "modules/management/ManagementSettingsLayout";
import type { FC } from "react";
import { useQuery } from "react-query";
import { useLocation, useParams } from "react-router-dom";
import {
canEditOrganization,
useOrganizationSettings,
} from "./ManagementSettingsLayout";
import { type OrganizationWithPermissions, SidebarView } from "./SidebarView";

/**
Expand All @@ -20,8 +20,7 @@ import { type OrganizationWithPermissions, SidebarView } from "./SidebarView";
export const Sidebar: FC = () => {
const location = useLocation();
const { permissions } = useAuthenticated();
const { experiments } = useDashboard();
const { organizations } = useOrganizationSettings();
const { organizations } = useManagementSettings();
const { organization: organizationName } = useParams() as {
organization?: string;
};
Expand Down Expand Up @@ -56,7 +55,6 @@ export const Sidebar: FC = () => {
activeOrganizationName={organizationName}
organizations={editableOrgs}
permissions={permissions}
experiments={experiments}
/>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ import { withDashboardProvider } from "testHelpers/storybook";
import { SidebarView } from "./SidebarView";

const meta: Meta<typeof SidebarView> = {
title: "components/MultiOrgSidebarView",
title: "modules/management/SidebarView",
component: SidebarView,
decorators: [withDashboardProvider],
parameters: { showOrganizations: true },
args: {
activeSettings: true,
activeOrganizationName: undefined,
Expand All @@ -35,7 +36,6 @@ const meta: Meta<typeof SidebarView> = {
},
],
permissions: MockPermissions,
experiments: ["notifications"],
},
};

Expand Down Expand Up @@ -223,3 +223,9 @@ export const SelectedMultiOrgAdminAndUserAdmin: Story = {
],
},
};

export const OrgsDisabled: Story = {
parameters: {
showOrganizations: false,
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ interface SidebarProps {
organizations: OrganizationWithPermissions[] | undefined;
/** Site-wide permissions. */
permissions: AuthorizationResponse;
/** Active experiments */
experiments: Experiments;
}

/**
Expand All @@ -43,25 +41,29 @@ export const SidebarView: FC<SidebarProps> = ({
activeOrganizationName,
organizations,
permissions,
experiments,
}) => {
const { showOrganizations } = useDashboard();

// TODO: Do something nice to scroll to the active org.
return (
<BaseSidebar>
<header>
<h2 css={styles.sidebarHeader}>Deployment</h2>
</header>
{showOrganizations && (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would showOrganizations need to be true to show only the Deployment header? Is there some reason to show DeploymentSettingsNavigation without the Header text?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it just looks weird for it to say deployment twice without the organizations section below. it's weird, but we'll get rid of this when we adopt the designs we went over with Christin.

<header>
<h2 css={styles.sidebarHeader}>Deployment</h2>
</header>
)}

<DeploymentSettingsNavigation
active={!activeOrganizationName && activeSettings}
experiments={experiments}
permissions={permissions}
/>
<OrganizationsSettingsNavigation
activeOrganizationName={activeOrganizationName}
organizations={organizations}
permissions={permissions}
/>
{showOrganizations && (
<OrganizationsSettingsNavigation
activeOrganizationName={activeOrganizationName}
organizations={organizations}
permissions={permissions}
/>
)}
</BaseSidebar>
);
};
Expand All @@ -71,8 +73,6 @@ interface DeploymentSettingsNavigationProps {
active: boolean;
/** Site-wide permissions. */
permissions: AuthorizationResponse;
/** Active experiments */
experiments: Experiments;
}

/**
Expand All @@ -85,7 +85,6 @@ interface DeploymentSettingsNavigationProps {
const DeploymentSettingsNavigation: FC<DeploymentSettingsNavigationProps> = ({
active,
permissions,
experiments,
}) => {
return (
<div css={{ paddingBottom: 12 }}>
Expand Down Expand Up @@ -144,16 +143,14 @@ const DeploymentSettingsNavigation: FC<DeploymentSettingsNavigationProps> = ({
</SidebarNavSubItem>
)}
{permissions.viewAllUsers && (
<SidebarNavSubItem href={linkToUsers.slice(1)}>
Users
</SidebarNavSubItem>
<SidebarNavSubItem href="users">Users</SidebarNavSubItem>
)}
<Stack direction="row" alignItems="center" css={{ gap: 0 }}>
<SidebarNavSubItem href="notifications">
Notifications
</SidebarNavSubItem>
<FeatureStageBadge contentType="beta" size="sm" />
</Stack>
<SidebarNavSubItem href="notifications">
<Stack direction="row" alignItems="center" spacing={1}>
<span>Notifications</span>
<FeatureStageBadge contentType="beta" size="sm" />
</Stack>
</SidebarNavSubItem>
</Stack>
)}
</div>
Expand Down Expand Up @@ -387,49 +384,49 @@ const styles = {

const classNames = {
link: (css, theme) => css`
color: inherit;
display: block;
font-size: 14px;
text-decoration: none;
padding: 10px 12px 10px 16px;
border-radius: 4px;
transition: background-color 0.15s ease-in-out;
position: relative;

&:hover {
background-color: ${theme.palette.action.hover};
}

border-left: 3px solid transparent;
`,
color: inherit;
display: block;
font-size: 14px;
text-decoration: none;
padding: 10px 12px 10px 16px;
border-radius: 4px;
transition: background-color 0.15s ease-in-out;
position: relative;

&:hover {
background-color: ${theme.palette.action.hover};
}

border-left: 3px solid transparent;
`,

activeLink: (css, theme) => css`
border-left-color: ${theme.palette.primary.main};
border-top-left-radius: 0;
border-bottom-left-radius: 0;
`,
border-left-color: ${theme.palette.primary.main};
border-top-left-radius: 0;
border-bottom-left-radius: 0;
`,

subLink: (css, theme) => css`
color: ${theme.palette.text.secondary};
text-decoration: none;

display: block;
font-size: 13px;
margin-left: 44px;
padding: 4px 12px;
border-radius: 4px;
transition: background-color 0.15s ease-in-out;
margin-bottom: 1px;
position: relative;

&:hover {
color: ${theme.palette.text.primary};
background-color: ${theme.palette.action.hover};
}
`,
color: ${theme.palette.text.secondary};
text-decoration: none;

display: block;
font-size: 13px;
margin-left: 44px;
padding: 4px 12px;
border-radius: 4px;
transition: background-color 0.15s ease-in-out;
margin-bottom: 1px;
position: relative;

&:hover {
color: ${theme.palette.text.primary};
background-color: ${theme.palette.action.hover};
}
`,

activeSubLink: (css, theme) => css`
color: ${theme.palette.text.primary};
font-weight: 600;
`,
color: ${theme.palette.text.primary};
font-weight: 600;
`,
} satisfies Record<string, ClassName>;
Loading
Loading