From a0015278b3eafdc86958e075d65cf8e69e3b73e6 Mon Sep 17 00:00:00 2001 From: Teale Fristoe Date: Wed, 11 Dec 2024 08:37:30 -0800 Subject: [PATCH] 188646708 v3 Update Graph API (#1675) * Include attribute ids in get requests. * Allow attributes to be specified by id when creating graph. * Support update graph requests. --- .../graph/component-handler-graph.test.ts | 150 +++++++++- v3/src/components/graph/components/graph.tsx | 18 +- .../graph/graph-component-handler.ts | 279 ++++++++++++++---- .../graph/models/graph-content-model.ts | 13 +- .../models/graph-data-configuration-model.ts | 21 +- .../data-interactive-component-types.ts | 24 +- .../handlers/attribute-handler.test.ts | 8 +- .../attribute-location-handler.test.ts | 8 +- .../handlers/collection-handler.test.ts | 6 +- .../handlers/handler-test-utils.ts | 15 +- .../handlers/item-by-case-id-handler.test.ts | 2 +- .../handlers/item-by-id-handler.test.ts | 2 +- .../handlers/item-handler.test.ts | 2 +- 13 files changed, 421 insertions(+), 127 deletions(-) diff --git a/v3/src/components/graph/component-handler-graph.test.ts b/v3/src/components/graph/component-handler-graph.test.ts index ab3736e5f5..6bd306b260 100644 --- a/v3/src/components/graph/component-handler-graph.test.ts +++ b/v3/src/components/graph/component-handler-graph.test.ts @@ -1,12 +1,14 @@ +// TODO Rename this file after the PR is approved + import { getSnapshot } from "mobx-state-tree" import { IBaseNumericAxisModel } from "../axis/models/axis-model" -import { V2GetGraph } from "../../data-interactive/data-interactive-component-types" +import { V2GetGraph, V2Graph } from "../../data-interactive/data-interactive-component-types" import { DIComponentInfo } from "../../data-interactive/data-interactive-types" import { diComponentHandler } from "../../data-interactive/handlers/component-handler" import { setupTestDataset, testCases } from "../../data-interactive/handlers/handler-test-utils" import { testGetComponent } from "../../data-interactive/handlers/component-handler-test-utils" import { appState } from "../../models/app-state" -import { toV3Id } from "../../utilities/codap-utils" +import { toV2Id, toV3Id } from "../../utilities/codap-utils" import { kGraphIdPrefix } from "./graph-defs" import "./graph-registration" import { IGraphContentModel, isGraphContentModel } from "./models/graph-content-model" @@ -21,6 +23,7 @@ describe("DataInteractive ComponentHandler Graph", () => { const a1 = dataset.getAttributeByName("a1")! const a2 = dataset.getAttributeByName("a2")! const a3 = dataset.getAttributeByName("a3")! + const a4 = dataset.getAttributeByName("a4")! it("create and get graph work", async () => { // Create a graph tile with no options @@ -46,9 +49,44 @@ describe("DataInteractive ComponentHandler Graph", () => { type: "graph", dataContext: "data", rightSplitAttributeName: "a3", xAttributeName: "a3" }).success).toBe(false) + // Create a graph with ids + const resultIds = handler.create!({}, { + type: "graph", cannotClose: true, dataContext: "data", xAttributeID: toV2Id(a3.id), yAttributeID: toV2Id(a2.id), + legendAttributeID: toV2Id(a1.id), captionAttributeID: toV2Id(a2.id), rightNumericAttributeID: toV2Id(a3.id), + rightSplitAttributeID: toV2Id(a1.id), topSplitAttributeID: toV2Id(a2.id), topSplitAttributeName: "a3", + enableNumberToggle: true, numberToggleLastMode: true + }) + expect(resultIds.success).toBe(true) + expect(documentContent.tileMap.size).toBe(1) + const resultIdsValues = resultIds.values as DIComponentInfo + const tileIds = documentContent.tileMap.get(toV3Id(kGraphIdPrefix, resultIdsValues.id!))! + expect(tileIds).toBeDefined() + expect(isGraphContentModel(tileIds.content)).toBe(true) + const tileContentIds = tileIds.content as IGraphContentModel + expect(tileIds.cannotClose).toBe(true) + expect(tileContentIds.dataConfiguration.attributeDescriptionForRole("x")?.attributeID).toBe(a3.id) + expect(tileContentIds.dataConfiguration.attributeDescriptionForRole("y")?.attributeID).toBe(a2.id) + expect(tileContentIds.dataConfiguration.attributeDescriptionForRole("legend")?.attributeID).toBe(a1.id) + expect(tileContentIds.dataConfiguration.attributeDescriptionForRole("caption")?.attributeID).toBe(a2.id) + expect(tileContentIds.dataConfiguration.attributeDescriptionForRole("rightNumeric")?.attributeID).toBe(a3.id) + expect(tileContentIds.dataConfiguration.attributeDescriptionForRole("rightSplit")?.attributeID).toBe(a1.id) + // Id should trump name for topSplit + expect(tileContentIds.dataConfiguration.attributeDescriptionForRole("topSplit")?.attributeID).toBe(a2.id) + expect(tileContentIds.showParentToggles).toBe(true) + // Make sure numberToggleLastMode hid all appropriate cases + tileContentIds.layers.forEach(layer => { + const lastCaseId = dataset.itemIds[dataset.itemIds.length - 1] + dataset.itemIds.forEach(itemId => { + expect(layer.dataConfiguration.hiddenCases.includes(itemId)).toBe(itemId !== lastCaseId) + }) + }) + // Delete the graph when we're finished + handler.delete!({ component: tileIds }) + expect(documentContent.tileMap.size).toBe(0) + // Create a graph with options const result = handler.create!({}, { - type: "graph", cannotClose: true, dataContext: "data", xAttributeName: "a3", yAttributeName: "a2", + type: "graph", cannotClose: true, dataContext: "data", xAttributeName: "a3", yAttributeName: "a4", legendAttributeName: "a1", captionAttributeName: "a2", rightNumericAttributeName: "a3", rightSplitAttributeName: "a1", topSplitAttributeName: "a2", enableNumberToggle: true, numberToggleLastMode: true }) @@ -59,14 +97,15 @@ describe("DataInteractive ComponentHandler Graph", () => { expect(tile).toBeDefined() expect(isGraphContentModel(tile.content)).toBe(true) const tileContent = tile.content as IGraphContentModel + const dataConfig = tileContent.dataConfiguration expect(tile.cannotClose).toBe(true) - expect(tileContent.dataConfiguration.attributeDescriptionForRole("x")?.attributeID).toBe(a3.id) - expect(tileContent.dataConfiguration.attributeDescriptionForRole("y")?.attributeID).toBe(a2.id) - expect(tileContent.dataConfiguration.attributeDescriptionForRole("legend")?.attributeID).toBe(a1.id) - expect(tileContent.dataConfiguration.attributeDescriptionForRole("caption")?.attributeID).toBe(a2.id) - expect(tileContent.dataConfiguration.attributeDescriptionForRole("rightNumeric")?.attributeID).toBe(a3.id) - expect(tileContent.dataConfiguration.attributeDescriptionForRole("rightSplit")?.attributeID).toBe(a1.id) - expect(tileContent.dataConfiguration.attributeDescriptionForRole("topSplit")?.attributeID).toBe(a2.id) + expect(dataConfig.attributeDescriptionForRole("x")?.attributeID).toBe(a3.id) + expect(dataConfig.attributeDescriptionForRole("y")?.attributeID).toBe(a4.id) + expect(dataConfig.attributeDescriptionForRole("legend")?.attributeID).toBe(a1.id) + expect(dataConfig.attributeDescriptionForRole("caption")?.attributeID).toBe(a2.id) + expect(dataConfig.attributeDescriptionForRole("rightNumeric")?.attributeID).toBe(a3.id) + expect(dataConfig.attributeDescriptionForRole("rightSplit")?.attributeID).toBe(a1.id) + expect(dataConfig.attributeDescriptionForRole("topSplit")?.attributeID).toBe(a2.id) expect(tileContent.showParentToggles).toBe(true) // Make sure numberToggleLastMode hid all appropriate cases tileContent.layers.forEach(layer => { @@ -76,12 +115,87 @@ describe("DataInteractive ComponentHandler Graph", () => { }) }) + // Update a graph's axis bounds + const xAxis = tileContent.getAxis("bottom") as IBaseNumericAxisModel + const yAxis = tileContent.getAxis("left") as IBaseNumericAxisModel + const y2Axis = tileContent.getAxis("rightNumeric") as IBaseNumericAxisModel + expect(xAxis.min).toBe(-.5) + expect(xAxis.max).toBe(7.5) + expect(yAxis.min).toBe(-6.5) + expect(yAxis.max).toBe(1.5) + expect(y2Axis.min).toBe(-.5) + expect(y2Axis.max).toBe(7.5) + const updateBoundsResult = handler.update!({ component: tile }, { + xLowerBound: 2, xUpperBound: 6, yLowerBound: -20, yUpperBound: 10, y2LowerBound: -3, y2UpperBound: 13 + }) + expect(updateBoundsResult.success).toBe(true) + expect(xAxis.min).toBe(2) + expect(xAxis.max).toBe(6) + expect(yAxis.min).toBe(-20) + expect(yAxis.max).toBe(10) + expect(y2Axis.min).toBe(-3) + expect(y2Axis.max).toBe(13) + + // Update a graph to switch attributes + const updateResult1 = handler.update!({ component: tile }, { + xAttributeName: "a2", yAttributeName: "a1", legendAttributeName: "a2", captionAttributeName: "a1", + rightNumericAttributeName: "a4", rightSplitAttributeName: "a1", topSplitAttributeName: "a1", + enableNumberToggle: false, numberToggleLastMode: false + }) + expect(updateResult1.success).toBe(true) + expect(dataConfig.attributeDescriptionForRole("x")?.attributeID).toBe(a2.id) + expect(dataConfig.attributeDescriptionForRole("y")?.attributeID).toBe(a1.id) + expect(dataConfig.attributeDescriptionForRole("legend")?.attributeID).toBe(a2.id) + expect(dataConfig.attributeDescriptionForRole("caption")?.attributeID).toBe(a1.id) + expect(dataConfig.attributeDescriptionForRole("rightNumeric")?.attributeID).toBe(a4.id) + expect(dataConfig.attributeDescriptionForRole("rightSplit")?.attributeID).toBe(a1.id) + expect(dataConfig.attributeDescriptionForRole("topSplit")?.attributeID).toBe(a1.id) + expect(tileContent.showParentToggles).toBe(false) + + // Update a graph to remove attributes + const updateResult2 = handler.update!({ component: tile }, { + xAttributeName: null, yAttributeName: null, legendAttributeName: null, captionAttributeID: null, + rightNumericAttributeName: null, rightSplitAttributeName: null, topSplitAttributeName: null + } as V2Graph) + expect(updateResult2.success).toBe(true) + expect(dataConfig.attributeDescriptionForRole("x")?.attributeID).toBeUndefined() + expect(dataConfig.attributeDescriptionForRole("y")?.attributeID).toBeUndefined() + expect(dataConfig.attributeDescriptionForRole("legend")?.attributeID).toBeUndefined() + expect(dataConfig.attributeDescriptionForRole("caption")?.attributeID).toBeUndefined() + expect(dataConfig.attributeDescriptionForRole("rightNumeric")?.attributeID).toBeUndefined() + expect(dataConfig.attributeDescriptionForRole("rightSplit")?.attributeID).toBeUndefined() + expect(dataConfig.attributeDescriptionForRole("topSplit")?.attributeID).toBeUndefined() + + // Update a graph to add attributes + const updateResult3 = handler.update!({ component: tile }, { + xAttributeID: toV2Id(a3.id), xAttributeName: "a2", yAttributeID: toV2Id(a2.id), legendAttributeID: toV2Id(a3.id), + captionAttributeID: toV2Id(a2.id), rightSplitAttributeID: toV2Id(a1.id), topSplitAttributeID: toV2Id(a2.id), + enableNumberToggle: true, numberToggleLastMode: true + }) + expect(updateResult3.success).toBe(true) + // Id should trump name + expect(dataConfig.attributeDescriptionForRole("x")?.attributeID).toBe(a3.id) + expect(dataConfig.attributeDescriptionForRole("y")?.attributeID).toBe(a2.id) + expect(dataConfig.attributeDescriptionForRole("legend")?.attributeID).toBe(a3.id) + expect(dataConfig.attributeDescriptionForRole("caption")?.attributeID).toBe(a2.id) + expect(dataConfig.attributeDescriptionForRole("rightSplit")?.attributeID).toBe(a1.id) + expect(dataConfig.attributeDescriptionForRole("topSplit")?.attributeID).toBe(a2.id) + expect(tileContent.showParentToggles).toBe(true) + + // We have to set a numeric x attribute before we can set the rightNumeric attribute + handler.update!({ component: tile }, { + rightNumericAttributeID: toV2Id(a3.id) + }) + expect(dataConfig.attributeDescriptionForRole("rightNumeric")?.attributeID).toBe(a3.id) + // Get graph testGetComponent(tile, handler, (graphTile, values) => { const { - dataContext, enableNumberToggle, numberToggleLastMode, captionAttributeName, legendAttributeName, - rightSplitAttributeName, topSplitAttributeName, xAttributeName, xLowerBound, xUpperBound, - yAttributeName, yLowerBound, yUpperBound, y2AttributeName, y2LowerBound, y2UpperBound + dataContext, enableNumberToggle, numberToggleLastMode, captionAttributeID, captionAttributeName, + legendAttributeID, legendAttributeName, rightSplitAttributeID, rightSplitAttributeName, + topSplitAttributeID, topSplitAttributeName, xAttributeID, xAttributeName, xLowerBound, xUpperBound, + yAttributeID, yAttributeName, yLowerBound, yUpperBound, + y2AttributeID, y2AttributeName, y2LowerBound, y2UpperBound } = values as V2GetGraph const content = graphTile.content as IGraphContentModel const graphDataset = content.dataset! @@ -91,32 +205,36 @@ describe("DataInteractive ComponentHandler Graph", () => { expect(numberToggleLastMode).toBe(content.showOnlyLastCase) const captionAttributeId = dataConfiguration.attributeDescriptionForRole("caption")!.attributeID + expect(captionAttributeID).toBe(captionAttributeId) expect(captionAttributeName).toBe(graphDataset.getAttribute(captionAttributeId)?.name) const legendAttributeId = dataConfiguration.attributeDescriptionForRole("legend")!.attributeID + expect(legendAttributeID).toBe(legendAttributeId) expect(legendAttributeName).toBe(graphDataset.getAttribute(legendAttributeId)?.name) const rightSplitId = dataConfiguration.attributeDescriptionForRole("rightSplit")!.attributeID + expect(rightSplitAttributeID).toBe(rightSplitId) expect(rightSplitAttributeName).toBe(graphDataset.getAttribute(rightSplitId)?.name) const topSplitId = dataConfiguration.attributeDescriptionForRole("topSplit")!.attributeID + expect(topSplitAttributeID).toBe(topSplitId) expect(topSplitAttributeName).toBe(graphDataset.getAttribute(topSplitId)?.name) const xAttributeId = dataConfiguration.attributeDescriptionForRole("x")!.attributeID + expect(xAttributeID).toBe(xAttributeId) expect(xAttributeName).toBe(graphDataset.getAttribute(xAttributeId)?.name) - const xAxis = content.getAxis("bottom") as IBaseNumericAxisModel expect(xLowerBound).toBe(xAxis.min) expect(xUpperBound).toBe(xAxis.max) const yAttributeId = dataConfiguration.attributeDescriptionForRole("y")!.attributeID + expect(yAttributeID).toBe(yAttributeId) expect(yAttributeName).toBe(graphDataset.getAttribute(yAttributeId)?.name) - const yAxis = content.getAxis("left") as IBaseNumericAxisModel expect(yLowerBound).toBe(yAxis.min) expect(yUpperBound).toBe(yAxis.max) const y2AttributeId = dataConfiguration.attributeDescriptionForRole("rightNumeric")!.attributeID + expect(y2AttributeID).toBe(y2AttributeId) expect(y2AttributeName).toBe(graphDataset.getAttribute(y2AttributeId)?.name) - const y2Axis = content.getAxis("rightNumeric") as IBaseNumericAxisModel expect(y2LowerBound).toBe(y2Axis.min) expect(y2UpperBound).toBe(y2Axis.max) }) diff --git a/v3/src/components/graph/components/graph.tsx b/v3/src/components/graph/components/graph.tsx index b82d17172d..0e84ecd63d 100644 --- a/v3/src/components/graph/components/graph.tsx +++ b/v3/src/components/graph/components/graph.tsx @@ -1,6 +1,6 @@ import {comparer} from "mobx" import {observer} from "mobx-react-lite" -import {isAlive} from "mobx-state-tree" +import {IDisposer, isAlive} from "mobx-state-tree" import React, {MutableRefObject, useCallback, useEffect, useMemo, useRef} from "react" import {select} from "d3" import {clsx} from "clsx" @@ -233,12 +233,16 @@ export const Graph = observer(function Graph({graphController, graphRef, pixiPoi // respond to assignment of new attribute ID useEffect(function handleNewAttributeID() { - const disposer = graphModel && onAnyAction(graphModel, action => { - if (isSetAttributeIDAction(action)) { - startAnimation() - graphController?.handleAttributeAssignment() - } - }) + let disposer: Maybe + if (graphModel) { + disposer = onAnyAction(graphModel, action => { + const dataConfigActions = ["setAttribute", "replaceYAttribute", "removeYAttributeAtIndex"] + if (dataConfigActions.includes(action.name) || isSetAttributeIDAction(action)) { + startAnimation() + graphController?.handleAttributeAssignment() + } + }) + } return () => disposer?.() }, [graphController, layout, graphModel, startAnimation]) diff --git a/v3/src/components/graph/graph-component-handler.ts b/v3/src/components/graph/graph-component-handler.ts index 60435b53f9..8e2ffc90b1 100644 --- a/v3/src/components/graph/graph-component-handler.ts +++ b/v3/src/components/graph/graph-component-handler.ts @@ -1,13 +1,17 @@ import { getSnapshot } from "mobx-state-tree" -import { V2Graph } from "../../data-interactive/data-interactive-component-types" +import { V2GetGraph, V2Graph } from "../../data-interactive/data-interactive-component-types" +import { DIValues } from "../../data-interactive/data-interactive-types" import { DIComponentHandler } from "../../data-interactive/handlers/component-handler" import { errorResult } from "../../data-interactive/handlers/di-results" import { appState } from "../../models/app-state" +import { IAttribute } from "../../models/data/attribute" import { IDataSet } from "../../models/data/data-set" import { ISharedCaseMetadata } from "../../models/shared/shared-case-metadata" import { getSharedCaseMetadataFromDataset, getSharedDataSets } from "../../models/shared/shared-data-utils" import { ITileContentModel, ITileContentSnapshotWithType } from "../../models/tiles/tile-content" +import { toV3AttrId, toV3DataSetId } from "../../utilities/codap-utils" import { t } from "../../utilities/translation/translate" +import { AxisPlace } from "../axis/axis-types" import { isNumericAxisModel } from "../axis/models/axis-model" import { attrRoleToGraphPlace, GraphAttrRole } from "../data-display/data-display-types" import { IAttributeDescriptionSnapshot } from "../data-display/models/data-configuration-model" @@ -18,27 +22,59 @@ import { GraphLayout } from "./models/graph-layout" import { syncModelWithAttributeConfiguration } from "./models/graph-model-utils" import { IGraphPointLayerModelSnapshot, kGraphPointLayerType } from "./models/graph-point-layer-model" +interface AttributeInfo { + id?: string | null + name?: string | null +} +function packageAttribute(id?: string | null, name?: string | null): Maybe { + if (id || id === null || name || name === null) { + return { id, name } + } +} +function getAttributeInfo(values?: DIValues): Record> { + const { + captionAttributeID, captionAttributeName, legendAttributeID, legendAttributeName, rightNumericAttributeID, + rightNumericAttributeName, rightSplitAttributeID, rightSplitAttributeName, topSplitAttributeID, + topSplitAttributeName, xAttributeID, xAttributeName, y2AttributeID, y2AttributeName + } = values as V2Graph + return { + caption: packageAttribute(captionAttributeID, captionAttributeName), + legend: packageAttribute(legendAttributeID, legendAttributeName), + rightNumeric: packageAttribute(rightNumericAttributeID, rightNumericAttributeName), + rightSplit: packageAttribute(rightSplitAttributeID, rightSplitAttributeName), + topSplit: packageAttribute(topSplitAttributeID, topSplitAttributeName), + x: packageAttribute(xAttributeID, xAttributeName), + y2: packageAttribute(y2AttributeID, y2AttributeName) + } +} +function getAttributeFromInfo(dataset: IDataSet, info?: AttributeInfo) { + let attribute: Maybe + if (info?.id != null) { + attribute = dataset.getAttribute(toV3AttrId(info.id)) + } + if (!attribute && info?.name != null) { + attribute = dataset.getAttributeByName(info.name) + } + return attribute +} +const roleFromAttrKey: Record = { + x: "x", + y: "y", + y2: "rightNumeric", + rightNumeric: "rightNumeric", + caption: "caption", + legend: "legend", + topSplit: "topSplit", + rightSplit: "rightSplit" +} + export const graphComponentHandler: DIComponentHandler = { create({ values }) { const { - captionAttributeName, dataContext: _dataContext, enableNumberToggle: showParentToggles, legendAttributeName, - numberToggleLastMode: showOnlyLastCase, rightNumericAttributeName, rightSplitAttributeName, - topSplitAttributeName, xAttributeName, yAttributeName, y2AttributeName + dataContext: _dataContext, enableNumberToggle: showParentToggles, numberToggleLastMode: showOnlyLastCase, + yAttributeID, yAttributeName, } = values as V2Graph - const attributeNames: Record = { - captionAttributeName, legendAttributeName, rightNumericAttributeName, rightSplitAttributeName, - topSplitAttributeName, xAttributeName, y2AttributeName - } - const roleFromAttrKey: Record = { - xAttributeName: "x", - yAttributeName: "y", - y2AttributeName: "rightNumeric", - rightNumericAttributeName: "rightNumeric", - captionAttributeName: "caption", - legendAttributeName: "legend", - topSplitAttributeName: "topSplit", - rightSplitAttributeName: "rightSplit" - } + const attributeInfo = getAttributeInfo(values) let layerIndex = 0 const layers: Array = [] @@ -54,24 +90,25 @@ export const graphComponentHandler: DIComponentHandler = { if (dataset.name === _dataContext) { provisionalDataSet = dataset provisionalMetadata = metadata - for (const attributeType in attributeNames) { - const attributeName = attributeNames[attributeType] - if (attributeName) { - const attribute = dataset.getAttributeByName(attributeName) - if (attribute) { - const attributeRole = roleFromAttrKey[attributeType] - if (attributeRole) { - _attributeDescriptions[attributeRole] = { attributeID: attribute.id, type: attribute.type } - } + for (const attributeType in attributeInfo) { + const attribute = getAttributeFromInfo(dataset, attributeInfo[attributeType]) + if (attribute) { + const attributeRole = roleFromAttrKey[attributeType] + if (attributeRole) { + _attributeDescriptions[attributeRole] = { attributeID: attribute.id, type: attribute.type } } } } - if (yAttributeName) { - const yAttribute = dataset.getAttributeByName(yAttributeName) - if (yAttribute) { - _yAttributeDescriptions.push({ attributeID: yAttribute.id, type: yAttribute.type }) - } + let yAttribute: Maybe + if (yAttributeID != null) { + yAttribute = dataset.getAttribute(toV3AttrId(yAttributeID)) + } + if (!yAttribute && yAttributeName != null) { + yAttribute = dataset.getAttributeByName(yAttributeName) + } + if (yAttribute) { + _yAttributeDescriptions.push({ attributeID: yAttribute.id, type: yAttribute.type }) } if (showOnlyLastCase) { @@ -117,20 +154,20 @@ export const graphComponentHandler: DIComponentHandler = { const { dataset } = dataConfiguration if (dataset && dataset.name === _dataContext) { // Make sure all attributes can legally fulfill their specified roles - for (const attributeType in attributeNames) { - const attributeName = attributeNames[attributeType] - if (attributeName) { - const attribute = dataset.getAttributeByName(attributeName) - if (attribute) { - const attributeRole = roleFromAttrKey[attributeType] - const attributePlace = attrRoleToGraphPlace[attributeRole] - if (attributePlace && !dataConfiguration.placeCanAcceptAttributeIDDrop( - attributePlace, dataset, attribute.id, { allowSameAttr: true } - )) { - return errorResult( - t("V3.DI.Error.illegalAttributeAssignment", { vars: [attributeName, attributeRole] }) - ) - } + for (const attributeType in attributeInfo) { + const attributePackage = attributeInfo[attributeType] + const attribute = getAttributeFromInfo(dataset, attributePackage) + if (attribute) { + const attributeRole = roleFromAttrKey[attributeType] + const attributePlace = attrRoleToGraphPlace[attributeRole] + if (attributePlace && !dataConfiguration.placeCanAcceptAttributeIDDrop( + attributePlace, dataset, attribute.id, { allowSameAttr: true } + )) { + return errorResult( + t("V3.DI.Error.illegalAttributeAssignment", { + vars: [attributePackage?.id ?? attributePackage?.name ?? "", attributeRole] + }) + ) } } } @@ -150,6 +187,7 @@ export const graphComponentHandler: DIComponentHandler = { } return { content: { ...getSnapshot(graphModel), layers: finalLayers } as ITileContentSnapshotWithType } }, + get(content: ITileContentModel) { if (isGraphContentModel(content)) { const dataset = content.dataset @@ -157,34 +195,36 @@ export const graphComponentHandler: DIComponentHandler = { const { dataConfiguration } = content.graphPointLayerModel const { showParentToggles: enableNumberToggle, showOnlyLastCase: numberToggleLastMode } = content - const captionAttributeId = dataConfiguration.attributeDescriptionForRole("caption")?.attributeID - const captionAttributeName = captionAttributeId ? dataset?.getAttribute(captionAttributeId)?.name : undefined + const captionAttributeID = dataConfiguration.attributeDescriptionForRole("caption")?.attributeID + const captionAttributeName = captionAttributeID ? dataset?.getAttribute(captionAttributeID)?.name : undefined - const legendAttributeId = dataConfiguration.attributeDescriptionForRole("legend")?.attributeID - const legendAttributeName = legendAttributeId ? dataset?.getAttribute(legendAttributeId)?.name : undefined + const legendAttributeID = dataConfiguration.attributeDescriptionForRole("legend")?.attributeID + const legendAttributeName = legendAttributeID ? dataset?.getAttribute(legendAttributeID)?.name : undefined - const rightSplitId = dataConfiguration.attributeDescriptionForRole("rightSplit")?.attributeID - const rightSplitAttributeName = rightSplitId ? dataset?.getAttribute(rightSplitId)?.name : undefined + const rightSplitAttributeID = dataConfiguration.attributeDescriptionForRole("rightSplit")?.attributeID + const rightSplitAttributeName = rightSplitAttributeID + ? dataset?.getAttribute(rightSplitAttributeID)?.name + : undefined - const topSplitId = dataConfiguration.attributeDescriptionForRole("topSplit")?.attributeID - const topSplitAttributeName = topSplitId ? dataset?.getAttribute(topSplitId)?.name : undefined + const topSplitAttributeID = dataConfiguration.attributeDescriptionForRole("topSplit")?.attributeID + const topSplitAttributeName = topSplitAttributeID ? dataset?.getAttribute(topSplitAttributeID)?.name : undefined - const xAttributeId = dataConfiguration.attributeDescriptionForRole("x")?.attributeID - const xAttributeName = xAttributeId ? dataset?.getAttribute(xAttributeId)?.name : undefined + const xAttributeID = dataConfiguration.attributeDescriptionForRole("x")?.attributeID + const xAttributeName = xAttributeID ? dataset?.getAttribute(xAttributeID)?.name : undefined const xAxis = content.getAxis("bottom") const xNumericAxis = isNumericAxisModel(xAxis) ? xAxis : undefined const xLowerBound = xNumericAxis?.min const xUpperBound = xNumericAxis?.max - const yAttributeId = dataConfiguration.attributeDescriptionForRole("y")?.attributeID - const yAttributeName = yAttributeId ? dataset?.getAttribute(yAttributeId)?.name : undefined + const yAttributeID = dataConfiguration.attributeDescriptionForRole("y")?.attributeID + const yAttributeName = yAttributeID ? dataset?.getAttribute(yAttributeID)?.name : undefined const yAxis = content.getAxis("left") const yNumericAxis = isNumericAxisModel(yAxis) ? yAxis : undefined const yLowerBound = yNumericAxis?.min const yUpperBound = yNumericAxis?.max - const y2AttributeId = dataConfiguration.attributeDescriptionForRole("rightNumeric")?.attributeID - const y2AttributeName = y2AttributeId ? dataset?.getAttribute(y2AttributeId)?.name : undefined + const y2AttributeID = dataConfiguration.attributeDescriptionForRole("rightNumeric")?.attributeID + const y2AttributeName = y2AttributeID ? dataset?.getAttribute(y2AttributeID)?.name : undefined const y2Axis = content.getAxis("rightNumeric") const y2NumericAxis = isNumericAxisModel(y2Axis) ? y2Axis : undefined const y2LowerBound = y2NumericAxis?.min @@ -192,12 +232,121 @@ export const graphComponentHandler: DIComponentHandler = { return { dataContext, enableNumberToggle, numberToggleLastMode, - captionAttributeName, legendAttributeName, - rightSplitAttributeName, topSplitAttributeName, - xAttributeName, xLowerBound, xUpperBound, - yAttributeName, yLowerBound, yUpperBound, - y2AttributeName, y2LowerBound, y2UpperBound + captionAttributeID, captionAttributeName, legendAttributeID, legendAttributeName, + rightSplitAttributeID, rightSplitAttributeName, topSplitAttributeID, topSplitAttributeName, + xAttributeID, xAttributeName, xLowerBound, xUpperBound, + yAttributeID, yAttributeName, yLowerBound, yUpperBound, + y2AttributeID, y2AttributeName, y2LowerBound, y2UpperBound } } + }, + + update(content: ITileContentModel, values: DIValues) { + if (!isGraphContentModel(content)) return { success: false } + + const { + dataContext: _dataContext, enableNumberToggle: showParentToggles, numberToggleLastMode: showOnlyLastCase, + xLowerBound, xUpperBound, yAttributeID, yAttributeName, yLowerBound, yUpperBound, y2LowerBound, y2UpperBound + } = values as V2GetGraph + const attributeInfo = getAttributeInfo(values) + + // Determine which dataset to work with + let dataSet: Maybe + if (_dataContext) { + getSharedDataSets(appState.document).forEach(sharedDataSet => { + if (sharedDataSet.dataSet.name === _dataContext || sharedDataSet.dataSet.id === toV3DataSetId(_dataContext)) { + dataSet = sharedDataSet.dataSet + } + }) + } + if (!dataSet) { + dataSet = content.dataset ?? getSharedDataSets(appState.document)[0].dataSet + } + + // Ensure that all specified attributes are legal for their roles before we actually update anything + // NOTE: This isn't perfect. It compares each new attribute assignment to the current configuration, but + // other attributes changed at the same time could make an assignment illegal. + const updatedAttributes: Record = {} + for (const attributeType in attributeInfo) { + const attributePackage = attributeInfo[attributeType] + if (attributePackage?.id === null || (attributePackage?.id === undefined && attributePackage?.name === null)) { + updatedAttributes[attributeType] = null + } else { + const attribute = getAttributeFromInfo(dataSet, attributePackage) + if (attribute) { + const role = roleFromAttrKey[attributeType] + const place = attrRoleToGraphPlace[role] + if (place && !content.dataConfiguration.placeCanAcceptAttributeIDDrop( + place, dataSet, attribute.id, { allowSameAttr: true } + )) { + return errorResult( + t("V3.DI.Error.illegalAttributeAssignment", { + vars: [attributePackage?.id ?? attributePackage?.name ?? "", role] + }) + ) + } + updatedAttributes[attributeType] = attribute + } + } + } + + // Actually update dataSet (which will clear any old attribute assignments) + if (dataSet && dataSet !== content.dataset) { + content.setDataSet(dataSet.id) + } + + // Actually update attributes + for (const attributeType in updatedAttributes) { + const attribute = updatedAttributes[attributeType] + const role = roleFromAttrKey[attributeType] + if (role) { + if (attribute) { + content.dataConfiguration.setAttribute(role, { attributeID: attribute.id }) + } else { + content.dataConfiguration.setAttribute(role) + } + } + } + + // Any attribute can be put on the y axis, so we don't check to make sure the attribute is legal first + // We don't use dataConfiguration.setAttribute() to make the change because that clears additional y attributes + if (yAttributeID !== undefined) { + if (yAttributeID) { + content.dataConfiguration.replaceYAttribute({ attributeID: toV3AttrId(yAttributeID) }, 0) + } else { + content.dataConfiguration.removeYAttributeAtIndex(0) + } + } else if (yAttributeName !== undefined) { + if (yAttributeName !== null) { + const attribute = dataSet?.getAttributeByName(yAttributeName) + if (attribute) content.dataConfiguration.replaceYAttribute({ attributeID: attribute.id }, 0) + } else { + content.dataConfiguration.removeYAttributeAtIndex(0) + } + } + + // Update lower and upper bounds + const updateBounds = (place: AxisPlace, lower?: number, upper?: number) => { + if (lower != null || upper != null) { + const axis = content.getAxis(place) + if (isNumericAxisModel(axis)) { + if (lower != null) axis.setMinimum(lower) + if (upper != null) axis.setMaximum(upper) + } + } + } + updateBounds("bottom", xLowerBound, xUpperBound) + updateBounds("left", yLowerBound, yUpperBound) + updateBounds("rightNumeric", y2LowerBound, y2UpperBound) + + // Update odd features + if (showParentToggles != null) { + content.setShowParentToggles(showParentToggles) + } + if (showOnlyLastCase != null) { + content.setShowOnlyLastCase(showOnlyLastCase) + } + + return { success: true } } } diff --git a/v3/src/components/graph/models/graph-content-model.ts b/v3/src/components/graph/models/graph-content-model.ts index 136f1db4ee..ef8f700ee5 100644 --- a/v3/src/components/graph/models/graph-content-model.ts +++ b/v3/src/components/graph/models/graph-content-model.ts @@ -324,6 +324,13 @@ export const GraphContentModel = DataDisplayContentModel binCount++ } return { tickValues, tickLabels } + }, + setDataSet(dataSetID: string) { + const newDataSet = getDataSetFromId(self, dataSetID) + if (newDataSet && newDataSet !== self.dataConfiguration.dataset) { + self.dataConfiguration.clearAttributes() + self.dataConfiguration.setDataset(newDataSet, getSharedCaseMetadataFromDataset(newDataSet)) + } } })) .views(self => ({ @@ -582,11 +589,7 @@ export const GraphContentModel = DataDisplayContentModel self.axes.delete(place) }, setAttributeID(role: GraphAttrRole, dataSetID: string, id: string) { - const newDataSet = getDataSetFromId(self, dataSetID) - if (newDataSet && newDataSet !== self.dataConfiguration.dataset) { - self.dataConfiguration.clearAttributes() - self.dataConfiguration.setDataset(newDataSet, getSharedCaseMetadataFromDataset(newDataSet)) - } + self.setDataSet(dataSetID) if (role === 'yPlus') { self.dataConfiguration.addYAttribute({attributeID: id}) } else { diff --git a/v3/src/components/graph/models/graph-data-configuration-model.ts b/v3/src/components/graph/models/graph-data-configuration-model.ts index 32888d135a..23d0a1c373 100644 --- a/v3/src/components/graph/models/graph-data-configuration-model.ts +++ b/v3/src/components/graph/models/graph-data-configuration-model.ts @@ -604,8 +604,12 @@ export const GraphDataConfigurationModel = DataConfigurationModel self._setAttributeDescription(role, desc) } }, - addYAttribute(desc: IAttributeDescriptionSnapshot) { - self._yAttributeDescriptions.push(desc) + addYAttribute(desc: IAttributeDescriptionSnapshot, index?: number) { + if (index != null && index >= 0) { + self._yAttributeDescriptions.splice(index, 0, desc) + } else { + self._yAttributeDescriptions.push(desc) + } }, setY2Attribute(desc?: IAttributeDescriptionSnapshot) { self._setAttributeDescription('rightNumeric', desc) @@ -616,8 +620,7 @@ export const GraphDataConfigurationModel = DataConfigurationModel existingFilteredCases?.invalidateCases() } }, - removeYAttributeWithID(id: string) { - const index = self._yAttributeDescriptions.findIndex((aDesc) => aDesc.attributeID === id) + removeYAttributeAtIndex(index: number) { if (index >= 0) { self._yAttributeDescriptions.splice(index, 1) // remove and destroy the filtered cases for the y attribute @@ -636,9 +639,17 @@ export const GraphDataConfigurationModel = DataConfigurationModel self._attributeDescriptions.get(role)?.setType(type) } self._setAttributeType(type, plotNumber) - }, + } })) .actions(self => ({ + replaceYAttribute(desc?: IAttributeDescriptionSnapshot, plotNumber = 0) { + if (self._yAttributeDescriptions[plotNumber]) self.removeYAttributeAtIndex(plotNumber) + if (desc) self.addYAttribute(desc, plotNumber) + }, + removeYAttributeWithID(id: string) { + const index = self._yAttributeDescriptions.findIndex((aDesc) => aDesc.attributeID === id) + self.removeYAttributeAtIndex(index) + }, clearGraphSpecificCasesCache() { self.allPlottedCases.invalidate() self.subPlotCases.invalidateAll() diff --git a/v3/src/data-interactive/data-interactive-component-types.ts b/v3/src/data-interactive/data-interactive-component-types.ts index d9247f7c96..7174271e9d 100644 --- a/v3/src/data-interactive/data-interactive-component-types.ts +++ b/v3/src/data-interactive/data-interactive-component-types.ts @@ -64,18 +64,26 @@ export interface V2Game extends V2Component { URL?: string } export interface V2Graph extends V2Component { - captionAttributeName?: string + captionAttributeID?: string | null + captionAttributeName?: string | null dataContext?: string enableNumberToggle?: boolean - legendAttributeName?: string + legendAttributeID?: string | null + legendAttributeName?: string | null numberToggleLastMode?: boolean - rightNumericAttributeName?: string - rightSplitAttributeName?: string - topSplitAttributeName?: string + rightNumericAttributeID?: string | null + rightNumericAttributeName?: string | null + rightSplitAttributeID?: string | null + rightSplitAttributeName?: string | null + topSplitAttributeID?: string | null + topSplitAttributeName?: string | null type: "graph" - xAttributeName?: string - yAttributeName?: string - y2AttributeName?: string + xAttributeID?: string | null + xAttributeName?: string | null + yAttributeID?: string | null + yAttributeName?: string | null + y2AttributeID?: string | null + y2AttributeName?: string | null } export interface V2GetGraph extends V2Graph { xLowerBound?: number diff --git a/v3/src/data-interactive/handlers/attribute-handler.test.ts b/v3/src/data-interactive/handlers/attribute-handler.test.ts index 18a9dd4646..f0ef2a2d04 100644 --- a/v3/src/data-interactive/handlers/attribute-handler.test.ts +++ b/v3/src/data-interactive/handlers/attribute-handler.test.ts @@ -24,11 +24,11 @@ describe("DataInteractive AttributeHandler", () => { expect(create(resources).success).toEqual(false) expect(create({ dataContext }, { name: "noCollection" }).success).toEqual(false) - expect(dataContext.attributes.length).toBe(3) + expect(dataContext.attributes.length).toBe(4) expect(c1.attributes.length).toBe(1) const name1 = "test" expect(create(resources, { name: name1 }).success).toEqual(true) - expect(dataContext.attributes.length).toBe(4) + expect(dataContext.attributes.length).toBe(5) expect(c1.attributes.length).toBe(2) const testAttr = c1.attributes[1]! expect(testAttr.name).toBe(name1) @@ -41,13 +41,13 @@ describe("DataInteractive AttributeHandler", () => { const name2 = "test2" expect(create(resources, [{ name: name2 }, {}]).success).toEqual(false) - expect(dataContext.attributes.length).toBe(4) + expect(dataContext.attributes.length).toBe(5) const name3 = "test3" const results = create(resources, [{ name: name2 }, { name: name3 }]) expect(results.success).toEqual(true) expect((results.values as DIResultAttributes).attrs.length).toBe(2) - expect(dataContext.attributes.length).toBe(6) + expect(dataContext.attributes.length).toBe(7) expect(c1.attributes[2]!.name).toBe(name2) expect(c1.attributes[3]!.name).toBe(name3) }) 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 5486c17719..8e19178f86 100644 --- a/v3/src/data-interactive/handlers/attribute-location-handler.test.ts +++ b/v3/src/data-interactive/handlers/attribute-location-handler.test.ts @@ -7,7 +7,7 @@ describe("DataInteractive AttributeLocationHandler", () => { it("update works as expected", () => { const { dataset, c1, c2, a1, a2 } = setupTestDataset() - const a4 = dataset.addAttribute({ name: "a4" }, { collection: c1.id }) + const a4 = dataset.addAttribute({ name: "a4b" }, { collection: c1.id }) const a5 = dataset.addAttribute({ name: "a5" }, { collection: c2.id }) const a6 = dataset.addAttribute({ name: "a6" }) const dataContext = dataset @@ -29,11 +29,11 @@ describe("DataInteractive AttributeLocationHandler", () => { // Move attribute within the ungrouped collection // Indexes snap to the end of the array - expect(dataset.childCollection.attributes[1]!.id).toBe(a6.id) + expect(dataset.childCollection.attributes[2]!.id).toBe(a6.id) expect(handler.update?.({ attributeLocation: a6, dataContext }, { position: -1 }).success).toBe(true) expect(dataset.childCollection.attributes[0]!.id).toBe(a6.id) expect(handler.update?.({ attributeLocation: a6, dataContext }, { position: 10 }).success).toBe(true) - expect(dataset.childCollection.attributes[1]!.id).toBe(a6.id) + expect(dataset.childCollection.attributes[2]!.id).toBe(a6.id) // Move attribute within a grouped collection // If not specified, move the attribute to the far right @@ -49,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.childCollection.attributes.length).toBe(1) + expect(dataset.childCollection.attributes.length).toBe(2) expect(collectionAttributes(c1)?.[1]?.id).toBe(a6.id) expect(collectionAttributes(c1)?.[2]?.id).toBe(a4.id) diff --git a/v3/src/data-interactive/handlers/collection-handler.test.ts b/v3/src/data-interactive/handlers/collection-handler.test.ts index e9ab45044e..bb63828a02 100644 --- a/v3/src/data-interactive/handlers/collection-handler.test.ts +++ b/v3/src/data-interactive/handlers/collection-handler.test.ts @@ -32,12 +32,12 @@ describe("DataInteractive CollectionHandler", () => { // Add a right-most collection // Add attributes with attributes field - const rightResult = handler.create?.({ dataContext }, { name: "right", attributes: [{ name: "a4" }] }) + const rightResult = handler.create?.({ dataContext }, { name: "right", attributes: [{ name: "a4b" }] }) expect(rightResult?.success).toBe(true) expect(dataset.collections.length).toBe(4) expect(dataset.collections[3].name).toBe("right") expect(dataset.collections[3].attributes.length).toBe(1) - expect(dataset.collections[3].attributes[0]?.name).toBe("a4") + expect(dataset.collections[3].attributes[0]?.name).toBe("a4b") // Add a left-most collection // Add attributes with attrs field @@ -76,7 +76,7 @@ describe("DataInteractive CollectionHandler", () => { const result = handler.delete?.({ dataContext, collection }) expect(result?.success).toBe(true) expect((result?.values as DIDeleteCollectionResult).collections?.[0]).toBe(toV2Id(collectionId)) - expect(dataContext.attributes.length).toBe(2) + expect(dataContext.attributes.length).toBe(3) expect(dataContext.collections.length).toBe(2) expect(dataContext.getCollection(collectionId)).toBeUndefined() }) diff --git a/v3/src/data-interactive/handlers/handler-test-utils.ts b/v3/src/data-interactive/handlers/handler-test-utils.ts index aeacbbc9cc..4fc60b39b3 100644 --- a/v3/src/data-interactive/handlers/handler-test-utils.ts +++ b/v3/src/data-interactive/handlers/handler-test-utils.ts @@ -1,12 +1,12 @@ import { DataSet } from "../../models/data/data-set" export const testCases = [ - { a1: "a", a2: "x", a3: 1 }, - { a1: "b", a2: "y", a3: 2 }, - { a1: "a", a2: "z", a3: 3 }, - { a1: "b", a2: "z", a3: 4 }, - { a1: "a", a2: "x", a3: 5 }, - { a1: "b", a2: "y", a3: 6 }, + { a1: "a", a2: "x", a3: 1, a4: -1 }, + { a1: "b", a2: "y", a3: 2, a4: -2 }, + { a1: "a", a2: "z", a3: 3, a4: -3 }, + { a1: "b", a2: "z", a3: 4, a4: -4 }, + { a1: "a", a2: "x", a3: 5, a4: -5 }, + { a1: "b", a2: "y", a3: 6, a4: -6 }, ] interface ITestDatasetOptions { datasetName?: string @@ -24,9 +24,10 @@ export const setupTestDataset = (options?: ITestDatasetOptions) => { const a1 = dataset.addAttribute({ name: "a1" }, { collection: c1.id }) const a2 = dataset.addAttribute({ name: "a2" }, { collection: c2.id }) const a3 = dataset.addAttribute({ name: "a3" }) + const a4 = dataset.addAttribute({ name: "a4" }) dataset.addCases(testCases, { canonicalize: true }) dataset.validateCases() - return { dataset, c1, c2, a1, a2, a3 } + return { dataset, c1, c2, a1, a2, a3, a4 } } export function setupForCaseTest() { diff --git a/v3/src/data-interactive/handlers/item-by-case-id-handler.test.ts b/v3/src/data-interactive/handlers/item-by-case-id-handler.test.ts index 821ebcbcc0..fb911fa83e 100644 --- a/v3/src/data-interactive/handlers/item-by-case-id-handler.test.ts +++ b/v3/src/data-interactive/handlers/item-by-case-id-handler.test.ts @@ -34,7 +34,7 @@ describe("DataInteractive ItemByCaseIDHandler", () => { expect(result.success).toBe(true) const values = result.values as DIFullCase expect(values.id).toBe(toV2Id(itemByCaseID.__id__)) - expect(Object.keys(values.values!).length).toBe(3) + expect(Object.keys(values.values!).length).toBe(4) expect(values.values?.a1).toBe(a1.value(0)) }) diff --git a/v3/src/data-interactive/handlers/item-by-id-handler.test.ts b/v3/src/data-interactive/handlers/item-by-id-handler.test.ts index 9e574f6b6f..2d2c3e99f7 100644 --- a/v3/src/data-interactive/handlers/item-by-id-handler.test.ts +++ b/v3/src/data-interactive/handlers/item-by-id-handler.test.ts @@ -34,7 +34,7 @@ describe("DataInteractive ItemByIDHandler", () => { expect(result.success).toBe(true) const values = result.values as DIFullCase expect(values.id).toBe(toV2Id(itemByID.__id__)) - expect(Object.keys(values.values!).length).toBe(3) + expect(Object.keys(values.values!).length).toBe(4) expect(values.values?.a1).toBe(a1.value(0)) }) diff --git a/v3/src/data-interactive/handlers/item-handler.test.ts b/v3/src/data-interactive/handlers/item-handler.test.ts index 36e8b007a3..713ccd9c27 100644 --- a/v3/src/data-interactive/handlers/item-handler.test.ts +++ b/v3/src/data-interactive/handlers/item-handler.test.ts @@ -75,7 +75,7 @@ describe("DataInteractive ItemHandler", () => { expect(result.success).toBe(true) const values = result.values as DIFullCase expect(values.id).toBe(toV2Id(item.__id__)) - expect(Object.keys(values.values!).length).toBe(3) + expect(Object.keys(values.values!).length).toBe(4) expect(values.values?.a1).toBe(a1.value(0)) })