diff --git a/app/packages/core/src/components/Modal/Sample.tsx b/app/packages/core/src/components/Modal/Sample.tsx index 53a6cd74c1..7811e97389 100644 --- a/app/packages/core/src/components/Modal/Sample.tsx +++ b/app/packages/core/src/components/Modal/Sample.tsx @@ -41,27 +41,25 @@ export const SampleWrapper = ({ const hoveringRef = useRef(false); const sample = useRecoilValue(sampleAtom); const isGroup = useRecoilValue(isDynamicGroup); - const hover = useHoveredSample(sample.sample, { + const { handlers: hoverEventHandlers } = useHoveredSample(sample.sample, { update, clear, }); - if (isGroup) { - return <>{children}; - } - return (
- + {!isGroup && ( + + )} {children}
); diff --git a/app/packages/state/src/hooks/useCreateLooker.ts b/app/packages/state/src/hooks/useCreateLooker.ts index 9d135dc6b8..4ce6586604 100644 --- a/app/packages/state/src/hooks/useCreateLooker.ts +++ b/app/packages/state/src/hooks/useCreateLooker.ts @@ -13,6 +13,7 @@ import { BaseState, ImaVidConfig } from "@fiftyone/looker/src/state"; import { EMBEDDED_DOCUMENT_FIELD, LIST_FIELD, + getFieldInfo, getMimeType, isNullish, } from "@fiftyone/utilities"; @@ -31,7 +32,7 @@ import * as dynamicGroupAtoms from "../recoil/dynamicGroups"; import * as schemaAtoms from "../recoil/schema"; import { datasetName } from "../recoil/selectors"; import { State } from "../recoil/types"; -import { getSampleSrc, getSanitizedGroupByExpression } from "../recoil/utils"; +import { getSampleSrc } from "../recoil/utils"; import * as viewAtoms from "../recoil/view"; import { getStandardizedUrls } from "../utils"; @@ -176,9 +177,10 @@ export default >( const { groupBy } = snapshot .getLoadable(dynamicGroupAtoms.dynamicGroupParameters) .valueMaybe(); + const groupByKeyFieldInfo = getFieldInfo(groupBy, fieldSchema); const groupByFieldValue = get( sample, - getSanitizedGroupByExpression(groupBy) + groupByKeyFieldInfo.pathWithDbField ); const groupByFieldValueTransformed = groupByFieldValue !== null ? String(groupByFieldValue) : null; diff --git a/app/packages/state/src/hooks/useExpandSample.ts b/app/packages/state/src/hooks/useExpandSample.ts index 652dbcfcbe..fcfe0f7531 100644 --- a/app/packages/state/src/hooks/useExpandSample.ts +++ b/app/packages/state/src/hooks/useExpandSample.ts @@ -1,7 +1,9 @@ import { FlashlightConfig } from "@fiftyone/flashlight"; +import { getFieldInfo } from "@fiftyone/utilities"; import { get } from "lodash"; import { useRelayEnvironment } from "react-relay"; import { CallbackInterface, RecoilState, useRecoilCallback } from "recoil"; +import { State } from "../recoil"; import * as atoms from "../recoil/atoms"; import * as dynamicGroupAtoms from "../recoil/dynamicGroups"; import * as filterAtoms from "../recoil/filters"; @@ -10,7 +12,6 @@ import * as modalAtoms from "../recoil/modal"; import * as schemaAtoms from "../recoil/schema"; import * as selectors from "../recoil/selectors"; import * as sidebarAtoms from "../recoil/sidebar"; -import { getSanitizedGroupByExpression } from "../recoil/utils"; import { LookerStore, Lookers } from "./useLookerStore"; import useSetExpandedSample from "./useSetExpandedSample"; @@ -138,12 +139,15 @@ export default (store: LookerStore) => { let groupByFieldValue: string; if (dynamicGroupParameters?.groupBy) { - groupByFieldValue = String( - get( - sample.sample, - getSanitizedGroupByExpression(dynamicGroupParameters.groupBy) - ) + const fieldSchema = await snapshot.getPromise( + schemaAtoms.fieldSchema({ space: State.SPACE.SAMPLE }) ); + const fieldInfo = getFieldInfo( + dynamicGroupParameters.groupBy, + fieldSchema + ); + const groupByKeyDbField = fieldInfo.pathWithDbField; + groupByFieldValue = String(get(sample.sample, groupByKeyDbField)); } return { id, groupId, groupByFieldValue }; diff --git a/app/packages/state/src/hooks/useHoveredSample.ts b/app/packages/state/src/hooks/useHoveredSample.ts index feae96f606..7f00196fc0 100644 --- a/app/packages/state/src/hooks/useHoveredSample.ts +++ b/app/packages/state/src/hooks/useHoveredSample.ts @@ -4,21 +4,20 @@ import * as fos from "../.."; export default function useHoveredSample( sample: Sample, - auxHandlers: any = {} + args?: { update?: () => void; clear?: () => void } ) { const setSample = useSetRecoilState(fos.hoveredSample); - const { update, clear } = auxHandlers; function onMouseEnter() { setSample(sample); - update && update(); + args?.update?.(); } function onMouseLeave() { setSample(null); - clear && clear(); + args?.clear?.(); } function onMouseMove() { setSample(sample); - update && update(); + args?.update?.(); } return { handlers: { onMouseEnter, onMouseLeave, onMouseMove } }; diff --git a/app/packages/state/src/recoil/dynamicGroups.ts b/app/packages/state/src/recoil/dynamicGroups.ts index fcb5e5c37d..bb75192443 100644 --- a/app/packages/state/src/recoil/dynamicGroups.ts +++ b/app/packages/state/src/recoil/dynamicGroups.ts @@ -4,6 +4,7 @@ import { EMBEDDED_DOCUMENT_FIELD, GROUP, LIST_FIELD, + getFieldInfo, } from "@fiftyone/utilities"; import { atom, atomFamily, selector, selectorFamily } from "recoil"; import { @@ -14,10 +15,9 @@ import { } from "./groups"; import { modalLooker } from "./modal"; import { dynamicGroupsViewMode } from "./options"; -import { fieldPaths } from "./schema"; +import { fieldPaths, fieldSchema } from "./schema"; import { datasetName, parentMediaTypeSelector } from "./selectors"; import { State } from "./types"; -import { getSanitizedGroupByExpression } from "./utils"; import { GROUP_BY_VIEW_STAGE, LIMIT_VIEW_STAGE, @@ -136,9 +136,8 @@ export const dynamicGroupViewQuery = selectorFamily< const { groupBy, orderBy } = params; - // todo: fix sample_id issue - // todo: sanitize expressions - const groupBySanitized = getSanitizedGroupByExpression(groupBy); + const schema = get(fieldSchema({ space: State.SPACE.SAMPLE })); + const groupByFieldKeyInfo = getFieldInfo(groupBy, schema); let groupByValue; @@ -161,7 +160,7 @@ export const dynamicGroupViewQuery = selectorFamily< $expr: { $let: { vars: { - expr: `$${groupBySanitized}`, + expr: `$${groupByFieldKeyInfo.pathWithDbField}`, }, in: { $in: [ diff --git a/app/packages/state/src/recoil/utils.ts b/app/packages/state/src/recoil/utils.ts index 5c2e74def9..3bfd56c741 100644 --- a/app/packages/state/src/recoil/utils.ts +++ b/app/packages/state/src/recoil/utils.ts @@ -24,14 +24,6 @@ export const getSampleSrc = (url: string) => { return `${params.origin}${path}?filepath=${encodeURIComponent(url)}`; }; -export const getSanitizedGroupByExpression = (expression: string) => { - // todo: why this special case for sample_id...? - if (expression === "sample_id") { - return "_sample_id"; - } - return expression; -}; - export const mapSampleResponse = < T extends Nullable<{ readonly sample?: Sample; diff --git a/app/packages/utilities/src/schema.test.ts b/app/packages/utilities/src/schema.test.ts index c74ba6b823..f1d9624663 100644 --- a/app/packages/utilities/src/schema.test.ts +++ b/app/packages/utilities/src/schema.test.ts @@ -35,6 +35,30 @@ const SCHEMA: schema.Schema = { }, }, }, + embeddedWithDbFields: { + dbField: "embeddedWithDbFields", + description: "description", + embeddedDocType: + "fiftyone.core.odm.embedded_document.DynamicEmbeddedDocument", + ftype: "fiftyone.core.fields.EmbeddedDocumentField", + info: {}, + name: "top", + path: "embeddedWithDbFields", + subfield: null, + fields: { + sample_id: { + dbField: "_sample_id", + pathWithDbField: "", + description: "description", + embeddedDocType: "fiftyone.core.labels.EmbeddedLabel", + ftype: "fiftyone.core.fields.EmbeddedDocumentField", + info: {}, + name: "sample_id", + path: "sample_id", + subfield: null, + }, + }, + }, }; describe("schema", () => { @@ -57,13 +81,17 @@ describe("schema", () => { describe("getFieldInfo ", () => { it("should get top level field info", () => { - expect(schema.getFieldInfo("top", SCHEMA)).toBe(SCHEMA.top); + expect(schema.getFieldInfo("top", SCHEMA)).toEqual({ + ...SCHEMA.top, + pathWithDbField: "", + }); }); it("should get embedded field info", () => { - expect(schema.getFieldInfo("embedded.field", SCHEMA)).toBe( - SCHEMA.embedded.fields.field - ); + expect(schema.getFieldInfo("embedded.field", SCHEMA)).toEqual({ + ...SCHEMA.embedded.fields.field, + pathWithDbField: "", + }); }); it("should return undefined for missing field paths", () => { @@ -72,5 +100,13 @@ describe("schema", () => { expect(schema.getCls("missing.path", {})).toBe(undefined); expect(schema.getFieldInfo("missing.path", SCHEMA)).toBe(undefined); }); + + it("should return correct pathWithDbField", () => { + const field = schema.getFieldInfo( + "embeddedWithDbFields.sample_id", + SCHEMA + ); + expect(field?.pathWithDbField).toBe("embeddedWithDbFields._sample_id"); + }); }); }); diff --git a/app/packages/utilities/src/schema.ts b/app/packages/utilities/src/schema.ts index 02dce4bfba..d65d2d9c7f 100644 --- a/app/packages/utilities/src/schema.ts +++ b/app/packages/utilities/src/schema.ts @@ -8,6 +8,7 @@ export interface Field { subfield: string | null; path: string; fields?: Schema; + pathWithDbField?: string | null; } export interface StrictField extends Omit { @@ -24,13 +25,20 @@ export function getFieldInfo( ): Field | undefined { const keys = fieldPath.split("."); let field: Field; + + const dbFields = []; + for (let index = 0; index < keys.length; index++) { - if (!schema) return undefined; + if (!schema || !schema[keys[index]]) return undefined; - field = schema[keys[index]]; + field = { ...schema[keys[index]] }; schema = field?.fields; + + dbFields.push(field?.dbField); } + field.pathWithDbField = dbFields.filter(Boolean).join("."); + return field; }