8000 fix: fix loading states and permissions checks in organization settings by aslilac · Pull Request #16465 · coder/coder · GitHub
[go: up one dir, main page]

Skip to content

fix: fix loading states and permissions checks in organization settings #16465

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 25 commits into from
Feb 19, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
yes
  • Loading branch information
aslilac committed Feb 8, 2025
commit b841aacae17c8c4f8a2bf53d3b300b7d74637e9d
3 changes: 2 additions & 1 deletion site/src/contexts/auth/RequirePermission.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ export const RequirePermission: FC<RequirePermissionProps> = ({
isFeatureVisible,
}) => {
if (!isFeatureVisible) {
return <Navigate to="/workspaces" />;
// return <Navigate to="/workspaces" />;
return <h1>oh fuck</h1>;
}

return <>{children}</>;
Expand Down
98 changes: 48 additions & 50 deletions site/src/modules/management/OrganizationSettingsLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,12 @@ const OrganizationSettingsLayout: FC = () => {
organizationsPermissions(organizations?.map((o) => o.id)),
);

if (orgPermissionsQuery.isLoading) {
return <Loader />;
if (orgPermissionsQuery.isError) {
return <ErrorAlert error={orgPermissionsQuery.error} />;
}

if (!orgPermissionsQuery.data) {
return <ErrorAlert error={orgPermissionsQuery.error} />;
return <Loader />;
}

const viewableOrganizations = organizations.filter((org) =>
Expand All @@ -84,54 +84,52 @@ const OrganizationSettingsLayout: FC = () => {
}

return (
<RequirePermission isFeatureVisible={canViewOrganizationSettings}>
<OrganizationSettingsContext.Provider
value={{
organizations: viewableOrganizations,
organizationPermissionsByOrganizationId: orgPermissionsQuery.data,
organization,
organizationPermissions,
}}
>
<div>
<Breadcrumb>
<BreadcrumbList>
<BreadcrumbItem>
<BreadcrumbPage>Admin Settings</BreadcrumbPage>
</BreadcrumbItem>
<BreadcrumbSeparator />
<BreadcrumbItem>
<BreadcrumbPage className="flex items-center gap-2">
Organizations
</BreadcrumbPage>
</BreadcrumbItem>
{organization && (
<>
<BreadcrumbSeparator />
<BreadcrumbItem>
<BreadcrumbPage className="text-content-primary">
<Avatar
key={organization.id}
size="sm"
fallback={organization.display_name}
src={organization.icon}
/>
{organization.display_name}
</BreadcrumbPage>
</BreadcrumbItem>
</>
)}
</BreadcrumbList>
</Breadcrumb>
<hr className="h-px border-none bg-border" />
<div className="px-10 max-w-screen-2xl">
<Suspense fallback={<Loader />}>
<Outlet />
</Suspense>
</div>
<OrganizationSettingsContext.Provider
value={{
organizations: viewableOrganizations,
organizationPermissionsByOrganizationId: orgPermissionsQuery.data,
organization,
organizationPermissions,
}}
>
<div>
<Breadcrumb>
<BreadcrumbList>
<BreadcrumbItem>
<BreadcrumbPage>Admin Settings</BreadcrumbPage>
</BreadcrumbItem>
<BreadcrumbSeparator />
<BreadcrumbItem>
<BreadcrumbPage className="flex items-center gap-2">
Organizations
</BreadcrumbPage>
</BreadcrumbItem>
{organization && (
<>
<BreadcrumbSeparator />
<BreadcrumbItem>
<BreadcrumbPage className="text-content-primary">
<Avatar
key={organization.id}
size="sm"
fallback={organization.display_name}
src={organization.icon}
/>
{organization.display_name}
</BreadcrumbPage>
</BreadcrumbItem>
</>
)}
</BreadcrumbList>
</Breadcrumb>
<hr className="h-px border-none bg-border" />
<div className="px-10 max-w-screen-2xl">
<Suspense fallback={<Loader />}>
<Outlet />
</Suspense>
</div>
</OrganizationSettingsContext.Provider>
</RequirePermission>
</div>
</OrganizationSettingsContext.Provider>
);
};

Expand Down
26 changes: 16 additions & 10 deletions site/src/modules/management/OrganizationSidebarView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ export const OrganizationSidebarView: FC<SidebarProps> = ({
]
: organizations;

console.log(organizations);

const [isPopoverOpen, setIsPopoverOpen] = useState(false);
const navigate = useNavigate();

Expand All @@ -60,16 +62,20 @@ export const OrganizationSidebarView: FC<SidebarProps> = ({
className="w-60 justify-between p-2 h-11"
>
<div className="flex flex-row gap-2 items-center p-2 truncate">
{activeOrganization && (
<Avatar
size="sm"
src={activeOrganization.icon}
fallback={activeOrganization.display_name}
/>
{activeOrganization ? (
<>
<Avatar
size="sm"
src={activeOrganization.icon}
fallback={activeOrganization.display_name}
/>
<span className="truncate">
{activeOrganization.display_name || activeOrganization.name}
</span>
</>
) : (
<span className="truncate">No organization selected</span>
)}
<span className="truncate">
{activeOrganization?.display_name || activeOrganization?.name}
</span>
</div>
<ChevronDown />
</Button>
Expand All @@ -78,7 +84,7 @@ export const OrganizationSidebarView: FC<SidebarProps> = ({
<Command loop>
<CommandList>
<CommandGroup className="pb-2">
{sortedOrganizations.length > 1 ? (
{sortedOrganizations.length > (activeOrganization ? 1 : 0) ? (
<div className="flex flex-col max-h-[260px] overflow-y-auto">
{sortedOrganizations.map((organization) => (
<CommandItem
Expand Down
18 changes: 13 additions & 5 deletions site/src/modules/management/organizationPermissions.tsx
8000
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export const organizationPermissionChecks = (organizationId: string) =>
},
action: "update",
},
editOrganization: {
editSettings: {
object: {
resource_type: "organization",
organization_id: organizationId,
Expand Down Expand Up @@ -87,6 +87,13 @@ export const organizationPermissionChecks = (organizationId: string) =>
},
action: "read",
},
editIdpSyncSettings: {
object: {
resource_type: "idpsync_settings",
organization_id: organizationId,
},
action: "update",
},
}) as const satisfies Record<string, AuthorizationCheck>;

/**
Expand Down Expand Up @@ -116,8 +123,9 @@ export const canEditOrganization = (
permissions !== undefined &&
(permissions.editMembers ||
permissions.editGroups ||
permissions.editOrganization ||
permissions.editSettings ||
permissions.assignOrgRoles ||
permissions.editIdpSyncSettings ||
permissions.createOrgRoles)
);
};
Expand Down Expand Up @@ -151,12 +159,12 @@ export const anyOrganizationPermissionChecks = {
},
action: "assign",
},
editAnyIdpSyncSettings: {
viewAnyIdpSyncSettings: {
object: {
resource_type: "idpsync_settings",
any_org: true,
},
action: "update",
action: "read",
},
editAnySettings: {
object: {
Expand All @@ -179,7 +187,7 @@ export const canViewAnyOrganization = (
(permissions.viewAnyMembers ||
permissions.editAnyGroups ||
permissions.assignAnyRoles ||
permissions.editAnyIdpSyncSettings ||
permissions.viewAnyIdpSyncSettings ||
permissions.editAnySettings)
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { useMutation, useQuery, useQueryClient } from "react-query";
import { useParams } from "react-router-dom";
import { pageTitle } from "utils/page";
import { OrganizationMembersPageView } from "./OrganizationMembersPageView";
import { EmptyState } from "components/EmptyState/EmptyState";

const OrganizationMembersPage: FC = () => {
const queryClient = useQueryClient();
Expand Down Expand Up @@ -54,11 +55,11 @@ const OrganizationMembersPage: FC = () => {
const [memberToDelete, setMemberToDelete] =
useState<OrganizationMemberWithUserData>();

if (!organizationPermissions) {
return <Loader />;
if (!organization || !organizationPermissions) {
return <EmptyState message="Organization not found" />;
}

const helmet = organization && (
const helmet = (
<Helmet>
<title>
{pageTitle("Members", organization.display_name || organization.name)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,29 @@ import {
waitForLoaderToBeRemoved,
} from "testHelpers/renderHelpers";
import { server } from "testHelpers/server";
import OrganizationSettingsPage from "./OrganizationSettingsPage";
import OrganizationRedirect from "./OrganizationRedirect";

jest.spyOn(console, "error").mockImplementation(() => {});

const renderPage = async () => {
renderWithOrganizationSettingsLayout(<OrganizationSettingsPage />, {
route: "/organizations",
path: "/organizations/:organization?",
});
const { router } = renderWithOrganizationSettingsLayout(
<OrganizationRedirect />,
{
route: "/organizations",
path: "/organizations",
extraRoutes: [
{
path: "/organizations/:organization",
element: <h1>Organization Settings</h1>,
},
],
},
);
await waitForLoaderToBeRemoved();
return router;
};

describe("OrganizationSettingsPage", () => {
describe("OrganizationRedirect", () => {
it("has no editable organizations", async () => {
server.use(
http.get("/api/v2/entitlements", () => {
Expand All @@ -32,9 +42,7 @@ describe("OrganizationSettingsPage", () => {
return HttpResponse.json([MockDefaultOrganization, MockOrganization2]);
}),
http.post("/api/v2/authcheck", async () => {
return HttpResponse.json({
viewDeploymentValues: true,
});
return HttpResponse.json({});
}),
);
await renderPage();
Expand All @@ -52,16 +60,19 @@ describe("OrganizationSettingsPage", () => {
}),
http.post("/api/v2/authcheck", async () => {
return HttpResponse.json({
[`${MockDefaultOrganization.id}.editOrganization`]: true,
[`${MockOrganization2.id}.editOrganization`]: true,
viewDeploymentValues: true,
viewAnyMembers: true,
[`${MockDefaultOrganization.id}.viewMembers`]: true,
[`${MockDefaultOrganization.id}.editMembers`]: true,
[`${MockOrganization2.id}.viewMembers`]: true,
[`${MockOrganization2.id}.editMembers`]: true,
});
}),
);
await renderPage();
const form = screen.getByTestId("org-settings-form");
expect(within(form).getByRole("textbox", { name: "Slug" })).toHaveValue(
MockDefaultOrganization.name,
const router = await renderPage();
const form = screen.getByText("Organization Settings");
expect(form).toBeInTheDocument();
expect(router.state.location.pathname).toBe(
`/organizations/${MockDefaultOrganization.name}`,
);
});

Expand All @@ -75,15 +86,18 @@ describe("OrganizationSettingsPage", () => {
}),
http.post("/api/v2/authcheck", async () => {
return HttpResponse.json({
[`${MockOrganization2.id}.editOrganization`]: true,
viewDeploymentValues: true,
viewAnyMembers: true,
[`${MockDefaultOrganization.id}.viewMembers`]: true,
[`${MockOrganization2.id}.viewMembers`]: true,
[`${MockOrganization2.id}.editMembers`]: true,
});
}),
);
await renderPage();
const form = screen.getByTestId("org-settings-form");
expect(within(form).getByRole("textbox", { name: "Slug" })).toHaveValue(
MockOrganization2.name,
const router = await renderPage();
const form = screen.getByText("Organization Settings");
expect(form).toBeInTheDocument();
expect(router.state.location.pathname).toBe(
`/organizations/${MockOrganization2.name}`,
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { canEditOrganization } from "modules/management/organizationPermissions"
import type { FC } from "react";
import { Navigate } from "react-router-dom";

const DefaultOrganizationRedirect: FC = () => {
const OrganizationRedirect: FC = () => {
const {
organizations,
organizationPermissionsByOrganizationId: organizationPermissions,
Expand All @@ -27,4 +27,4 @@ const DefaultOrganizationRedirect: FC = () => {
return <EmptyState message="No organizations found" />;
};

export default DefaultOrganizationRedirect;
export default OrganizationRedirect;
Loading
Loading
0