From eb42ad3c3b4088fdd4811ff81a57e9aee45fd10c Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Mon, 5 Aug 2024 11:48:31 -0400 Subject: [PATCH 1/4] id fixes, testing updates --- app/packages/app/src/useEventSource.ts | 2 +- .../app/src/useEvents/useStateUpdate.ts | 6 +- app/packages/app/src/useEvents/utils.ts | 116 ++++++++++++++---- .../app/src/useSetters/onSetDataset.ts | 2 +- app/packages/utilities/src/index.ts | 5 +- e2e-pw/src/oss/fixtures/loader.ts | 16 +-- .../src/oss/specs/modal/group-linking.spec.ts | 0 .../oss/specs/modal/sample-linking.spec.ts | 33 +++++ e2e-pw/src/shared/abstract-loader.ts | 4 +- 9 files changed, 144 insertions(+), 40 deletions(-) create mode 100644 e2e-pw/src/oss/specs/modal/group-linking.spec.ts create mode 100644 e2e-pw/src/oss/specs/modal/sample-linking.spec.ts diff --git a/app/packages/app/src/useEventSource.ts b/app/packages/app/src/useEventSource.ts index 8893099f06..439a2536bf 100644 --- a/app/packages/app/src/useEventSource.ts +++ b/app/packages/app/src/useEventSource.ts @@ -58,7 +58,7 @@ const useEventSource = ( dataset: getDatasetName(), group_id: getParam("groupId"), group_slice: getParam("slice"), - sample_id: getParam("sampleId"), + sample_id: getParam("id"), view: getParam("view"), workspace: getParam("workspace"), }, diff --git a/app/packages/app/src/useEvents/useStateUpdate.ts b/app/packages/app/src/useEvents/useStateUpdate.ts index 5d8ce4bcd7..6594fc5ac1 100644 --- a/app/packages/app/src/useEvents/useStateUpdate.ts +++ b/app/packages/app/src/useEvents/useStateUpdate.ts @@ -29,12 +29,14 @@ const useStateUpdate: EventHandlerHook = ({ : (payload.state.saved_view_slug as string), extra: { groupId: state.modalSelector?.groupId || null, - slice: state.groupSlice || null, - sampleId: state.modalSelector?.id || null, + id: state.modalSelector?.id || null, + slice: stateless ? getParam("slice") : state.groupSlice || null, workspace: state.workspace?._name || null, }, }); + console.log(path); + if (readyStateRef.current !== AppReadyState.OPEN) { router.history.replace(path, state); router.load().then(() => setReadyState(AppReadyState.OPEN)); diff --git a/app/packages/app/src/useEvents/utils.ts b/app/packages/app/src/useEvents/utils.ts index 7d16721413..b432419195 100644 --- a/app/packages/app/src/useEvents/utils.ts +++ b/app/packages/app/src/useEvents/utils.ts @@ -7,6 +7,7 @@ import { env, toCamelCase } from "@fiftyone/utilities"; import { atom } from "recoil"; import type { DatasetPageQuery } from "../pages/datasets/__generated__/DatasetPageQuery.graphql"; import type { LocationState } from "../routing"; +import { getParam } from "../utils"; import { AppReadyState } from "./registerEvent"; export const appReadyState = atom({ @@ -23,43 +24,108 @@ export const processState = ( state.color_scheme as ColorSchemeInput ); - if (env().VITE_NO_STATE) { + if (env().VITE_NO_STATE && !groupSlice) { session.sessionGroupSlice = data.dataset?.defaultGroupSlice || undefined; unsubscribe(); return; } - session.sessionGroupSlice = state.group_slice as string; - session.selectedLabels = toCamelCase( - state.selected_labels as object - ) as State.SelectedLabel[]; - session.selectedSamples = new Set(state.selected as string[]); - session.sessionSpaces = (state.spaces || - session.sessionSpaces) as SpaceNodeJSON; - session.fieldVisibilityStage = - (state.field_visibility_stage as State.FieldVisibilityStage) || undefined; + session.sessionGroupSlice = + groupSlice || data.dataset?.defaultGroupSlice || undefined; + session.selectedLabels = resolveSelectedLabels(state); + session.selectedSamples = resolveSelected(state); + session.sessionSpaces = workspace; + session.fieldVisibilityStage = fieldVisibility; session.modalSelector = modalSelector; unsubscribe(); }); - if (env().VITE_NO_STATE) { - return { view: [], fieldVisibility: undefined }; - } - - const modalSelector = - state.group_id || state.sample_id - ? { - groupId: state.group_id as string, - id: state.sample_id as string, - } - : undefined; + const fieldVisibility = resolveFieldVisibility(state); + const groupSlice = resolveGroupSlice(state); + const modalSelector = resolveModalSelector(state); + const view = resolveView(state); + const workspace = resolveWorkspace(session, state); return { - fieldVisibility: state.field_visibility_stage as State.FieldVisibilityStage, - groupSlice: state.group_slice as string, + fieldVisibility, + groupSlice, modalSelector, - view: state.view || [], - workspace: state.spaces as SpaceNodeJSON, + view, + workspace, }; }; +const resolveSelected = (state: { selected?: string[] }) => { + if (env().VITE_NO_STATE) { + return new Set(); + } + + return new Set(state.selected || []); +}; + +const resolveSelectedLabels = (state: { selected_labels?: string[] }) => { + if (env().VITE_NO_STATE) { + return []; + } + + return ( + (toCamelCase(state.selected_labels as object) as State.SelectedLabel[]) || + [] + ); +}; + +const resolveFieldVisibility = (state: { + field_visibility?: State.FieldVisibilityStage; +}) => { + if (env().VITE_NO_STATE) { + return undefined; + } + + return state.field_visibility; +}; + +const resolveGroupSlice = (state: { group_slice?: string }) => { + if (env().VITE_NO_STATE) { + return getParam("slice") || undefined; + } + + return state.group_slice; +}; + +const resolveModalSelector = (state: { + group_id?: string; + sample_id?: string; +}) => { + if (env().VITE_NO_STATE) { + const groupId = getParam("groupId") || undefined; + const id = getParam("id") || undefined; + + return groupId || id ? { groupId, id } : undefined; + } + + return state.group_id || state.sample_id + ? { + groupId: state.group_id as string, + id: state.sample_id as string, + } + : undefined; +}; + +const resolveView = (state: { view?: object[] }) => { + if (env().VITE_NO_STATE) { + return []; + } + + return state.view || []; +}; + +const resolveWorkspace = ( + session: Session, + state: { spaces?: SpaceNodeJSON } +) => { + if (env().VITE_NO_STATE) { + return session.sessionSpaces; + } + + return state.spaces || session.sessionSpaces; +}; diff --git a/app/packages/app/src/useSetters/onSetDataset.ts b/app/packages/app/src/useSetters/onSetDataset.ts index 93c7a5740f..3e0308d02a 100644 --- a/app/packages/app/src/useSetters/onSetDataset.ts +++ b/app/packages/app/src/useSetters/onSetDataset.ts @@ -48,7 +48,7 @@ const onSetDataset: RegisteredSetter = nextDataset: datasetName || null, extra: { groupId: null, - sampleId: null, + id: null, slice: null, workspace: null, }, diff --git a/app/packages/utilities/src/index.ts b/app/packages/utilities/src/index.ts index b5fbe3c66f..a67a6386b2 100644 --- a/app/packages/utilities/src/index.ts +++ b/app/packages/utilities/src/index.ts @@ -4,16 +4,16 @@ import mime from "mime"; import { isElectron } from "./electron"; import { Field } from "./schema"; -export * from "./Resource"; export * from "./color"; export * from "./electron"; export * from "./errors"; export * from "./fetch"; +export * from "./order"; export * from "./paths"; +export * from "./Resource"; export * from "./schema"; export * from "./styles"; export * from "./type-check"; -export * from "./order"; interface O { [key: string]: O | any; @@ -696,6 +696,7 @@ export function pluralize( // @fiftyone/utilities does not use the plugin, so this helper // is defined export const env = (): ImportMetaEnv => { + return { VITE_NO_STATE: true }; return import.meta.env; }; diff --git a/e2e-pw/src/oss/fixtures/loader.ts b/e2e-pw/src/oss/fixtures/loader.ts index 5bcbafc281..b794922ef8 100644 --- a/e2e-pw/src/oss/fixtures/loader.ts +++ b/e2e-pw/src/oss/fixtures/loader.ts @@ -121,15 +121,15 @@ export class OssLoader extends AbstractFiftyoneLoader { datasetName: string, options?: WaitUntilGridVisibleOptions ) { - const { isEmptyDataset, savedView, withGrid } = options ?? { + const { isEmptyDataset, searchParams, withGrid } = options ?? { isEmptyDataset: false, - savedView: undefined, + searchParams: undefined, withGrid: true, }; const forceDatasetFromSelector = async () => { await page.goto("/"); - await page.getByTestId(`selector-Select dataset`).click(); + await page.getByTestId("selector-Select dataset").click(); if (datasetName) { await page.getByTestId(`selector-result-${datasetName}`).click(); @@ -141,8 +141,9 @@ export class OssLoader extends AbstractFiftyoneLoader { } }; - if (savedView) { - await page.goto(`/datasets/${datasetName}?view=${savedView}`); + const search = searchParams ? searchParams.toString() : undefined; + if (search) { + await page.goto(`/datasets/${datasetName}?${search}`); } else { await page.goto(`/datasets/${datasetName}`); } @@ -152,11 +153,12 @@ export class OssLoader extends AbstractFiftyoneLoader { await forceDatasetFromSelector(); } - if (savedView) { + const view = searchParams?.get("view"); + if (view) { const search = await page.evaluate(() => window.location.search); const params = new URLSearchParams(search); - if (params.get("view") !== savedView) { + if (params.get("view") !== view) { throw new Error(`wrong view: '${params.get("view")}'`); } } diff --git a/e2e-pw/src/oss/specs/modal/group-linking.spec.ts b/e2e-pw/src/oss/specs/modal/group-linking.spec.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/e2e-pw/src/oss/specs/modal/sample-linking.spec.ts b/e2e-pw/src/oss/specs/modal/sample-linking.spec.ts new file mode 100644 index 0000000000..da498300a3 --- /dev/null +++ b/e2e-pw/src/oss/specs/modal/sample-linking.spec.ts @@ -0,0 +1,33 @@ +import { test as base } from "src/oss/fixtures"; +import { ModalPom } from "src/oss/poms/modal"; +import { getUniqueDatasetNameWithPrefix } from "src/oss/utils"; + +const test = base.extend<{ modal: ModalPom }>({ + modal: async ({ page, eventUtils }, use) => { + await use(new ModalPom(page, eventUtils)); + }, +}); + +const datasetName = getUniqueDatasetNameWithPrefix("sample-linking"); + +const sampleId = "000000000000000000000000"; + +test.beforeAll(async ({ fiftyoneLoader }) => { + await fiftyoneLoader.executePythonCode(` + import fiftyone as fo + + dataset = fo.Dataset("${datasetName}") + dataset.persistent = True + + dataset.add_sample( + fo.Sample(id=${sampleId}, filepath="sample.png") + )`); +}); + +test(`sample linking`, async ({ page, fiftyoneLoader, modal }) => { + await fiftyoneLoader.waitUntilGridVisible(page, datasetName, { + searchParams: new URLSearchParams({ id: sampleId }), + }); + + await modal.assert.isOpen(); +}); diff --git a/e2e-pw/src/shared/abstract-loader.ts b/e2e-pw/src/shared/abstract-loader.ts index f1c97fb036..41ed010e98 100644 --- a/e2e-pw/src/shared/abstract-loader.ts +++ b/e2e-pw/src/shared/abstract-loader.ts @@ -8,9 +8,9 @@ export type WaitUntilGridVisibleOptions = { isEmptyDataset?: boolean; /** - * Name of the saved view to be loaded. + * Search parameters to include */ - savedView?: string; + searchParams?: URLSearchParams; /** * Whether to wait for the grid to be visible. From 9474525e6dee315a3a158a8877da9c318f3a2409 Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Mon, 5 Aug 2024 15:33:40 -0400 Subject: [PATCH 2/4] cleanup --- .../app/src/useEvents/useStateUpdate.ts | 2 - app/packages/app/src/useEvents/utils.ts | 6 -- .../src/oss/specs/modal/group-linking.spec.ts | 0 e2e-pw/src/oss/specs/modal/linking.spec.ts | 61 +++++++++++++++++++ .../oss/specs/modal/sample-linking.spec.ts | 33 ---------- 5 files changed, 61 insertions(+), 41 deletions(-) delete mode 100644 e2e-pw/src/oss/specs/modal/group-linking.spec.ts create mode 100644 e2e-pw/src/oss/specs/modal/linking.spec.ts delete mode 100644 e2e-pw/src/oss/specs/modal/sample-linking.spec.ts diff --git a/app/packages/app/src/useEvents/useStateUpdate.ts b/app/packages/app/src/useEvents/useStateUpdate.ts index 6594fc5ac1..f87572ed5e 100644 --- a/app/packages/app/src/useEvents/useStateUpdate.ts +++ b/app/packages/app/src/useEvents/useStateUpdate.ts @@ -35,8 +35,6 @@ const useStateUpdate: EventHandlerHook = ({ }, }); - console.log(path); - if (readyStateRef.current !== AppReadyState.OPEN) { router.history.replace(path, state); router.load().then(() => setReadyState(AppReadyState.OPEN)); diff --git a/app/packages/app/src/useEvents/utils.ts b/app/packages/app/src/useEvents/utils.ts index b432419195..58976bb7ea 100644 --- a/app/packages/app/src/useEvents/utils.ts +++ b/app/packages/app/src/useEvents/utils.ts @@ -24,12 +24,6 @@ export const processState = ( state.color_scheme as ColorSchemeInput ); - if (env().VITE_NO_STATE && !groupSlice) { - session.sessionGroupSlice = data.dataset?.defaultGroupSlice || undefined; - unsubscribe(); - return; - } - session.sessionGroupSlice = groupSlice || data.dataset?.defaultGroupSlice || undefined; session.selectedLabels = resolveSelectedLabels(state); diff --git a/e2e-pw/src/oss/specs/modal/group-linking.spec.ts b/e2e-pw/src/oss/specs/modal/group-linking.spec.ts deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/e2e-pw/src/oss/specs/modal/linking.spec.ts b/e2e-pw/src/oss/specs/modal/linking.spec.ts new file mode 100644 index 0000000000..1181ccbe8b --- /dev/null +++ b/e2e-pw/src/oss/specs/modal/linking.spec.ts @@ -0,0 +1,61 @@ +import { test as base } from "src/oss/fixtures"; +import { ModalPom } from "src/oss/poms/modal"; +import { getUniqueDatasetNameWithPrefix } from "src/oss/utils"; + +const test = base.extend<{ modal: ModalPom }>({ + modal: async ({ page, eventUtils }, use) => { + await use(new ModalPom(page, eventUtils)); + }, +}); + +const datasetName = getUniqueDatasetNameWithPrefix("linking"); +const groupDatasetName = getUniqueDatasetNameWithPrefix("group-linking"); + +const id = "000000000000000000000000"; + +test.beforeAll(async ({ fiftyoneLoader }) => { + await fiftyoneLoader.executePythonCode(` + from bson import ObjectId + + import fiftyone as fo + + dataset = fo.Dataset("${datasetName}") + dataset.persistent = True + + sample = fo.Sample(_id=ObjectId("${id}"), filepath="sample.png") + dataset._sample_collection.insert_many( + [dataset._make_dict(sample, include_id=True)] + ) + + + group_dataset = fo.Dataset("${groupDatasetName}") + group_dataset.persistent = True + + group = fo.Group(id="${id}") + group_sample = fo.Sample( + filepath="group_sample.png", group=group.element("only") + ) + group_dataset.add_sample(group_sample)`); +}); + +test(`sample linking`, async ({ page, fiftyoneLoader, modal }) => { + await fiftyoneLoader.waitUntilGridVisible(page, datasetName, { + searchParams: new URLSearchParams({ id }), + }); + + await modal.waitForSampleLoadDomAttribute(true); + + await modal.assert.isOpen(); + await modal.sidebar.assert.verifySidebarEntryText("id", id); +}); + +test(`group linking`, async ({ page, fiftyoneLoader, modal }) => { + await fiftyoneLoader.waitUntilGridVisible(page, groupDatasetName, { + searchParams: new URLSearchParams({ groupId: id }), + }); + + await modal.waitForSampleLoadDomAttribute(true); + + await modal.assert.isOpen(); + await modal.sidebar.assert.verifySidebarEntryText("group.id", id); +}); diff --git a/e2e-pw/src/oss/specs/modal/sample-linking.spec.ts b/e2e-pw/src/oss/specs/modal/sample-linking.spec.ts deleted file mode 100644 index da498300a3..0000000000 --- a/e2e-pw/src/oss/specs/modal/sample-linking.spec.ts +++ /dev/null @@ -1,33 +0,0 @@ -import { test as base } from "src/oss/fixtures"; -import { ModalPom } from "src/oss/poms/modal"; -import { getUniqueDatasetNameWithPrefix } from "src/oss/utils"; - -const test = base.extend<{ modal: ModalPom }>({ - modal: async ({ page, eventUtils }, use) => { - await use(new ModalPom(page, eventUtils)); - }, -}); - -const datasetName = getUniqueDatasetNameWithPrefix("sample-linking"); - -const sampleId = "000000000000000000000000"; - -test.beforeAll(async ({ fiftyoneLoader }) => { - await fiftyoneLoader.executePythonCode(` - import fiftyone as fo - - dataset = fo.Dataset("${datasetName}") - dataset.persistent = True - - dataset.add_sample( - fo.Sample(id=${sampleId}, filepath="sample.png") - )`); -}); - -test(`sample linking`, async ({ page, fiftyoneLoader, modal }) => { - await fiftyoneLoader.waitUntilGridVisible(page, datasetName, { - searchParams: new URLSearchParams({ id: sampleId }), - }); - - await modal.assert.isOpen(); -}); From 3a72198465edb8470f509e752bddfb174702c336 Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Mon, 5 Aug 2024 15:38:37 -0400 Subject: [PATCH 3/4] update for searchParams page load --- e2e-pw/src/oss/specs/groups/dynamic-groups.spec.ts | 4 ++-- e2e-pw/src/oss/specs/groups/sparse-dynamic-groups.spec.ts | 2 +- e2e-pw/src/oss/specs/regression-tests/grid-page.spec.ts | 2 +- .../oss/specs/smoke-tests/quickstart-groups-dynamic.spec.ts | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/e2e-pw/src/oss/specs/groups/dynamic-groups.spec.ts b/e2e-pw/src/oss/specs/groups/dynamic-groups.spec.ts index 5b266d80b0..bb0d767b4f 100644 --- a/e2e-pw/src/oss/specs/groups/dynamic-groups.spec.ts +++ b/e2e-pw/src/oss/specs/groups/dynamic-groups.spec.ts @@ -54,7 +54,7 @@ extensionDatasetNamePairs.forEach(([extension, datasetName]) => { modal, }) => { await fiftyoneLoader.waitUntilGridVisible(page, datasetName, { - savedView: "dynamic-group", + searchParams: new URLSearchParams({ view: "dynamic-group" }), }); await grid.assert.isEntryCountTextEqualTo("10 groups"); @@ -74,7 +74,7 @@ extensionDatasetNamePairs.forEach(([extension, datasetName]) => { modal, }) => { await fiftyoneLoader.waitUntilGridVisible(page, datasetName, { - savedView: "dynamic-group", + searchParams: new URLSearchParams({ view: "dynamic-group" }), }); await grid.openFirstSample(); diff --git a/e2e-pw/src/oss/specs/groups/sparse-dynamic-groups.spec.ts b/e2e-pw/src/oss/specs/groups/sparse-dynamic-groups.spec.ts index d17688c402..9e11346e24 100644 --- a/e2e-pw/src/oss/specs/groups/sparse-dynamic-groups.spec.ts +++ b/e2e-pw/src/oss/specs/groups/sparse-dynamic-groups.spec.ts @@ -87,7 +87,7 @@ test(`left slice (default)`, async ({ fiftyoneLoader, page, grid, modal }) => { test(`right slice`, async ({ fiftyoneLoader, page, grid, modal }) => { await fiftyoneLoader.waitUntilGridVisible(page, datasetName, { - savedView: "group", + searchParams: new URLSearchParams({ view: "group" }), }); await grid.selectSlice("right"); await grid.assert.isEntryCountTextEqualTo("1 group with slice"); diff --git a/e2e-pw/src/oss/specs/regression-tests/grid-page.spec.ts b/e2e-pw/src/oss/specs/regression-tests/grid-page.spec.ts index b339b17b06..9a9a75eff1 100644 --- a/e2e-pw/src/oss/specs/regression-tests/grid-page.spec.ts +++ b/e2e-pw/src/oss/specs/regression-tests/grid-page.spec.ts @@ -69,7 +69,7 @@ test("modal dynamic group carousel has correct second page (all 21 samples)", as page, }) => { await fiftyoneLoader.waitUntilGridVisible(page, datasetName, { - savedView: "group", + searchParams: new URLSearchParams({ view: "group" }), }); await grid.openFirstSample(); await modal.group.setDynamicGroupsNavigationMode("carousel"); diff --git a/e2e-pw/src/oss/specs/smoke-tests/quickstart-groups-dynamic.spec.ts b/e2e-pw/src/oss/specs/smoke-tests/quickstart-groups-dynamic.spec.ts index 66c6463f37..87ded69cbd 100644 --- a/e2e-pw/src/oss/specs/smoke-tests/quickstart-groups-dynamic.spec.ts +++ b/e2e-pw/src/oss/specs/smoke-tests/quickstart-groups-dynamic.spec.ts @@ -54,7 +54,7 @@ test.describe("quickstart-groups", () => { test.beforeEach(async ({ page, fiftyoneLoader }) => { await fiftyoneLoader.waitUntilGridVisible(page, datasetName, { - savedView: "dynamic", + searchParams: new URLSearchParams({ view: "dynamic" }), }); }); From f96164a7516e1d9baefa3f22f68e6405477b3cdd Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Mon, 5 Aug 2024 15:44:29 -0400 Subject: [PATCH 4/4] undo --- app/packages/utilities/src/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/app/packages/utilities/src/index.ts b/app/packages/utilities/src/index.ts index a67a6386b2..b7918836c8 100644 --- a/app/packages/utilities/src/index.ts +++ b/app/packages/utilities/src/index.ts @@ -696,7 +696,6 @@ export function pluralize( // @fiftyone/utilities does not use the plugin, so this helper // is defined export const env = (): ImportMetaEnv => { - return { VITE_NO_STATE: true }; return import.meta.env; };