Skip to content
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

Add feature flag to restrict access to the inventory report button to sysadmins. (PP-1329) #115

Merged
merged 5 commits into from
Jun 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
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={"[email protected]"}>
<ContextProvider
csrfToken={""}
featureFlags={{}}
email={"[email protected]"}
>
<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={"[email protected]"}>
<ContextProvider
csrfToken={""}
featureFlags={{}}
email={"[email protected]"}
>
<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