Fix dynamic groups modal shortcuts + dynamic groups refactoring to use schema

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 (
     <div
       style={{ width: "100%", height: "100%", position: "relative" }}
-      {...hover.handlers}
+      {...hoverEventHandlers}
     >
-      <SampleBar
-        sampleId={sample.sample._id}
-        lookerRef={lookerRef}
-        visible={hovering}
-        hoveringRef={hoveringRef}
-        actions={actions}
-      />
+      {!isGroup && (
+        <SampleBar
+          sampleId={sample.sample._id}
+          lookerRef={lookerRef}
+          visible={hovering}
+          hoveringRef={hoveringRef}
+          actions={actions}
+        />
+      )}
       {children}
     </div>
   );

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 <T extends AbstractLooker<BaseState>>(
           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;

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 <T extends Lookers>(store: LookerStore<T>) => {
 
           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 };

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 } };

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: [

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;

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");
+    });
   });
 });

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<Field, "fields"> {
@@ -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;
 }