Skip to content

Commit

Permalink
Add feature flag to restrict access to the inventory report button to…
Browse files Browse the repository at this point in the history
… sysadmins. (PP-1329) (#115)

* Add feature flags to context.

* Use `reportsOnlyForSysadmins` to control access to inventory reports.
  • Loading branch information
tdilauro authored Jun 12, 2024
1 parent 5fe560a commit 24dde11
Show file tree
Hide file tree
Showing 14 changed files with 140 additions and 15 deletions.
4 changes: 3 additions & 1 deletion src/components/ContextProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export interface ContextProviderProps extends React.Props<ContextProvider> {
role: string;
library?: string;
}[];
featureFlags?: FeatureFlags;
featureFlags: FeatureFlags;
}

/** Provides a redux store, configuration options, and a function to create URLs
Expand Down Expand Up @@ -81,6 +81,7 @@ export default class ContextProvider extends React.Component<
showCircEventsDownload: PropTypes.bool.isRequired,
settingUp: PropTypes.bool.isRequired,
admin: PropTypes.object.isRequired,
featureFlags: PropTypes.object.isRequired,
};

getChildContext() {
Expand All @@ -90,6 +91,7 @@ export default class ContextProvider extends React.Component<
showCircEventsDownload: this.props.showCircEventsDownload || false,
settingUp: this.props.settingUp || false,
admin: this.admin,
featureFlags: this.props.featureFlags,
};
}

Expand Down
5 changes: 3 additions & 2 deletions src/components/InventoryReportRequestModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,9 @@ const CANCEL_BUTTON_TITLE = "Cancel Report Request";
export const ACK_RESPONSE_BUTTON_CONTENT = "Ok";
const ACK_RESPONSE_BUTTON_TITLE = "Acknowledge Response";

// Create a modal to request an inventory report library and email address and to report on success.
// *** To use the legacy context here, we need to create a `contextProps` property on this function object:
// Create a modal to request an inventory report and to describe outcome.
// *** To use the legacy context here, we need to create a `contextTypes` property on this function object
// *** and add `context` types to the function definition.
// *** InventoryReportRequestModal.contextTypes = { email: PropTypes.string }
// *** See: https://legacy.reactjs.org/docs/legacy-context.html#referencing-context-in-stateless-function-components
const InventoryReportRequestModal = (
Expand Down
35 changes: 31 additions & 4 deletions src/components/LibraryStats.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as React from "react";
import { useState } from "react";
import * as numeral from "numeral";
import {
FeatureFlags,
InventoryStatistics,
LibraryStatistics,
PatronStatistics,
Expand All @@ -18,6 +19,8 @@ import {
import { Button } from "library-simplified-reusable-components";
import InventoryReportRequestModal from "./InventoryReportRequestModal";
import SingleStatListItem from "./SingleStatListItem";
import * as PropTypes from "prop-types";
import Admin from "../models/Admin";

export interface LibraryStatsProps {
stats: LibraryStatistics;
Expand Down Expand Up @@ -48,7 +51,14 @@ const inventoryKeyToLabelMap = {

/** Displays statistics about patrons, licenses, and collections from the server,
for a single library or all libraries the admin has access to. */
const LibraryStats = (props: LibraryStatsProps) => {
// *** To use the legacy context here, we need to create a `contextTypes` property on this function object
// *** and add `context` types to the function definition.
// *** LibraryStats.contextTypes = { email: PropTypes.string }
// *** See: https://legacy.reactjs.org/docs/legacy-context.html#referencing-context-in-stateless-function-components
const LibraryStats = (
props: LibraryStatsProps,
context: { admin: Admin; featureFlags: FeatureFlags }
) => {
const { stats, library } = props;
const {
name: libraryName,
Expand All @@ -58,6 +68,11 @@ const LibraryStats = (props: LibraryStatsProps) => {
patronStatistics: patrons,
} = stats || {};

// A feature flag controls whether to show the inventory report form.
const inventoryReportRequestEnabled =
!context.featureFlags.reportsOnlyForSysadmins ||
context.admin.isSystemAdmin();

const chartItems = collections
?.map(({ name, inventory, inventoryByMedium }) => ({
name,
Expand All @@ -77,7 +92,11 @@ const LibraryStats = (props: LibraryStatsProps) => {
<li className="stat-group">{renderPatronsGroup(patrons)}</li>
<li className="stat-group">{renderCirculationsGroup(patrons)}</li>
<li className="stat-group">
{renderInventoryGroup(inventory, library)}
{renderInventoryGroup(
inventory,
inventoryReportRequestEnabled,
library
)}
</li>
<li className="stat-group stat-group-wide">
{renderCollectionsGroup(chartItems)}
Expand All @@ -86,6 +105,13 @@ const LibraryStats = (props: LibraryStatsProps) => {
</div>
);
};
// TODO: This is needed to support legacy context provider on this component (see above).
// The overall approach should be replaced with another mechanism (e.g., `useContext` or
// `useSelector` if we move `email` to new context provider or Redux, respectively).
LibraryStats.contextTypes = {
admin: PropTypes.object.isRequired,
featureFlags: PropTypes.object.isRequired,
};

const renderPatronsGroup = (patrons: PatronStatistics) => {
return (
Expand Down Expand Up @@ -137,13 +163,14 @@ const renderCirculationsGroup = (patrons: PatronStatistics) => {

const renderInventoryGroup = (
inventory: InventoryStatistics,
inventoryReportsEnabled: boolean,
library?: string
) => {
const [showReportForm, setShowReportForm] = useState(false);

return (
<>
{library && (
{inventoryReportsEnabled && library && (
<InventoryReportRequestModal
show={showReportForm}
onHide={() => setShowReportForm(false)}
Expand All @@ -152,7 +179,7 @@ const renderInventoryGroup = (
)}
<h3>
<span className="stat-grouping-label">Inventory</span>
{library && (
{inventoryReportsEnabled && library && (
<Button
callback={(() => setShowReportForm(true)) as any}
content="⬇︎"
Expand Down
3 changes: 2 additions & 1 deletion src/components/__tests__/ContextProvider-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ describe("ContextProvider", () => {
wrapper = shallow(
<ContextProvider
csrfToken="token"
featureFlags={{}}
roles={[{ role: "system" }]}
email="email"
>
Expand Down Expand Up @@ -89,7 +90,7 @@ describe("ContextProvider", () => {
}

const mockProvider = mount(
<MockContextProvider csrfToken="token">
<MockContextProvider csrfToken="token" featureFlags={{}}>
<Child />
</MockContextProvider>
);
Expand Down
6 changes: 5 additions & 1 deletion src/components/__tests__/LibraryStats-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ import {
const AllProviders = ({ children }) => {
const queryClient = new QueryClient();
return (
<ContextProvider csrfToken={""} email={"user@example.org"}>
<ContextProvider
csrfToken={""}
featureFlags={{}}
email={"user@example.org"}
>
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
</ContextProvider>
);
Expand Down
6 changes: 5 additions & 1 deletion src/components/__tests__/Stats-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,11 @@ describe("Stats", () => {
const AllProviders = ({ children, store }) => {
return (
<Provider store={store ?? mockStore(statsState({}))}>
<ContextProvider csrfToken={""} email={"user@example.org"}>
<ContextProvider
csrfToken={""}
featureFlags={{}}
email={"user@example.org"}
>
<QueryClientProvider client={new QueryClient()}>
{children}
</QueryClientProvider>
Expand Down
6 changes: 4 additions & 2 deletions src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import SetupPage from "./components/SetupPage";
import ManagePatrons from "./components/ManagePatrons";
import TroubleshootingPage from "./components/TroubleshootingPage";
import { FeatureFlags } from "./interfaces";
import { defaultFeatureFlags } from "./utils/featureFlags";

interface ConfigurationSettings {
/** A token generated by the server to prevent Cross-Site Request Forgery.
Expand All @@ -39,7 +40,7 @@ interface ConfigurationSettings {
/** `email` will be the email address of the currently logged in admin. */
email?: string;

/** `roles` contains the logged in admin's roles: system admininstrator,
/** `roles` contains the logged in admin's roles: system administrator,
or library manager or librarian for one or more libraries. */
roles?: {
role: string;
Expand All @@ -49,7 +50,7 @@ interface ConfigurationSettings {
tos_link_text?: string;
tos_link_href?: string;

featureFlags?: FeatureFlags;
featureFlags: FeatureFlags;
}

/** The main admin interface application. Create an instance of this class
Expand All @@ -59,6 +60,7 @@ class CirculationAdmin {
const div = document.createElement("div");
div.id = "opds-catalog";
div.className = "palace";
config.featureFlags = { ...defaultFeatureFlags, ...config.featureFlags };
document.getElementsByTagName("body")[0].appendChild(div);

const catalogEditorPath =
Expand Down
1 change: 1 addition & 0 deletions src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

export interface FeatureFlags {
enableAutoList?: boolean;
reportsOnlyForSysadmins?: boolean;
}

export interface Navigate {
Expand Down
9 changes: 9 additions & 0 deletions src/utils/featureFlags.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { FeatureFlags } from "../interfaces";

/**
* Default values for our feature flags.
*/
export const defaultFeatureFlags: FeatureFlags = {
enableAutoList: true,
reportsOnlyForSysadmins: true,
};
4 changes: 4 additions & 0 deletions tests/jest/components/CustomLists.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ describe("CustomLists", () => {

const contextProviderProps = {
csrfToken: "",
featureFlags: {},
roles: [{ role: "system" }],
};

Expand Down Expand Up @@ -70,6 +71,7 @@ describe("CustomLists", () => {

const contextProviderProps = {
csrfToken: "",
featureFlags: {},
roles: [{ role: "system" }],
};

Expand Down Expand Up @@ -121,6 +123,7 @@ describe("CustomLists", () => {

const contextProviderProps = {
csrfToken: "",
featureFlags: {},
roles: [{ role: "system" }],
};

Expand Down Expand Up @@ -164,6 +167,7 @@ describe("CustomLists", () => {

const contextProviderProps = {
csrfToken: "",
featureFlags: {},
roles: [{ role: "system" }],
};

Expand Down
1 change: 1 addition & 0 deletions tests/jest/components/IndividualAdminEditForm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ describe("IndividualAdminEditForm", () => {

const contextProviderProps = {
csrfToken: "",
featureFlags: {},
roles: [
{
role: "system",
Expand Down
1 change: 1 addition & 0 deletions tests/jest/components/QuicksightDashboard.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ describe("QuicksightDashboard", () => {
it("embed url is retrieved and set in iframe", async () => {
const contextProviderProps = {
csrfToken: "",
featureFlags: {},
roles: [{ role: "system" }],
};

Expand Down
63 changes: 62 additions & 1 deletion tests/jest/components/Stats.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
import * as React from "react";
import { render } from "@testing-library/react";
import { CustomTooltip } from "../../../src/components/LibraryStats";
import LibraryStats, {
CustomTooltip,
} from "../../../src/components/LibraryStats";
import { renderWithProviders } from "../testUtils/withProviders";
import { ContextProviderProps } from "../../../src/components/ContextProvider";

import {
statisticsApiResponseData,
testLibraryKey as sampleLibraryKey,
} from "../../__data__/statisticsApiResponseData";
import { normalizeStatistics } from "../../../src/components/Stats";

describe("Dashboard Statistics", () => {
// NB: This adds test to the already existing tests in:
Expand All @@ -11,6 +21,57 @@ describe("Dashboard Statistics", () => {
// Those tests should eventually be migrated here and
// adapted to the Jest/React Testing Library paradigm.

describe("requesting inventory reports", () => {
// Convert from the API format to our in-app format.
const statisticsData = normalizeStatistics(statisticsApiResponseData);
const librariesStatsTestDataByKey = statisticsData.libraries.reduce(
(map, library) => ({ ...map, [library.key]: library }),
{}
);
const sampleStatsData = librariesStatsTestDataByKey[sampleLibraryKey];

const systemAdmin = [{ role: "system" }];
const managerAll = [{ role: "manager-all" }];
const librarianAll = [{ role: "librarian-all" }];

const baseContextProviderProps = {
csrfToken: "",
featureFlags: { reportsOnlyForSysadmins: false },
};

const renderFor = (
onlySysadmins: boolean,
roles: { role: string; library?: string }[]
) => {
const contextProviderProps: ContextProviderProps = {
...baseContextProviderProps,
featureFlags: { reportsOnlyForSysadmins: onlySysadmins },
roles,
};

const { container, queryByRole } = renderWithProviders(
<LibraryStats stats={sampleStatsData} library={sampleLibraryKey} />,
{ contextProviderProps }
);

const result = queryByRole("button", { name: "⬇︎" });
// Clean up the container after each render.
document.body.removeChild(container);
return result;
};

it("shows inventory reports only for sysadmins, if feature flag set", async () => {
// If the feature flag is set, the button should be visible only to sysadmins.
expect(renderFor(true, systemAdmin)).not.toBeNull();
expect(renderFor(true, managerAll)).toBeNull();
expect(renderFor(true, librarianAll)).toBeNull();
// If the feature flag is false, the button should be visible to all users.
expect(renderFor(false, systemAdmin)).not.toBeNull();
expect(renderFor(false, managerAll)).not.toBeNull();
expect(renderFor(false, librarianAll)).not.toBeNull();
});
});

describe("charting - custom tooltip", () => {
const defaultLabel = "Collection X";
const summaryInventory = {
Expand Down
11 changes: 9 additions & 2 deletions tests/jest/testUtils/withProviders.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import ContextProvider, {
} from "../../../src/components/ContextProvider";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import { render, RenderOptions, RenderResult } from "@testing-library/react";
import { defaultFeatureFlags } from "../../../src/utils/featureFlags";

export type TestProviderWrapperOptions = {
contextProviderProps?: Partial<ContextProviderProps>;
Expand All @@ -15,7 +16,10 @@ export type TestRenderWrapperOptions = TestProviderWrapperOptions & {

// The `csrfToken` context provider prop is required, so we provide
// a default value here, so it can be easily merged with other props.
const defaultContextProviderProps = { csrfToken: "" };
const defaultContextProviderProps: ContextProviderProps = {
csrfToken: "",
featureFlags: defaultFeatureFlags,
};

/**
* Returns a component, composed with our providers, that can be used to wrap
Expand All @@ -27,7 +31,10 @@ const defaultContextProviderProps = { csrfToken: "" };
* @returns {React.FunctionComponent} A React component that wraps children with our providers
*/
export const componentWithProviders = ({
contextProviderProps = { csrfToken: "" },
contextProviderProps = {
csrfToken: "",
featureFlags: defaultFeatureFlags,
},
queryClient = new QueryClient(),
}: TestProviderWrapperOptions): React.FunctionComponent => {
const effectiveContextProviderProps = {
Expand Down

0 comments on commit 24dde11

Please sign in to comment.