refactor: collections and attributes representation in DataSets (#1283

)

v3/cypress/e2e/graph.spec.ts

@@ -539,16 +539,16 @@ context("Graph UI", () => {
         expect(valueNum).to.be.closeTo(4, 0.1)
       })
     })
-    it("shows a bar graph when there's one categorical attr on primary axis and 'Fuse Dots into Bars' is checked", () => {
+    it("shows a bar graph with one categorical attr on primary axis and 'Fuse Dots into Bars' is checked", () => {
       cy.dragAttributeToTarget("table", "Habitat", "bottom")
       cy.get("[data-testid=bar-cover]").should("not.exist")
       graph.getDisplayConfigButton().click()
       cy.get("[data-testid=bar-chart-checkbox]").find("input").should("exist").and("have.attr", "type", "checkbox")
         .and("not.be.checked")
       cy.get("[data-testid=bar-chart-checkbox]").click()
       cy.get("[data-testid=bar-chart-checkbox]").find("input").should("be.checked")
-      // TODO: It would be better to check for the exact number of bars, but the number seems to vary depending on whether
-      // you're running the test locally or in CI for some mysterious reason.
+      // TODO: It would be better to check for the exact number of bars, but the number seems to vary depending
+      // on whether you're running the test locally or in CI for some mysterious reason.
       // cy.get("[data-testid=bar-cover]").should("exist").and("have.length", 3)
       cy.get("[data-testid=bar-cover]").should("exist")
       cy.get(".axis-wrapper.left").find("[data-testid=attribute-label]").should("exist").and("have.text", "Count")

v3/cypress/e2e/hierarchical-table.spec.ts

@@ -12,12 +12,13 @@ context("hierarchical collections", () => {
   hierarchical.tests.forEach((h) => {
     it(`${h.testName}`, () => {
       const collections = h.collections
-      collections.forEach(collection => {
+      collections.forEach((collection, index) => {
+        cy.log("Testing collection:", index, "name:", collection.name)
         collection.attributes.forEach(attribute => {
+          cy.log("Moving attribute:", attribute.name)
           table.moveAttributeToParent(attribute.name, attribute.move)
           table.getColumnHeaders(collection.index+1).should("not.contain", attribute.name)
           table.getAttribute(attribute.name, collection.index).should("have.text", attribute.name)
-          // 
           cy.wait(2000)
         })
         table.getCollectionTitle(collection.index).should("have.text", collection.name)
@@ -26,6 +27,7 @@ context("hierarchical collections", () => {
         table.verifyAttributeValues(collection.attributes, values, collection.index)
         cy.wait(2000)
 
+        cy.log("Testing expanding/collapsing...")
         table.verifyCollapseAllGroupsButton(collection.index+1)
         table.collapseAllGroups(collection.index+1)
         table.getNumOfRows(collection.index+1).should("contain", collection.cases+1)

v3/src/components/case-card/case-card.tsx

@@ -52,7 +52,7 @@ export const CaseCard = observer(function CaseCard({ setNodeRef }: IProps) {
   if (!cardModel || !data) return null
 
   // access observable properties that should trigger re-renders
-  data.collectionModels.map(({ name }) => name)
+  data.collections.map(({ name }) => name)
   data.attributes.map(({ name }) => name)
   data.cases.map(({ __id__ }) => __id__)
   data.selectionChanges   // eslint-disable-line no-unused-expressions

v3/src/components/case-card/case-card.v2.test.tsx

@@ -3,8 +3,8 @@ import { render, screen } from "@testing-library/react"
 import { userEvent } from '@testing-library/user-event'
 import React from "react"
 import { DG } from "../../v2/dg-compat.v2"
+import { createDataSet } from "../../models/data/data-set-conversion"
 import { DGDataContext } from "../../models/v2/dg-data-context"
-import { DataSet, LEGACY_ATTRIBUTES_ARRAY_ANY } from "../../models/data/data-set"
 import { t } from "../../utilities/translation/translate"
 import "./case-card.v2"
 const { CaseCard } = DG.React as any
@@ -22,10 +22,8 @@ describe("CaseCard component", () => {
   })
 
   it("renders a flat data set", async () => {
-    const data = DataSet.create({
-      attributes: [
-        { id: "AttrId", name: "AttrName" }
-      ] as LEGACY_ATTRIBUTES_ARRAY_ANY
+    const data = createDataSet({
+      attributes: [{ id: "AttrId", name: "AttrName" }]
     })
     data.addCases([{ __id__: "Case1", AttrId: "foo" }, { __id__: "Case2", AttrId: "bar" }])
     const context = new DGDataContext(data)
@@ -135,7 +133,7 @@ describe("CaseCard component", () => {
         isSelectedCallback={() => mockIsSelected()}
       />
     )
-    expect(screen.getByText("1 selected of 3")).toBeInTheDocument()
+    expect(screen.getByText("1 selected of 3 Cases")).toBeInTheDocument()
 
     // clicking attribute name brings up menu
     await user.click(attrName)
@@ -155,11 +153,11 @@ describe("CaseCard component", () => {
   })
 
   it("renders a hierarchical data set", async () => {
-    const data = DataSet.create({
+    const data = createDataSet({
       attributes: [
         { id: "Attr1Id", name: "Attr1Name" },
         { id: "Attr2Id", name: "Attr2Name" }
-      ] as LEGACY_ATTRIBUTES_ARRAY_ANY
+      ]
     })
     data.addCases([
       { __id__: "Case1", Attr1Id: "foo", Attr2Id: 1 },

v3/src/components/case-table/case-table.tsx

@@ -69,24 +69,15 @@ export const CaseTable = observer(function CaseTable({ setNodeRef }: IProps) {
       let collection: ICollectionModel | undefined
 
       // Determine if the old collection will become empty and therefore get removed
-      // TODO Revisit this after collection overhaul. Can we just use dataSet.getCollection(oldCollectionId)
-      // to determine if the old collection still exists?
       const oldCollectionId = dataSet.getCollectionForAttribute(attrId)?.id
       let removedOldCollection = false
-      if (oldCollectionId) {
-        if (oldCollectionId === dataSet.ungrouped.id) {
-          if (dataSet.ungroupedAttributes.length <= 1) removedOldCollection = true
-        } else {
-          const oldCollectionLength = dataSet.getGroupedCollection(oldCollectionId)?.attributes.length
-          if (oldCollectionLength && oldCollectionLength <= 1) removedOldCollection = true
-        }
-      }
 
       dataSet.applyModelChange(() => {
         collection = dataSet.moveAttributeToNewCollection(attrId, beforeCollectionId)
         if (collection) {
           lastNewCollectionDrop.current = { newCollectionId: collection.id, beforeCollectionId }
         }
+        removedOldCollection = !!(oldCollectionId && !dataSet.getCollection(oldCollectionId))
       }, {
         notifications: () => {
           const notifications: INotification[] = []
@@ -107,7 +98,7 @@ export const CaseTable = observer(function CaseTable({ setNodeRef }: IProps) {
 
     if (!tableModel || !data) return null
 
-    const collections = data.collectionModels
+    const collections = data.collections
     const handleHorizontalScroll: React.UIEventHandler<HTMLDivElement> = () => {
       tableModel?.setScrollLeft(contentRef.current?.scrollLeft ?? 0)
     }

v3/src/components/case-table/collection-table-spacer.tsx

@@ -66,7 +66,12 @@ export const CollectionTableSpacer = observer(function CollectionTableSpacer({ o
   // }
 
   function handleTopClick() {
-    parentCases?.forEach((value) => caseMetadata?.setIsCollapsed(value.__id__, !everyCaseIsCollapsed))
+    caseMetadata?.applyModelChange(() => {
+      parentCases?.forEach((value) => caseMetadata?.setIsCollapsed(value.__id__, !everyCaseIsCollapsed))
+    }, {
+      undoStringKey: "DG.Undo.caseTable.groupToggleExpandCollapseAll",
+      redoStringKey: "DG.Redo.caseTable.groupToggleExpandCollapseAll"
+    })
   }
 
   function handleExpandCollapseClick(parentCaseId: string) {

v3/src/components/case-table/use-index-column.tsx

@@ -9,7 +9,7 @@ import { useRdgCellFocus } from "./use-rdg-cell-focus"
 import { useCaseMetadata } from "../../hooks/use-case-metadata"
 import { useCollectionContext } from "../../hooks/use-collection-context"
 import { useDataSetContext } from "../../hooks/use-data-set-context"
-import { ICollectionPropsModel } from "../../models/data/collection"
+import { ICollectionModel } from "../../models/data/collection"
 import { IDataSet } from "../../models/data/data-set"
 import { symIndex, symParent } from "../../models/data/data-set-types"
 import { getCollectionAttrs } from "../../models/data/data-set-utils"
@@ -19,7 +19,7 @@ import { t } from "../../utilities/translation/translate"
 interface IColSpanProps {
   data?: IDataSet
   metadata?: ISharedCaseMetadata
-  collection: ICollectionPropsModel
+  collection: ICollectionModel
 }
 function indexColumnSpan(args: TColSpanArgs, { data, metadata, collection }: IColSpanProps) {
   // collapsed rows span the entire table

v3/src/components/data-display/models/data-configuration-model.ts

@@ -80,14 +80,14 @@ export const DataConfigurationModel = types
             .filter(id => !!id),
           childmostCollectionID = idOfChildmostCollectionForAttributes(attrIDs, self.dataset)
         if (childmostCollectionID) {
-          const childmostCollection = self.dataset?.getGroupedCollection(childmostCollectionID),
+          const childmostCollection = self.dataset?.getCollection(childmostCollectionID),
             childmostCollectionAttributes = childmostCollection?.attributes
           if (childmostCollectionAttributes?.length) {
             const firstAttribute = childmostCollectionAttributes[0]
             return firstAttribute?.id
           }
         }
-        return self.dataset?.ungroupedAttributes[0]?.id
+        return self.dataset?.childCollection.attributes[0]?.id
       }
 
       let attrID = this.attributeDescriptionForRole(role)?.attributeID || ""

v3/src/components/graph/components/scatterdots.tsx

@@ -4,6 +4,7 @@ import React, {useCallback, useEffect, useRef, useState} from "react"
 import { autorun } from "mobx"
 import { observer } from "mobx-react-lite"
 import {appState} from "../../../models/app-state"
+import { firstVisibleParentAttribute, idOfChildmostCollectionForAttributes } from "../../../models/data/data-set-utils"
 import {ScaleNumericBaseType} from "../../axis/axis-types"
 import {CaseData} from "../../data-display/d3-types"
 import {PlotProps} from "../graphing-types"
@@ -176,7 +177,8 @@ export const ScatterDots = observer(function ScatterDots(props: PlotProps) {
     if (!showConnectingLines && !connectingLinesActivatedRef.current) return
     const { connectingLinesForCases } = scatterPlotFuncs(layout, dataConfiguration)
     const connectingLines = connectingLinesForCases()
-    const parentAttr = dataset?.collections[0]?.attributes[0]
+    const childmostCollectionId = idOfChildmostCollectionForAttributes(dataConfiguration?.attributes ?? [], dataset)
+    const parentAttr = firstVisibleParentAttribute(dataset, childmostCollectionId)
     const parentAttrID = parentAttr?.id
     const parentAttrName = parentAttr?.name
     const cellKeys = dataConfiguration?.getAllCellKeys()
@@ -199,7 +201,7 @@ export const ScatterDots = observer(function ScatterDots(props: PlotProps) {
       await pixiPoints?.setAllPointsScale(scaleFactor, transitionDuration)
       graphModel.pointDescription.setPointSizeMultiplier(origPointSizeMultiplier.current)
     }
-  }, [showConnectingLines, layout, dataConfiguration, dataset?.collections, pointSizeMultiplier, renderConnectingLines,
+  }, [showConnectingLines, layout, dataConfiguration, dataset, pointSizeMultiplier, renderConnectingLines,
       graphModel, pixiPoints])
 
   const refreshSquares = useCallback(() => {

v3/src/components/graph/models/graph-controller.test.ts

@@ -2,7 +2,8 @@ import { applySnapshot, getSnapshot, SnapshotIn, types } from "mobx-state-tree"
 import { GraphContentModel } from "./graph-content-model"
 import { GraphController } from "./graph-controller"
 import { GraphLayout } from "./graph-layout"
-import { DataSet, LEGACY_ATTRIBUTES_ARRAY_ANY } from "../../../models/data/data-set"
+import { DataSet } from "../../../models/data/data-set"
+import { createDataSet } from "../../../models/data/data-set-conversion"
 import { SharedCaseMetadata } from "../../../models/shared/shared-case-metadata"
 import { attrRoleToGraphPlace, GraphAttrRole } from "../../data-display/data-display-types"
 import { isCategoricalAxisModel, isEmptyAxisModel, isNumericAxisModel } from "../../axis/models/axis-model"
@@ -29,13 +30,13 @@ describe("GraphController", () => {
 
   const Tree = types.model("Tree", {
     model: types.optional(GraphContentModel, () => GraphContentModel.create()),
-    data: types.optional(DataSet, () => DataSet.create({
+    data: types.optional(DataSet, () => createDataSet({
       attributes: [
         { id: "xId", name: "x", values: ["1", "2", "3"] },
         { id: "yId", name: "y", values: ["4", "5", "6"] },
         { id: "y2Id", name: "y2", values: ["7", "8", "9"] },
         { id: "cId", name: "c", values: ["a", "b", "c"] }
-      ] as LEGACY_ATTRIBUTES_ARRAY_ANY
+      ]
     })),
     metadata: types.optional(SharedCaseMetadata, () => SharedCaseMetadata.create())
   })

v3/src/components/map/components/map-point-layer.tsx

@@ -3,12 +3,14 @@ import {comparer, reaction} from "mobx"
 import { observer } from "mobx-react-lite"
 import { isAlive } from "mobx-state-tree"
 import * as PIXI from "pixi.js"
-import {mstReaction} from "../../../utilities/mst-reaction"
-import {onAnyAction} from "../../../utilities/mst-utils"
-import {useDebouncedCallback} from "use-debounce"
 import {useMap} from "react-leaflet"
+import {useDebouncedCallback} from "use-debounce"
 import {isSelectionAction, isSetCaseValuesAction} from "../../../models/data/data-set-actions"
+import { firstVisibleParentAttribute, idOfChildmostCollectionForAttributes } from "../../../models/data/data-set-utils"
 import {defaultSelectedStroke, defaultSelectedStrokeWidth, defaultStrokeWidth} from "../../../utilities/color-utils"
+import { mstAutorun } from "../../../utilities/mst-autorun"
+import {mstReaction} from "../../../utilities/mst-reaction"
+import {onAnyAction} from "../../../utilities/mst-utils"
 import {DataTip} from "../../data-display/components/data-tip"
 import {CaseData} from "../../data-display/d3-types"
 import {
@@ -23,7 +25,6 @@ import {IPixiPointMetadata, PixiPoints} from "../../data-display/pixi/pixi-point
 import {useMapModelContext} from "../hooks/use-map-model-context"
 import {IMapPointLayerModel} from "../models/map-point-layer-model"
 import {MapPointGrid} from "./map-point-grid"
-import { mstAutorun } from "../../../utilities/mst-autorun"
 import { useInstanceIdContext } from "../../../hooks/use-instance-id-context"
 import { useConnectingLines } from "../../data-display/hooks/use-connecting-lines"
 
@@ -145,14 +146,15 @@ export const MapPointLayer = observer(function MapPointLayer({mapLayerModel, onS
   const refreshConnectingLines = useCallback(() => {
     if (!showConnectingLines && !connectingLinesActivatedRef.current) return
     const connectingLines = connectingLinesForCases()
-    const parentAttr = dataset?.collections[0]?.attributes[0]
+    const childmostCollectionId = idOfChildmostCollectionForAttributes(dataConfiguration?.attributes ?? [], dataset)
+    const parentAttr = firstVisibleParentAttribute(dataset, childmostCollectionId)
     const parentAttrID = parentAttr?.id
     const parentAttrName = parentAttr?.name
     const pointColorAtIndex = mapModel.pointDescription.pointColorAtIndex
 
     renderConnectingLines({ connectingLines, parentAttrID, parentAttrName, pointColorAtIndex, showConnectingLines })
-  }, [connectingLinesForCases, dataset?.collections, mapModel.pointDescription.pointColorAtIndex, renderConnectingLines,
-      showConnectingLines])
+  }, [connectingLinesForCases, dataConfiguration, dataset, mapModel.pointDescription.pointColorAtIndex,
+      renderConnectingLines, showConnectingLines])
 
   const callMatchCirclesToData = useCallback(() => {
     if (mapLayerModel && dataConfiguration && layout && pixiPointsRef.current) {

v3/src/data-interactive/data-interactive-type-utils.ts

@@ -1,6 +1,6 @@
 import { getSnapshot } from "mobx-state-tree"
 import { IAttribute, IAttributeSnapshot } from "../models/data/attribute"
-import { ICollectionModel, ICollectionPropsModel } from "../models/data/collection"
+import { ICollectionModel } from "../models/data/collection"
 import { IDataSet } from "../models/data/data-set"
 import { ICase } from "../models/data/data-set-types"
 import { v2ModelSnapshotFromV2ModelStorage } from "../models/data/v2-model"
@@ -44,14 +44,14 @@ export function convertCaseToV2FullCase(c: ICase, dataContext: IDataSet) {
     name: dataContext.name
   }
 
+  const _collection = dataContext.getCollectionForCase(caseId)
+  const collectionId = _collection?.id
   const caseGroup = dataContext.pseudoCaseMap.get(caseId)
-  const collectionId = caseGroup?.collectionId ?? dataContext.ungrouped.id
 
   const parent = maybeToV2Id(dataContext.getParentCase(caseId, collectionId)?.pseudoCase.__id__)
 
-  const _collection = dataContext.getCollection(collectionId)
-  const collectionIndex = dataContext.getCollectionIndex(collectionId)
-  const parentCollection = dataContext.collections[collectionIndex - 1]
+  const collectionIndex = collectionId ? dataContext.getCollectionIndex(collectionId) : -1
+  const parentCollection = collectionIndex > 0 ? dataContext.collections[collectionIndex - 1] : undefined
   const parentCollectionInfo = parentCollection ? {
     id: toV2Id(parentCollection.id),
     name: parentCollection.name
@@ -62,7 +62,7 @@ export function convertCaseToV2FullCase(c: ICase, dataContext: IDataSet) {
     parent: parentCollectionInfo
   } : undefined
 
-  const values = getCaseValues(caseId, dataContext, collectionId)
+  const values = collectionId ? getCaseValues(caseId, dataContext, collectionId) : undefined
 
   return {
     id: maybeToV2Id(caseGroup?.pseudoCase.__id__),
@@ -79,7 +79,7 @@ export function getCaseRequestResultValues(c: ICase, dataContext: IDataSet): DIG
 
   const id = toV2Id(caseId)
 
-  const collectionId = dataContext.pseudoCaseMap.get(caseId)?.collectionId ?? dataContext.ungrouped.id
+  const collectionId = dataContext.pseudoCaseMap.get(caseId)?.collectionId ?? dataContext.childCollection.id
 
   const parent = maybeToV2Id(dataContext.getParentCase(caseId, collectionId)?.pseudoCase.__id__)
 
@@ -94,7 +94,7 @@ export function getCaseRequestResultValues(c: ICase, dataContext: IDataSet): DIG
   const pseudoCase = dataContext.pseudoCaseMap.get(caseId)
   const children = pseudoCase?.childPseudoCaseIds?.map(cId => toV2Id(cId)) ??
     pseudoCase?.childCaseIds?.map(cId => toV2Id(cId)) ?? []
-  
+
   const caseIndex = dataContext.getCasesForCollection(collectionId).findIndex(aCase => aCase.__id__ === caseId)
 
   return {
@@ -164,34 +164,12 @@ export function convertCollectionToV2(collection: ICollectionModel, dataContext?
   }
 }
 
-export function convertUngroupedCollectionToV2(dataContext: IDataSet): ICodapV2CollectionV3 | undefined {
-  // TODO This will probably need to be reworked after upcoming v3 collection overhaul,
-  // so I'm leaving it bare bones for now.
-  const { name, title, id, labels: _labels } = dataContext.ungrouped
-  const v2Id = toV2Id(id)
-  const labels = _labels ? getSnapshot(_labels) : undefined
-  const ungroupedAttributes = dataContext.ungroupedAttributes
-  if (ungroupedAttributes.length > 0) {
-    return {
-      guid: v2Id,
-      id: v2Id,
-      labels,
-      name,
-      title,
-      attrs: ungroupedAttributes.map(attr => convertAttributeToV2(attr, dataContext)),
-      type: "DG.Collection"
-    }
-  }
-}
-
 export function convertDataSetToV2(dataSet: IDataSet, docId: number | string): ICodapV2DataContextV3 {
   const { name, title, id, description } = dataSet
   const v2Id = toV2Id(id)
 
   const collections: ICodapV2CollectionV3[] =
-    dataSet.collectionGroups.map(collectionGroup => convertCollectionToV2(collectionGroup.collection, dataSet))
-  const ungroupedCollection = convertUngroupedCollectionToV2(dataSet)
-  if (ungroupedCollection) collections.push(ungroupedCollection)
+    dataSet.collections.map(collection => convertCollectionToV2(collection, dataSet))
 
   return {
     type: "DG.DataContext",
@@ -230,7 +208,7 @@ export function basicAttributeInfo(attribute: IAttribute) {
   return { name, id: toV2Id(id), title }
 }
 
-export function basicCollectionInfo(collection: ICollectionPropsModel) {
+export function basicCollectionInfo(collection: ICollectionModel) {
   const { name, id, title } = collection
   const v2Id = toV2Id(id)
   return { name, guid: v2Id, title, id: v2Id }