diff --git a/v3/cypress/e2e/graph.spec.ts b/v3/cypress/e2e/graph.spec.ts index 3d4bb26d26..597f5875c5 100644 --- a/v3/cypress/e2e/graph.spec.ts +++ b/v3/cypress/e2e/graph.spec.ts @@ -539,7 +539,7 @@ 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() @@ -547,8 +547,8 @@ context("Graph UI", () => { .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") diff --git a/v3/cypress/e2e/hierarchical-table.spec.ts b/v3/cypress/e2e/hierarchical-table.spec.ts index 960ff3b45c..6f068ac8a8 100644 --- a/v3/cypress/e2e/hierarchical-table.spec.ts +++ b/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) diff --git a/v3/src/components/case-card/case-card.tsx b/v3/src/components/case-card/case-card.tsx index c797aac693..cd02af4ca5 100644 --- a/v3/src/components/case-card/case-card.tsx +++ b/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 diff --git a/v3/src/components/case-card/case-card.v2.test.tsx b/v3/src/components/case-card/case-card.v2.test.tsx index c726b2ae00..b7ea546dbf 100644 --- a/v3/src/components/case-card/case-card.v2.test.tsx +++ b/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 }, diff --git a/v3/src/components/case-table/case-table.tsx b/v3/src/components/case-table/case-table.tsx index 0090a29404..0272246ad0 100644 --- a/v3/src/components/case-table/case-table.tsx +++ b/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 = () => { tableModel?.setScrollLeft(contentRef.current?.scrollLeft ?? 0) } diff --git a/v3/src/components/case-table/collection-table-spacer.tsx b/v3/src/components/case-table/collection-table-spacer.tsx index cc45dc9093..48f8de7d1d 100644 --- a/v3/src/components/case-table/collection-table-spacer.tsx +++ b/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) { diff --git a/v3/src/components/case-table/use-index-column.tsx b/v3/src/components/case-table/use-index-column.tsx index c84085908b..932d5d1d12 100644 --- a/v3/src/components/case-table/use-index-column.tsx +++ b/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 diff --git a/v3/src/components/data-display/models/data-configuration-model.ts b/v3/src/components/data-display/models/data-configuration-model.ts index 58a0276fc9..cbdeedbe18 100644 --- a/v3/src/components/data-display/models/data-configuration-model.ts +++ b/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 || "" diff --git a/v3/src/components/graph/components/scatterdots.tsx b/v3/src/components/graph/components/scatterdots.tsx index 592439e58b..d5d41a3f2b 100644 --- a/v3/src/components/graph/components/scatterdots.tsx +++ b/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(() => { diff --git a/v3/src/components/graph/models/graph-controller.test.ts b/v3/src/components/graph/models/graph-controller.test.ts index d053464ab7..144a80363d 100644 --- a/v3/src/components/graph/models/graph-controller.test.ts +++ b/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()) }) diff --git a/v3/src/components/map/components/map-point-layer.tsx b/v3/src/components/map/components/map-point-layer.tsx index 63175e6d34..9167679e53 100644 --- a/v3/src/components/map/components/map-point-layer.tsx +++ b/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) { diff --git a/v3/src/data-interactive/data-interactive-type-utils.ts b/v3/src/data-interactive/data-interactive-type-utils.ts index 6b5d944d07..98ee04f669 100644 --- a/v3/src/data-interactive/data-interactive-type-utils.ts +++ b/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 } diff --git a/v3/src/data-interactive/data-interactive-types.ts b/v3/src/data-interactive/data-interactive-types.ts index 6628e19876..0f98ae2e04 100644 --- a/v3/src/data-interactive/data-interactive-types.ts +++ b/v3/src/data-interactive/data-interactive-types.ts @@ -5,7 +5,7 @@ import { IDataSet } from "../models/data/data-set" import { ICase, ICaseID } from "../models/data/data-set-types" import { IGlobalValue } from "../models/global/global-value" import { ITileModel } from "../models/tiles/tile-model" -import { ICollectionLabels, ICollectionPropsModel } from "../models/data/collection" +import { ICollectionLabels, ICollectionModel } from "../models/data/collection" import { V2Component } from "./data-interactive-component-types" export type DICaseValue = string | number | boolean | undefined @@ -137,8 +137,8 @@ export interface DIResources { caseByIndex?: ICase caseFormulaSearch?: DICase[] caseSearch?: DICase[] - collection?: ICollectionPropsModel - collectionList?: ICollectionPropsModel[] + collection?: ICollectionModel + collectionList?: ICollectionModel[] component?: DIComponent dataContext?: IDataSet dataContextList?: IDataSet[] diff --git a/v3/src/data-interactive/data-interactive-utils.ts b/v3/src/data-interactive/data-interactive-utils.ts index a25d838b6f..06c7b3c75a 100644 --- a/v3/src/data-interactive/data-interactive-utils.ts +++ b/v3/src/data-interactive/data-interactive-utils.ts @@ -22,8 +22,8 @@ export function canonicalizeAttributeName(name: string, iCanonicalize = true) { export function getCaseValues(caseId: string, dataSet: IDataSet, collectionId?: string) { const attributes = collectionId - ? dataSet.getGroupedCollection(collectionId)?.attributes ?? dataSet.ungroupedAttributes - : dataSet.attributes + ? dataSet.getCollection(collectionId)?.attributes ?? [] + : dataSet.attributes const values: DICaseValues = {} const actualCaseIndex = dataSet.caseIDMap.get(caseId) ?? -1 diff --git a/v3/src/data-interactive/handlers/all-cases-handler.test.ts b/v3/src/data-interactive/handlers/all-cases-handler.test.ts index c8785dae32..cb7acd6375 100644 --- a/v3/src/data-interactive/handlers/all-cases-handler.test.ts +++ b/v3/src/data-interactive/handlers/all-cases-handler.test.ts @@ -46,7 +46,7 @@ describe("DataInteractive AllCasesHandler", () => { const c2c4 = getCase(result2, 3) checkCase(c2c4, "a2", "z", 1, c1c2.id) - const result3 = handler.get?.({ dataContext: dataset, collection: dataset?.ungrouped }) as GetAllCasesResult + const result3 = handler.get?.({ dataContext: dataset, collection: dataset?.childCollection }) as GetAllCasesResult expect(result3.success).toBe(true) expect(result3.values.cases?.length).toBe(6) checkCase(getCase(result3, 0), "a3", "1", 0, c2c1.id) diff --git a/v3/src/data-interactive/handlers/all-cases-handler.ts b/v3/src/data-interactive/handlers/all-cases-handler.ts index 2656029fc6..54a9ae99cc 100644 --- a/v3/src/data-interactive/handlers/all-cases-handler.ts +++ b/v3/src/data-interactive/handlers/all-cases-handler.ts @@ -1,6 +1,6 @@ import { registerDIHandler } from "../data-interactive-handler" -import { getCaseValues } from "../data-interactive-utils" import { DIHandler, DIResources } from "../data-interactive-types" +import { IValueType } from "../../models/data/attribute" import { maybeToV2Id, toV2Id } from "../../utilities/codap-utils" import { collectionNotFoundResult, dataContextNotFoundResult } from "./di-results" @@ -22,21 +22,23 @@ export const diAllCasesHandler: DIHandler = { if (!dataContext) return dataContextNotFoundResult if (!collection) return collectionNotFoundResult - const cases = dataContext.getGroupsForCollection(collection.id)?.map((c, caseIndex) => { - const id = c.pseudoCase.__id__ - - const parent = maybeToV2Id(dataContext.getParentCase(id, collection.id)?.pseudoCase.__id__) - - // iphone-frame was throwing an error when Array.from() wasn't used here for some reason. - const childPseudoCaseIds = c.childPseudoCaseIds && Array.from(c.childPseudoCaseIds) - const childCaseIds = c.childCaseIds && Array.from(c.childCaseIds) - const children = childPseudoCaseIds ?? childCaseIds ?? [] - - const values = getCaseValues(id, dataContext, collection.id) + const attributes = collection.attributes ?? [] + const cases = dataContext.getCasesForCollection(collection.id).map(({ __id__ }, index) => { + const pseudoCase = dataContext.pseudoCaseMap.get(__id__) + const parentCase = dataContext.getParentCase(__id__, collection.id) + const parent = maybeToV2Id(parentCase?.pseudoCase.__id__) + const children: number[] = [] + if (pseudoCase) { + children.push(...(pseudoCase.childPseudoCaseIds ?? pseudoCase.childCaseIds).map(childId => toV2Id(childId))) + } + const values: Record = {} + attributes.forEach(attr => { + if (attr) values[attr.name] = dataContext.getValue(__id__, attr.id) + }) return { - case: { id: toV2Id(id), parent, children: children.map(child => toV2Id(child)), values }, - caseIndex + case: { id: toV2Id(__id__), parent, children, values }, + caseIndex: index } }) diff --git a/v3/src/data-interactive/handlers/attribute-handler.ts b/v3/src/data-interactive/handlers/attribute-handler.ts index d8ab3f5b6a..8880693419 100644 --- a/v3/src/data-interactive/handlers/attribute-handler.ts +++ b/v3/src/data-interactive/handlers/attribute-handler.ts @@ -34,7 +34,7 @@ export const diAttributeHandler: DIHandler = { dataContext.applyModelChange(() => { attributeValues.forEach(attributeValue => { if (attributeValue) { - const attribute = createAttribute(attributeValue, dataContext, metadata, collection) + const attribute = createAttribute(attributeValue, dataContext, collection, metadata) if (attribute) attributes.push(attribute) } }) diff --git a/v3/src/data-interactive/handlers/attribute-location-handler.test.ts b/v3/src/data-interactive/handlers/attribute-location-handler.test.ts index df2d5dea5c..5486c17719 100644 --- a/v3/src/data-interactive/handlers/attribute-location-handler.test.ts +++ b/v3/src/data-interactive/handlers/attribute-location-handler.test.ts @@ -1,4 +1,4 @@ -import { ICollectionPropsModel } from "../../models/data/collection" +import { ICollectionModel } from "../../models/data/collection" import { diAttributeLocationHandler } from "./attribute-location-handler" import { setupTestDataset } from "./handler-test-utils" @@ -13,8 +13,7 @@ describe("DataInteractive AttributeLocationHandler", () => { const dataContext = dataset const resources = { attributeLocation: a1, dataContext } - const collectionAttributes = (collection: ICollectionPropsModel) => - dataset.getGroupedCollection(collection.id)?.attributes + const collectionAttributes = (collection: ICollectionModel) => dataset.getCollection(collection.id)?.attributes expect(handler.update?.({})?.success).toBe(false) // Missing dataContext @@ -30,11 +29,11 @@ describe("DataInteractive AttributeLocationHandler", () => { // Move attribute within the ungrouped collection // Indexes snap to the end of the array - expect(dataset.ungroupedAttributes[1].id).toBe(a6.id) + expect(dataset.childCollection.attributes[1]!.id).toBe(a6.id) expect(handler.update?.({ attributeLocation: a6, dataContext }, { position: -1 }).success).toBe(true) - expect(dataset.ungroupedAttributes[0].id).toBe(a6.id) + expect(dataset.childCollection.attributes[0]!.id).toBe(a6.id) expect(handler.update?.({ attributeLocation: a6, dataContext }, { position: 10 }).success).toBe(true) - expect(dataset.ungroupedAttributes[1].id).toBe(a6.id) + expect(dataset.childCollection.attributes[1]!.id).toBe(a6.id) // Move attribute within a grouped collection // If not specified, move the attribute to the far right @@ -50,7 +49,7 @@ describe("DataInteractive AttributeLocationHandler", () => { expect(handler.update?.({ attributeLocation: a6, dataContext }, { collection: c1.name, position: 1 }).success) .toBe(true) expect(collectionAttributes(c1)?.length).toBe(3) - expect(dataset.ungroupedAttributes.length).toBe(1) + expect(dataset.childCollection.attributes.length).toBe(1) expect(collectionAttributes(c1)?.[1]?.id).toBe(a6.id) expect(collectionAttributes(c1)?.[2]?.id).toBe(a4.id) diff --git a/v3/src/data-interactive/handlers/attribute-location-handler.ts b/v3/src/data-interactive/handlers/attribute-location-handler.ts index dc43ae2cd3..2dba2e3934 100644 --- a/v3/src/data-interactive/handlers/attribute-location-handler.ts +++ b/v3/src/data-interactive/handlers/attribute-location-handler.ts @@ -17,8 +17,7 @@ export const diAttributeLocationHandler: DIHandler = { : getCollection(dataContext, collection ? `${collection}` : undefined) ?? sourceCollection if (!targetCollection) return collectionNotFoundResult - const targetAttrs = - dataContext.getGroupedCollection(targetCollection.id)?.attributes ?? dataContext.ungroupedAttributes + const targetAttrs = dataContext.getCollection(targetCollection.id)?.attributes ?? [] const numPos = Number(position) // If the position isn't specified or isn't a number, make the attribute the right-most diff --git a/v3/src/data-interactive/handlers/case-count-handler.test.ts b/v3/src/data-interactive/handlers/case-count-handler.test.ts index c6c2a2fc20..879c63d28f 100644 --- a/v3/src/data-interactive/handlers/case-count-handler.test.ts +++ b/v3/src/data-interactive/handlers/case-count-handler.test.ts @@ -19,6 +19,6 @@ describe("DataInteractive CaseCountHandler", () => { expect(handler.get?.({ collection: dataset.collections[0] })?.success).toBe(false) expect(handler.get?.({ dataContext: dataset, collection: dataset.collections[0] })?.values).toBe(2) - expect(handler.get?.({ dataContext: dataset, collection: dataset.ungrouped })?.values).toBe(4) + expect(handler.get?.({ dataContext: dataset, collection: dataset.childCollection })?.values).toBe(4) }) }) diff --git a/v3/src/data-interactive/handlers/collection-handler.test.ts b/v3/src/data-interactive/handlers/collection-handler.test.ts index 25b281aacb..f9a0fbb1ed 100644 --- a/v3/src/data-interactive/handlers/collection-handler.test.ts +++ b/v3/src/data-interactive/handlers/collection-handler.test.ts @@ -14,24 +14,24 @@ describe("DataInteractive CollectionHandler", () => { expect(handler.create?.({ dataContext }).success).toBe(false) // Collections require names - expect(dataset.collections.length).toBe(2) + expect(dataset.collections.length).toBe(3) const noNameResult = handler.create?.({ dataContext }, {}) expect(noNameResult?.success).toBe(true) expect(noNameResult?.values).toEqual([]) - expect(dataset.collections.length).toBe(2) + expect(dataset.collections.length).toBe(3) // Return information for existing collection when there's a name collision const sameNameResult = handler.create?.({ dataContext }, { name: c1.name }) expect(sameNameResult?.success).toBe(true) const sameNameValues = sameNameResult?.values as DICollection[] expect(toV3CollectionId(sameNameValues[0].id!)).toBe(c1.id) - expect(dataset.collections.length).toBe(2) + expect(dataset.collections.length).toBe(3) // Add a right-most collection // Add attributes with attributes field const rightResult = handler.create?.({ dataContext }, { name: "right", attributes: [{ name: "a4" }] }) expect(rightResult?.success).toBe(true) - expect(dataset.collections.length).toBe(3) + expect(dataset.collections.length).toBe(4) expect(dataset.collections[2].name).toBe("right") expect(dataset.collections[2].attributes.length).toBe(1) expect(dataset.collections[2].attributes[0]?.name).toBe("a4") @@ -40,7 +40,7 @@ describe("DataInteractive CollectionHandler", () => { // Add attributes with attrs field const leftResult = handler.create?.({ dataContext }, { name: "left", parent: "root", attrs: [{ name: "a5"}] }) expect(leftResult?.success).toBe(true) - expect(dataset.collections.length).toBe(4) + expect(dataset.collections.length).toBe(5) expect(dataset.collections[0].name).toBe("left") expect(dataset.collections[0].attributes.length).toBe(1) expect(dataset.collections[0].attributes[0]?.name).toBe("a5") @@ -53,7 +53,7 @@ describe("DataInteractive CollectionHandler", () => { { name: "c4", parent: "c3", attrs: [{ name: "a7" }], attributes: [{ name: "a6" }] } ]) expect(twoResult?.success).toBe(true) - expect(dataset.collections.length).toBe(6) + expect(dataset.collections.length).toBe(7) expect(dataset.collections[3].name).toBe("c3") expect(dataset.collections[3].attributes.length).toBe(0) expect(dataset.collections[4].name).toBe("c4") @@ -72,7 +72,7 @@ describe("DataInteractive CollectionHandler", () => { expect(result?.success).toBe(true) expect((result?.values as DIDeleteCollectionResult).collections?.[0]).toBe(toV2Id(collectionId)) expect(dataContext.attributes.length).toBe(2) - expect(dataContext.collections.length).toBe(1) + expect(dataContext.collections.length).toBe(2) expect(dataContext.getCollection(collectionId)).toBeUndefined() }) @@ -91,14 +91,14 @@ describe("DataInteractive CollectionHandler", () => { expect(groupedValues.attrs.length).toEqual(c1.attributes.length) expect(groupedValues.labels?.singleCase).toBe("singleCase") - // Ungrouped collection - const ungrouped = dataset.ungrouped - const ungroupedResult = handler.get?.({ dataContext: dataset, collection: ungrouped }) - expect(ungroupedResult?.success).toBe(true) - const ungroupedValues = ungroupedResult?.values as ICodapV2CollectionV3 - expect(ungroupedValues.name).toEqual(ungrouped.name) - expect(ungroupedValues.id).toEqual(toV2Id(ungrouped.id)) - expect(ungroupedValues.attrs.length).toEqual(dataset.ungroupedAttributes.length) + // child collection + const childCollection = dataset.childCollection + const childCollectionResult = handler.get?.({ dataContext: dataset, collection: childCollection }) + expect(childCollectionResult?.success).toBe(true) + const childCollectionValues = childCollectionResult?.values as ICodapV2CollectionV3 + expect(childCollectionValues.name).toEqual(childCollection.name) + expect(childCollectionValues.id).toEqual(toV2Id(childCollection.id)) + expect(childCollectionValues.attrs.length).toEqual(dataset.childCollection.attributes.length) }) it("update works", () => { diff --git a/v3/src/data-interactive/handlers/collection-handler.ts b/v3/src/data-interactive/handlers/collection-handler.ts index 467605c399..e0449f0bbd 100644 --- a/v3/src/data-interactive/handlers/collection-handler.ts +++ b/v3/src/data-interactive/handlers/collection-handler.ts @@ -1,4 +1,4 @@ -import { CollectionModel, ICollectionModel, isCollectionModel } from "../../models/data/collection" +import { ICollectionModel } from "../../models/data/collection" import { createCollectionNotification } from "../../models/data/data-set-notifications" import { getSharedCaseMetadataFromDataset } from "../../models/shared/shared-data-utils" import { toV2Id } from "../../utilities/codap-utils" @@ -6,7 +6,7 @@ import { registerDIHandler } from "../data-interactive-handler" import { DIHandler, DIResources, DIValues, DICreateCollection, DICollection, DIUpdateCollection } from "../data-interactive-types" -import { convertCollectionToV2, convertUngroupedCollectionToV2 } from "../data-interactive-type-utils" +import { convertCollectionToV2 } from "../data-interactive-type-utils" import { getCollection } from "../data-interactive-utils" import { createAttribute } from "./di-handler-utils" import { collectionNotFoundResult, dataContextNotFoundResult, valuesRequiredResult } from "./di-results" @@ -54,9 +54,8 @@ export const diCollectionHandler: DIHandler = { } } - const newCollection = CollectionModel.create({ name, _title: title }) + const newCollection = dataContext.addCollection({ name, _title: title }, { before: beforeCollectionId }) newCollections.push(newCollection) - dataContext.addCollection(newCollection, beforeCollectionId) // Attributes can be specified in both attributes and attrs const _attributes = Array.isArray(attributes) ? attributes : [] @@ -65,7 +64,7 @@ export const diCollectionHandler: DIHandler = { newAttributes.forEach(newAttribute => { // Each attribute must have a unique name if (newAttribute.name && !dataContext.getAttributeByName(newAttribute.name)) { - createAttribute(newAttribute, dataContext, metadata, newCollection) + createAttribute(newAttribute, dataContext, newCollection, metadata) } }) @@ -84,7 +83,7 @@ export const diCollectionHandler: DIHandler = { if (!_collection) return collectionNotFoundResult const collectionId = _collection.id // For now, it's only possible to delete a grouped collection. - const collection = dataContext.getGroupedCollection(collectionId) + const collection = dataContext.getCollection(collectionId) if (!collection) return collectionNotFoundResult dataContext.applyModelChange(() => { @@ -99,9 +98,7 @@ export const diCollectionHandler: DIHandler = { if (!dataContext) return dataContextNotFoundResult if (!collection) return collectionNotFoundResult - const v2Collection = isCollectionModel(collection) - ? convertCollectionToV2(collection) - : convertUngroupedCollectionToV2(dataContext) + const v2Collection = convertCollectionToV2(collection) return { success: true, values: v2Collection diff --git a/v3/src/data-interactive/handlers/collection-list-handler.test.ts b/v3/src/data-interactive/handlers/collection-list-handler.test.ts index 1bcda5b973..329c6dd330 100644 --- a/v3/src/data-interactive/handlers/collection-list-handler.test.ts +++ b/v3/src/data-interactive/handlers/collection-list-handler.test.ts @@ -8,8 +8,8 @@ describe("DataInteractive AttributeListHandler", () => { it("get works", () => { const { dataset, c1, c2 } = setupTestDataset() - const { ungrouped } = dataset - const collectionList = [c1, c2, ungrouped] + const { childCollection } = dataset + const collectionList = [c1, c2, childCollection] expect(handler.get?.({})?.success).toBe(false) expect(handler.get?.({ dataContext: dataset }).success).toBe(false) @@ -23,8 +23,8 @@ describe("DataInteractive AttributeListHandler", () => { expect(collection1.title).toBe(c1.title) expect(collection1.id).toBe(toV2Id(c1.id)) const collection3 = resultCollectionList[2] - expect(collection3.name).toBe(ungrouped.name) - expect(collection3.title).toBe(ungrouped.title) - expect(collection3.id).toBe(toV2Id(ungrouped.id)) + expect(collection3.name).toBe(childCollection.name) + expect(collection3.title).toBe(childCollection.title) + expect(collection3.id).toBe(toV2Id(childCollection.id)) }) }) diff --git a/v3/src/data-interactive/handlers/data-context-handler.ts b/v3/src/data-interactive/handlers/data-context-handler.ts index 712a11d69f..79a3cf706a 100644 --- a/v3/src/data-interactive/handlers/data-context-handler.ts +++ b/v3/src/data-interactive/handlers/data-context-handler.ts @@ -29,8 +29,13 @@ export const diDataContextHandler: DIHandler = { gDataBroker.addDataSet(dataSet) const metadata = getSharedCaseMetadataFromDataset(dataSet) - // Create and add collections and attributes - collections?.forEach(v2collection => createCollection(v2collection, dataSet, metadata)) + if (collections?.length) { + // remove the default collection + dataSet.removeCollection(dataSet.collections[0]) + + // Create and add collections and attributes + collections.forEach(v2collection => createCollection(v2collection, dataSet, metadata)) + } return { success: true, diff --git a/v3/src/data-interactive/handlers/di-handler-utils.ts b/v3/src/data-interactive/handlers/di-handler-utils.ts index 20e36d6664..5ed35fa174 100644 --- a/v3/src/data-interactive/handlers/di-handler-utils.ts +++ b/v3/src/data-interactive/handlers/di-handler-utils.ts @@ -1,13 +1,13 @@ -import { CollectionModel, ICollectionPropsModel } from "../../models/data/collection" +import { ICollectionModel } from "../../models/data/collection" import { IDataSet } from "../../models/data/data-set" +import { IAddCollectionOptions } from "../../models/data/data-set-types" import { v2NameTitleToV3Title } from "../../models/data/v2-model" import { ISharedCaseMetadata } from "../../models/shared/shared-case-metadata" import { DIAttribute, DICollection } from "../data-interactive-types" import { convertValuesToAttributeSnapshot } from "../data-interactive-type-utils" -export function createAttribute( - value: DIAttribute, dataContext: IDataSet, metadata?: ISharedCaseMetadata, collection?: ICollectionPropsModel -) { +export function createAttribute(value: DIAttribute, dataContext: IDataSet, collection?: ICollectionModel, + metadata?: ISharedCaseMetadata) { const attributeSnapshot = convertValuesToAttributeSnapshot(value) if (attributeSnapshot) { const attribute = dataContext.addAttribute(attributeSnapshot, { collection: collection?.id }) @@ -23,11 +23,11 @@ export function createCollection(v2collection: DICollection, dataContext: IDataS // TODO Handle labels const { attrs, name: collectionName, title: collectionTitle } = v2collection const _title = v2NameTitleToV3Title(collectionName ?? "", collectionTitle) - const collection = CollectionModel.create({ name: collectionName, _title }) - dataContext.addCollection(collection) + const options: IAddCollectionOptions = { after: dataContext.childCollection?.id } + const collection = dataContext.addCollection({ name: collectionName, _title }, options) attrs?.forEach(attr => { - createAttribute(attr, dataContext, metadata, collection) + createAttribute(attr, dataContext, collection, metadata) }) return collection diff --git a/v3/src/data-interactive/handlers/handler-test-utils.ts b/v3/src/data-interactive/handlers/handler-test-utils.ts index 3a4e3a1681..b840e703e8 100644 --- a/v3/src/data-interactive/handlers/handler-test-utils.ts +++ b/v3/src/data-interactive/handlers/handler-test-utils.ts @@ -1,4 +1,3 @@ -import { CollectionModel } from "../../models/data/collection" import { DataSet } from "../../models/data/data-set" export const testCases = [ @@ -10,11 +9,15 @@ export const testCases = [ { a1: "b", a2: "y", a3: 6 }, ] export const setupTestDataset = () => { - const dataset = DataSet.create({ name: "data" }) - const c1 = CollectionModel.create({ name: "collection1" }) - const c2 = CollectionModel.create({ name: "collection2" }) - dataset.addCollection(c1) - dataset.addCollection(c2) + const dataset = DataSet.create({ + name: "data", + collections: [ + { name: "collection1" }, + { name: "collection2" }, + { name: "collection3" } + ] + }) + const [c1, c2] = dataset.collections const a1 = dataset.addAttribute({ name: "a1" }, { collection: c1.id }) const a2 = dataset.addAttribute({ name: "a2" }, { collection: c2.id }) const a3 = dataset.addAttribute({ name: "a3" }) diff --git a/v3/src/data-interactive/resource-parser.test.ts b/v3/src/data-interactive/resource-parser.test.ts index ec6f363e7a..e26d3c6ebc 100644 --- a/v3/src/data-interactive/resource-parser.test.ts +++ b/v3/src/data-interactive/resource-parser.test.ts @@ -23,7 +23,7 @@ describe("DataInteractive ResourceParser", () => { dataset.collectionGroups // set up the pseudoCases const tile = content!.createOrShowTile(kWebViewTileType)! const resolve = (resource: string) => resolveResources(resource, "get", tile) - + it("finds dataContext", () => { expect(resolve("").dataContext?.id).toBe(dataset.id) expect(resolve("dataContext[data]").dataContext?.id).toBe(dataset.id) @@ -56,7 +56,7 @@ describe("DataInteractive ResourceParser", () => { const { collectionList } = resolve("dataContext[data].collectionList") expect(collectionList?.length).toBe(3) expect(collectionList?.[0].id).toBe(dataset.collections[0].id) - expect(collectionList?.[2].id).toBe(dataset.ungrouped.id) + expect(collectionList?.[2].id).toBe(dataset.collections[2].id) }) it("finds collection", () => { @@ -96,8 +96,9 @@ describe("DataInteractive ResourceParser", () => { expect(resolve(`dataContext[data].collection[unknown].caseByIndex[0]`).caseByIndex).toBeUndefined() const itemId = dataset.getCaseAtIndex(0)!.__id__ - const ungroupedId = toV2Id(dataset.ungrouped.id) - expect(resolve(`dataContext[data].collection[${ungroupedId}].caseByIndex[0]`).caseByIndex?.__id__).toBe(itemId) + const childCollectionId = toV2Id(dataset.childCollection.id) + expect(resolve(`dataContext[data].collection[${childCollectionId}].caseByIndex[0]`).caseByIndex?.__id__) + .toBe(itemId) const caseId = Array.from(dataset.pseudoCaseMap.values())[0].pseudoCase.__id__ expect(resolve(`dataContext[data].collection[${collectionId}].caseByIndex[0]`).caseByIndex?.__id__).toBe(caseId) diff --git a/v3/src/data-interactive/resource-parser.ts b/v3/src/data-interactive/resource-parser.ts index 2d82523831..b5f55de714 100644 --- a/v3/src/data-interactive/resource-parser.ts +++ b/v3/src/data-interactive/resource-parser.ts @@ -123,7 +123,7 @@ export function resolveResources( if ("collectionList" in resourceSelector) { if (dataContext) { - result.collectionList = [...dataContext.collectionModels] + result.collectionList = [...dataContext.collections] } } @@ -142,7 +142,7 @@ export function resolveResources( if ("attributeList" in resourceSelector) { const attributeList: IAttribute[] = [] - const attributes = collectionModel?.attributes ?? dataContext?.ungroupedAttributes ?? [] + const attributes = collectionModel?.attributes ?? [] attributes.forEach(attribute => { if (attribute) attributeList.push(attribute) }) @@ -151,7 +151,7 @@ export function resolveResources( const getCaseById = (caseId: string) => dataContext?.pseudoCaseMap.get(caseId)?.pseudoCase ?? dataContext?.getCase(caseId) - + if (resourceSelector.caseByID) { const caseId = toV3CaseId(resourceSelector.caseByID) result.caseByID = getCaseById(caseId) diff --git a/v3/src/models/codap/create-codap-document.test.ts b/v3/src/models/codap/create-codap-document.test.ts index 3ccd71d4a4..bc31a10af6 100644 --- a/v3/src/models/codap/create-codap-document.test.ts +++ b/v3/src/models/codap/create-codap-document.test.ts @@ -87,10 +87,13 @@ describe("createCodapDocument", () => { values: ["1", "2", "3"] } }, - attributes: ["test-8"], cases: [{ __id__: "test-9" }, { __id__: "test-10" }, { __id__: "test-11" }], - collections: [], - ungrouped: { id: "test-6", name: "Cases" }, + collections: [{ + id: "test-7", + name: "Cases", + attributes: ["test-8"], + caseIds: [] + }], id: "test-5", snapSelection: [] }, diff --git a/v3/src/models/data/collection.test.ts b/v3/src/models/data/collection.test.ts index a28c710220..bde09521e1 100644 --- a/v3/src/models/data/collection.test.ts +++ b/v3/src/models/data/collection.test.ts @@ -1,6 +1,6 @@ import { destroy, isAlive, types } from "mobx-state-tree" import { Attribute, IAttribute } from "./attribute" -import { CollectionModel, CollectionPropsModel, ICollectionModel, isCollectionModel } from "./collection" +import { CollectionModel, ICollectionModel, isCollectionModel } from "./collection" const Tree = types.model("Tree", { attributes: types.array(Attribute), @@ -21,21 +21,21 @@ const Tree = types.model("Tree", { describe("CollectionModel", () => { it("Simple properties work as expected", () => { - const withName = CollectionPropsModel.create({ name: "name" }) + const withName = CollectionModel.create({ name: "name" }) expect(withName.title).toBe("name") withName.setTitle("newName") expect(withName.title).toBe("newName") - expect(isCollectionModel(withName)).toBe(false) + expect(isCollectionModel(withName)).toBe(true) - const withNameAndTitle = CollectionPropsModel.create({ name: "name", _title: "title" }) + const withNameAndTitle = CollectionModel.create({ name: "name", _title: "title" }) expect(withNameAndTitle.title).toBe("title") withNameAndTitle.setTitle("newTitle") expect(withNameAndTitle.title).toBe("newTitle") - expect(isCollectionModel(withNameAndTitle)).toBe(false) + expect(isCollectionModel(withNameAndTitle)).toBe(true) }) it("labels work as expected", () => { - const c1 = CollectionPropsModel.create({ name: "c1" }) + const c1 = CollectionModel.create({ name: "c1" }) expect(c1.labels).toBeUndefined() c1.setSingleCase("singleCase") expect(c1.labels?.singleCase).toBe("singleCase") @@ -51,24 +51,24 @@ describe("CollectionModel", () => { expect(c1.labels?.setOfCases).toBe("setOfCases") expect(c1.labels?.setOfCasesWithArticle).toBe("setOfCasesWithArticle") - const c2 = CollectionPropsModel.create({ name: "c2" }) + const c2 = CollectionModel.create({ name: "c2" }) expect(c2.labels).toBeUndefined() c2.setPluralCase("pluralCase") expect(c2.labels?.pluralCase).toBe("pluralCase") c2.setSingleCase("singleCase") expect(c2.labels?.singleCase).toBe("singleCase") - const c3 = CollectionPropsModel.create({ name: "c3" }) + const c3 = CollectionModel.create({ name: "c3" }) expect(c3.labels).toBeUndefined() c3.setSingleCaseWithArticle("singleCaseWithArticles") expect(c3.labels?.singleCaseWithArticle).toBe("singleCaseWithArticles") - const c4 = CollectionPropsModel.create({ name: "c4" }) + const c4 = CollectionModel.create({ name: "c4" }) expect(c4.labels).toBeUndefined() c4.setSetOfCases("setOfCases") expect(c4.labels?.setOfCases).toBe("setOfCases") - const c5 = CollectionPropsModel.create({ name: "c5" }) + const c5 = CollectionModel.create({ name: "c5" }) expect(c5.labels).toBeUndefined() c5.setSetOfCasesWithArticle("setOfCasesWithArticle") expect(c5.labels?.setOfCasesWithArticle).toBe("setOfCasesWithArticle") @@ -97,6 +97,7 @@ describe("CollectionModel", () => { collection.addAttribute(undefined as any as IAttribute) expect(collection.getAttribute("a")).toBeUndefined() expect(collection.getAttributeIndex("a")).toBe(-1) + expect(collection.getAttributeByName("a")).toBeUndefined() }) it("can add/move/remove attributes", () => { @@ -143,4 +144,37 @@ describe("CollectionModel", () => { expect(collection.attributes.length).toBe(0) }) + it("can add/remove cases", () => { + const collection = CollectionModel.create() + expect(isCollectionModel(collection)).toBe(true) + expect(collection.caseIds.length).toBe(0) + expect(collection.caseIdToIndexMap.size).toBe(0) + collection.addCases(["case3", "case6"]) + expect(collection.hasCase("case2")).toBe(false) + expect(collection.hasCase("case3")).toBe(true) + expect(collection.caseIds.length).toBe(2) + expect(collection.caseIdToIndexMap.size).toBe(2) + collection.addCases(["case1", "case2"], { before: "case3" }) + expect(collection.hasCase("case2")).toBe(true) + expect(collection.caseIds.length).toBe(4) + expect(collection.caseIdToIndexMap.size).toBe(4) + expect(collection.caseIds).toEqual(["case1", "case2", "case3", "case6"]) + collection.addCases(["case4", "case5"], { after: "case3" }) + expect(collection.caseIds.length).toBe(6) + expect(collection.caseIdToIndexMap.size).toBe(6) + expect(collection.caseIds).toEqual(["case1", "case2", "case3", "case4", "case5", "case6"]) + + collection.removeCases(["case1", "case3", "case5"]) + expect(collection.caseIds.length).toBe(3) + expect(collection.caseIdToIndexMap.size).toBe(3) + expect(collection.hasCase("case2")).toBe(true) + expect(collection.hasCase("case3")).toBe(false) + + collection.removeCases(["case2", "case4", "case6"]) + expect(collection.caseIds.length).toBe(0) + expect(collection.caseIdToIndexMap.size).toBe(0) + expect(collection.hasCase("case2")).toBe(false) + expect(collection.hasCase("case3")).toBe(false) + }) + }) diff --git a/v3/src/models/data/collection.ts b/v3/src/models/data/collection.ts index abfb41d65b..5d6f680202 100644 --- a/v3/src/models/data/collection.ts +++ b/v3/src/models/data/collection.ts @@ -1,6 +1,6 @@ import { getType, IAnyStateTreeNode, Instance, SnapshotIn, types } from "mobx-state-tree" import { Attribute, IAttribute } from "./attribute" -import { IMoveAttributeOptions } from "./data-set-types" +import { IAddCasesOptions, IMoveAttributeOptions } from "./data-set-types" import { V2Model } from "./v2-model" import { kCollectionIdPrefix, typeV3Id } from "../../utilities/codap-utils" @@ -13,10 +13,37 @@ export const CollectionLabels = types.model("CollectionLabels", { }) export interface ICollectionLabels extends Instance {} -export const CollectionPropsModel = V2Model.named("CollectionProps").props({ + +export const CollectionModel = V2Model +.named("Collection") +.props({ id: typeV3Id(kCollectionIdPrefix), - labels: types.maybe(CollectionLabels) + labels: types.maybe(CollectionLabels), + // attributes in left-to-right order + attributes: types.array(types.safeReference(Attribute)), + caseIds: types.array(types.string) }) +.views(self => ({ + getAttribute(attrId: string) { + return self.attributes.find(attribute => attribute?.id === attrId) + }, + getAttributeIndex(attrId: string) { + return self.attributes.findIndex(attribute => attribute?.id === attrId) + }, + getAttributeByName(name: string) { + return self.attributes.find(attribute => attribute?.name === name) + }, + get caseIdToIndexMap() { + const idMap: Map = new Map() + self.caseIds.forEach((caseId, index) => idMap.set(caseId, index)) + return idMap + } +})) +.views(self => ({ + hasCase(caseId: string) { + return self.caseIdToIndexMap.get(caseId) != null + } +})) .actions(self => ({ setSingleCase(singleCase: string) { if (self.labels) { @@ -63,25 +90,6 @@ export const CollectionPropsModel = V2Model.named("CollectionProps").props({ if (labels.setOfCasesWithArticle) self.setSetOfCasesWithArticle(labels.setOfCasesWithArticle) } })) -export interface ICollectionPropsModel extends Instance {} - -export const CollectionModel = CollectionPropsModel -.named("Collection") -.props({ - // grouping attributes in left-to-right order - attributes: types.array(types.safeReference(Attribute)) -}) -.views(self => ({ - getAttribute(attrId: string) { - return self.attributes.find(attr => attr?.id === attrId) - }, - getAttributeIndex(attrId: string) { - return self.attributes.findIndex(attr => attr?.id === attrId) - }, - getAttributeByName(name: string) { - return self.attributes.find(attribute => attribute?.name === name) - } -})) .actions(self => ({ addAttribute(attr: IAttribute, options?: IMoveAttributeOptions) { const beforeIndex = options?.before ? self.getAttributeIndex(options.before) : -1 @@ -101,6 +109,43 @@ export const CollectionModel = CollectionPropsModel attr && self.attributes.remove(attr) } })) +.actions(self => ({ + addCases(caseIds: string[], options?: IAddCasesOptions) { + if (options?.before) { + const beforeIndex = self.caseIdToIndexMap.get(options.before) + if (beforeIndex != null) { + self.caseIds.splice(beforeIndex, 0, ...caseIds) + return + } + } + if (options?.after) { + const afterIndex = self.caseIdToIndexMap.get(options.after) + if (afterIndex != null && afterIndex < self.caseIds.length - 1) { + self.caseIds.splice(afterIndex + 1, 0, ...caseIds) + return + } + } + self.caseIds.push(...caseIds) + }, + removeCases(caseIds: string[]) { + const entries: Array<{ caseId: string, index: number }> = [] + caseIds.forEach(caseId => { + const index = self.caseIdToIndexMap.get(caseId) + if (index != null) { + entries.push({ caseId, index }) + } + }) + // remove the cases from the rear so that prior indices aren't affected + entries.sort((a, b) => b.index - a.index) + entries.forEach(({ caseId, index }) => { + if (self.caseIds[index] !== caseId) { + /* istanbul ignore next */ + console.warn("Collection.removeCases encountered case id indexing inconsistency") + } + self.caseIds.splice(index, 1) + }) + } +})) .actions(self => ({ moveAttribute(attrId: string, options?: IMoveAttributeOptions) { const attr = self.getAttribute(attrId) diff --git a/v3/src/models/data/data-set-collection-groups.test.ts b/v3/src/models/data/data-set-collection-groups.test.ts index 4a03a27331..7db90018be 100644 --- a/v3/src/models/data/data-set-collection-groups.test.ts +++ b/v3/src/models/data/data-set-collection-groups.test.ts @@ -1,5 +1,5 @@ import { types } from "mobx-state-tree" -import { CollectionModel } from "./collection" +import { ICollectionModel } from "./collection" import { DataSet, IDataSet } from "./data-set" // eslint-disable-next-line no-var @@ -44,19 +44,19 @@ describe("CollectionGroups", () => { data.collections.forEach(collection => { attrs.push(collection.attributes.map(attr => attr!.id)) }) - attrs.push(data.ungroupedAttributes.map(attr => attr.id)) return attrs } it("handles ungrouped data", () => { expect(data.collectionGroups).toEqual([]) - expect(data.getCasesForCollection("foo")).toEqual(data.cases) + expect(data.getCasesForCollection("foo")).toEqual([]) + expect(data.getCasesForCollection(data.collections[0].id)).toEqual(data.cases) expect(data.getCasesForAttributes(["aId"])).toEqual(data.cases) expect(data.getCasesForAttributes(["bId"])).toEqual(data.cases) expect(data.getCasesForAttributes(["cId"])).toEqual(data.cases) - expect(data.groupedAttributes).toEqual([]) - expect(data.ungroupedAttributes.map(attr => attr.id)).toEqual(["aId", "bId", "cId"]) + expect(data.collections.length).toEqual(1) + expect(data.childCollection.attributes.map(attr => attr!.id)).toEqual(["aId", "bId", "cId"]) // case caches are updated when cases are added/removed const allCases = data.cases.map(({ __id__ }) => ({ __id__ })) const childCases = data.childCases() @@ -75,16 +75,14 @@ describe("CollectionGroups", () => { }) it("handles grouping by a single attribute", () => { - const collection = CollectionModel.create() - collection.addAttribute(data.attrFromID("aId")!) - data.addCollection(collection) - expect(data.groupedAttributes.map(attr => attr.id)).toEqual(["aId"]) - expect(data.ungroupedAttributes.map(attr => attr.id)).toEqual(["bId", "cId"]) + const collection = data.addCollection({ attributes: ["aId"] }) + expect(data.collections[0].attributes.map(attr => attr!.id)).toEqual(["aId"]) + expect(data.childCollection.attributes.map(attr => attr!.id)).toEqual(["bId", "cId"]) expect(collection.id).toBe("test-3") expect(data.collectionGroups.length).toBe(1) expect(attributesByCollection()).toEqual([["aId"], ["bId", "cId"]]) - expect(data.getGroupedCollection(collection.id)).toBe(collection) + expect(data.getCollection(collection.id)).toBe(collection) const aCases = data.getCasesForAttributes(["aId"]) expect(data.getCasesForCollection(collection.id)).toEqual(aCases) expect(aCases.length).toBe(3) @@ -94,12 +92,10 @@ describe("CollectionGroups", () => { }) it("handles grouping by multiple attributes", () => { - const collection = CollectionModel.create() - collection.addAttribute(data.attrFromID("aId")!) - collection.addAttribute(data.attrFromID("bId")!) - data.addCollection(collection) - expect(data.groupedAttributes.map(attr => attr.id)).toEqual(["aId", "bId"]) - expect(data.ungroupedAttributes.map(attr => attr.id)).toEqual(["cId"]) + const collection: ICollectionModel = data.moveAttributeToNewCollection("aId")! + data.moveAttribute("bId", { collection: collection.id }) + expect(data.collections[0].attributes.map(attr => attr!.id)).toEqual(["aId", "bId"]) + expect(data.childCollection.attributes.map(attr => attr!.id)).toEqual(["cId"]) expect(attributesByCollection()).toEqual([["aId", "bId"], ["cId"]]) expect(collection.id).toBe("test-3") @@ -116,15 +112,13 @@ describe("CollectionGroups", () => { }) it("handles multiple groupings", () => { - const collection1 = CollectionModel.create() - collection1.addAttribute(data.attrFromID("aId")!) - data.addCollection(collection1) + const collection1 = data.addCollection({ attributes: ["aId"] }) + expect(data.collections.length).toBe(2) expect(data.collectionGroups.length).toBe(1) - const collection2 = CollectionModel.create() - collection2.addAttribute(data.attrFromID("bId")!) - data.addCollection(collection2) - expect(data.groupedAttributes.map(attr => attr.id)).toEqual(["aId", "bId"]) - expect(data.ungroupedAttributes.map(attr => attr.id)).toEqual(["cId"]) + const collection2 = data.addCollection({ attributes: ["bId"] }) + expect(data.collections.length).toBe(3) + expect(data.collections[0].attributes.map(attr => attr!.id)).toEqual(["aId"]) + expect(data.childCollection.attributes.map(attr => attr!.id)).toEqual(["cId"]) expect(attributesByCollection()).toEqual([["aId"], ["bId"], ["cId"]]) expect(data.collectionGroups.length).toBe(2) @@ -161,29 +155,29 @@ describe("CollectionGroups", () => { // move attr "b" to a new collection (parent to collection with "a") data.moveAttributeToNewCollection("bId", data.collections[0].id) expect(attributesByCollection()).toEqual([["bId"], ["aId"], ["cId"]]) - expect(data.collections.length).toBe(2) + expect(data.collections.length).toBe(3) // move attr "a" from its collection to the collection with "b", // leaving only the one collection with "a" and "b" - data.setCollectionForAttribute("aId", { collection: data.collections[0].id, before: "bId" }) - expect(data.collections.length).toBe(1) + data.moveAttribute("aId", { collection: data.collections[0].id, before: "bId" }) + expect(data.collections.length).toBe(2) expect(attributesByCollection()).toEqual([["aId", "bId"], ["cId"]]) // move attr "b" to a new collection (child to collection with "a") data.moveAttributeToNewCollection("bId") - expect(data.collections.length).toBe(2) + expect(data.collections.length).toBe(3) expect(attributesByCollection()).toEqual([["aId"], ["bId"], ["cId"]]) const bCases = data.getCasesForCollection(data.collections[1].id) expect(data.isCaseSelected(bCases[0].__id__)).toBe(false) // move attr "c" to collection with "b", leaving no un-grouped attributes // the child-most collection is then removed, leaving those attributes un-grouped - data.setCollectionForAttribute("cId", { collection: data.collections[1].id }) + data.moveAttribute("cId", { collection: data.collections[1].id }) expect(attributesByCollection()).toEqual([["aId"], ["bId", "cId"]]) - expect(data.collections.length).toBe(1) + expect(data.collections.length).toBe(2) expect(data.collections[0].attributes.length).toBe(1) expect(data.collections[0].attributes[0]!.id).toBe("aId") // move attr "a" out of its collection back into data set - data.setCollectionForAttribute("aId", { before: "bId"}) + data.moveAttribute("aId", { before: "bId"}) expect(attributesByCollection()).toEqual([["aId", "bId", "cId"]]) - expect(data.collections.length).toBe(0) + expect(data.collections.length).toBe(1) expect(data.attrIndexFromID("aId")).toBe(0) expect(data.attrIndexFromID("bId")).toBe(1) expect(data.attrIndexFromID("cId")).toBe(2) @@ -213,21 +207,21 @@ describe("CollectionGroups", () => { it("removes attributes from collections when they're removed from the data set", () => { data.moveAttributeToNewCollection("aId") - expect(data.collections.length).toBe(1) + expect(data.collections.length).toBe(2) const collection = data.collections[0] - data.setCollectionForAttribute("bId", { collection: collection.id }) + data.moveAttribute("bId", { collection: collection.id }) data.removeAttribute("aId") - expect(data.collections.length).toBe(1) + expect(data.collections.length).toBe(2) data.removeAttribute("bId") - expect(data.collections.length).toBe(0) + expect(data.collections.length).toBe(1) }) it("doesn't take formula evaluated values into account when grouping", () => { const aAttr = data.attrFromID("aId") aAttr?.setDisplayExpression("foo * bar") data.moveAttributeToNewCollection("aId") - expect(data.groupedAttributes.map(attr => attr.id)).toEqual(["aId"]) - expect(data.ungroupedAttributes.map(attr => attr.id)).toEqual(["bId", "cId"]) + expect(data.collections[0].attributes.map(attr => attr!.id)).toEqual(["aId"]) + expect(data.childCollection.attributes.map(attr => attr!.id)).toEqual(["bId", "cId"]) expect(data.collectionGroups.length).toBe(1) expect(attributesByCollection()).toEqual([["aId"], ["bId", "cId"]]) const aCases = data.getCasesForAttributes(["aId"]) diff --git a/v3/src/models/data/data-set-conversion.ts b/v3/src/models/data/data-set-conversion.ts new file mode 100644 index 0000000000..c6c44ad219 --- /dev/null +++ b/v3/src/models/data/data-set-conversion.ts @@ -0,0 +1,34 @@ +import { IAttributeSnapshot } from "./attribute" +import { ICollectionModelSnapshot } from "./collection" +// eslint-disable-next-line import/no-cycle +import { DataSet, IDataSetSnapshot } from "./data-set" + +// originally, `attributesMap` didn't exist and `attributes` was an array of attributes +interface IOriginalDataSetSnap { + collections: Array + attributes: Array + ungrouped: Omit +} +export function isOriginalDataSetSnap(snap: IDataSetSnapshot): snap is IOriginalDataSetSnap { + return (!("attributesMap" in snap) && "attributes" in snap && + Array.isArray(snap.attributes) && (typeof snap.attributes[0] === "object")) +} + +// temporarily, `attributesMap` existed and `attributes` was an array of references +interface ITempDataSetSnap extends Omit { + attributes: Array +} +export function isTempDataSetSnap(snap: IDataSetSnapshot): snap is ITempDataSetSnap { + return ("attributesMap" in snap && "attributes" in snap && + Array.isArray(snap.attributes) && (typeof snap.attributes[0] === "string")) +} + +export function isLegacyDataSetSnap(snap: IDataSetSnapshot): snap is (IOriginalDataSetSnap | ITempDataSetSnap) { + return isOriginalDataSetSnap(snap) || isTempDataSetSnap(snap) +} +// eventually, `attributesMap` continued to exist, `attributes` became a view, and `ungrouped` became a collection + +export function createDataSet(snap: IDataSetSnapshot | IOriginalDataSetSnap) { + // preProcessSnapshot handler will perform the necessary conversion internally + return DataSet.create(snap as IDataSetSnapshot) +} diff --git a/v3/src/models/data/data-set-notifications.ts b/v3/src/models/data/data-set-notifications.ts index d646e568bc..24c800d63c 100644 --- a/v3/src/models/data/data-set-notifications.ts +++ b/v3/src/models/data/data-set-notifications.ts @@ -4,7 +4,7 @@ import { toV2Id } from "../../utilities/codap-utils" import { IAttribute } from "./attribute" import { IDataSet } from "./data-set" import { ICase } from "./data-set-types" -import { ICollectionModel, ICollectionPropsModel } from "./collection" +import { ICollectionModel } from "./collection" const action = "notify" function makeCallback(operation: string, other?: any) { @@ -48,7 +48,7 @@ export function deleteCollectionNotification(dataSet?: IDataSet) { return notification("deleteCollection", result, dataSet) } -export function updateCollectionNotification(collection?: ICollectionPropsModel, dataSet?: IDataSet) { +export function updateCollectionNotification(collection?: ICollectionModel, dataSet?: IDataSet) { const result = { success: true, properties: { name: collection?.name } } return notification("updateCollection", result, dataSet) } @@ -94,12 +94,12 @@ export function updateCasesNotification(data: IDataSet, cases?: ICase[]) { return notification("updateCases", result, data) } -// selectCasesNotificaiton returns a function that will later be called to determine if the selection +// selectCasesNotification returns a function that will later be called to determine if the selection // actually changed and a notification is necessary to broadcast export function selectCasesNotification(dataset: IDataSet, extend?: boolean) { const oldSelection = Array.from(dataset.selection) const oldSelectionSet = new Set(oldSelection) - + return () => { const newSelection = Array.from(dataset.selection) const newSelectionSet = new Set(newSelection) diff --git a/v3/src/models/data/data-set-types.ts b/v3/src/models/data/data-set-types.ts index 4a83d3457e..5d4c7dc173 100644 --- a/v3/src/models/data/data-set-types.ts +++ b/v3/src/models/data/data-set-types.ts @@ -1,4 +1,5 @@ import { Instance, types } from "mobx-state-tree" +import { RequireAtLeastOne } from "type-fest" import { IValueType } from "./attribute" import { kCaseIdPrefix, v3Id } from "../../utilities/codap-utils" @@ -51,13 +52,19 @@ export interface IAddAttributeOptions { export interface IMoveAttributeOptions { before?: string; // id of attribute before which the moved attribute should be placed after?: string; // id of attribute after which the moved attribute should be placed - withoutCustomUndo?: boolean; } -export interface IMoveAttributeCollectionOptions extends IMoveAttributeOptions { - collection?: string // id of destination collection; undefined => no collection (ungrouped) +export interface IAddCollectionOptions { + before?: string; // id of collection before which the new collection should be placed + after?: string; // id of collection after which the new collection should be placed } +export interface IMoveAttributeCollectionOptionsBase extends IMoveAttributeOptions { + collection?: string // id of destination collection +} +export type IMoveAttributeCollectionOptions = + RequireAtLeastOne + export interface IAttributeChangeResult { removedCollectionId?: string } diff --git a/v3/src/models/data/data-set-utils.test.ts b/v3/src/models/data/data-set-utils.test.ts index 82f98139de..19a656672c 100644 --- a/v3/src/models/data/data-set-utils.test.ts +++ b/v3/src/models/data/data-set-utils.test.ts @@ -1,5 +1,5 @@ import { IAttribute } from "./attribute" -import { CollectionModel, ICollectionPropsModel } from "./collection" +import { ICollectionModel } from "./collection" import { DataSet, IDataSet } from "./data-set" import { getCollectionAttrs } from "./data-set-utils" @@ -8,28 +8,26 @@ describe("DataSetUtils", () => { function names(attrs: IAttribute[]) { return attrs.map(({ name }) => name) } - function getCollectionAttrNames(_collection: ICollectionPropsModel, _data?: IDataSet) { + function getCollectionAttrNames(_collection: ICollectionModel, _data?: IDataSet) { return names(getCollectionAttrs(_collection, _data)) } const data = DataSet.create() - const collection = CollectionModel.create({ id: "cId" }) - data.addCollection(collection) + const origChildCollectionId = data.childCollection.id + const collection = data.addCollection({ id: "cId" }) data.addAttribute({ id: "aId", name: "a" }) data.addAttribute({ id: "bId", name: "b" }) expect(getCollectionAttrNames(collection, data)).toEqual([]) - expect(getCollectionAttrNames(data.ungrouped, data)).toEqual(["a", "b"]) + expect(getCollectionAttrNames(data.childCollection, data)).toEqual(["a", "b"]) - data.setCollectionForAttribute("aId", { collection: "cId" }) + data.moveAttribute("aId", { collection: "cId" }) expect(getCollectionAttrNames(collection, data)).toEqual(["a"]) - expect(getCollectionAttrNames(data.ungrouped, data)).toEqual(["b"]) + expect(getCollectionAttrNames(data.childCollection, data)).toEqual(["b"]) - // Moving the last attribute out of the ungrouped collection will cause the collection - // to be destroyed and the attributes to return to being ungrouped. - data.setCollectionForAttribute("bId", { collection: "cId" }) - jestSpyConsole("warn", spy => { - expect(getCollectionAttrNames(collection, data)).toEqual([]) - expect(spy).toHaveBeenCalledTimes(1) - }) - expect(getCollectionAttrNames(data.ungrouped, data)).toEqual(["a", "b"]) + // Moving the last attribute out of the child collection will cause it + // to be destroyed and the parent collection to become the child collection. + data.moveAttribute("bId", { collection: "cId" }) + expect(data.childCollection).toBe(collection) + expect(data.getCollection(origChildCollectionId)).toBeUndefined() + expect(getCollectionAttrNames(data.childCollection, data)).toEqual(["a", "b"]) }) }) diff --git a/v3/src/models/data/data-set-utils.ts b/v3/src/models/data/data-set-utils.ts index 8a4477e56a..4ff4e31ee3 100644 --- a/v3/src/models/data/data-set-utils.ts +++ b/v3/src/models/data/data-set-utils.ts @@ -1,21 +1,20 @@ import { isAlive } from "mobx-state-tree" import { kIndexColumnKey } from "../../components/case-table/case-table-types" -import {IAttribute} from "./attribute" -import {ICollectionPropsModel, isCollectionModel} from "./collection" -import {IDataSet} from "./data-set" +import { getSharedCaseMetadataFromDataset } from "../shared/shared-data-utils" +import { IAttribute } from "./attribute" +import { ICollectionModel } from "./collection" +import { IDataSet } from "./data-set" import { deleteCollectionNotification, moveAttributeNotification, selectCasesNotification } from "./data-set-notifications" import { IAttributeChangeResult, IMoveAttributeOptions } from "./data-set-types" -export function getCollectionAttrs(collection: ICollectionPropsModel, data?: IDataSet) { +export function getCollectionAttrs(collection: ICollectionModel, data?: IDataSet): IAttribute[] { if (collection && !isAlive(collection)) { console.warn("DataSetUtils.getCollectionAttrs called for defunct collection") return [] } - return (isCollectionModel(collection) - ? Array.from(collection.attributes) as IAttribute[] - : data?.ungroupedAttributes) ?? [] + return Array.from(collection.attributes) as IAttribute[] } export function collectionCaseIdFromIndex(index: number, data?: IDataSet, collectionId?: string) { @@ -39,7 +38,6 @@ export function collectionCaseIndexFromId(caseId: string, data?: IDataSet, colle */ export function idOfChildmostCollectionForAttributes(attrIDs: string[], data?: IDataSet) { if (!data) return undefined - if (data.ungroupedAttributes.some(attr => attrIDs.includes(attr.id))) return undefined const collections = data.collections for (let i = collections.length - 1; i >= 0; --i) { const collection = collections[i] @@ -47,13 +45,20 @@ export function idOfChildmostCollectionForAttributes(attrIDs: string[], data?: I } } +export function firstVisibleParentAttribute(data?: IDataSet, collectionId?: string): IAttribute | undefined { + if (!collectionId) return + const metadata = data && getSharedCaseMetadataFromDataset(data) + const parentCollection = data?.getParentCollection(collectionId) + return parentCollection?.attributes.find(attr => attr && !metadata?.isHidden(attr.id)) +} + interface IMoveAttributeParameters { afterAttrId?: string attrId: string dataset: IDataSet includeNotifications?: boolean - sourceCollection?: ICollectionPropsModel - targetCollection: ICollectionPropsModel + sourceCollection?: ICollectionModel + targetCollection: ICollectionModel undoable?: boolean } export function moveAttribute({ @@ -69,20 +74,11 @@ export function moveAttribute({ const modelChangeOptions = { notifications, undoStringKey, redoStringKey } if (targetCollection.id === sourceCollection?.id) { - if (isCollectionModel(targetCollection)) { - // move the attribute within a collection - dataset.applyModelChange( - () => targetCollection.moveAttribute(attrId, options), - modelChangeOptions - ) - } - else { - // move an ungrouped attribute within the DataSet - dataset.applyModelChange( - () => dataset.moveAttribute(attrId, options), - modelChangeOptions - ) - } + // move the attribute within a collection + dataset.applyModelChange( + () => targetCollection.moveAttribute(attrId, options), + modelChangeOptions + ) } else { // move the attribute to a new collection @@ -95,7 +91,7 @@ export function moveAttribute({ dataset.applyModelChange( () => { - result = dataset.setCollectionForAttribute(attrId, { collection: targetCollection?.id, ...options }) + result = dataset.moveAttribute(attrId, { collection: targetCollection?.id, ...options }) }, { notifications: _notifications, undoStringKey, redoStringKey } ) diff --git a/v3/src/models/data/data-set.test.ts b/v3/src/models/data/data-set.test.ts index cd6670ed4e..ed2ebcef73 100644 --- a/v3/src/models/data/data-set.test.ts +++ b/v3/src/models/data/data-set.test.ts @@ -1,8 +1,10 @@ import { isEqual, isEqualWith } from "lodash" import { applyAction, clone, destroy, getSnapshot, onAction, onSnapshot } from "mobx-state-tree" import { uniqueName } from "../../utilities/js-utils" -import { CollectionModel, CollectionPropsModel } from "./collection" -import { DataSet, LEGACY_ATTRIBUTES_ARRAY_ANY, fromCanonical, toCanonical } from "./data-set" +import { IAttributeSnapshot } from "./attribute" +import { ICollectionModelSnapshot } from "./collection" +import { DataSet, fromCanonical, toCanonical } from "./data-set" +import { createDataSet } from "./data-set-conversion" import { CaseID, ICaseID } from "./data-set-types" let message = () => "" @@ -92,6 +94,63 @@ test("Canonicalization", () => { mockConsole.mockRestore() }) +test("DataSet original flat snapshot conversion", () => { + const ungrouped: ICollectionModelSnapshot = { name: "Ungrouped" } + const attributes: IAttributeSnapshot[] = [ + { name: "a1" }, + { name: "a2" }, + { name: "a3" } + ] + const data = DataSet.create({ + name: "Data", + ungrouped, + attributes + } as any) + expect(data.collections.length).toBe(1) + expect(data.childCollection.name).toBe("Ungrouped") + expect(data.attributesMap.size).toBe(3) + expect(data.attributes.length).toBe(3) +}) + +test("DataSet original hierarchical snapshot conversion", () => { + const ungrouped: ICollectionModelSnapshot = { name: "Ungrouped" } + const attributes: IAttributeSnapshot[] = [ + { id: "a1Id", name: "a1" }, + { id: "a2Id", name: "a2" }, + { id: "a3Id", name: "a3" } + ] + const collections: ICollectionModelSnapshot[] = [ + { name: "Collection1", attributes: ["a1Id"] } + ] + const data = DataSet.create({ + name: "Data", + collections, + ungrouped, + attributes + } as any) + expect(data.collections.length).toBe(2) + expect(data.collections[0].attributes.length).toBe(1) + expect(data.childCollection.name).toBe("Ungrouped") + expect(data.childCollection.attributes.length).toBe(2) + expect(data.attributesMap.size).toBe(3) + expect(data.attributes.length).toBe(3) +}) + +test("DataSet temporary flat snapshot conversion", () => { + const attributes: string[] = ["a1Id", "a2Id", "a3Id"] + const data = DataSet.create({ + name: "Data", + attributesMap: { + a1Id: { id: "a1Id", name: "a1" }, + a2Id: { id: "a2Id", name: "a2" }, + a3Id: { id: "a3Id", name: "a3" } + }, + attributes + } as any) + expect(data.attributesMap.size).toBe(3) + expect(data.attributes.length).toBe(3) +}) + test("DataSet basic functionality", () => { const dataset = DataSet.create({ name: "data" }) expect(dataset.id).toBeDefined() @@ -387,28 +446,28 @@ test("hierarchical collection support", () => { const data = DataSet.create({ name: "data" }) expect(data.id).toBeDefined() - expect(data.ungrouped.name).toBe("") - expect(data.collectionIds).toEqual([data.ungrouped.id]) - expect(data.collectionModels).toEqual([data.ungrouped]) - data.setUngroupedCollection(CollectionPropsModel.create({ name: "Cases" })) - expect(data.ungrouped.name).toBe("Cases") - expect(data.collectionIds).toEqual([data.ungrouped.id]) - expect(data.collectionModels).toEqual([data.ungrouped]) + expect(data.collections.length).toBe(1) + expect(data.collectionIds).toEqual([data.childCollection.id]) + expect(data.collections).toEqual([data.childCollection]) + expect(data.childCollection.name).toBe("Cases") - const collection = CollectionModel.create({ name: "ParentCollection" }) - const collectionId = collection.id - data.addCollection(collection) - expect(data.collectionIds).toEqual([collection.id, data.ungrouped.id]) - expect(data.collectionModels).toEqual([collection, data.ungrouped]) + const parentCollection = data.addCollection({ name: "ParentCollection" }) + const parentCollectionId = parentCollection.id + expect(data.collectionIds).toEqual([parentCollection.id, data.childCollection.id]) + expect(data.collections).toEqual([parentCollection, data.childCollection]) + expect(data.getParentCollection(data.childCollection.id)).toBe(parentCollection) + expect(data.getParentCollection(parentCollection.id)).toBeUndefined() + expect(data.getChildCollection(parentCollection.id)).toBe(data.childCollection) + expect(data.getChildCollection(data.childCollection.id)).toBeUndefined() const childAttr = data.addAttribute({ name: "childAttr" }) - const parentAttr = data.addAttribute({ name: "parentAttr" }, { collection: collectionId }) - expect(collection.getAttribute(childAttr.id)).toBeUndefined() - expect(collection.getAttribute(parentAttr.id)).toBe(parentAttr) + const parentAttr = data.addAttribute({ name: "parentAttr" }, { collection: parentCollectionId }) + expect(parentCollection.getAttribute(childAttr.id)).toBeUndefined() + expect(parentCollection.getAttribute(parentAttr.id)).toBe(parentAttr) destroy(data) jestSpyConsole("warn", spy => { - data.getCollection(collectionId) + data.getCollection(parentCollectionId) expect(spy).toHaveBeenCalledTimes(1) data.getCollectionByName("ParentCollection") @@ -417,9 +476,9 @@ test("hierarchical collection support", () => { }) test("Canonical case functionality", () => { - const dataset = DataSet.create({ + const dataset = createDataSet({ name: "data", - attributes: [{ name: "str" }, { name: "num" }] as LEGACY_ATTRIBUTES_ARRAY_ANY + attributes: [{ name: "str" }, { name: "num" }] }), strAttrID = dataset.attributes[0].id, numAttrID = dataset.attributes[1].id @@ -649,13 +708,11 @@ test("DataSet collection helpers", () => { const ds = DataSet.create({ name: "data" }) ds.addAttribute({ name: "attr1" }) ds.addAttribute({ name: "attr2" }) - expect(ds.ungrouped).toBeDefined() - expect(ds.collections.length).toBe(0) - ds.moveAttributeToNewCollection(ds.attributes[0].id) expect(ds.collections.length).toBe(1) + ds.moveAttributeToNewCollection(ds.attributes[0].id) + expect(ds.collections.length).toBe(2) // Test collection helpers using the actual, grouped collection (instance of CollectionModel). - expect(ds.getGroupedCollection(ds.collections[0].id)).toBeDefined() expect(ds.getCollection(ds.collections[0].id)).toBeDefined() expect(ds.getCollectionIndex(ds.collections[0].id)).toEqual(0) expect(ds.getCollectionForAttribute(ds.attributes[0].id)).toBe(ds.collections[0]) @@ -663,10 +720,9 @@ test("DataSet collection helpers", () => { // Test collection helpers using the the ungrouped collection stand-in. It's not considered be a grouped collection, // but other collection-related helpers handle it as expected. - expect(ds.getGroupedCollection(ds.ungrouped.id)).not.toBeDefined() - expect(ds.getCollection(ds.ungrouped.id)).toBeDefined() - expect(ds.getCollectionIndex(ds.ungrouped.id)).toEqual(1) - expect(ds.getCollectionForAttribute(ds.attributes[1].id)).toBe(ds.ungrouped) + expect(ds.getCollection(ds.childCollection.id)).toBeDefined() + expect(ds.getCollectionIndex(ds.childCollection.id)).toEqual(1) + expect(ds.getCollectionForAttribute(ds.attributes[1].id)).toBe(ds.childCollection) expect(ds.getCollectionForAttribute("non-existent-attr-ID")).toBeUndefined() }) diff --git a/v3/src/models/data/data-set.ts b/v3/src/models/data/data-set.ts index c9d696afd0..0f6d6910f8 100644 --- a/v3/src/models/data/data-set.ts +++ b/v3/src/models/data/data-set.ts @@ -43,33 +43,29 @@ */ import { observable, reaction, runInAction } from "mobx" -import { addDisposer, addMiddleware, getEnv, hasEnv, Instance, isAlive, SnapshotIn, types } from "mobx-state-tree" +import { + addDisposer, addMiddleware, getEnv, getSnapshot, hasEnv, Instance, isAlive, ReferenceIdentifier, SnapshotIn, types +} from "mobx-state-tree" import pluralize from "pluralize" import { Attribute, IAttribute, IAttributeSnapshot } from "./attribute" +import { CollectionModel, ICollectionModel, ICollectionModelSnapshot, isCollectionModel } from "./collection" import { - CollectionModel, CollectionPropsModel, ICollectionModel, ICollectionPropsModel, isCollectionModel -} from "./collection" -import { - CaseGroup, CaseID, IAddAttributeOptions, IAddCasesOptions, IAttributeChangeResult, ICase, ICaseCreation, - IDerivationSpec, IGetCaseOptions, IGetCasesOptions, IGroupedCase, IMoveAttributeCollectionOptions, - IMoveAttributeOptions, symIndex, symParent + CaseGroup, CaseID, IAddAttributeOptions, IAddCasesOptions, IAddCollectionOptions, IAttributeChangeResult, ICase, + ICaseCreation, IDerivationSpec, IGetCaseOptions, IGetCasesOptions, IGroupedCase, IMoveAttributeCollectionOptions, + symIndex, symParent } from "./data-set-types" -// eslint-disable-next-line import/no-cycle +/* eslint-disable import/no-cycle */ +import { isLegacyDataSetSnap, isOriginalDataSetSnap, isTempDataSetSnap } from "./data-set-conversion" import { ISetCaseValuesCustomPatch, setCaseValuesCustomUndoRedo } from "./data-set-undo" +/* eslint-enable import/no-cycle */ import { applyModelChange } from "../history/apply-model-change" import { withCustomUndoRedo } from "../history/with-custom-undo-redo" import { withoutUndo } from "../history/without-undo" import { kAttrIdPrefix, kCaseIdPrefix, typeV3Id, v3Id } from "../../utilities/codap-utils" import { prf } from "../../utilities/profiler" +import { t } from "../../utilities/translation/translate" import { V2Model } from "./v2-model" -// For testing purposes, it is convenient to build a DataSet using just the legacy `attributes` -// array format, trusting that the preProcessSnapshot handler will convert the array to its -// corresponding `attributesMap` and `attributes` references array properties. Rather than -// forcing those tests to be rewritten, this type makes it explicit that these tests are relying -// on this mechanism in a way that allows the resulting code to pass the TypeScript compiler. -export type LEGACY_ATTRIBUTES_ARRAY_ANY = any - // remnant of derived DataSet implementation that isn't in active use interface IEnvContext { srcDataSet: IDataSet; @@ -144,12 +140,9 @@ export interface CollectionGroup { export const DataSet = V2Model.named("DataSet").props({ id: typeV3Id("DATA"), sourceID: types.maybe(types.string), - // ordered parent-most to child-most; no explicit collection for ungrouped (child-most) attributes + // ordered parent-most to child-most collections: types.array(CollectionModel), - // ungrouped (child-most) collection has properties, but no grouping attributes - ungrouped: types.optional(CollectionPropsModel, () => CollectionPropsModel.create()), attributesMap: types.map(Attribute), - attributes: types.array(types.reference(Attribute)), cases: types.array(CaseID), sourceName: types.maybe(types.string), description: types.maybe(types.string), @@ -193,31 +186,83 @@ export const DataSet = V2Model.named("DataSet").props({ } }) .preProcessSnapshot(snap => { - // convert legacy array of attribute models to attributesMap and array of references - if (snap.attributes?.length && typeof snap.attributes[0] === "object") { - const { attributes: _attributes, ...others } = snap - const legacyAttributes = _attributes as unknown as Array + // convert legacy collections/attributes implementation to current + if (isLegacyDataSetSnap(snap)) { + const { collections: _collections = [], attributes: _legacyAttributes, ungrouped, ...others } = snap + + const attributeIds: string[] = [] + + // build the attributesMap (if necessary) const attributesMap: Record = {} - const attributes: string[] = [] - legacyAttributes.forEach(attr => { - const attrId = attr.id || v3Id(kAttrIdPrefix) - attributesMap[attrId] = { id: attrId, ...attr } - attributes.push(attrId) + if (isOriginalDataSetSnap(snap)) { + const { attributes: _attributes } = snap + _attributes.forEach(attr => { + const attrId = attr.id || v3Id(kAttrIdPrefix) + attributeIds.push(attrId) + attributesMap[attrId] = { id: attrId, ...attr } + }) + } + // extract the attribute ids + else if (isTempDataSetSnap(snap)) { + const { attributes: _attributes } = snap + attributeIds.push(..._attributes) + } + + const collections: ICollectionModelSnapshot[] = [..._collections] + + // identify parent attributes that shouldn't be in child collection + const parentAttrs = new Set() + collections.forEach(collection => { + collection.attributes?.forEach(attrId => { + attrId && parentAttrs.add(attrId) + }) }) - return { attributesMap, attributes, ...others } + // identify child collection attributes + const childAttrs: ReferenceIdentifier[] = [] + attributeIds?.forEach(attrId => { + if (!parentAttrs.has(attrId)) { + childAttrs.push(attrId) + } + }) + + // create child collection + const childCollection: ICollectionModelSnapshot = { + name: t("DG.AppController.createDataSet.collectionName"), + ...ungrouped, + attributes: childAttrs + } + collections.push(childCollection) + + return { attributesMap, collections, ...others } } return snap }) +.views(self => ({ + get attributes() { + const attrs: IAttribute[] = [] + self.collections.forEach(collection => { + collection.attributes.forEach(attr => { + attr && attrs.push(attr) + }) + }) + return attrs + }, + get parentCollections() { + const _parentCollections = [...self.collections] + _parentCollections.splice(_parentCollections.length - 1, 1) + return _parentCollections + } +})) .views(self => ({ attrIndexFromID(id: string) { const index = self.attributes.findIndex(attr => attr.id === id) return index >= 0 ? index : undefined }, get collectionIds() { - return [...self.collections.map(collection => collection.id), self.ungrouped.id] + return self.collections.map(collection => collection.id) }, - get collectionModels(): ICollectionPropsModel[] { - return [...self.collections, self.ungrouped] + get childCollection(): ICollectionModel { + return self.collections[self.collections.length - 1] } })) .views(self => ({ @@ -228,59 +273,6 @@ export const DataSet = V2Model.named("DataSet").props({ return self.attributesMap.get(self.attrNameMap.get(name) ?? "") } })) -.actions(self => ({ - // change the attribute order within the data set itself; doesn't handle collections - moveAttribute(attributeID: string, options?: IMoveAttributeOptions) { - const attribute = self.getAttribute(attributeID) - const attrIndex = self.attrIndexFromID(attributeID) - if (attribute && attrIndex != null) { - if (options?.before) { - let beforeAttrIndex = self.attrIndexFromID(options.before) - if (beforeAttrIndex != null) { - if (beforeAttrIndex !== attrIndex && beforeAttrIndex !== attrIndex + 1) { - self.attributes.remove(attribute) - if (attrIndex < beforeAttrIndex) --beforeAttrIndex - self.attributes.splice(beforeAttrIndex, 0, attribute) - } - return - } - } - if (options?.after) { - let afterAttrIndex = self.attrIndexFromID(options.after) - if (afterAttrIndex != null) { - if (afterAttrIndex !== attrIndex && afterAttrIndex !== attrIndex - 1) { - self.attributes.remove(attribute) - if (attrIndex > afterAttrIndex) ++afterAttrIndex - self.attributes.splice(afterAttrIndex, 0, attribute) - } - return - } - } - // no before/after -- move attribute to end - if (attrIndex !== self.attributes.length - 1) { - self.attributes.remove(attribute) - self.attributes.push(attribute) - } - } - } -})) -.views(self => ({ - // array of attributes that are grouped into collections - get groupedAttributes() { - const groupedAttrs: IAttribute[] = [] - self.collections.forEach(collection => { - collection.attributes.forEach(attr => attr && groupedAttrs.push(attr)) - }) - return groupedAttrs - } -})) -.views(self => ({ - // array of attributes _not_ grouped into collections - get ungroupedAttributes(): IAttribute[] { - const grouped = new Set(self.groupedAttributes.map(attr => attr.id)) - return self.attributes.filter(attr => attr && !grouped.has(attr.id)) - }, -})) .extend(self => { // we do our own caching because MST's auto-caching wasn't working as expected const _collectionGroups = observable.box([]) @@ -288,57 +280,46 @@ export const DataSet = V2Model.named("DataSet").props({ let _childCases: IGroupedCase[] = [] const isValidCollectionGroups = observable.box(false) - function getGroupedCollection(collectionId: string): ICollectionModel | undefined { - return self.collections.find(coll => coll.id === collectionId) - } - - function getCollection(collectionId: string): ICollectionPropsModel | undefined { + function getCollection(collectionId: string): ICollectionModel | undefined { if (!isAlive(self)) { console.warn("DataSet.getCollection called on a defunct DataSet") return } - return collectionId === self.ungrouped.id ? self.ungrouped : getGroupedCollection(collectionId) + return self.collections.find(({ id }) => id === collectionId) } - function getGroupedCollectionByName(name: string): ICollectionModel | undefined { - return self.collections.find(collection => collection.name === name) - } - - function getCollectionByName(name: string): ICollectionPropsModel | undefined { + function getCollectionByName(collectionName: string): ICollectionModel | undefined { if (!isAlive(self)) { console.warn("DataSet.getCollectionByName called on a defunct DataSet") return } - return name === self.ungrouped.name ? self.ungrouped : getGroupedCollectionByName(name) + return self.collections.find(({ name }) => name === collectionName) } function getCollectionIndex(collectionId: string) { - // For consistency, treat ungrouped as the last / child-most collection - return collectionId === self.ungrouped.id - ? self.collections.length - : self.collections.findIndex(coll => coll.id === collectionId) + return self.collections.findIndex(({ id }) => id === collectionId) } - function getCollectionForAttribute(attributeId: string): ICollectionPropsModel | undefined { - return self.collections.find(coll => coll.getAttribute(attributeId)) ?? - (self.attributes.find(attr => attr.id === attributeId) ? self.ungrouped : undefined) + function getCollectionForAttribute(attributeId: string): ICollectionModel | undefined { + return self.collections.find(coll => coll.getAttribute(attributeId)) + } + + function getCollectionForCase(caseId: string): ICollectionModel | undefined { + return self.collections.find(coll => coll.hasCase(caseId)) } return { views: { - // get real collection from id (ungrouped collection is not considered to be a real collection) - getGroupedCollection, - // get collection from id (including ungrouped collection) + // get collection from id getCollection, - // get real collection from name (ungrouped collection is not considered to be a real collection) - getGroupedCollectionByName, - // get collection from name (including ungrouped collection) + // get collection from name getCollectionByName, - // get index from collection (including ungrouped collection) + // get index from collection getCollectionIndex, - // get collection from attribute. Ungrouped collection is returned for ungrouped attributes. + // get collection from attribute // undefined => attribute not present in dataset getCollectionForAttribute, + getCollectionForCase, // leaf-most child cases (i.e. those not grouped in a collection) childCases() { if (!isValidCollectionGroups.get()) { @@ -351,9 +332,9 @@ export const DataSet = V2Model.named("DataSet").props({ get collectionGroups() { if (isValidCollectionGroups.get()) return _collectionGroups.get() - // create groups for each collection (does not included ungrouped) + // create groups for each parent collection const newCollectionGroups: CollectionGroup[] = - self.collections.map(collection => ({ collection, groups: [], groupsMap: {} })) + self.parentCollections.map(collection => ({ collection, groups: [], groupsMap: {} })) prf.measure("DataSet.collectionGroups", () => { self.cases.forEach(aCase => { @@ -483,83 +464,107 @@ export const DataSet = V2Model.named("DataSet").props({ invalidateCollectionGroups() { isValidCollectionGroups.set(false) }, - setUngroupedCollection(collection: ICollectionPropsModel) { - self.ungrouped = collection - }, - addCollection(collection: ICollectionModel, beforeCollectionId?: string) { - const beforeIndex = beforeCollectionId ? getCollectionIndex(beforeCollectionId) : -1 + addCollection(collection: ICollectionModelSnapshot, options?: IAddCollectionOptions) { + // place the collection in the correct location + let beforeIndex = options?.before ? getCollectionIndex(options.before) : -1 + if (beforeIndex < 0 && options?.after) { + beforeIndex = getCollectionIndex(options.after) + if (beforeIndex >= 0 && beforeIndex < self.collections.length) { + ++beforeIndex + } + } + // by default, new collections are added before the default child collection + if (beforeIndex < 0 && self.collections.length > 0) { + beforeIndex = self.collections.length - 1 + } if (beforeIndex >= 0) { self.collections.splice(beforeIndex, 0, collection) } else { + // by default, new collections are added as the childmost collection + beforeIndex = self.collections.length self.collections.push(collection) } + const newCollection = self.collections[beforeIndex] + // remove any attributes from other collections + const attrIds: Array = [...(collection.attributes ?? [])] + attrIds?.forEach(attrId => { + const attrCollection = self.collections.find(_collection => { + return attrId && _collection !== newCollection && _collection.getAttribute(`${attrId}`) + }) + if (attrId && attrCollection) attrCollection.removeAttribute(`${attrId}`) + }) + // recalculate groups this.invalidateCollectionGroups() + return newCollection }, removeCollection(collection: ICollectionModel) { self.collections.remove(collection) }, - setCollectionForAttribute(attributeId: string, options?: IMoveAttributeCollectionOptions) { - const result: IAttributeChangeResult = {} - const attribute = self.attributes.find(attr => attr.id === attributeId) - const newCollection = options?.collection ? getGroupedCollection(options.collection) : undefined - const oldCollection = getCollectionForAttribute(attributeId) - if (attribute && oldCollection !== newCollection) { - if (attribute.hasFormula) { - // If the attribute has a formula, we need to reset all the calculated values to blank values so that they - // are not taken into account while calculating case grouping. After the grouping is done, the formula will - // be re-evaluated, and the values will be updated to the correct values again. - attribute.clearValues() - } - if (isCollectionModel(oldCollection)) { - // remove it from previous collection (if any) - if (oldCollection?.attributes.length > 1) { - oldCollection.removeAttribute(attributeId) - } - // remove the entire collection if it was the last attribute - else { - result.removedCollectionId = oldCollection.id - this.removeCollection(oldCollection) - } - } - if (newCollection) { - // add it to the new collection - newCollection.addAttribute(attribute, options) - } - else if (options?.before || options?.after) { - // move it within the data set - self.moveAttribute(attributeId, { withoutCustomUndo: true, ...options }) - } - if (!isCollectionModel(oldCollection)) { - // if the last ungrouped attribute was moved into a collection, then eliminate - // the last collection, thus un-grouping the child-most attributes - const allAttrCount = self.attributes.length - const collectionAttrCount = self.collections - .reduce((sum, collection) => sum += collection.attributes.length, 0) - if (collectionAttrCount >= allAttrCount) { - result.removedCollectionId = self.ungrouped.id - self.collections.splice(self.collections.length - 1, 1) - } - } - this.invalidateCollectionGroups() - } - return result - }, - // if beforeCollectionId is not specified, new collection is last (child-most) - moveAttributeToNewCollection(attributeId: string, beforeCollectionId?: string) { - const attribute = self.getAttribute(attributeId) - if (attribute) { - const name = pluralize(attribute.name) - const collection = CollectionModel.create({ name }) - this.addCollection(collection, beforeCollectionId) - this.setCollectionForAttribute(attributeId, { collection: collection.id }) - return collection - } - } } } }) +.actions(self => ({ + moveAttribute(attributeID: string, options?: IMoveAttributeCollectionOptions): IAttributeChangeResult { + let removedCollectionId: string | undefined + const attribute = self.getAttribute(attributeID) + const srcCollection = self.getCollectionForAttribute(attributeID) + const dstCollection = options?.before + ? self.getCollectionForAttribute(options.before) + : options?.after + ? self.getCollectionForAttribute(options.after) + : options?.collection + ? self.getCollection(options.collection) + : undefined + if (!attribute || !srcCollection) return {} + if (!dstCollection || srcCollection === dstCollection) { + srcCollection?.moveAttribute(attributeID, options) + } + else { + if (attribute.hasFormula) { + // If the attribute has a formula, we need to reset all the calculated values to blank values so that they + // are not taken into account while calculating case grouping. After the grouping is done, the formula will + // be re-evaluated, and the values will be updated to the correct values again. + attribute.clearValues() + } + if (srcCollection.getAttribute(attributeID) && srcCollection.attributes.length === 1) { + removedCollectionId = srcCollection.id + self.removeCollection(srcCollection) + } + else { + srcCollection.removeAttribute(attributeID) + } + dstCollection.addAttribute(attribute, options) + // update grouping + self.invalidateCollectionGroups() + } + return { removedCollectionId } + } +})) +.actions(self => ({ + // if beforeCollectionId is not specified, new collection is parent of the child-most collection + moveAttributeToNewCollection(attributeId: string, beforeCollectionId?: string) { + const attribute = self.getAttribute(attributeId) + if (attribute) { + const name = pluralize(attribute.name) + const collectionSnap: ICollectionModelSnapshot = { name } + const collection = self.addCollection(collectionSnap, { before: beforeCollectionId }) + self.moveAttribute(attributeId, { collection: collection.id }) + return collection + } + } +})) .views(self => ({ + getParentCollection(collectionId?: string) { + const foundIndex = self.collections.findIndex(collection => collection.id === collectionId) + const parentIndex = foundIndex > 0 ? foundIndex - 1 : -1 + return parentIndex >= 0 ? self.collections[parentIndex] : undefined + }, + getChildCollection(collectionId?: string) { + const foundIndex = self.collections.findIndex(collection => collection.id === collectionId) + const childIndex = foundIndex < self.collections.length - 1 ? foundIndex + 1 : -1 + return childIndex >= 0 ? self.collections[childIndex] : undefined + }, getGroupsForCollection(collectionId?: string) { if (collectionId && self.getCollection(collectionId)) { for (let i = self.collectionGroups.length - 1; i >= 0; --i) { @@ -573,37 +578,46 @@ export const DataSet = V2Model.named("DataSet").props({ } } } - return self.childCases().map(c => ({ - childCaseIds: [] as string[], - childPseudoCaseIds: [] as string[], - collectionId: self.ungrouped.id, - pseudoCase: c - })) }, getParentCollectionGroup(collectionId?: string) { if (collectionId && self.collectionGroups.length) { - if (self.ungrouped.id === collectionId) { - return self.collectionGroups[self.collectionGroups.length - 1] - } else if (self.getCollection(collectionId)) { + if (self.getCollection(collectionId)) { + const parentCollection = this.getParentCollection(collectionId) return self.collectionGroups.find((_collectionGroup, index) => { - return index < self.collectionGroups.length - 1 && - self.collectionGroups[index + 1].collection.id === collectionId + return _collectionGroup.collection.id === parentCollection?.id }) } } } })) .views(self => ({ - getCasesForCollection(collectionId?: string) { - return self.getGroupsForCollection(collectionId)?.map(group => group.pseudoCase) + getCasesForCollection(collectionId?: string): ICase[] { + if (!collectionId || !self.getCollection(collectionId)) return [] + // parent collection cases can be retrieved from the groups + const collectionGroups = self.getGroupsForCollection(collectionId) + if (collectionGroups) return collectionGroups.map(group => group.pseudoCase) + // child collection cases can be ordered by parent + const parentCollection = self.getParentCollection(collectionId || self.childCollection.id) + const parentGroups = parentCollection ? self.getGroupsForCollection(parentCollection.id) : undefined + if (parentGroups) { + const cases: ICase[] = [] + parentGroups.forEach(group => { + const caseCount = cases.length + cases.push(...group.childCaseIds.map((__id__, index) => ({ + __id__, + [symParent]: group.pseudoCase.__id__, + [symIndex]: caseCount + index + }))) + }) + return cases + } + // return child cases in data set order + return getSnapshot(self.cases) as ICase[] }, getParentCase(caseId: string, collectionId?: string) { const parentCollectionGroup = self.getParentCollectionGroup(collectionId) return parentCollectionGroup?.groups.find(group => - collectionId === self.ungrouped.id - ? group.childCaseIds.includes(caseId) - : group.childPseudoCaseIds?.includes(caseId) - ) + (group.childPseudoCaseIds ?? group.childCaseIds)?.includes(caseId)) }, getCollectionGroupForAttributes(attributeIds: string[]) { // finds the child-most collection (if any) among the specified attributes @@ -829,6 +843,11 @@ export const DataSet = V2Model.named("DataSet").props({ attr.setLength(self.cases.length) }) + // add initial collection if not already present + if (!self.collections.length) { + self.addCollection({ name: t("DG.AppController.createDataSet.collectionName") }) + } + if (!srcDataSet) { // set up middleware to add ids to inserted attributes and cases // adding the ids in middleware makes them available as action arguments @@ -888,6 +907,7 @@ export const DataSet = V2Model.named("DataSet").props({ const { before: beforeID, collection: collectionId } = options || {} // add attribute to attributesMap + let collection: ICollectionModel | undefined const attribute = self.attributesMap.put(snapshot) // add attribute to attrNameMap @@ -895,21 +915,26 @@ export const DataSet = V2Model.named("DataSet").props({ // add attribute reference to attributes array const beforeIndex = beforeID ? self.attrIndexFromID(beforeID) ?? -1 : -1 - if (beforeIndex >= 0) { - self.attributes.splice(beforeIndex, 0, attribute.id) - } - else { - self.attributes.push(attribute.id) + if (beforeID && beforeIndex >= 0) { + collection = self.getCollectionForAttribute(beforeID) + const collectionBeforeIndex = collection?.attributes.findIndex(attr => attr?.id === beforeID) ?? -1 + if (collectionBeforeIndex >= 0) { + collection?.attributes.splice(collectionBeforeIndex, 0, attribute.id) + } + return attribute } // fill out any missing values - attribute.setLength(self.cases.length) + for (let i = attribute.strValues.length; i < self.cases.length; ++i) { + attribute.addValue() + } - // add the attribute to the specified collection (if any) - if (collectionId) { - const collection = self.getGroupedCollection(collectionId) - collection?.addAttribute(attribute) + // add the attribute to the specified collection (if any) or the childmost collection + if (!collection && collectionId) { + collection = self.getCollection(collectionId) } + if (!collection) collection = self.childCollection + collection.addAttribute(attribute) return attribute }, @@ -936,14 +961,12 @@ export const DataSet = V2Model.named("DataSet").props({ if (collection.attributes.length > 1) { collection.removeAttribute(attributeID) } - else { + else if (self.collections.length > 1) { result.removedCollectionId = collection.id self.removeCollection(collection) } } - // remove attribute from attributes array - self.attributes.remove(attribute) // remove attribute from attrNameMap self.attrNameMap.delete(attribute.name) // remove attribute from attributesMap diff --git a/v3/src/models/document/document-content.ts b/v3/src/models/document/document-content.ts index 70174af035..e19b117c50 100644 --- a/v3/src/models/document/document-content.ts +++ b/v3/src/models/document/document-content.ts @@ -105,15 +105,13 @@ export const DocumentContentModel = BaseDocumentContentModel const sharedModelManager = getSharedModelManager(self) // DataSets must have unique names const baseDataSetName = snapshot?.name || t("DG.AppController.createDataSet.name") - const baseCollectionName = snapshot?.ungrouped?.name || t("DG.AppController.createDataSet.collectionName") - const ungrouped = { name: baseCollectionName, ...snapshot?.ungrouped } let name = baseDataSetName const existingNames = sharedModelManager?.getSharedModelsByType(kSharedDataSetType) .map((sharedModel: ISharedDataSet) => sharedModel.dataSet.name) ?? [] for (let i = 2; existingNames.includes(name); ++i) { name = `${baseDataSetName} ${i}` } - const dataSet: IDataSetSnapshot = { ...snapshot, name, ungrouped } + const dataSet: IDataSetSnapshot = { ...snapshot, name } const sharedDataSet = SharedDataSet.create({ providerId, dataSet }) sharedModelManager?.addSharedModel(sharedDataSet) diff --git a/v3/src/models/formula/attribute-formula-adapter.test.ts b/v3/src/models/formula/attribute-formula-adapter.test.ts index 3903ce2f6d..54b9ce4fe5 100644 --- a/v3/src/models/formula/attribute-formula-adapter.test.ts +++ b/v3/src/models/formula/attribute-formula-adapter.test.ts @@ -1,10 +1,11 @@ -import { DataSet, IDataSet, LEGACY_ATTRIBUTES_ARRAY_ANY } from "../data/data-set" +import { IDataSet } from "../data/data-set" +import { createDataSet } from "../data/data-set-conversion" import { AttributeFormulaAdapter } from "./attribute-formula-adapter" import { localAttrIdToCanonical } from "./utils/name-mapping-utils" const getTestEnv = () => { - const dataSet = DataSet.create({ - attributes: [{ name: "foo", formula: { display: "1 + 2", canonical: "1 + 2" } }] as LEGACY_ATTRIBUTES_ARRAY_ANY + const dataSet = createDataSet({ + attributes: [{ name: "foo", formula: { display: "1 + 2", canonical: "1 + 2" } }] }) dataSet.addCases([{ __id__: "1" }]) const attribute = dataSet.attributes[0] @@ -52,11 +53,11 @@ describe("AttributeFormulaAdapter", () => { describe("getFormulaError", () => { it("should detect dependency cycles", () => { - const dataSet = DataSet.create({ + const dataSet = createDataSet({ attributes: [ { name: "foo", formula: { display: "bar + 1" } }, { name: "bar", formula: { display: "foo + 1" } } - ] as LEGACY_ATTRIBUTES_ARRAY_ANY + ] }) dataSet.attributes[0].formula!.setCanonicalExpression( `${localAttrIdToCanonical(dataSet.attrIDFromName("bar")!)} + 1` @@ -84,11 +85,11 @@ describe("AttributeFormulaAdapter", () => { describe("setupFormulaObservers", () => { it("should setup observer detecting hierarchy updates", () => { - const dataSet = DataSet.create({ + const dataSet = createDataSet({ attributes: [ { name: "foo", formula: { display: "1 + 2", canonical: "1 + 2" } }, { name: "bar" } - ] as LEGACY_ATTRIBUTES_ARRAY_ANY + ] }) dataSet.addCases([{ __id__: "1" }]) const attribute = dataSet.attributes[0] diff --git a/v3/src/models/formula/attribute-formula-adapter.ts b/v3/src/models/formula/attribute-formula-adapter.ts index 1cc05e3075..bbe180d839 100644 --- a/v3/src/models/formula/attribute-formula-adapter.ts +++ b/v3/src/models/formula/attribute-formula-adapter.ts @@ -62,11 +62,12 @@ export class AttributeFormulaAdapter implements IFormulaManagerAdapter { } const calculateChildCollectionGroups = () => { - for (let i = collectionIndex + 1; i < dataSet.collections.length; i++) { + // process children of requested collection that are themselves parent collections + for (let i = collectionIndex + 1; i < dataSet.collections.length - 1; i++) { const collectionGroup = dataSet.collectionGroups[i] collectionGroup.groups.forEach((group: CaseGroup) => processCase(group.pseudoCase)) } - // Note that the child cases are never in any collection and they require separate processing. + // process child cases dataSet.childCases().forEach(childCase => processCase(childCase)) } diff --git a/v3/src/models/formula/formula-manager.test.ts b/v3/src/models/formula/formula-manager.test.ts index 3556fb9727..64b7fe594c 100644 --- a/v3/src/models/formula/formula-manager.test.ts +++ b/v3/src/models/formula/formula-manager.test.ts @@ -1,5 +1,6 @@ import { observable, runInAction } from "mobx" -import { DataSet, IDataSet, LEGACY_ATTRIBUTES_ARRAY_ANY } from "../data/data-set" +import { IDataSet } from "../data/data-set" +import { createDataSet } from "../data/data-set-conversion" import { Formula, IFormula } from "./formula" import { FormulaManager } from "./formula-manager" import { CASE_INDEX_FAKE_ATTR_ID, localAttrIdToCanonical } from "./utils/name-mapping-utils" @@ -26,7 +27,7 @@ const getFakeAdapter = (formula: IFormula, dataSet: IDataSet) => { } const getManagerWithFakeAdapter = () => { - const dataSet = DataSet.create({ attributes: [{ name: "foo" }] as LEGACY_ATTRIBUTES_ARRAY_ANY }) + const dataSet = createDataSet({ attributes: [{ name: "foo" }] }) const formula = Formula.create({ display: formulaDisplay }) const adapter = getFakeAdapter(formula, dataSet) const manager = new FormulaManager() @@ -75,15 +76,15 @@ describe("FormulaManager", () => { }) describe("when formula has a circular dependency", () => { - it("registers formulas in a way that lets cirlcular detection algorithm to work correctly", () => { + it("registers formulas in a way that lets circular detection algorithm to work correctly", () => { // This test is a bit specific ot the attribute formula adapter, but it's pretty important as it ensures // that FormulaManager.registerAllFormulas is implemented in a way that delays error detection until all // the formulas are registered (necessary for circular dependency detection to work correctly). - const dataSet = DataSet.create({ + const dataSet = createDataSet({ attributes: [ { name: "foo", formula: { display: "bar + 1" } }, { name: "bar", formula: { display: "foo + 1" } } - ] as LEGACY_ATTRIBUTES_ARRAY_ANY + ] }) dataSet.addCases([{ __id__: "1" }]) const manager = new FormulaManager() diff --git a/v3/src/models/formula/formula-observers.test.ts b/v3/src/models/formula/formula-observers.test.ts index f9b60d2890..c3a2cb25de 100644 --- a/v3/src/models/formula/formula-observers.test.ts +++ b/v3/src/models/formula/formula-observers.test.ts @@ -247,7 +247,6 @@ describe("observeDatasetHierarchyChanges", () => { dataSet.collections.forEach(collection => { attrs.push(collection.attributes.map(attr => attr!.id)) }) - attrs.push(dataSet.ungroupedAttributes.map(attr => attr.id)) return attrs } it("should call recalculateCallback with ALL_CASES when attribute is moved between collections", () => { diff --git a/v3/src/models/formula/plotted-function-formula-adapter.test.ts b/v3/src/models/formula/plotted-function-formula-adapter.test.ts index 07d9e23343..119270becb 100644 --- a/v3/src/models/formula/plotted-function-formula-adapter.test.ts +++ b/v3/src/models/formula/plotted-function-formula-adapter.test.ts @@ -1,4 +1,5 @@ -import { DataSet, IDataSet, LEGACY_ATTRIBUTES_ARRAY_ANY } from "../data/data-set" +import { IDataSet } from "../data/data-set" +import { createDataSet } from "../data/data-set-conversion" import { PlottedFunctionFormulaAdapter } from "./plotted-function-formula-adapter" import { localAttrIdToCanonical } from "./utils/name-mapping-utils" import { GraphDataConfigurationModel } from "../../components/graph/models/graph-data-configuration-model" @@ -7,7 +8,7 @@ import { } from "../../components/graph/adornments/plotted-function/plotted-function-adornment-model" const getTestEnv = () => { - const dataSet = DataSet.create({ attributes: [{ name: "foo" }] as LEGACY_ATTRIBUTES_ARRAY_ANY }) + const dataSet = createDataSet({ attributes: [{ name: "foo" }] }) dataSet.addCases([{ __id__: "1" }]) const attribute = dataSet.attributes[0] const adornment = PlottedFunctionAdornmentModel.create({ formula: { display: "1 + 2 + x", canonical: "1 + 2 + x" }}) diff --git a/v3/src/models/formula/plotted-value-formula-adapter.test.ts b/v3/src/models/formula/plotted-value-formula-adapter.test.ts index 8cde6c43a9..43822d17c5 100644 --- a/v3/src/models/formula/plotted-value-formula-adapter.test.ts +++ b/v3/src/models/formula/plotted-value-formula-adapter.test.ts @@ -1,13 +1,14 @@ import { PlottedValueAdornmentModel } from "../../components/graph/adornments/univariate-measures/plotted-value/plotted-value-adornment-model" -import { DataSet, IDataSet, LEGACY_ATTRIBUTES_ARRAY_ANY } from "../data/data-set" +import { GraphDataConfigurationModel } from "../../components/graph/models/graph-data-configuration-model" +import { IDataSet } from "../data/data-set" +import { createDataSet } from "../data/data-set-conversion" import { PlottedValueFormulaAdapter } from "./plotted-value-formula-adapter" import { localAttrIdToCanonical } from "./utils/name-mapping-utils" -import { GraphDataConfigurationModel } from "../../components/graph/models/graph-data-configuration-model" const getTestEnv = () => { - const dataSet = DataSet.create({ attributes: [{ name: "foo" }] as LEGACY_ATTRIBUTES_ARRAY_ANY }) + const dataSet = createDataSet({ attributes: [{ name: "foo" }] }) dataSet.addCases([{ __id__: "1" }]) const attribute = dataSet.attributes[0] const adornment = PlottedValueAdornmentModel.create({ formula: { display: "1 + 2", canonical: "1 + 2" }}) diff --git a/v3/src/models/formula/utils/name-mapping-utils.test.ts b/v3/src/models/formula/utils/name-mapping-utils.test.ts index 385902b881..2b00fe7b22 100644 --- a/v3/src/models/formula/utils/name-mapping-utils.test.ts +++ b/v3/src/models/formula/utils/name-mapping-utils.test.ts @@ -2,8 +2,8 @@ import { CANONICAL_NAME, CASE_INDEX_FAKE_ATTR_ID, getCanonicalNameMap, getDisplayNameMap, globalValueIdToCanonical, idToCanonical, isCanonicalName, localAttrIdToCanonical, rmCanonicalPrefix, safeSymbolName } from "./name-mapping-utils" +import { createDataSet } from "../../data/data-set-conversion" import { getFormulaTestEnv } from "../test-utils/formula-test-utils" -import { DataSet, LEGACY_ATTRIBUTES_ARRAY_ANY } from "../../data/data-set" import { GlobalValueManager } from "../../global/global-value-manager" import { basicCanonicalNameToDependency } from "./formula-dependency-utils" @@ -131,12 +131,12 @@ describe("getDisplayNameMap", () => { describe("when there are local attributes or globals with the name 'caseIndex'", () => { it("resolves 'caseIndex' to a special value (special value takes precedence over anything else)", () => { - const dataSet = DataSet.create({ + const dataSet = createDataSet({ id: "dataSet", name: "dataSet", attributes: [ { id: "DATA_SET_ATTR_ID", name: "caseIndex" }, - ] as LEGACY_ATTRIBUTES_ARRAY_ANY + ] }) const nameMap = getDisplayNameMap({ localDataSet: dataSet, @@ -155,12 +155,12 @@ describe("getDisplayNameMap", () => { describe("when a local attribute and a global value have the same name", () => { it("resolves this name to the local attribute (local attributes take precedence over global values)", () => { - const dataSet = DataSet.create({ + const dataSet = createDataSet({ id: "dataSet", name: "dataSet", attributes: [ { id: "DATA_SET_ATTR_ID", name: "fooBar" }, - ] as LEGACY_ATTRIBUTES_ARRAY_ANY + ] }) const nameMap = getDisplayNameMap({ localDataSet: dataSet, diff --git a/v3/src/models/v2/dg-collection-client.ts b/v3/src/models/v2/dg-collection-client.ts index 33ad0c4d59..38574857c0 100644 --- a/v3/src/models/v2/dg-collection-client.ts +++ b/v3/src/models/v2/dg-collection-client.ts @@ -1,4 +1,4 @@ -import { ICollectionPropsModel } from "../data/collection" +import { ICollectionModel } from "../data/collection" import { IDataSet } from "../data/data-set" import { DGCase } from "./dg-case" import { DGCollection } from "./dg-collection" @@ -26,7 +26,7 @@ export class DGCollectionClient extends SCObject { collection: DGCollection casesController: DGCasesController - constructor(readonly data: IDataSet, collection: ICollectionPropsModel, readonly api: DGDataContextAPI) { + constructor(readonly data: IDataSet, collection: ICollectionModel, readonly api: DGDataContextAPI) { super() this.collection = new DGCollection(data, collection, api) this.casesController = new DGCasesController(data, api) diff --git a/v3/src/models/v2/dg-collection.ts b/v3/src/models/v2/dg-collection.ts index cd517f968a..e41db33e4b 100644 --- a/v3/src/models/v2/dg-collection.ts +++ b/v3/src/models/v2/dg-collection.ts @@ -1,4 +1,4 @@ -import { ICollectionPropsModel, isCollectionModel } from "../data/collection" +import { ICollectionModel, isCollectionModel } from "../data/collection" import { IDataSet } from "../data/data-set" import { mstAutorun } from "../../utilities/mst-autorun" // eslint-disable-next-line import/no-cycle @@ -11,14 +11,12 @@ export class DGCollection extends SCObject { attrs: DGAttribute[] = [] disposer: () => void - constructor(readonly data: IDataSet, readonly collection: ICollectionPropsModel, readonly api: DGDataContextAPI) { + constructor(readonly data: IDataSet, readonly collection: ICollectionModel, readonly api: DGDataContextAPI) { super() this.disposer = mstAutorun(() => { const prevAttrs = this.attrs - const dsAttributes = isCollectionModel(this.collection) - ? this.collection.attributes - : this.data.ungroupedAttributes + const dsAttributes = this.collection.attributes const newAttrs: DGAttribute[] = [] dsAttributes.forEach(dsAttr => { if (dsAttr) { diff --git a/v3/src/models/v2/dg-data-context.ts b/v3/src/models/v2/dg-data-context.ts index d104bb4e13..035502133a 100644 --- a/v3/src/models/v2/dg-data-context.ts +++ b/v3/src/models/v2/dg-data-context.ts @@ -24,7 +24,7 @@ export class DGDataContext extends SCObject implements DGDataContextAPI { this.disposer = mstAutorun(() => { const prevCollections = this.collections - this.collections = this.data.collectionModels.map(dsCollection => { + this.collections = this.data.collections.map(dsCollection => { const foundCollection = prevCollections.find(dgCollection => dgCollection.id === dsCollection.id) return foundCollection ?? new DGCollectionClient(data, dsCollection, this) }) @@ -61,7 +61,7 @@ export class DGDataContext extends SCObject implements DGDataContextAPI { const pseudoCase = this.data.pseudoCaseMap.get(caseId) const dsCollection = pseudoCase ? this.data.getCollection(pseudoCase.collectionId) - : this.data.ungrouped + : this.data.childCollection const collectionClient = this.getCollectionByID(dsCollection?.id ?? "") return collectionClient?.get("collection") } diff --git a/v3/src/v2/codap-v2-document.ts b/v3/src/v2/codap-v2-document.ts index e69eb23f24..77e7be47f7 100644 --- a/v3/src/v2/codap-v2-document.ts +++ b/v3/src/v2/codap-v2-document.ts @@ -1,5 +1,6 @@ +import { SetRequired } from "type-fest" import { IAttribute } from "../models/data/attribute" -import { CollectionModel, CollectionPropsModel } from "../models/data/collection" +import { ICollectionModel, ICollectionModelSnapshot } from "../models/data/collection" import { IDataSet, toCanonical } from "../models/data/data-set" import { v2NameTitleToV3Title } from "../models/data/v2-model" import { ISharedCaseMetadata, SharedCaseMetadata } from "../models/shared/shared-case-metadata" @@ -88,6 +89,7 @@ export class CodapV2Document { } registerCollections(data: IDataSet, metadata: ISharedCaseMetadata, collections: ICodapV2Collection[]) { + let prevCollection: ICollectionModel | undefined collections.forEach((collection, index) => { const { attrs = [], cases = [], guid, name = "", title, type = "DG.Collection" } = collection const _title = v2NameTitleToV3Title(name, title) @@ -98,17 +100,23 @@ export class CodapV2Document { this.registerAttributes(data, metadata, attrs, level) this.registerCases(data, cases, level) - if (level > 0) { - const collectionModel = CollectionModel.create({ id: toV3CollectionId(guid), name, _title }) - attrs.forEach(attr => { - const attrModel = data.attrFromName(attr.name) - attrModel && collectionModel.addAttribute(attrModel) - }) - data.addCollection(collectionModel) + const attributes = attrs.map(attr => { + const attrModel = data.attrFromName(attr.name) + return attrModel?.id + }).filter(attrId => !!attrId) as string[] + + const collectionSnap: SetRequired = { + id: toV3CollectionId(guid), + name, + _title, + attributes } - else { - data.setUngroupedCollection(CollectionPropsModel.create({ id: toV3CollectionId(guid), name, _title })) + // remove default collection + if (index === 0) { + data.removeCollection(data.collections[0]) } + // add the imported collections + prevCollection = data.addCollection(collectionSnap, { after: prevCollection?.id }) }) } diff --git a/v3/src/v2/codap-v2-import.test.ts b/v3/src/v2/codap-v2-import.test.ts index 30a8770f43..5c6b177d36 100644 --- a/v3/src/v2/codap-v2-import.test.ts +++ b/v3/src/v2/codap-v2-import.test.ts @@ -73,7 +73,7 @@ describe(`V2 "mammals.codap"`, () => { const collection = context.collections[0] const data = mammals.dataSets[0].dataSet expect(data.id).toBe(`DATA${context.guid}`) - expect(data.ungrouped.id).toBe(`COLL${collection.guid}`) + expect(data.collections[0].id).toBe(`COLL${collection.guid}`) expect(data.attributes.length).toBe(9) expect(data.attributes[0].id).toBe(`ATTR${collection.attrs[0].guid}`) expect(data.cases.length).toBe(27) @@ -118,7 +118,7 @@ describe(`V2 "24cats.codap"`, () => { const context = cats.contexts[0] const [v2ParentCollection, v2ChildCollection] = context.collections const data = cats.dataSets[0].dataSet - expect(data.collections.length).toBe(1) // plus ungrouped makes two + expect(data.collections.length).toBe(2) expect(data.collections[0].attributes.length).toBe(1) expect(data.attributes.length).toBe(9) expect(data.cases.length).toBe(24)