From cd6afece623eb7f0bf921ed07d7e33cb44abc880 Mon Sep 17 00:00:00 2001 From: Jackie Quach Date: Fri, 8 Jul 2022 15:56:05 -0400 Subject: [PATCH 1/7] implement feature flags --- src/context/FeatureFlagContext.tsx | 59 +++++++++++++++++++++++++++ src/pages/_app.tsx | 5 ++- src/util/LinkUtils.ts | 11 +++++ src/util/__tests__/LinkUtils.test.tsx | 30 +++++++++++++- 4 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 src/context/FeatureFlagContext.tsx diff --git a/src/context/FeatureFlagContext.tsx b/src/context/FeatureFlagContext.tsx new file mode 100644 index 00000000..28ddb0b9 --- /dev/null +++ b/src/context/FeatureFlagContext.tsx @@ -0,0 +1,59 @@ +import { useRouter } from "next/router"; +import React, { createContext, useContext, useEffect, useState } from "react"; +import { extractQueryParam } from "../util/LinkUtils"; +import { flattenDeep } from "../util/Util"; + +type FeatureFlagContextType = { + activeFeatureFlags: string[]; + setActiveFeatureFlags: (activeFeatureFlags: string[]) => void; + isFlagActive: (flag: string) => boolean; +}; + +export const FeatureFlagContext = + createContext(undefined); + +export const FeatureFlagProvider: React.FC = ({ children }) => { + const [activeFeatureFlags, setActiveFeatureFlags] = useState([]); + const isFlagActive = (flag: string) => { + return activeFeatureFlags.includes(flag); + }; + + const router = useRouter(); + const { query } = router; + + useEffect(() => { + const storedFeatureFlagsStr = sessionStorage.getItem("features"); + let storedFeatureFlags: string[] = []; + if (storedFeatureFlagsStr) { + try { + storedFeatureFlags = JSON.parse(storedFeatureFlagsStr); + } catch (e) { + throw new Error(e); + } + } + const newFeatureFlags = flattenDeep([ + ...storedFeatureFlags, + ...extractQueryParam(query, "feature"), + ]); + sessionStorage.setItem("features", JSON.stringify(newFeatureFlags)); + setActiveFeatureFlags(newFeatureFlags); + }, [query]); + + return ( + + {children} + + ); +}; + +export default function useFeatureFlags() { + const context = useContext(FeatureFlagContext); + if (typeof context === "undefined") { + throw new Error( + "useFeatureFlags must be used within a FeatureFlagProvider" + ); + } + return context; +} diff --git a/src/pages/_app.tsx b/src/pages/_app.tsx index 86728b1d..1f7335c4 100644 --- a/src/pages/_app.tsx +++ b/src/pages/_app.tsx @@ -9,6 +9,7 @@ import Head from "next/head"; import appConfig from "~/config/appConfig"; import { documentTitles } from "../constants/labels"; import "@nypl/web-reader/dist/esm/index.css"; +import { FeatureFlagProvider } from "../context/FeatureFlagContext"; /** * Determines if we are running on server or in the client. @@ -75,7 +76,9 @@ function MyApp({ Component, pageProps }: AppProps) { /> {/* */} - + + + ); } diff --git a/src/util/LinkUtils.ts b/src/util/LinkUtils.ts index 95e83f9d..bbbe6098 100644 --- a/src/util/LinkUtils.ts +++ b/src/util/LinkUtils.ts @@ -1,3 +1,5 @@ +import { ParsedUrlQuery } from "querystring"; + const isRefererInternal = (referer: string, host: string) => { return referer && referer.includes(host); }; @@ -11,3 +13,12 @@ export const getBackToSearchUrl = (referer: string, host: string) => { ? referer : null; }; + +export const extractQueryParam = ( + query: ParsedUrlQuery, + param: string +): string[] => { + const extracted = query?.[param]; + + return extracted ? (Array.isArray(extracted) ? extracted : [extracted]) : []; +}; diff --git a/src/util/__tests__/LinkUtils.test.tsx b/src/util/__tests__/LinkUtils.test.tsx index cbcb7c7c..d2345769 100644 --- a/src/util/__tests__/LinkUtils.test.tsx +++ b/src/util/__tests__/LinkUtils.test.tsx @@ -1,4 +1,10 @@ -import { getBackToSearchUrl, getBackUrl } from "../LinkUtils"; +import { + extractQueryParam, + getBackToSearchUrl, + getBackUrl, +} from "../LinkUtils"; +import mockRouter from "next-router-mock"; +jest.mock("next/router", () => require("next-router-mock")); describe("Generates back url", () => { const host = "drb-qa.nypl.org"; @@ -31,3 +37,25 @@ describe("Generate back to serach url", () => { expect(backUrl).toEqual(referer); }); }); + +describe("Extracts query parameter from url", () => { + test("extractQueryParam returns array with a single flag", () => { + mockRouter.setCurrentUrl( + "https://drb-qa.nypl.org/edition/1780467?feature=new_feature" + ); + const features = extractQueryParam(mockRouter.query, "feature"); + expect(features).toEqual(["new_feature"]); + }); + test("extractQueryParam returns array with multiple flags", () => { + mockRouter.setCurrentUrl( + "https://drb-qa.nypl.org/edition/1780467?feature=new_feature&feature=new_feature2" + ); + const features = extractQueryParam(mockRouter.query, "feature"); + expect(features).toEqual(["new_feature", "new_feature2"]); + }); + test("extractQueryParam returns empty array if query doesn't exist", () => { + mockRouter.setCurrentUrl("https://drb-qa.nypl.org/edition/1780467"); + const features = extractQueryParam(mockRouter.query, "feature"); + expect(features).toEqual([]); + }); +}); From 8086833526b8688f662bd4da0c90f983d457fd1f Mon Sep 17 00:00:00 2001 From: Jackie Quach Date: Fri, 8 Jul 2022 15:58:04 -0400 Subject: [PATCH 2/7] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c416bbc..4df449ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Change format filters to "Readable", "Downloadable", and "Requestable" - Upgrade to Web Reader version 3.1.0 - Adds author and work title to edition detail page +- Adds feature flag functionality ## [0.13.1] - Added "Back to search results" link for works and editions From 094838280cd86e81e2e521dbf6b4fad1f4011d17 Mon Sep 17 00:00:00 2001 From: Jackie Quach Date: Fri, 8 Jul 2022 17:25:06 -0400 Subject: [PATCH 3/7] make flags toggleable --- src/context/FeatureFlagContext.tsx | 53 +++++++++++++++++++-------- src/util/LinkUtils.ts | 4 +- src/util/__tests__/LinkUtils.test.tsx | 12 +++--- 3 files changed, 46 insertions(+), 23 deletions(-) diff --git a/src/context/FeatureFlagContext.tsx b/src/context/FeatureFlagContext.tsx index 28ddb0b9..2a144532 100644 --- a/src/context/FeatureFlagContext.tsx +++ b/src/context/FeatureFlagContext.tsx @@ -1,47 +1,70 @@ import { useRouter } from "next/router"; +import { ParsedUrlQuery } from "querystring"; import React, { createContext, useContext, useEffect, useState } from "react"; import { extractQueryParam } from "../util/LinkUtils"; -import { flattenDeep } from "../util/Util"; + +interface FeatureFlag { + [flag: string]: boolean; +} type FeatureFlagContextType = { - activeFeatureFlags: string[]; - setActiveFeatureFlags: (activeFeatureFlags: string[]) => void; + featureFlags: FeatureFlag; + setFeatureFlags: (featureFlags: FeatureFlag) => void; isFlagActive: (flag: string) => boolean; }; export const FeatureFlagContext = createContext(undefined); +const extractFeatureFlagParams = (query: ParsedUrlQuery) => { + const featureFlags = {}; + for (const param in query) { + if (param.includes("featureFlag_")) { + const featureFlag = param.split("_")[1]; + featureFlags[featureFlag] = JSON.parse(extractQueryParam(query, param)); + } + } + return featureFlags; +}; + export const FeatureFlagProvider: React.FC = ({ children }) => { - const [activeFeatureFlags, setActiveFeatureFlags] = useState([]); + const [featureFlags, setFeatureFlags] = useState({}); const isFlagActive = (flag: string) => { - return activeFeatureFlags.includes(flag); + return featureFlags[flag]; }; const router = useRouter(); const { query } = router; useEffect(() => { - const storedFeatureFlagsStr = sessionStorage.getItem("features"); - let storedFeatureFlags: string[] = []; + const storedFeatureFlagsStr = sessionStorage.getItem("featureFlags"); + let storedFeatureFlags: FeatureFlag = {}; if (storedFeatureFlagsStr) { try { storedFeatureFlags = JSON.parse(storedFeatureFlagsStr); + for (const flag in storedFeatureFlags) { + const featureFlag = "featureFlag_" + flag; + if (!query[featureFlag]) { + router.push({ query: { [featureFlag]: storedFeatureFlags[flag] } }); + } + } } catch (e) { throw new Error(e); } } - const newFeatureFlags = flattenDeep([ - ...storedFeatureFlags, - ...extractQueryParam(query, "feature"), - ]); - sessionStorage.setItem("features", JSON.stringify(newFeatureFlags)); - setActiveFeatureFlags(newFeatureFlags); - }, [query]); + + const newFeatureFlags = extractFeatureFlagParams(query); + sessionStorage.setItem("featureFlags", JSON.stringify(newFeatureFlags)); + setFeatureFlags(newFeatureFlags); + }, [query, router]); return ( {children} diff --git a/src/util/LinkUtils.ts b/src/util/LinkUtils.ts index bbbe6098..aae8b84a 100644 --- a/src/util/LinkUtils.ts +++ b/src/util/LinkUtils.ts @@ -17,8 +17,8 @@ export const getBackToSearchUrl = (referer: string, host: string) => { export const extractQueryParam = ( query: ParsedUrlQuery, param: string -): string[] => { +): string | undefined => { const extracted = query?.[param]; - return extracted ? (Array.isArray(extracted) ? extracted : [extracted]) : []; + return typeof extracted === "string" ? extracted : undefined; }; diff --git a/src/util/__tests__/LinkUtils.test.tsx b/src/util/__tests__/LinkUtils.test.tsx index d2345769..6bafe7a0 100644 --- a/src/util/__tests__/LinkUtils.test.tsx +++ b/src/util/__tests__/LinkUtils.test.tsx @@ -39,23 +39,23 @@ describe("Generate back to serach url", () => { }); describe("Extracts query parameter from url", () => { - test("extractQueryParam returns array with a single flag", () => { + test("extractQueryParam returns a single flag", () => { mockRouter.setCurrentUrl( "https://drb-qa.nypl.org/edition/1780467?feature=new_feature" ); const features = extractQueryParam(mockRouter.query, "feature"); - expect(features).toEqual(["new_feature"]); + expect(features).toEqual("new_feature"); }); - test("extractQueryParam returns array with multiple flags", () => { + test("extractQueryParam returns undefined if multiple flags", () => { mockRouter.setCurrentUrl( "https://drb-qa.nypl.org/edition/1780467?feature=new_feature&feature=new_feature2" ); const features = extractQueryParam(mockRouter.query, "feature"); - expect(features).toEqual(["new_feature", "new_feature2"]); + expect(features).toBeUndefined(); }); - test("extractQueryParam returns empty array if query doesn't exist", () => { + test("extractQueryParam returns undefined if query doesn't exist", () => { mockRouter.setCurrentUrl("https://drb-qa.nypl.org/edition/1780467"); const features = extractQueryParam(mockRouter.query, "feature"); - expect(features).toEqual([]); + expect(features).toBeUndefined(); }); }); From eb9abb1d6e7ffd489264cad86648e348889aa9fe Mon Sep 17 00:00:00 2001 From: Jackie Quach Date: Mon, 11 Jul 2022 14:53:31 -0400 Subject: [PATCH 4/7] add total count test --- src/__tests__/Search.test.tsx | 37 +++++++++++++++++++++++++++++- src/__tests__/testUtils/render.tsx | 13 +++++++++++ src/components/Search/Search.tsx | 8 +++++++ src/context/FeatureFlagContext.tsx | 14 +++++++---- 4 files changed, 66 insertions(+), 6 deletions(-) create mode 100644 src/__tests__/testUtils/render.tsx diff --git a/src/__tests__/Search.test.tsx b/src/__tests__/Search.test.tsx index 570120aa..f4b84b97 100644 --- a/src/__tests__/Search.test.tsx +++ b/src/__tests__/Search.test.tsx @@ -1,5 +1,6 @@ import React from "react"; -import { act, fireEvent, render, screen, within } from "@testing-library/react"; +import { act, fireEvent, screen, within } from "@testing-library/react"; +import { render } from "./testUtils/render"; import SearchResults from "../components/Search/Search"; import { FacetItem, SearchField } from "../types/DataModel"; import { ApiSearchResult, SearchQuery } from "../types/SearchQuery"; @@ -623,3 +624,37 @@ describe("Renders search header correctly when viaf search is passed", () => { ); }); }); + +describe("Renders total works correctly when feature flag is set", () => { + beforeEach(() => { + act(() => { + resizeWindow(300, 1000); + }); + }); + test("Total works is shown when feature flag query is true", () => { + mockRouter.push("?feature_totalCount=true"); + render( + + ); + expect(screen.getByText("Total number of works: 26")).toBeInTheDocument(); + }); + + test("Total works is not shown when feature flag query is false", () => { + mockRouter.push("?feature_totalCount=false"); + render( + + ); + expect( + screen.queryByText("Total number of works: 26") + ).not.toBeInTheDocument(); + }); + + test("Total works is not shown when feature flag query is not passed", () => { + render( + + ); + expect( + screen.queryByText("Total number of works: 26") + ).not.toBeInTheDocument(); + }); +}); diff --git a/src/__tests__/testUtils/render.tsx b/src/__tests__/testUtils/render.tsx new file mode 100644 index 00000000..d7e10444 --- /dev/null +++ b/src/__tests__/testUtils/render.tsx @@ -0,0 +1,13 @@ +import { render, RenderOptions } from "@testing-library/react"; +import { ReactElement } from "react"; +import { FeatureFlagProvider } from "~/src/context/FeatureFlagContext"; + +const customRender = ( + ui: ReactElement, + options?: Omit +) => { + render(ui, { wrapper: FeatureFlagProvider, ...options }); +}; + +export * from "@testing-library/react"; +export { customRender as render }; diff --git a/src/components/Search/Search.tsx b/src/components/Search/Search.tsx index 8988b50e..a2b2028a 100644 --- a/src/components/Search/Search.tsx +++ b/src/components/Search/Search.tsx @@ -33,6 +33,8 @@ import ResultsSorts from "../ResultsSorts/ResultsSorts"; import { defaultBreadcrumbs } from "~/src/constants/labels"; import SearchHeader from "../SearchHeader/SearchHeader"; import { ApiWork } from "~/src/types/WorkQuery"; +import useFeatureFlags from "~/src/context/FeatureFlagContext"; +import TotalWorks from "../TotalWorks/TotalWorks"; const SearchResults: React.FC<{ searchQuery: SearchQuery; @@ -43,6 +45,7 @@ const SearchResults: React.FC<{ ...SearchQueryDefaults, ...props.searchQuery, }); + const { isFlagActive } = useFeatureFlags(); const { onClose, onOpen, Modal } = useModal(); @@ -173,6 +176,11 @@ const SearchResults: React.FC<{ + {isFlagActive("totalCount") && ( + + + + )} diff --git a/src/context/FeatureFlagContext.tsx b/src/context/FeatureFlagContext.tsx index 2a144532..87716aad 100644 --- a/src/context/FeatureFlagContext.tsx +++ b/src/context/FeatureFlagContext.tsx @@ -19,7 +19,7 @@ export const FeatureFlagContext = const extractFeatureFlagParams = (query: ParsedUrlQuery) => { const featureFlags = {}; for (const param in query) { - if (param.includes("featureFlag_")) { + if (param.includes("feature_")) { const featureFlag = param.split("_")[1]; featureFlags[featureFlag] = JSON.parse(extractQueryParam(query, param)); } @@ -43,9 +43,11 @@ export const FeatureFlagProvider: React.FC = ({ children }) => { try { storedFeatureFlags = JSON.parse(storedFeatureFlagsStr); for (const flag in storedFeatureFlags) { - const featureFlag = "featureFlag_" + flag; + const featureFlag = "feature_" + flag; if (!query[featureFlag]) { - router.push({ query: { [featureFlag]: storedFeatureFlags[flag] } }); + router.push({ + query: { ...query, [featureFlag]: storedFeatureFlags[flag] }, + }); } } } catch (e) { @@ -71,7 +73,7 @@ export const FeatureFlagProvider: React.FC = ({ children }) => { ); }; -export default function useFeatureFlags() { +export const useFeatureFlags = () => { const context = useContext(FeatureFlagContext); if (typeof context === "undefined") { throw new Error( @@ -79,4 +81,6 @@ export default function useFeatureFlags() { ); } return context; -} +}; + +export default useFeatureFlags; From 37702e9d25508362349a5279a77e43800b55ca07 Mon Sep 17 00:00:00 2001 From: Jackie Quach Date: Tue, 12 Jul 2022 11:30:42 -0400 Subject: [PATCH 5/7] change interface to type --- src/context/FeatureFlagContext.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/context/FeatureFlagContext.tsx b/src/context/FeatureFlagContext.tsx index 87716aad..1853f527 100644 --- a/src/context/FeatureFlagContext.tsx +++ b/src/context/FeatureFlagContext.tsx @@ -3,9 +3,9 @@ import { ParsedUrlQuery } from "querystring"; import React, { createContext, useContext, useEffect, useState } from "react"; import { extractQueryParam } from "../util/LinkUtils"; -interface FeatureFlag { +type FeatureFlag = { [flag: string]: boolean; -} +}; type FeatureFlagContextType = { featureFlags: FeatureFlag; From cb19d4744be65edf61af221725ac834883e80e5f Mon Sep 17 00:00:00 2001 From: Jackie Quach Date: Wed, 13 Jul 2022 12:01:38 -0400 Subject: [PATCH 6/7] update test name --- src/util/__tests__/LinkUtils.test.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/util/__tests__/LinkUtils.test.tsx b/src/util/__tests__/LinkUtils.test.tsx index 6bafe7a0..5cce22db 100644 --- a/src/util/__tests__/LinkUtils.test.tsx +++ b/src/util/__tests__/LinkUtils.test.tsx @@ -41,14 +41,14 @@ describe("Generate back to serach url", () => { describe("Extracts query parameter from url", () => { test("extractQueryParam returns a single flag", () => { mockRouter.setCurrentUrl( - "https://drb-qa.nypl.org/edition/1780467?feature=new_feature" + "https://drb-qa.nypl.org/edition/1780467?feature=true" ); const features = extractQueryParam(mockRouter.query, "feature"); - expect(features).toEqual("new_feature"); + expect(features).toEqual("true"); }); - test("extractQueryParam returns undefined if multiple flags", () => { + test("extractQueryParam returns undefined if multiple flags have the same name", () => { mockRouter.setCurrentUrl( - "https://drb-qa.nypl.org/edition/1780467?feature=new_feature&feature=new_feature2" + "https://drb-qa.nypl.org/edition/1780467?feature=false&feature=true" ); const features = extractQueryParam(mockRouter.query, "feature"); expect(features).toBeUndefined(); From b71216582946c28f3b89446d3290cb253e6671d7 Mon Sep 17 00:00:00 2001 From: Jackie Quach Date: Thu, 14 Jul 2022 13:54:50 -0400 Subject: [PATCH 7/7] add tests for sessionStorage --- src/__tests__/Search.test.tsx | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/src/__tests__/Search.test.tsx b/src/__tests__/Search.test.tsx index f4b84b97..5a486bb3 100644 --- a/src/__tests__/Search.test.tsx +++ b/src/__tests__/Search.test.tsx @@ -629,30 +629,52 @@ describe("Renders total works correctly when feature flag is set", () => { beforeEach(() => { act(() => { resizeWindow(300, 1000); + Object.defineProperty(window, "sessionStorage", { + value: { + getItem: jest.fn(() => null), + setItem: jest.fn(() => null), + }, + writable: true, + }); }); }); - test("Total works is shown when feature flag query is true", () => { + + test("Shown when feature flag query is true", () => { mockRouter.push("?feature_totalCount=true"); render( ); + expect(window.sessionStorage.getItem).toHaveBeenCalledTimes(1); + expect(window.sessionStorage.setItem).toHaveBeenCalledTimes(1); + expect(window.sessionStorage.setItem).toHaveBeenCalledWith( + "featureFlags", + JSON.stringify({ totalCount: true }) + ); expect(screen.getByText("Total number of works: 26")).toBeInTheDocument(); }); - test("Total works is not shown when feature flag query is false", () => { + test("Not shown when feature flag query is false", () => { mockRouter.push("?feature_totalCount=false"); render( ); + expect(window.sessionStorage.getItem).toHaveBeenCalledTimes(1); + expect(window.sessionStorage.setItem).toHaveBeenCalledTimes(1); + expect(window.sessionStorage.setItem).toHaveBeenCalledWith( + "featureFlags", + JSON.stringify({ totalCount: false }) + ); expect( screen.queryByText("Total number of works: 26") ).not.toBeInTheDocument(); }); - test("Total works is not shown when feature flag query is not passed", () => { + test("Not shown when feature flag query is not passed", () => { render( ); + expect(window.sessionStorage.getItem).toHaveBeenCalledTimes(1); + expect(window.sessionStorage.setItem).toHaveBeenCalledTimes(1); expect( screen.queryByText("Total number of works: 26") ).not.toBeInTheDocument();