From 1df67c20ee232434f05ab051944883ad973ff456 Mon Sep 17 00:00:00 2001
From: Benjamin Kane <ben@voxel51.com>
Date: Tue, 11 Jun 2024 12:49:53 -0400
Subject: [PATCH] fix return to index page, add index-page e2e spec (#4478)

---
 app/packages/app/src/Renderer.tsx             |  2 +-
 .../app/src/components/DatasetSelector.tsx    |  1 +
 app/packages/app/src/pages/IndexPage.tsx      |  7 +-
 .../app/src/pages/datasets/DatasetPage.tsx    | 24 ++++---
 app/packages/app/src/routing/RouterContext.ts |  9 +--
 .../app/src/useEvents/useStateUpdate.ts       |  1 +
 e2e-pw/src/oss/poms/page.ts                   | 65 +++++++++++++++++++
 .../specs/regression-tests/index-page.spec.ts | 36 ++++++++++
 8 files changed, 126 insertions(+), 19 deletions(-)
 create mode 100644 e2e-pw/src/oss/poms/page.ts
 create mode 100644 e2e-pw/src/oss/specs/regression-tests/index-page.spec.ts

diff --git a/app/packages/app/src/Renderer.tsx b/app/packages/app/src/Renderer.tsx
index d18d77ce88..c78eb4e3f3 100644
--- a/app/packages/app/src/Renderer.tsx
+++ b/app/packages/app/src/Renderer.tsx
@@ -45,7 +45,7 @@ const Renderer = () => {
   useEffect(() => {
     router.load().then(setRouteEntry);
     subscribe((_, { set }) => {
-      set(entry, router.get());
+      set(entry, router.get(true));
       set(pendingEntry, false);
     });
   }, [router, setRouteEntry]);
diff --git a/app/packages/app/src/components/DatasetSelector.tsx b/app/packages/app/src/components/DatasetSelector.tsx
index 57eb85b95c..14b5f84431 100644
--- a/app/packages/app/src/components/DatasetSelector.tsx
+++ b/app/packages/app/src/components/DatasetSelector.tsx
@@ -22,6 +22,7 @@ const DatasetSelector: React.FC<{
 
   return (
     <Selector<string>
+      cy={"dataset"}
       component={DatasetLink}
       placeholder={"Select dataset"}
       inputStyle={{ height: 40, maxWidth: 300 }}
diff --git a/app/packages/app/src/pages/IndexPage.tsx b/app/packages/app/src/pages/IndexPage.tsx
index b5a596080d..1bf331dc13 100644
--- a/app/packages/app/src/pages/IndexPage.tsx
+++ b/app/packages/app/src/pages/IndexPage.tsx
@@ -5,6 +5,7 @@ import { graphql } from "relay-runtime";
 import Nav from "../components/Nav";
 import { Route } from "../routing";
 import { IndexPageQuery } from "./__generated__/IndexPageQuery.graphql";
+import style from "./index.module.css";
 
 const IndexPageQueryNode = graphql`
   query IndexPageQuery($search: String = "", $count: Int, $cursor: String) {
@@ -28,7 +29,11 @@ const IndexPage: Route<IndexPageQuery> = ({ prepared }) => {
   return (
     <>
       <Nav fragment={queryRef} hasDataset={false} />
-      <Starter mode={totalDatasets === 0 ? "ADD_DATASET" : "SELECT_DATASET"} />
+      <div className={style.page} data-cy={"index-page"}>
+        <Starter
+          mode={totalDatasets === 0 ? "ADD_DATASET" : "SELECT_DATASET"}
+        />
+      </div>
       <Snackbar />
     </>
   );
diff --git a/app/packages/app/src/pages/datasets/DatasetPage.tsx b/app/packages/app/src/pages/datasets/DatasetPage.tsx
index 32fe08f1db..9d13e019e5 100644
--- a/app/packages/app/src/pages/datasets/DatasetPage.tsx
+++ b/app/packages/app/src/pages/datasets/DatasetPage.tsx
@@ -112,19 +112,17 @@ const DatasetPage: Route<DatasetPageQuery> = ({ prepared }) => {
   return (
     <>
       <Nav fragment={data} hasDataset={!isEmpty} />
-      {isEmpty ? (
-        <Starter mode="ADD_SAMPLE" />
-      ) : (
-        <>
-          <div className={style.page}>
-            <datasetQueryContext.Provider value={data}>
-              <OperatorCore />
-              <Dataset />
-            </datasetQueryContext.Provider>
-          </div>
-          <Snackbar />
-        </>
-      )}
+      <div className={style.page} data-cy={"dataset-page"}>
+        {isEmpty ? (
+          <Starter mode="ADD_SAMPLE" />
+        ) : (
+          <datasetQueryContext.Provider value={data}>
+            <OperatorCore />
+            <Dataset />
+          </datasetQueryContext.Provider>
+        )}
+      </div>
+      <Snackbar />
     </>
   );
 };
diff --git a/app/packages/app/src/routing/RouterContext.ts b/app/packages/app/src/routing/RouterContext.ts
index da08a58e9e..cfd3bd1505 100644
--- a/app/packages/app/src/routing/RouterContext.ts
+++ b/app/packages/app/src/routing/RouterContext.ts
@@ -54,7 +54,7 @@ type Subscribe<T extends OperationType> = (
 
 export interface RoutingContext<T extends OperationType> {
   history: ReturnType<typeof createBrowserHistory>;
-  get: () => Entry<T>;
+  get: (next: boolean) => Entry<T>;
   load: (hard?: boolean) => Promise<Entry<T>>;
   subscribe: Subscribe<T>;
 }
@@ -131,11 +131,12 @@ export const createRouter = <T extends OperationType>(
       runUpdate && update(history.location as FiftyOneLocation);
       return currentEntryResource.load();
     },
-    get() {
-      if (!currentEntryResource) {
+    get(next = false) {
+      const resource = next ? nextCurrentEntryResource : currentEntryResource;
+      if (!resource) {
         throw new Error("no entry loaded");
       }
-      const entry = currentEntryResource.get();
+      const entry = resource.get();
       if (!entry) {
         throw new Error("entry is loading");
       }
diff --git a/app/packages/app/src/useEvents/useStateUpdate.ts b/app/packages/app/src/useEvents/useStateUpdate.ts
index 0f9421e434..dd53e1246d 100644
--- a/app/packages/app/src/useEvents/useStateUpdate.ts
+++ b/app/packages/app/src/useEvents/useStateUpdate.ts
@@ -27,6 +27,7 @@ const useStateUpdate: EventHandlerHook = ({
           workspace: state.workspace?._name || null,
         },
       });
+
       if (readyStateRef.current !== AppReadyState.OPEN) {
         router.history.replace(path, state);
         router.load().then(() => setReadyState(AppReadyState.OPEN));
diff --git a/e2e-pw/src/oss/poms/page.ts b/e2e-pw/src/oss/poms/page.ts
new file mode 100644
index 0000000000..0a45d5c545
--- /dev/null
+++ b/e2e-pw/src/oss/poms/page.ts
@@ -0,0 +1,65 @@
+import { Locator, Page, expect } from "src/oss/fixtures";
+import { EventUtils } from "src/shared/event-utils";
+import { SelectorPom } from "./selector";
+
+export class PagePom {
+  readonly assert: PageAsserter;
+  readonly datasetSelector: SelectorPom;
+  readonly locator: Locator;
+
+  constructor(
+    private readonly page: Page,
+    private readonly eventUtils: EventUtils
+  ) {
+    this.assert = new PageAsserter(this);
+    this.datasetSelector = new SelectorPom(page, eventUtils, "dataset");
+  }
+
+  get pathname() {
+    return this.url.pathname;
+  }
+
+  get url() {
+    return new URL(this.page.url());
+  }
+
+  getPage(pagename: string) {
+    return this.page.getByTestId(`${pagename}-page`);
+  }
+
+  async loadDataset(dataset?: string) {
+    if (!dataset) {
+      await this.page.goto("/");
+    } else {
+      await this.datasetSelector.openResults();
+      await this.datasetSelector.selectResult(dataset);
+    }
+    await this.page.waitForSelector(
+      `[data-cy=${dataset ? "dataset" : "index"}-page]`,
+      {
+        state: "visible",
+      }
+    );
+  }
+}
+
+class PageAsserter {
+  constructor(private readonly pagePom: PagePom) {}
+
+  async verifyPage(pagename: string) {
+    await expect(this.pagePom.getPage(pagename)).toBeVisible();
+  }
+
+  async verifyPathname(pathname: string) {
+    await expect(this.pagePom.pathname).toEqual(pathname);
+  }
+
+  async verifyDataset(datasetName: string) {
+    await this.pagePom.datasetSelector.assert.verifyValue(datasetName);
+    await expect(this.pagePom.pathname).toEqual(`/datasets/${datasetName}`);
+  }
+
+  async verifyDatasets(datasetNames: string[]) {
+    await this.pagePom.datasetSelector.assert.verifyResults(datasetNames);
+  }
+}
diff --git a/e2e-pw/src/oss/specs/regression-tests/index-page.spec.ts b/e2e-pw/src/oss/specs/regression-tests/index-page.spec.ts
new file mode 100644
index 0000000000..378048f82f
--- /dev/null
+++ b/e2e-pw/src/oss/specs/regression-tests/index-page.spec.ts
@@ -0,0 +1,36 @@
+import { test as base } from "src/oss/fixtures";
+import { PagePom } from "src/oss/poms/page";
+import { getUniqueDatasetNameWithPrefix } from "src/oss/utils";
+
+const datasetName = getUniqueDatasetNameWithPrefix(`index-page`);
+
+const test = base.extend<{
+  pagePom: PagePom;
+}>({
+  pagePom: async ({ eventUtils, page }, use) => {
+    await use(new PagePom(page, eventUtils));
+  },
+});
+
+test.beforeAll(async ({ fiftyoneLoader }) => {
+  await fiftyoneLoader.executePythonCode(`
+  import fiftyone as fo
+
+  dataset = fo.Dataset("${datasetName}")
+
+  dataset.persistent = True`);
+});
+
+test("index page", async ({ pagePom, page }) => {
+  await pagePom.loadDataset();
+  await pagePom.assert.verifyPage("index");
+  await pagePom.assert.verifyPathname("/");
+
+  await pagePom.loadDataset(datasetName);
+  await pagePom.assert.verifyPage("dataset");
+  await pagePom.assert.verifyPathname(`/datasets/${datasetName}`);
+
+  await page.goBack();
+  await pagePom.assert.verifyPage("index");
+  await pagePom.assert.verifyPathname("/");
+});