Skip to content

Commit

Permalink
Experience-7891: Disable organization-specific fetches as admin
Browse files Browse the repository at this point in the history
This changeset disables a few Organization-specific requests to mitigate the number of 404s we're getting as admins try to fetch Organization resources:
- Organization settings
- Deliveries
- Submissions

There's not a one-size-fits-all solution here given the different fetch mechanisms we already have and how the data is rendered, so I tried to add minimal changes to prevent disrupting anything down the line.  I think going forward, we can make a generic `useOrganizationQuery` hook that'll automatically be disabled if the user is logged in as an admin without impersonating an Organization.  There are a lot of layers with our fetching that in my opinion should be untangled, but that's out of the scope of this pull request.
  • Loading branch information
Stephen Kao committed Jan 20, 2023
1 parent 8da0878 commit f9e39cf
Show file tree
Hide file tree
Showing 18 changed files with 509 additions and 182 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import {
formattedDateFromTimestamp,
timeZoneAbbreviated,
} from "../../utils/DateTimeUtils";
import { StaticAlert } from "../StaticAlert";
import { StaticAlert, StaticAlertType } from "../StaticAlert";
import {
ResponseError,
ErrorCodeTranslation,
ResponseError,
} from "../../config/endpoints/waters";
import { Destination } from "../../resources/ActionDetailsResource";
import { USExtLink, USLink } from "../USLink";
Expand Down Expand Up @@ -38,7 +38,7 @@ export const FileSuccessDisplay = ({
return (
<>
<StaticAlert
type={"success slim"}
type={[StaticAlertType.Success, StaticAlertType.Slim]}
heading={heading}
message={message}
/>
Expand Down Expand Up @@ -129,7 +129,10 @@ export const RequestedChangesDisplay = ({
handlerType,
}: RequestedChangesDisplayProps) => {
const alertType = useMemo(
() => (title === RequestLevel.WARNING ? "warning" : "error"),
() =>
title === RequestLevel.WARNING
? StaticAlertType.Warning
: StaticAlertType.Error,
[title]
);
const showTable =
Expand Down Expand Up @@ -208,7 +211,13 @@ interface FileWarningBannerProps {
}

export const FileWarningBanner = ({ message }: FileWarningBannerProps) => {
return <StaticAlert type={"warning"} heading="Warning" message={message} />;
return (
<StaticAlert
type={StaticAlertType.Warning}
heading="Warning"
message={message}
/>
);
};

interface ErrorRowProps {
Expand Down Expand Up @@ -251,7 +260,7 @@ export const FileQualityFilterDisplay = ({
return (
<>
<StaticAlert
type={"error slim"}
type={[StaticAlertType.Error, StaticAlertType.Slim]}
heading={heading}
message={message}
/>
Expand Down
6 changes: 3 additions & 3 deletions frontend-react/src/components/StaticAlert.test.tsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import { screen, render } from "@testing-library/react";

import { StaticAlert } from "./StaticAlert";
import { StaticAlert, StaticAlertType } from "./StaticAlert";

describe("StaticAlert", () => {
test("renders correct class for success", async () => {
render(<StaticAlert type={"success"} heading={"any"} />);
render(<StaticAlert type={StaticAlertType.Success} heading={"any"} />);

const wrapper = await screen.findByRole("alert");
expect(wrapper).toHaveClass("usa-alert--success");
});

test("renders correct class for success", async () => {
render(<StaticAlert type={"error"} heading={"any"} />);
render(<StaticAlert type={StaticAlertType.Error} heading={"any"} />);

const wrapper = await screen.findByRole("alert");
expect(wrapper).toHaveClass("usa-alert--error");
Expand Down
19 changes: 14 additions & 5 deletions frontend-react/src/components/StaticAlert.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
import React, { ReactNode } from "react";
import classNames from "classnames";

export enum StaticAlertType {
Success = "success",
Error = "error",
Warning = "warning",
Slim = "slim",
}

interface StaticAlertProps {
type: string;
type: StaticAlertType | StaticAlertType[];
heading: string;
message?: string;
children?: ReactNode;
Expand All @@ -14,12 +21,14 @@ export const StaticAlert = ({
message,
children,
}: StaticAlertProps) => {
type = Array.isArray(type) ? type : [type];

const alertClasses = classNames({
"usa-alert": true,
"usa-alert--success": type.indexOf("success") > -1,
"usa-alert--error": type.indexOf("error") > -1,
"usa-alert--warning": type.indexOf("warning") > -1,
"usa-alert--slim": type.indexOf("slim") > -1,
"usa-alert--success": type.includes(StaticAlertType.Success),
"usa-alert--error": type.includes(StaticAlertType.Error),
"usa-alert--warning": type.includes(StaticAlertType.Warning),
"usa-alert--slim": type.includes(StaticAlertType.Slim),
});
return (
<div className={alertClasses} role="alert">
Expand Down
13 changes: 13 additions & 0 deletions frontend-react/src/components/alerts/AdminFetchAlert.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import React from "react";

import { StaticAlert, StaticAlertType } from "../StaticAlert";

export default function AdminFetchAlert() {
return (
<StaticAlert
type={StaticAlertType.Error}
heading="Cannot fetch Organization data as admin"
message="Please try again as an Organization"
/>
);
}
4 changes: 2 additions & 2 deletions frontend-react/src/components/alerts/NoServicesAlert.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from "react";

import { StaticAlert } from "../StaticAlert";
import { StaticAlert, StaticAlertType } from "../StaticAlert";
import { capitalizeFirst } from "../../utils/misc";

interface NoServicesBannerProps {
Expand All @@ -16,7 +16,7 @@ export const NoServicesBanner = ({
}: NoServicesBannerProps) => {
return (
<StaticAlert
type={"error"}
type={StaticAlertType.Error}
heading={`${capitalizeFirst(featureName || "feature")} unavailable`}
message={`No valid ${serviceType || "service"} found for ${
organization || "your organization"
Expand Down
11 changes: 8 additions & 3 deletions frontend-react/src/hooks/UseOrganizationReceivers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,30 @@ import { RSReceiver, servicesEndpoints } from "../config/endpoints/settings";
import { useAuthorizedFetch } from "../contexts/AuthorizedFetchContext";
import { useSessionContext } from "../contexts/SessionContext";

import { Organizations } from "./UseAdminSafeOrganizationName";

const { receivers } = servicesEndpoints;

export const useOrganizationReceivers = () => {
const { activeMembership } = useSessionContext();
const parsedName = activeMembership?.parsedName;

const { authorizedFetch, rsUseQuery } = useAuthorizedFetch<RSReceiver[]>();
const memoizedDataFetch = useCallback(
() =>
authorizedFetch(receivers, {
segments: {
orgName: activeMembership?.parsedName!!,
orgName: parsedName!!,
},
}),
[activeMembership?.parsedName, authorizedFetch]
[parsedName, authorizedFetch]
);
return rsUseQuery(
[receivers.queryKey, activeMembership],
memoizedDataFetch,
{
enabled: !!activeMembership?.parsedName,
enabled:
Boolean(parsedName) && parsedName !== Organizations.PRIMEADMINS,
}
);
};
116 changes: 81 additions & 35 deletions frontend-react/src/hooks/UseOrganizationReceiversFeed.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,52 +10,98 @@ import { mockSessionContext } from "../contexts/__mocks__/SessionContext";

import { MemberType } from "./UseOktaMemberships";
import { useOrganizationReceiversFeed } from "./UseOrganizationReceiversFeed";
import { Organizations } from "./UseAdminSafeOrganizationName";

describe("useOrganizationReceiversFeed", () => {
beforeAll(() => {
orgServer.listen();
});
afterEach(() => orgServer.resetHandlers());
afterAll(() => orgServer.close());
test("returns empty array if no active membership parsed name", () => {
mockSessionContext.mockReturnValue({
oktaToken: {
accessToken: "TOKEN",
},
activeMembership: undefined,
dispatch: () => {},
initialized: true,
describe("with no active membership parsed name", () => {
beforeEach(() => {
mockSessionContext.mockReturnValue({
oktaToken: {
accessToken: "TOKEN",
},
activeMembership: undefined,
dispatch: () => {},
initialized: true,
});
});
const { result } = renderHook(() => useOrganizationReceiversFeed(), {
wrapper: QueryWrapper(),

test("is disabled and returns an empty array", () => {
const { result } = renderHook(
() => useOrganizationReceiversFeed(),
{
wrapper: QueryWrapper(),
}
);
expect(result.current.services).toEqual([]);
expect(result.current.setActiveService).toBeDefined();
expect(result.current.activeService).toEqual(undefined);
expect(result.current.loadingServices).toEqual(false);
expect(result.current.isDisabled).toEqual(true);
});
expect(result.current.services).toEqual([]);
expect(result.current.setActiveService).toBeDefined();
expect(result.current.activeService).toEqual(undefined);
expect(result.current.loadingServices).toEqual(true);
});
test("returns correct organization receivers feed", async () => {
mockSessionContext.mockReturnValue({
oktaToken: {
accessToken: "TOKEN",
},
activeMembership: {
memberType: MemberType.RECEIVER,
parsedName: "testOrg",
service: "testReceiver",
},
dispatch: () => {},
initialized: true,

describe("with an admin parsed name", () => {
beforeEach(() => {
mockSessionContext.mockReturnValue({
oktaToken: {
accessToken: "TOKEN",
},
activeMembership: {
memberType: MemberType.PRIME_ADMIN,
parsedName: Organizations.PRIMEADMINS,
},
dispatch: () => {},
initialized: true,
});
});

test("is disabled and returns an empty array", () => {
const { result } = renderHook(
() => useOrganizationReceiversFeed(),
{
wrapper: QueryWrapper(),
}
);
expect(result.current.services).toEqual([]);
expect(result.current.setActiveService).toBeDefined();
expect(result.current.activeService).toEqual(undefined);
expect(result.current.loadingServices).toEqual(false);
expect(result.current.isDisabled).toEqual(true);
});
});

describe("with a non-admin parsed name", () => {
beforeEach(() => {
mockSessionContext.mockReturnValue({
oktaToken: {
accessToken: "TOKEN",
},
activeMembership: {
memberType: MemberType.RECEIVER,
parsedName: "testOrg",
service: "testReceiver",
},
dispatch: () => {},
initialized: true,
});
});

test("returns correct organization receivers feed", async () => {
const { result, waitForNextUpdate } = renderHook(
() => useOrganizationReceiversFeed(),
{ wrapper: QueryWrapper() }
);
await waitForNextUpdate();
expect(result.current.services).toEqual(dummyReceivers);
expect(result.current.setActiveService).toBeDefined();
expect(result.current.activeService).toEqual(dummyActiveReceiver);
expect(result.current.loadingServices).toEqual(false);
});
const { result, waitForNextUpdate } = renderHook(
() => useOrganizationReceiversFeed(),
{ wrapper: QueryWrapper() }
);
await waitForNextUpdate();
expect(result.current.services).toEqual(dummyReceivers);
expect(result.current.setActiveService).toBeDefined();
expect(result.current.activeService).toEqual(dummyActiveReceiver);
expect(result.current.loadingServices).toEqual(false);
});

test("setActiveService sets an active receiver", async () => {
Expand Down
10 changes: 8 additions & 2 deletions frontend-react/src/hooks/UseOrganizationReceiversFeed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,16 @@ interface ReceiverFeeds {
services: RSReceiver[];
activeService: RSReceiver | undefined;
setActiveService: Dispatch<SetStateAction<RSReceiver | undefined>>;
isDisabled: boolean;
}
/** Fetches a list of receivers for your active organization, and provides a controller to switch
* between them */
export const useOrganizationReceiversFeed = (): ReceiverFeeds => {
const { data: receivers, isLoading } = useOrganizationReceivers();
const {
data: receivers,
isLoading,
fetchStatus,
} = useOrganizationReceivers();
const [active, setActive] = useState<RSReceiver | undefined>();

useEffect(() => {
Expand All @@ -29,9 +34,10 @@ export const useOrganizationReceiversFeed = (): ReceiverFeeds => {
}, [receivers]);

return {
loadingServices: isLoading,
loadingServices: isLoading && fetchStatus !== "idle",
services: receivers || [],
activeService: active,
setActiveService: setActive,
isDisabled: isLoading && fetchStatus === "idle",
};
};
Loading

0 comments on commit f9e39cf

Please sign in to comment.