From e9260612aa4e25f49ef9165fc7a3c836997746c4 Mon Sep 17 00:00:00 2001 From: Tim DiLauro Date: Mon, 10 Jun 2024 14:09:50 -0400 Subject: [PATCH 1/5] Add feature flags to context. --- src/components/ContextProvider.tsx | 4 +++- src/components/__tests__/ContextProvider-test.tsx | 3 ++- src/components/__tests__/LibraryStats-test.tsx | 6 +++++- src/components/__tests__/Stats-test.tsx | 6 +++++- src/index.tsx | 6 ++++-- src/utils/featureFlags.ts | 12 ++++++++++++ tests/jest/components/CustomLists.test.tsx | 4 ++++ .../jest/components/IndividualAdminEditForm.test.tsx | 1 + tests/jest/components/QuicksightDashboard.test.tsx | 1 + tests/jest/testUtils/withProviders.tsx | 11 +++++++++-- 10 files changed, 46 insertions(+), 8 deletions(-) create mode 100644 src/utils/featureFlags.ts diff --git a/src/components/ContextProvider.tsx b/src/components/ContextProvider.tsx index 270af2dc1..e5487d406 100644 --- a/src/components/ContextProvider.tsx +++ b/src/components/ContextProvider.tsx @@ -17,7 +17,7 @@ export interface ContextProviderProps extends React.Props { role: string; library?: string; }[]; - featureFlags?: FeatureFlags; + featureFlags: FeatureFlags; } /** Provides a redux store, configuration options, and a function to create URLs @@ -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() { @@ -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, }; } diff --git a/src/components/__tests__/ContextProvider-test.tsx b/src/components/__tests__/ContextProvider-test.tsx index 8b31b7c20..eee88f6b8 100644 --- a/src/components/__tests__/ContextProvider-test.tsx +++ b/src/components/__tests__/ContextProvider-test.tsx @@ -19,6 +19,7 @@ describe("ContextProvider", () => { wrapper = shallow( @@ -89,7 +90,7 @@ describe("ContextProvider", () => { } const mockProvider = mount( - + ); diff --git a/src/components/__tests__/LibraryStats-test.tsx b/src/components/__tests__/LibraryStats-test.tsx index 8abf58107..9ff1213a6 100644 --- a/src/components/__tests__/LibraryStats-test.tsx +++ b/src/components/__tests__/LibraryStats-test.tsx @@ -20,7 +20,11 @@ import { const AllProviders = ({ children }) => { const queryClient = new QueryClient(); return ( - + {children} ); diff --git a/src/components/__tests__/Stats-test.tsx b/src/components/__tests__/Stats-test.tsx index f5f4cea8b..e4c04c306 100644 --- a/src/components/__tests__/Stats-test.tsx +++ b/src/components/__tests__/Stats-test.tsx @@ -43,7 +43,11 @@ describe("Stats", () => { const AllProviders = ({ children, store }) => { return ( - + {children} diff --git a/src/index.tsx b/src/index.tsx index e6a58f68b..7869e7292 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -18,6 +18,7 @@ import SetupPage from "./components/SetupPage"; import ManagePatrons from "./components/ManagePatrons"; import TroubleshootingPage from "./components/TroubleshootingPage"; import { FeatureFlags } from "./interfaces"; +import { featureFlagsWithDefaults } from "./utils/featureFlags"; interface ConfigurationSettings { /** A token generated by the server to prevent Cross-Site Request Forgery. @@ -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; @@ -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 @@ -59,6 +60,7 @@ class CirculationAdmin { const div = document.createElement("div"); div.id = "opds-catalog"; div.className = "palace"; + config.featureFlags = featureFlagsWithDefaults(config.featureFlags); document.getElementsByTagName("body")[0].appendChild(div); const catalogEditorPath = diff --git a/src/utils/featureFlags.ts b/src/utils/featureFlags.ts new file mode 100644 index 000000000..f30e9bf57 --- /dev/null +++ b/src/utils/featureFlags.ts @@ -0,0 +1,12 @@ +import { FeatureFlags } from "../interfaces"; + +/** + * Apply default values to each of the feature flags that we know about. + */ +export const featureFlagsWithDefaults = ({ + enableAutoList = true, + ...rest +}: Partial = {}): FeatureFlags => ({ + enableAutoList, + ...rest, +}); diff --git a/tests/jest/components/CustomLists.test.tsx b/tests/jest/components/CustomLists.test.tsx index 212498a3d..a3a5a8511 100644 --- a/tests/jest/components/CustomLists.test.tsx +++ b/tests/jest/components/CustomLists.test.tsx @@ -31,6 +31,7 @@ describe("CustomLists", () => { const contextProviderProps = { csrfToken: "", + featureFlags: {}, roles: [{ role: "system" }], }; @@ -70,6 +71,7 @@ describe("CustomLists", () => { const contextProviderProps = { csrfToken: "", + featureFlags: {}, roles: [{ role: "system" }], }; @@ -121,6 +123,7 @@ describe("CustomLists", () => { const contextProviderProps = { csrfToken: "", + featureFlags: {}, roles: [{ role: "system" }], }; @@ -164,6 +167,7 @@ describe("CustomLists", () => { const contextProviderProps = { csrfToken: "", + featureFlags: {}, roles: [{ role: "system" }], }; diff --git a/tests/jest/components/IndividualAdminEditForm.test.tsx b/tests/jest/components/IndividualAdminEditForm.test.tsx index 34194f837..13efdb774 100644 --- a/tests/jest/components/IndividualAdminEditForm.test.tsx +++ b/tests/jest/components/IndividualAdminEditForm.test.tsx @@ -10,6 +10,7 @@ describe("IndividualAdminEditForm", () => { const contextProviderProps = { csrfToken: "", + featureFlags: {}, roles: [ { role: "system", diff --git a/tests/jest/components/QuicksightDashboard.test.tsx b/tests/jest/components/QuicksightDashboard.test.tsx index 14fe5c73a..1e4cd17d6 100644 --- a/tests/jest/components/QuicksightDashboard.test.tsx +++ b/tests/jest/components/QuicksightDashboard.test.tsx @@ -37,6 +37,7 @@ describe("QuicksightDashboard", () => { it("embed url is retrieved and set in iframe", async () => { const contextProviderProps = { csrfToken: "", + featureFlags: {}, roles: [{ role: "system" }], }; diff --git a/tests/jest/testUtils/withProviders.tsx b/tests/jest/testUtils/withProviders.tsx index 2a6169846..9e914d11e 100644 --- a/tests/jest/testUtils/withProviders.tsx +++ b/tests/jest/testUtils/withProviders.tsx @@ -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 { featureFlagsWithDefaults } from "../../../src/utils/featureFlags"; export type TestProviderWrapperOptions = { contextProviderProps?: Partial; @@ -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: featureFlagsWithDefaults(), +}; /** * Returns a component, composed with our providers, that can be used to wrap @@ -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: featureFlagsWithDefaults(), + }, queryClient = new QueryClient(), }: TestProviderWrapperOptions): React.FunctionComponent => { const effectiveContextProviderProps = { From d95ce670e19c652fe24d8364042ea2718ee24178 Mon Sep 17 00:00:00 2001 From: Tim DiLauro Date: Mon, 10 Jun 2024 14:24:08 -0400 Subject: [PATCH 2/5] Use `reportsOnlyForSysadmins` to control access to inventory reports. --- src/components/LibraryStats.tsx | 34 +++++++++++++++++++++++--- src/index.tsx | 4 +-- src/interfaces.ts | 1 + src/utils/featureFlags.ts | 13 ++++------ tests/jest/testUtils/withProviders.tsx | 6 ++--- 5 files changed, 41 insertions(+), 17 deletions(-) diff --git a/src/components/LibraryStats.tsx b/src/components/LibraryStats.tsx index f149b72e9..88eb6e8fc 100644 --- a/src/components/LibraryStats.tsx +++ b/src/components/LibraryStats.tsx @@ -2,6 +2,7 @@ import * as React from "react"; import { useState } from "react"; import * as numeral from "numeral"; import { + FeatureFlags, InventoryStatistics, LibraryStatistics, PatronStatistics, @@ -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; @@ -48,7 +51,13 @@ 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 `contextProps` property on this function object: +// *** InventoryReportRequestModal.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, @@ -58,6 +67,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, @@ -77,7 +91,11 @@ const LibraryStats = (props: LibraryStatsProps) => {
  • {renderPatronsGroup(patrons)}
  • {renderCirculationsGroup(patrons)}
  • - {renderInventoryGroup(inventory, library)} + {renderInventoryGroup( + inventory, + inventoryReportRequestEnabled, + library + )}
  • {renderCollectionsGroup(chartItems)} @@ -86,6 +104,13 @@ const LibraryStats = (props: LibraryStatsProps) => { ); }; +// 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 ( @@ -137,13 +162,14 @@ const renderCirculationsGroup = (patrons: PatronStatistics) => { const renderInventoryGroup = ( inventory: InventoryStatistics, + inventoryReportsEnabled: boolean, library?: string ) => { const [showReportForm, setShowReportForm] = useState(false); return ( <> - {library && ( + {inventoryReportsEnabled && library && ( setShowReportForm(false)} @@ -152,7 +178,7 @@ const renderInventoryGroup = ( )}

    Inventory - {library && ( + {inventoryReportsEnabled && library && (