Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix modal linking #4618

Merged
merged 4 commits into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/packages/app/src/useEventSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
},
Expand Down
4 changes: 2 additions & 2 deletions app/packages/app/src/useEvents/useStateUpdate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ 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,
},
});
Expand Down
120 changes: 90 additions & 30 deletions app/packages/app/src/useEvents/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<AppReadyState>({
Expand All @@ -23,43 +24,102 @@ export const processState = (
state.color_scheme as ColorSchemeInput
);

if (env().VITE_NO_STATE) {
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<string>();
}

return new Set<string>(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;
};
2 changes: 1 addition & 1 deletion app/packages/app/src/useSetters/onSetDataset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ const onSetDataset: RegisteredSetter =
nextDataset: datasetName || null,
extra: {
groupId: null,
sampleId: null,
id: null,
slice: null,
workspace: null,
},
Expand Down
4 changes: 2 additions & 2 deletions app/packages/utilities/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
16 changes: 9 additions & 7 deletions e2e-pw/src/oss/fixtures/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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}`);
}
Expand All @@ -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")}'`);
}
}
Expand Down
4 changes: 2 additions & 2 deletions e2e-pw/src/oss/specs/groups/dynamic-groups.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { test as base } from "src/oss/fixtures";

Check failure on line 1 in e2e-pw/src/oss/specs/groups/dynamic-groups.spec.ts

View workflow job for this annotation

GitHub Actions / e2e / test-e2e

[chromium] › oss/specs/groups/dynamic-groups.spec.ts:70:3 › pcd dynamic group pagination bar

2) [chromium] › oss/specs/groups/dynamic-groups.spec.ts:70:3 › pcd dynamic group pagination bar ── Test timeout of 60000ms exceeded.

Check failure on line 1 in e2e-pw/src/oss/specs/groups/dynamic-groups.spec.ts

View workflow job for this annotation

GitHub Actions / e2e / test-e2e

[chromium] › oss/specs/groups/dynamic-groups.spec.ts:70:3 › png dynamic group pagination bar

3) [chromium] › oss/specs/groups/dynamic-groups.spec.ts:70:3 › png dynamic group pagination bar ── Test timeout of 60000ms exceeded.
import { GridPom } from "src/oss/poms/grid";
import { ModalPom } from "src/oss/poms/modal";
import { getUniqueDatasetNameWithPrefix } from "src/oss/utils";
Expand Down Expand Up @@ -54,7 +54,7 @@
modal,
}) => {
await fiftyoneLoader.waitUntilGridVisible(page, datasetName, {
savedView: "dynamic-group",
searchParams: new URLSearchParams({ view: "dynamic-group" }),
});

await grid.assert.isEntryCountTextEqualTo("10 groups");
Expand All @@ -74,7 +74,7 @@
modal,
}) => {
await fiftyoneLoader.waitUntilGridVisible(page, datasetName, {
savedView: "dynamic-group",
searchParams: new URLSearchParams({ view: "dynamic-group" }),
});
await grid.openFirstSample();

Expand Down
2 changes: 1 addition & 1 deletion e2e-pw/src/oss/specs/groups/sparse-dynamic-groups.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
61 changes: 61 additions & 0 deletions e2e-pw/src/oss/specs/modal/linking.spec.ts
Original file line number Diff line number Diff line change
@@ -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);
});
2 changes: 1 addition & 1 deletion e2e-pw/src/oss/specs/regression-tests/grid-page.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" }),
});
});

Expand Down
4 changes: 2 additions & 2 deletions e2e-pw/src/shared/abstract-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading