From 1abcb6eb7047255614fd13cc1c1d2e10fa44819e Mon Sep 17 00:00:00 2001 From: Les Orchard Date: Tue, 27 Jul 2021 16:39:10 -0700 Subject: [PATCH] fix #5957 feat(nimbus-ui): sortable directory columns Because: * We want to make the directory view table sortable by each column * Currently applied sorting should be reflected in the url query parameter. This commit: * Adds support for directory tab selection via query parameter * Adds experiment sorting to DirectoryTable based on location query parameters * Adds SortableColumnTitle component to DirectoryTable to cycle through column sorting modes and update location query parameters * Updates column definitions in DirectoryTable components to support sorting criteria * Adds sorting utilities to lib/experiment.ts * Tweaks mock data to add more sortable variety * Updates tests and stories with router context * Adds useSearchParamsState hook to read and update location query parameters --- .../PageHome/DirectoryTable/index.stories.tsx | 13 +- .../PageHome/DirectoryTable/index.test.tsx | 215 +++++++++++++++--- .../PageHome/DirectoryTable/index.tsx | 174 +++++++++++--- .../src/components/PageHome/index.stories.tsx | 68 +++--- .../src/components/PageHome/index.test.tsx | 48 +++- .../src/components/PageHome/index.tsx | 12 +- app/experimenter/nimbus-ui/src/hooks/index.ts | 1 + .../src/hooks/useSearchParamsState.test.tsx | 74 ++++++ .../src/hooks/useSearchParamsState.ts | 24 ++ .../nimbus-ui/src/lib/experiment.test.ts | 79 ++++++- .../nimbus-ui/src/lib/experiment.ts | 48 ++++ app/experimenter/nimbus-ui/src/lib/mocks.tsx | 59 ++++- .../nimbus-ui/src/lib/test-utils.tsx | 14 ++ .../nimbus/test_e2e_create_experiment.py | 6 +- 14 files changed, 715 insertions(+), 120 deletions(-) create mode 100644 app/experimenter/nimbus-ui/src/hooks/useSearchParamsState.test.tsx create mode 100644 app/experimenter/nimbus-ui/src/hooks/useSearchParamsState.ts diff --git a/app/experimenter/nimbus-ui/src/components/PageHome/DirectoryTable/index.stories.tsx b/app/experimenter/nimbus-ui/src/components/PageHome/DirectoryTable/index.stories.tsx index cfc5e7870c..8f2f879fec 100644 --- a/app/experimenter/nimbus-ui/src/components/PageHome/DirectoryTable/index.stories.tsx +++ b/app/experimenter/nimbus-ui/src/components/PageHome/DirectoryTable/index.stories.tsx @@ -10,15 +10,25 @@ import DirectoryTable, { DirectoryLiveTable, } from "."; import { mockDirectoryExperiments } from "../../../lib/mocks"; +import { CurrentLocation, RouterSlugProvider } from "../../../lib/test-utils"; import { NimbusExperimentPublishStatus, NimbusExperimentStatus, } from "../../../types/globalTypes"; +const withRouterAndCurrentUrl = (Story: React.FC) => ( + +
+ + +
+ +); + export default { title: "pages/Home/DirectoryTable", component: DirectoryTable, - decorators: [withLinks], + decorators: [withRouterAndCurrentUrl, withLinks], }; const experiments = mockDirectoryExperiments(); @@ -56,6 +66,7 @@ export const CustomComponent = () => ( columns={[ { label: "Testing column", + sortBy: ({ status }) => `Hello ${status}`, component: ({ status }) => Hello {status}, }, ]} diff --git a/app/experimenter/nimbus-ui/src/components/PageHome/DirectoryTable/index.test.tsx b/app/experimenter/nimbus-ui/src/components/PageHome/DirectoryTable/index.test.tsx index 21c60e3bcd..967b2c23e4 100644 --- a/app/experimenter/nimbus-ui/src/components/PageHome/DirectoryTable/index.test.tsx +++ b/app/experimenter/nimbus-ui/src/components/PageHome/DirectoryTable/index.test.tsx @@ -2,28 +2,42 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import { render, screen } from "@testing-library/react"; +import { fireEvent } from "@testing-library/dom"; +import { render, screen, waitFor } from "@testing-library/react"; import React from "react"; import DirectoryTable, { + Column, + ColumnComponent, + ColumnSortOrder, DirectoryColumnFeature, DirectoryColumnOwner, DirectoryColumnTitle, DirectoryCompleteTable, DirectoryDraftsTable, DirectoryLiveTable, + SortableColumnTitle, } from "."; +import { UpdateSearchParams } from "../../../hooks/useSearchParamsState"; import { getProposedEnrollmentRange, humanDate } from "../../../lib/dateUtils"; -import { mockSingleDirectoryExperiment } from "../../../lib/mocks"; +import { + mockDirectoryExperiments, + mockSingleDirectoryExperiment, +} from "../../../lib/mocks"; +import { RouterSlugProvider } from "../../../lib/test-utils"; const experiment = mockSingleDirectoryExperiment(); -const TestTable = ({ children }: { children: React.ReactNode }) => ( - - - {children} - -
-); +const TestTable = ({ children }: { children: React.ReactNode }) => { + return ( + + + + {children} + +
+ + ); +}; describe("DirectoryColumnTitle", () => { it("renders the experiment name and slug", () => { @@ -107,7 +121,11 @@ function expectTableCells(testId: string, cellTexts: string[]) { describe("DirectoryTable", () => { it("renders as expected with default columns", () => { const experiments = [experiment]; - render(); + render( + + + , + ); expectTableCells("directory-table-header", ["Name", "Owner", "Feature"]); expectTableCells("directory-table-cell", [ experiment.name, @@ -120,24 +138,35 @@ describe("DirectoryTable", () => { }); it("renders as expected without experiments", () => { - render(); + render( + + + , + ); expect(screen.getByTestId("no-experiments")).toBeInTheDocument(); }); it("renders as expected with custom columns", () => { render( - ( - {status} - ), - }, - ]} - />, + + ( + {status} + ), + }, + ]} + /> + , ); expectTableCells("directory-table-header", ["Cant think", "Of a title"]); expectTableCells("directory-table-cell", [ @@ -145,11 +174,55 @@ describe("DirectoryTable", () => { experiment.status!, ]); }); + + // TODO: not exhaustively testing all sort orders here, might be worth adding more? + it("supports sorting by name", async () => { + const experiments = mockDirectoryExperiments(); + const experimentNames = experiments.map((experiment) => experiment.name); + const getExperimentNamesFromTable = () => + screen.getAllByTestId("directory-title-name").map((el) => el.textContent); + + render( + + + , + ); + + expect(getExperimentNamesFromTable()).toEqual(experimentNames); + + const nameSortButton = screen + .getAllByTestId("sort-select") + .find((el) => el.title === "Name")!; + expect(nameSortButton).toBeDefined(); + + fireEvent.click(nameSortButton); + await waitFor(() => + expect(getExperimentNamesFromTable()).toEqual( + [...experimentNames].sort((a, b) => a.localeCompare(b)), + ), + ); + + fireEvent.click(nameSortButton); + await waitFor(() => + expect(getExperimentNamesFromTable()).toEqual( + [...experimentNames].sort((a, b) => b.localeCompare(a)), + ), + ); + + fireEvent.click(nameSortButton); + await waitFor(() => + expect(getExperimentNamesFromTable()).toEqual(experimentNames), + ); + }); }); describe("DirectoryLiveTable", () => { it("renders as expected with custom columns", () => { - render(); + render( + + + , + ); expectTableCells("directory-table-header", [ "Name", "Owner", @@ -175,7 +248,11 @@ describe("DirectoryLiveTable", () => { describe("DirectoryCompleteTable", () => { it("renders as expected with custom columns", () => { - render(); + render( + + + , + ); expectTableCells("directory-table-header", [ "Name", "Owner", @@ -197,7 +274,11 @@ describe("DirectoryCompleteTable", () => { describe("DirectoryDraftsTable", () => { it("renders as expected with custom columns", () => { - render(); + render( + + + , + ); expectTableCells("directory-table-header", ["Name", "Owner", "Feature"]); expectTableCells("directory-table-cell", [ experiment.name, @@ -206,3 +287,85 @@ describe("DirectoryDraftsTable", () => { ]); }); }); + +describe("SortableColumnTitle", () => { + const columnComponent: ColumnComponent = ({ name }) => ( + {name} + ); + + const column: Column = { + label: "Name", + sortBy: "name", + component: columnComponent, + }; + + let nextParams: URLSearchParams, updateSearchParams: UpdateSearchParams; + + beforeEach(() => { + nextParams = new URLSearchParams(); + updateSearchParams = jest.fn((updaterFn) => updaterFn(nextParams)); + }); + + const Subject = (props: { columnSortOrder: ColumnSortOrder }) => ( + + + + ); + + const expectClass = (element: HTMLElement, cls: string, toHave = true) => { + const matcher = toHave ? expect(element) : expect(element).not; + matcher.toHaveClass(cls); + }; + + const commonTestCase = + ( + initiallySelected = false, + initiallyDescending = false, + paramsWillHaveSelected = true, + paramsWillHaveDescending = false, + ) => + () => { + render( + , + ); + + const header = screen.getByTestId("directory-table-header"); + expectClass(header, "sort-selected", initiallySelected); + expectClass(header, "sort-descending", initiallyDescending); + + const button = screen.getByTestId("sort-select"); + expect(button).toHaveTextContent(column.label); + + fireEvent.click(button); + expect(updateSearchParams).toHaveBeenCalled(); + + if (paramsWillHaveSelected) { + expect(nextParams.get("sortByLabel")).toEqual(column.label); + } else { + expect(nextParams.has("sortByLabel")).toBeFalsy(); + } + expect(nextParams.has("descending"))[ + paramsWillHaveDescending ? "toBeTruthy" : "toBeFalsy" + ](); + }; + + it( + "renders default as expected and supports ascending sort selection", + commonTestCase(false, false, true, false), + ); + + it( + "renders ascending sort as expected and support descending sort selection", + commonTestCase(true, false, true, true), + ); + + it( + "renders descending sort as expected and support sort reset", + commonTestCase(true, true, false, false), + ); +}); diff --git a/app/experimenter/nimbus-ui/src/components/PageHome/DirectoryTable/index.tsx b/app/experimenter/nimbus-ui/src/components/PageHome/DirectoryTable/index.tsx index 88a8adb892..30dc51d51a 100644 --- a/app/experimenter/nimbus-ui/src/components/PageHome/DirectoryTable/index.tsx +++ b/app/experimenter/nimbus-ui/src/components/PageHome/DirectoryTable/index.tsx @@ -3,13 +3,24 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import { Link } from "@reach/router"; -import React from "react"; +import classNames from "classnames"; +import React, { useCallback, useMemo } from "react"; +import { Button } from "react-bootstrap"; +import { UpdateSearchParams, useSearchParamsState } from "../../../hooks"; import { getProposedEnrollmentRange, humanDate } from "../../../lib/dateUtils"; +import { + enrollmentSortSelector, + experimentSortComparator, + ExperimentSortSelector, + featureConfigNameSortSelector, + ownerUsernameSortSelector, + resultsReadySortSelector, +} from "../../../lib/experiment"; import { getAllExperiments_experiments } from "../../../types/getAllExperiments"; import LinkExternal from "../../LinkExternal"; import NotSet from "../../NotSet"; -// These are all render functions for column type sin the table. +// These are all render functions for column types in the table. export type ColumnComponent = React.FC; export const DirectoryColumnTitle: React.FC = ({ @@ -36,10 +47,12 @@ export const DirectoryColumnTitle: React.FC = ({ ); }; -export const DirectoryColumnOwner: ColumnComponent = ({ owner }) => ( +export const DirectoryColumnOwner: ColumnComponent = (experiment) => ( // #4380 made it so owner is never null, but we have experiments pre-this // that may be absent an owner, so keep this fallback in place. - {owner?.username || } + + {experiment.owner?.username || } + ); export const DirectoryColumnFeature: ColumnComponent = ({ featureConfig }) => ( @@ -63,46 +76,139 @@ export const DirectoryColumnFeature: ColumnComponent = ({ featureConfig }) => ( ); -interface Column { +export interface Column { /** The label of the column, which shows up in */ label: string; + /** Experiment property selector used for sorting the column */ + sortBy: ExperimentSortSelector; /** A component that renders a given the experiment data */ component: ColumnComponent; } +export interface ColumnSortOrder { + column: Column | undefined; + descending: boolean; +} + +interface SortableColumnTitleProps { + column: Column; + columnSortOrder: ColumnSortOrder; + updateSearchParams: UpdateSearchParams; +} + +export const SortableColumnTitle: React.FunctionComponent = + ({ column, columnSortOrder, updateSearchParams }) => { + const { label } = column; + const { descending } = columnSortOrder; + const selected = columnSortOrder.column === column; + + const onClick = useCallback(() => { + updateSearchParams((params) => { + // tri-state sort: ascending -> descending -> reset + if (!selected) { + // 1) ascending + params.set("sortByLabel", label); + params.delete("descending"); + } else { + if (!descending) { + // 2) descending + params.set("sortByLabel", label); + params.set("descending", "1"); + } else { + // 3) reset + params.delete("sortByLabel"); + params.delete("descending"); + } + } + }); + }, [label, selected, updateSearchParams]); + + return ( + + + + ); + }; + interface DirectoryTableProps { experiments: getAllExperiments_experiments[]; columns?: Column[]; } +const commonColumns: Column[] = [ + { label: "Name", sortBy: "name", component: DirectoryColumnTitle }, + { + label: "Owner", + sortBy: ownerUsernameSortSelector, + component: DirectoryColumnOwner, + }, + { + label: "Feature", + sortBy: featureConfigNameSortSelector, + component: DirectoryColumnFeature, + }, +]; + const DirectoryTable: React.FunctionComponent = ({ experiments, - columns: customColumns, + columns = commonColumns, }) => { - const columns = customColumns || [ - { label: "Name", component: DirectoryColumnTitle }, - { label: "Owner", component: DirectoryColumnOwner }, - { label: "Feature", component: DirectoryColumnFeature }, - ]; + const [searchParams, updateSearchParams] = useSearchParamsState(); + const columnSortOrder = useMemo( + () => ({ + column: columns.find( + (column) => column.label === searchParams.get("sortByLabel"), + ), + descending: searchParams.get("descending") === "1", + }), + [searchParams, columns], + ); + + const sortedExperiments = [...experiments]; + if (columnSortOrder.column) { + sortedExperiments.sort( + experimentSortComparator( + columnSortOrder.column.sortBy, + columnSortOrder.descending, + ), + ); + } + return (
{experiments.length ? ( - {columns.map(({ label }, i) => ( - + {columns.map((column, i) => ( + ))} - {experiments.map((experiment) => ( + {sortedExperiments.map((experiment) => ( {columns.map(({ label, component: ColumnComponent }, i) => { return ; @@ -122,17 +228,17 @@ export const DirectoryLiveTable: React.FC = (props) => ( ( ), }, { label: "Enrolling", + sortBy: enrollmentSortSelector, component: (experiment) => ( ), }, { label: "Ended", + sortBy: "computedEndDate", component: ({ computedEndDate: d }) => ( ), }, { label: "Results", + sortBy: resultsReadySortSelector, component: ({ slug }) => (
- {label} -
{d && humanDate(d)} {getProposedEnrollmentRange(experiment)} @@ -141,6 +247,7 @@ export const DirectoryLiveTable: React.FC = (props) => ( }, { label: "Ending", + sortBy: "computedEndDate", component: (experiment) => ( {humanDate(experiment.computedEndDate!)} @@ -149,6 +256,7 @@ export const DirectoryLiveTable: React.FC = (props) => ( }, { label: "Monitoring", + sortBy: "monitoringDashboardUrl", component: ({ monitoringDashboardUrl }) => ( {monitoringDashboardUrl && ( @@ -164,6 +272,7 @@ export const DirectoryLiveTable: React.FC = (props) => ( }, { label: "Results", + sortBy: resultsReadySortSelector, component: (experiment) => ( {experiment.resultsReady ? ( @@ -189,23 +298,24 @@ export const DirectoryCompleteTable: React.FC = ( ( {d && humanDate(d)}{d && humanDate(d)} @@ -219,17 +329,7 @@ export const DirectoryCompleteTable: React.FC = ( ); export const DirectoryDraftsTable: React.FC = (props) => ( - , - }, - { label: "Owner", component: DirectoryColumnOwner }, - { label: "Feature", component: DirectoryColumnFeature }, - ]} - /> + ); export default DirectoryTable; diff --git a/app/experimenter/nimbus-ui/src/components/PageHome/index.stories.tsx b/app/experimenter/nimbus-ui/src/components/PageHome/index.stories.tsx index a58156f57b..5185843124 100644 --- a/app/experimenter/nimbus-ui/src/components/PageHome/index.stories.tsx +++ b/app/experimenter/nimbus-ui/src/components/PageHome/index.stories.tsx @@ -5,45 +5,49 @@ import { withLinks } from "@storybook/addon-links"; import React from "react"; import PageHome from "."; -import { mockDirectoryExperimentsQuery, MockedCache } from "../../lib/mocks"; -import { RouterSlugProvider } from "../../lib/test-utils"; +import { mockDirectoryExperimentsQuery } from "../../lib/mocks"; +import { CurrentLocation, RouterSlugProvider } from "../../lib/test-utils"; import { NimbusExperimentStatus } from "../../types/globalTypes"; +interface StoryContext { + args: { + mocks: Parameters[0]["mocks"]; + }; +} + +const withRouterAndCurrentUrl = ( + Story: React.FC, + { args: { mocks = [mockDirectoryExperimentsQuery()] } }: StoryContext, +) => ( + + <> + + + + +); + export default { title: "pages/Home", component: PageHome, - decorators: [withLinks], + decorators: [withRouterAndCurrentUrl, withLinks], }; -export const Basic = () => ( - - - -); +const storyTemplate = (mocks: StoryContext["args"]["mocks"]) => { + return Object.assign(() => , { args: { mocks } }); +}; -export const Loading = () => ( - - - -); +export const Basic = storyTemplate([mockDirectoryExperimentsQuery()]); -export const NoExperiments = () => ( - - - -); +export const Loading = storyTemplate([]); -export const OnlyDrafts = () => ( - - - -); +export const NoExperiments = storyTemplate([mockDirectoryExperimentsQuery([])]); + +export const OnlyDrafts = storyTemplate([ + mockDirectoryExperimentsQuery([ + { status: NimbusExperimentStatus.DRAFT }, + { status: NimbusExperimentStatus.DRAFT }, + { status: NimbusExperimentStatus.DRAFT }, + { status: NimbusExperimentStatus.DRAFT }, + ]), +]); diff --git a/app/experimenter/nimbus-ui/src/components/PageHome/index.test.tsx b/app/experimenter/nimbus-ui/src/components/PageHome/index.test.tsx index ce79c263dd..c2eb54560f 100644 --- a/app/experimenter/nimbus-ui/src/components/PageHome/index.test.tsx +++ b/app/experimenter/nimbus-ui/src/components/PageHome/index.test.tsx @@ -4,13 +4,16 @@ import * as apollo from "@apollo/client"; import { + fireEvent, render, screen, + waitFor, waitForElementToBeRemoved, } from "@testing-library/react"; import React from "react"; import PageHome from "."; -import { mockDirectoryExperimentsQuery, MockedCache } from "../../lib/mocks"; +import { mockDirectoryExperimentsQuery } from "../../lib/mocks"; +import { CurrentLocation, RouterSlugProvider } from "../../lib/test-utils"; import { getAllExperiments_experiments } from "../../types/getAllExperiments"; describe("PageHome", () => { @@ -20,6 +23,7 @@ describe("PageHome", () => { expect(screen.getByTestId("PageHome")).toBeInTheDocument(); expect(screen.getByText("Create new")).toBeInTheDocument(); }); + it("displays loading when experiments are still loading", () => { (jest.spyOn(apollo, "useQuery") as jest.Mock).mockReturnValueOnce({ loading: true, @@ -29,10 +33,12 @@ describe("PageHome", () => { expect(screen.queryByTestId("page-loading")).toBeInTheDocument(); }); + it("displays loading when experiments are still loading", async () => { await renderAndWaitForLoaded([]); expect(screen.queryByText("No experiments found.")).toBeInTheDocument(); }); + it("renders the error alert when an error occurs", () => { const error = new Error("You done it now!"); @@ -43,14 +49,35 @@ describe("PageHome", () => { render(); expect(screen.queryByTestId("apollo-error-alert")).toBeInTheDocument(); }); + + const findTabs = () => + [ + ["review", screen.getByText("Review (1)")], + ["preview", screen.getByText("Preview (1)")], + ["completed", screen.getByText("Completed (4)")], + ["drafts", screen.getByText("Draft (3)")], + ["live", screen.getByText("Live (3)")], + ] as const; + it("displays five Directory Tables (one for each status type)", async () => { await renderAndWaitForLoaded(); expect(screen.queryAllByTestId("DirectoryTable")).toHaveLength(5); - expect(screen.getByText("Live (3)")).toBeInTheDocument(); - expect(screen.getByText("Review (1)")).toBeInTheDocument(); - expect(screen.getByText("Preview (1)")).toBeInTheDocument(); - expect(screen.getByText("Completed (4)")).toBeInTheDocument(); - expect(screen.getByText("Draft (1)")).toBeInTheDocument(); + for (const [tabKey, tab] of findTabs()) { + expect(tab).toBeInTheDocument(); + } + }); + + it("supports updating search params when tabs are clicked", async () => { + await renderAndWaitForLoaded(); + for (const [tabKey, tab] of findTabs()) { + fireEvent.click(tab); + await waitFor(() => { + expect(tab).toHaveClass("active"); + expect(screen.getByTestId("location")).toHaveTextContent( + `tab=${tabKey}`, + ); + }); + } }); }); @@ -59,9 +86,12 @@ const Subject = ({ }: { experiments?: getAllExperiments_experiments[]; }) => ( - - - + + <> + + + + ); const renderAndWaitForLoaded = async ( diff --git a/app/experimenter/nimbus-ui/src/components/PageHome/index.tsx b/app/experimenter/nimbus-ui/src/components/PageHome/index.tsx index c0211bac6b..ad09b51121 100644 --- a/app/experimenter/nimbus-ui/src/components/PageHome/index.tsx +++ b/app/experimenter/nimbus-ui/src/components/PageHome/index.tsx @@ -4,9 +4,10 @@ import { useQuery } from "@apollo/client"; import { Link, RouteComponentProps } from "@reach/router"; -import React from "react"; +import React, { useCallback } from "react"; import { Alert, Tab, Tabs } from "react-bootstrap"; import { GET_EXPERIMENTS_QUERY } from "../../gql/experiments"; +import { useSearchParamsState } from "../../hooks"; import { getAllExperiments_experiments } from "../../types/getAllExperiments"; import ApolloErrorAlert from "../ApolloErrorAlert"; import AppLayout from "../AppLayout"; @@ -23,10 +24,17 @@ import sortByStatus from "./sortByStatus"; type PageHomeProps = Record & RouteComponentProps; export const Body = () => { + const [searchParams, updateSearchParams] = useSearchParamsState(); const { data, loading, error } = useQuery<{ experiments: getAllExperiments_experiments[]; }>(GET_EXPERIMENTS_QUERY); + const selectedTab = searchParams.get("tab") || "live"; + const onSelectTab = useCallback( + (nextTab) => updateSearchParams((params) => params.set("tab", nextTab)), + [updateSearchParams], + ); + if (loading) { return ; } @@ -43,7 +51,7 @@ export const Body = () => { data.experiments, ); return ( - + diff --git a/app/experimenter/nimbus-ui/src/hooks/index.ts b/app/experimenter/nimbus-ui/src/hooks/index.ts index 42ab8dd972..22133e6e6a 100644 --- a/app/experimenter/nimbus-ui/src/hooks/index.ts +++ b/app/experimenter/nimbus-ui/src/hooks/index.ts @@ -13,3 +13,4 @@ export * from "./useExperiment"; export * from "./useFakeMutation"; export * from "./useOutcomes"; export * from "./useReviewCheck"; +export * from "./useSearchParamsState"; diff --git a/app/experimenter/nimbus-ui/src/hooks/useSearchParamsState.test.tsx b/app/experimenter/nimbus-ui/src/hooks/useSearchParamsState.test.tsx new file mode 100644 index 0000000000..6d3329e5c4 --- /dev/null +++ b/app/experimenter/nimbus-ui/src/hooks/useSearchParamsState.test.tsx @@ -0,0 +1,74 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import React from "react"; +import { RouterSlugProvider } from "../lib/test-utils"; +import useSearchParamsState from "./useSearchParamsState"; + +// TODO: Work out how to test this stuff with @testing-library/react-hooks? +// Depends on being wrapped by @reach/router, so that seems to make it difficult + +describe("hooks/useSearchParamsState", () => { + it("returns the current search parameters", async () => { + const expected = { foo: "bar", baz: "quux" }; + const params = new URLSearchParams(expected); + const path = `/xyzzy/edit?${params.toString()}`; + render(); + expect(screen.getByTestId("params")).toHaveTextContent(params.toString()); + }); + + it("updates the search parameters", async () => { + render( + , + ); + fireEvent.click(screen.getByTestId("setParams")); + await waitFor(() => { + expect(screen.getByTestId("params")).toHaveTextContent( + "wibble=wobble&beep=beep&three=four&one=two", + ); + }); + }); + + interface SubjectProps { + path: string; + paramsToDelete?: Array; + // TODO: This should be Array<[string, string]> but current eslint version trips on it + paramsToSet?: Array; + } + const Subject = (props: SubjectProps) => ( + + + + ); + const SubjectInner = (props: SubjectProps) => { + const [params, setParams] = useSearchParamsState(); + const onClick = () => { + const { paramsToDelete = [], paramsToSet = [] } = props; + setParams((params) => { + for (const name of paramsToDelete) { + params.delete(name); + } + for (const [name, value] of paramsToSet) { + params.set(name, value); + } + }); + }; + return ( +
+ {params.toString()} + +
+ ); + }; +}); diff --git a/app/experimenter/nimbus-ui/src/hooks/useSearchParamsState.ts b/app/experimenter/nimbus-ui/src/hooks/useSearchParamsState.ts new file mode 100644 index 0000000000..b784938bc1 --- /dev/null +++ b/app/experimenter/nimbus-ui/src/hooks/useSearchParamsState.ts @@ -0,0 +1,24 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import { useLocation, useNavigate } from "@reach/router"; +import { useMemo } from "react"; + +export function useSearchParamsState() { + const location = useLocation(); + const navigate = useNavigate(); + return useMemo(() => { + const params = new URLSearchParams(location.search); + const setParams = (updaterFn: (params: URLSearchParams) => void) => { + const nextParams = new URLSearchParams(location.search); + updaterFn(nextParams); + navigate(`${location.pathname}?${nextParams.toString()}`); + }; + return [params, setParams] as const; + }, [navigate, location.search]); +} + +export type UpdateSearchParams = ReturnType[1]; + +export default useSearchParamsState; diff --git a/app/experimenter/nimbus-ui/src/lib/experiment.test.ts b/app/experimenter/nimbus-ui/src/lib/experiment.test.ts index 406f01bda2..55adca4ac4 100644 --- a/app/experimenter/nimbus-ui/src/lib/experiment.test.ts +++ b/app/experimenter/nimbus-ui/src/lib/experiment.test.ts @@ -6,8 +6,21 @@ import { NimbusExperimentPublishStatus, NimbusExperimentStatus, } from "../types/globalTypes"; -import { editCommonRedirects, getStatus } from "./experiment"; -import { mockExperimentQuery } from "./mocks"; +import { + editCommonRedirects, + enrollmentSortSelector, + experimentSortComparator, + featureConfigNameSortSelector, + getStatus, + ownerUsernameSortSelector, + resultsReadySortSelector, + selectFromExperiment, +} from "./experiment"; +import { + mockDirectoryExperiments, + mockExperimentQuery, + mockSingleDirectoryExperiment, +} from "./mocks"; const { experiment } = mockExperimentQuery("boo"); @@ -65,3 +78,65 @@ describe("editCommonRedirects", () => { expect(mockedCall({ status: NimbusExperimentStatus.PREVIEW })).toEqual(""); }); }); + +describe("selectFromExperiment", () => { + const experiment = mockSingleDirectoryExperiment({ + startDate: "2021-06-29T00:00:00Z", + proposedEnrollment: 8, + }); + + it("returns a property selected by string name", () => { + expect(selectFromExperiment(experiment, "name")).toEqual(experiment.name); + }); + + it("returns a property selected via function", () => { + const selectorCases = [ + [featureConfigNameSortSelector, "New tab"], + [ownerUsernameSortSelector, "example@mozilla.com"], + [resultsReadySortSelector, "0"], + [enrollmentSortSelector, "2021-07-07T00:00:00.000Z"], + ] as const; + selectorCases.forEach(([selectBy, expected]) => + expect(selectFromExperiment(experiment, selectBy)).toEqual(expected), + ); + + expect( + selectFromExperiment( + { ...experiment, startDate: null }, + enrollmentSortSelector, + ), + ).toEqual("8"); + }); +}); + +describe("experimentSortComparator", () => { + const experiments = mockDirectoryExperiments().slice(0, 5); + + it("sorts by name ascending as expected", () => { + const sortedExperiments = [...experiments].sort( + experimentSortComparator("name", false), + ); + const names = sortedExperiments.map(({ name }) => name); + expect(names).toEqual([ + "Aliquam interdum ac lacus at dictum", + "Consectetur adipiscing elit", + "Dolor sit amet", + "Ipsum dolor sit amet", + "Lorem ipsum dolor sit amet", + ]); + }); + + it("sorts by name descending as expected", () => { + const sortedExperiments = [...experiments].sort( + experimentSortComparator("name", true), + ); + const names = sortedExperiments.map(({ name }) => name); + expect(names).toEqual([ + "Lorem ipsum dolor sit amet", + "Ipsum dolor sit amet", + "Dolor sit amet", + "Consectetur adipiscing elit", + "Aliquam interdum ac lacus at dictum", + ]); + }); +}); diff --git a/app/experimenter/nimbus-ui/src/lib/experiment.ts b/app/experimenter/nimbus-ui/src/lib/experiment.ts index 7a2aed78f2..d8f3266a34 100644 --- a/app/experimenter/nimbus-ui/src/lib/experiment.ts +++ b/app/experimenter/nimbus-ui/src/lib/experiment.ts @@ -77,3 +77,51 @@ export function getSummaryAction( } return ""; } + +export type ExperimentSortSelector = + | keyof getAllExperiments_experiments + | ((experiment: getAllExperiments_experiments) => string | undefined); + +export const featureConfigNameSortSelector: ExperimentSortSelector = ( + experiment, +) => experiment.featureConfig?.name; + +export const ownerUsernameSortSelector: ExperimentSortSelector = (experiment) => + experiment.owner?.username; + +export const enrollmentSortSelector: ExperimentSortSelector = ({ + startDate, + proposedEnrollment, +}) => { + if (startDate) { + const startTime = new Date(startDate).getTime(); + const enrollmentMS = proposedEnrollment * (1000 * 60 * 60 * 24); + return new Date(startTime + enrollmentMS).toISOString(); + } else { + return "" + proposedEnrollment; + } +}; + +export const resultsReadySortSelector: ExperimentSortSelector = (experiment) => + experiment.resultsReady ? "1" : "0"; + +export const selectFromExperiment = ( + experiment: getAllExperiments_experiments, + selectBy: ExperimentSortSelector, +) => + "" + + (typeof selectBy === "function" + ? selectBy(experiment) + : experiment[selectBy]); + +export const experimentSortComparator = + (sortBy: ExperimentSortSelector, descending: boolean) => + ( + experimentA: getAllExperiments_experiments, + experimentB: getAllExperiments_experiments, + ) => { + const orderBy = descending ? -1 : 1; + const propertyA = selectFromExperiment(experimentA, sortBy); + const propertyB = selectFromExperiment(experimentB, sortBy); + return orderBy * propertyA.localeCompare(propertyB); + }; diff --git a/app/experimenter/nimbus-ui/src/lib/mocks.tsx b/app/experimenter/nimbus-ui/src/lib/mocks.tsx index f741810ffd..f30c125e65 100644 --- a/app/experimenter/nimbus-ui/src/lib/mocks.tsx +++ b/app/experimenter/nimbus-ui/src/lib/mocks.tsx @@ -494,14 +494,16 @@ export const mockGetStatus = ( return getStatus(experiment); }; -const fiveDaysAgo = new Date(); -fiveDaysAgo.setDate(fiveDaysAgo.getDate() - 5); - /** Creates a single mock experiment suitable for getAllExperiments queries. */ export function mockSingleDirectoryExperiment( overrides: Partial = {}, slugIndex: number = Math.round(Math.random() * 100), ): getAllExperiments_experiments { + const now = Date.now(); + const oneDay = 1000 * 60 * 60 * 24; + const startTime = now - oneDay * 60 - 21 * oneDay * Math.random(); + const endTime = now - oneDay * 30 + 21 * oneDay * Math.random(); + return { slug: `some-experiment-${slugIndex}`, owner: { @@ -521,8 +523,8 @@ export function mockSingleDirectoryExperiment( isEnrollmentPausePending: false, proposedEnrollment: 7, proposedDuration: 28, - startDate: fiveDaysAgo.toISOString(), - computedEndDate: new Date(Date.now() + 12096e5).toISOString(), + startDate: new Date(startTime).toISOString(), + computedEndDate: new Date(endTime).toISOString(), resultsReady: false, ...overrides, }; @@ -531,42 +533,83 @@ export function mockSingleDirectoryExperiment( export function mockDirectoryExperiments( overrides: Partial[] = [ { + name: "Lorem ipsum dolor sit amet", + status: NimbusExperimentStatus.DRAFT, + owner: { username: "alpha-example@mozilla.com" }, + startDate: null, + computedEndDate: null, + }, + { + name: "Ipsum dolor sit amet", status: NimbusExperimentStatus.DRAFT, + owner: { username: "gamma-example@mozilla.com" }, + featureConfig: { slug: "foo", name: "Foo" }, startDate: null, computedEndDate: null, }, { + name: "Dolor sit amet", + status: NimbusExperimentStatus.DRAFT, + owner: { username: "beta-example@mozilla.com" }, + featureConfig: { slug: "bar", name: "Bar" }, + startDate: null, + computedEndDate: null, + }, + { + name: "Consectetur adipiscing elit", status: NimbusExperimentStatus.PREVIEW, + owner: { username: "alpha-example@mozilla.com" }, + featureConfig: { slug: "baz", name: "Baz" }, computedEndDate: null, }, { + name: "Aliquam interdum ac lacus at dictum", publishStatus: NimbusExperimentPublishStatus.APPROVED, + owner: { username: "beta-example@mozilla.com" }, + featureConfig: { slug: "foo", name: "Foo" }, computedEndDate: null, }, { + name: "Nam semper sit amet orci in imperdiet", publishStatus: NimbusExperimentPublishStatus.APPROVED, + owner: { username: "gamma-example@mozilla.com" }, }, { + name: "Duis ornare mollis sem.", status: NimbusExperimentStatus.LIVE, - computedEndDate: null, + owner: { username: "alpha-example@mozilla.com" }, + featureConfig: { slug: "bar", name: "Bar" }, }, { + name: "Nec suscipit mi accumsan id", status: NimbusExperimentStatus.LIVE, - computedEndDate: null, + owner: { username: "beta-example@mozilla.com" }, + featureConfig: { slug: "baz", name: "Baz" }, + resultsReady: true, }, { + name: "Etiam congue risus quis aliquet eleifend", status: NimbusExperimentStatus.LIVE, - computedEndDate: null, + owner: { username: "gamma-example@mozilla.com" }, }, { + name: "Nam gravida", status: NimbusExperimentStatus.COMPLETE, + owner: { username: "alpha-example@mozilla.com" }, + featureConfig: { slug: "foo", name: "Foo" }, + resultsReady: false, }, { + name: "Quam quis volutpat ornare", status: NimbusExperimentStatus.DRAFT, publishStatus: NimbusExperimentPublishStatus.REVIEW, + featureConfig: { slug: "Baz", name: "Bar" }, + owner: { username: "beta-example@mozilla.com" }, }, { + name: "Lorem arcu faucibus tortor", featureConfig: null, + owner: { username: "gamma-example@mozilla.com" }, }, ], ): getAllExperiments_experiments[] { diff --git a/app/experimenter/nimbus-ui/src/lib/test-utils.tsx b/app/experimenter/nimbus-ui/src/lib/test-utils.tsx index ab14b2c41d..8a58a30501 100644 --- a/app/experimenter/nimbus-ui/src/lib/test-utils.tsx +++ b/app/experimenter/nimbus-ui/src/lib/test-utils.tsx @@ -9,6 +9,7 @@ import { LocationProvider, RouteComponentProps, Router, + useLocation, } from "@reach/router"; import { render, screen } from "@testing-library/react"; import React from "react"; @@ -116,3 +117,16 @@ export const assertSerializerMessages = async ( } } }; + +export const CurrentLocation = () => { + const location = useLocation(); + return ( +

+ Location:{" "} + + {location.pathname} + {location.search} + +

+ ); +}; diff --git a/app/tests/integration/nimbus/test_e2e_create_experiment.py b/app/tests/integration/nimbus/test_e2e_create_experiment.py index 68b000da83..cbe2308591 100644 --- a/app/tests/integration/nimbus/test_e2e_create_experiment.py +++ b/app/tests/integration/nimbus/test_e2e_create_experiment.py @@ -193,12 +193,12 @@ def test_create_new_experiment_remote_settings_reject(selenium, base_url): modal.decline_changes() # Load home page and wait for experiment to show in the Drafts tab - selenium.get(base_url) + drafts_tab_url = f"{base_url}?tab=drafts" + selenium.get(drafts_tab_url) experiment_found = False for attempt in range(45): try: - home = HomePage(selenium, base_url).wait_for_page_to_load() - home.tabs[-1].click() + home = HomePage(selenium, drafts_tab_url) new_experiments = home.tables[0].experiments for item in new_experiments: if experiment_name in item.text: