From 72b1c32e6e14dde6777656d710ed3302e8882c5f Mon Sep 17 00:00:00 2001 From: William Finzer Date: Fri, 27 Dec 2024 09:00:49 -0800 Subject: [PATCH] Merge pull request #1694 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * * In this branch we're marking instances of TODO_V2_IMPORT with indic… * * Explicitly classified ~24 of the TODO_V2_IMPORT comments. ~49 remai… --- .../graph/adornments/v2-adornment-importer.ts | 10 ++-- v3/src/components/graph/v2-graph-importer.ts | 5 +- v3/src/components/map/v2-map-importer.ts | 6 +-- v3/src/v2/codap-v2-document.ts | 3 +- v3/src/v2/codap-v2-types.ts | 54 +++++++++++-------- 5 files changed, 44 insertions(+), 34 deletions(-) diff --git a/v3/src/components/graph/adornments/v2-adornment-importer.ts b/v3/src/components/graph/adornments/v2-adornment-importer.ts index ed6043af1..295870414 100644 --- a/v3/src/components/graph/adornments/v2-adornment-importer.ts +++ b/v3/src/components/graph/adornments/v2-adornment-importer.ts @@ -67,11 +67,11 @@ interface IInstanceKeysForAdornmentsProps { } const univariateMeasureInstances = (adornment: ICodapV2UnivariateAdornment, instanceKeys?: string[]) => { - // TODO_V2_IMPORT: most documents with an equationCoordsArray have multiple items in the array + // TODO_V2_IMPORT: [Story **#188695360**] most documents with an equationCoordsArray have multiple items in the array // we are only looking at the first item here const equationCoordsV2 = adornment.equationCoordsArray?.[0] - // TODO_V2_IMPORT: 93 files in cfm-shared have equationCoordsArray with values with + // TODO_V2_IMPORT: [Story: **#188695677**] 93 files in cfm-shared have equationCoordsArray with values with // proportionCenterX and proportionCenterY instead of proportionX and proportionY. // For now we are just skipping those and treating them as undefined // example doc: cfm-shared/1caDhoHFlpuNQgSfOdhh/file.json @@ -223,7 +223,7 @@ export const v2AdornmentImporter = ({data, plotModels, attributeDescriptions, yA const lines: Record = {} instanceKeys?.forEach((key: string) => { const lineInstance = { - // TODO_V2_IMPORT: equationCoords are not handled correctly, the model stores x and y + // TODO_V2_IMPORT: [Story: **#188695677**] equationCoords are not handled correctly, the model stores x and y // but the loaded equationCoords have proportionCenterX and proportionCenterY equationCoords: equationCoords ?? undefined, // The V2 default is null, but we want undefined intercept: movableLineAdornment.intercept, @@ -249,7 +249,7 @@ export const v2AdornmentImporter = ({data, plotModels, attributeDescriptions, yA const lsrlInstances: ILSRLInstanceSnapshot[] = [] lsrlAdornment.lsrls?.forEach((lsrl) => { const lsrlInstance = { - // TODO_V2_IMPORT: equationCoords are not handled correctly, the model stores x and y + // TODO_V2_IMPORT: [Story: **#188695677**] equationCoords are not handled correctly, the model stores x and y // but the loaded equationCoords have proportionCenterX and proportionCenterY equationCoords: lsrl.equationCoords ?? undefined // The V2 default is null, but we want undefined } @@ -377,7 +377,7 @@ export const v2AdornmentImporter = ({data, plotModels, attributeDescriptions, yA plotValues.push(value.value) }) - // TODO_V2_IMPORT: both valueModels and values might have `isVisible: false` + // [Story: #188699857] TODO_V2_IMPORT: both valueModels and values might have `isVisible: false` // we are currently just ignoring that values[key] = plotValues }) diff --git a/v3/src/components/graph/v2-graph-importer.ts b/v3/src/components/graph/v2-graph-importer.ts index bc128f114..aa6f2e19d 100644 --- a/v3/src/components/graph/v2-graph-importer.ts +++ b/v3/src/components/graph/v2-graph-importer.ts @@ -47,7 +47,8 @@ export function v2GraphImporter({v2Component, v2Document, sharedModelManager, in pointColor, strokeColor, pointSizeMultiplier, strokeSameAsFill, isTransparent, plotBackgroundImageLockInfo, - /* TODO_V2_IMPORT: The following are present in the componentStorage but not used in the V3 content model (yet): + /* TODO_V2_IMPORT: [Story: #188694812] + The following are present in the componentStorage but not used in the V3 content model (yet): displayOnlySelected, legendRole, legendAttributeType, numberOfLegendQuantiles, legendQuantilesAreLocked, plotBackgroundImage, transparency, strokeTransparency, plotBackgroundOpacity, @@ -132,7 +133,7 @@ export function v2GraphImporter({v2Component, v2Document, sharedModelManager, in axes[v3Place] = {place: v3Place, type: "categorical"} break case "DG.CellLinearAxisModel": - // TODO_V2_IMPORT when lowerBound or upperBound are undefined or null this is + // TODO_V2_IMPORT [Story:#188701144] when lowerBound or upperBound are undefined or null this is // not handled correctly. It likely will cause an MST exception and failure to load. // There are 966 instances of `xUpperBound: null` in cfm-shared axes[v3Place] = {place: v3Place, type: "numeric", min: lowerBound as any, max: upperBound as any} diff --git a/v3/src/components/map/v2-map-importer.ts b/v3/src/components/map/v2-map-importer.ts index 82299f209..60727f421 100644 --- a/v3/src/components/map/v2-map-importer.ts +++ b/v3/src/components/map/v2-map-importer.ts @@ -64,7 +64,7 @@ export function v2MapImporter({v2Component, v2Document, insertTile}: V2TileImpor const { pointColor, strokeColor, pointSizeMultiplier, grid, pointsShouldBeVisible, connectingLines - /* TODO_V2_IMPORT: Present in v2 layer model but not yet used in V3 layer model: + /* TODO_V2_IMPORT: [Story: #188694812] Present in v2 layer model but not yet used in V3 layer model: transparency, strokeTransparency */ } = v2LayerModel @@ -81,7 +81,7 @@ export function v2MapImporter({v2Component, v2Document, insertTile}: V2TileImpor dataset: data?.dataSet.id, metadata: metadata?.id, _attributeDescriptions, - // TODO_V2_IMPORT hiddenCases are not imported + // TODO_V2_IMPORT [Story: #188694826] hiddenCases are not imported // the array in a "modern" v2 document coming from `mapModelStorage.layerModels[]._links_.hiddenCases` // looks like { type: "DG.Case", id: number } // The MST type expects an array of strings. @@ -106,7 +106,7 @@ export function v2MapImporter({v2Component, v2Document, insertTile}: V2TileImpor else if (isV2MapPolygonLayerStorage(v2LayerModel)) { const { areaColor, areaStrokeColor, - /* TODO_V2_IMPORT: Present in v2 layer model but not yet used in V3 layer model: + /* TODO_V2_IMPORT: [Story: #188694812] Present in v2 layer model but not yet used in V3 layer model: areaTransparency, areaStrokeTransparency */ } = v2LayerModel diff --git a/v3/src/v2/codap-v2-document.ts b/v3/src/v2/codap-v2-document.ts index bbf9eed23..52453b9c1 100644 --- a/v3/src/v2/codap-v2-document.ts +++ b/v3/src/v2/codap-v2-document.ts @@ -116,8 +116,9 @@ export class CodapV2Document { registerContexts(contexts?: CodapV2Context[]) { contexts?.forEach(context => { - // TODO_V2_IMPORT: external contexts are not imported + // TODO_V2_IMPORT_IGNORE: external contexts are not imported // There are 75 cases of external contexts in cfm-shared + // This refers to an obsolescent form of document storage no longer used at all. Ignore! if (isV2ExternalContext(context)) return const { guid, type = "DG.DataContext", document, name = "", title, collections = [] } = context if (document && this.guidMap.get(document)?.type !== "DG.Document") { diff --git a/v3/src/v2/codap-v2-types.ts b/v3/src/v2/codap-v2-types.ts index 2235e387f..d6c72e7b1 100644 --- a/v3/src/v2/codap-v2-types.ts +++ b/v3/src/v2/codap-v2-types.ts @@ -25,7 +25,8 @@ export interface ICodapV2Attribute { deletedFormula?: string precision?: number | string | null unit?: string | null - // TODO_V2_IMPORT decimals is not imported + // TODO_V2_IMPORT_CARRY_OVER [Story:#188701222] decimals is not imported + // Defined as `precision` in v3 // it occurs 604 times in 36 files in cfm-shared // in 33 cases the value is "2" // the rest of the cases the value is "0" @@ -58,17 +59,17 @@ export interface ICodapV2Case { export interface ICodapV2Collection { attrs: ICodapV2Attribute[] cases: ICodapV2Case[] - // TODO_V2_IMPORT: caseName is not imported + // TODO_V2_IMPORT_EXTRACT: caseName is not imported // There are 2,500 cases where this has a value in cfm-shared caseName?: string | null - // TODO_V2_IMPORT: childAttrName is not imported + // TODO_V2_IMPORT_EXTRACT: childAttrName is not imported // There are 8,592 cases where this has a value in cfm-shared childAttrName?: string | null cid?: string - // TODO_V2_IMPORT: collapseChildren is not imported + // TODO_V2_IMPORT_EXTRACT: collapseChildren is not imported // There are 250 cases where this is true in cfm-shared collapseChildren?: boolean | null - // TODO_V2_IMPORT: defaults does not seem to be imported + // TODO_V2_IMPORT_EXTRACT: defaults does not seem to be imported // There are 825 cases where it is defined in cfm-shared defaults?: { xAttr?: string @@ -76,8 +77,9 @@ export interface ICodapV2Collection { } guid: number id?: number - // TODO_V2_IMPORT: labels seem to be handled by the plugin api, and stored in v3 structures but + // TODO_V2_IMPORT_CARRY_OVER: labels seem to be handled by the plugin api, and stored in v3 structures but // they don't seem to be imported when opening a v2 document + // There is a V3 interface for this — CollectionLabels labels?: { singleCase?: string pluralCase?: string @@ -89,10 +91,10 @@ export interface ICodapV2Collection { parent?: number title?: string | null type?: "DG.Collection" - // TODO_V2_IMPORT areParentChildLinksConfigured is not imported + // TODO_V2_IMPORT_IGNORE areParentChildLinksConfigured is not imported // it is true 5,000 times in 2,812 files in cfm-shared // it is false at least 20,000 times - // it might not be optional + // According to V2 documentation it is no longer used areParentChildLinksConfigured?: boolean } // when exporting a v3 collection to v2 @@ -115,33 +117,37 @@ export interface ICodapV2ExternalContext { } export interface ICodapV2DataContextMetadata { description?: string - // TODO_V2_IMPORT data context metadata source is not imported + // TODO_V2_IMPORT_CARRY_OVER data context metadata source is not imported // unknown how many instances in this location + // In V3 this is stored as sourceName in dataset source?: string - // TODO_V2_IMPORT data context metadata importDate is not imported + // TODO_V2_IMPORT_CARRY_OVER G data context metadata importDate is not imported // at least 20,000 instances in cfm-shared importDate?: string - // TODO_V2_IMPORT "import date" is not imported + // TODO_V2_IMPORT_EXTRACT "import date" is not imported // 2,025 instances in cfm-shared + // Assume that it is the same as importDate and convert "import date"?: string } export interface ICodapV2DataContext { type: "DG.DataContext" document?: number // id of containing document guid: number - // TODO_V2_IMPORT do we need to import id so we can export it - // it again? + // TODO_V2_IMPORT_STORE do we need to import id so we can export it again? // It is not always present in v2 files. id?: number - // TODO_V2_IMPORT flexibleGroupingChangeFlag is not imported + // TODO_V2_IMPORT_DEFINE_AND_IMPLEMENT flexibleGroupingChangeFlag is not imported // This is set to true in 11,000 cfm-shared files + // This flag is used in interaction with plugin to indicate that the user + // has moved an attribute in a way that invalidates the plugin's assumptions. + // V3 should define and implement, following the pattern set in V2 flexibleGroupingChangeFlag?: boolean | null name?: string title?: string collections: ICodapV2Collection[] description?: string metadata?: ICodapV2DataContextMetadata | null - // TODO_V2_IMPORT preventReorg is not imported + // TODO_V2_IMPORT_DEFINE_AND_IMPLEMENT preventReorg is not imported // it is false at least 20,000 times // it is true 969 times in 316 files preventReorg?: boolean @@ -153,7 +159,7 @@ export interface ICodapV2DataContext { // 259 of these arrays have a first object that doesn't have an "id" field setAsideItems?: ICodapV2SetAsideItem[] | Record[] contextStorage: ICodapV2DataContextStorage - // TODO_V2_IMPORT we are ignoring _permissions. It seems like a CFM + // TODO_V2_IMPORT_IGNORE we are ignoring _permissions. It seems like a CFM // artifact _permissions?: any } @@ -165,9 +171,10 @@ export interface ICodapV2DataContextSelectedCase { export interface ICodapV2DataContextStorage { _links_?: { - // TODO_V2_IMPORT selectedCases is not imported + // TODO_V2_IMPORT_EXTRACT selectedCases is not imported // they appear at lest 20,000 times perhaps both in game context and data context // The value is an empty array 11,300 times + // Note that V3 should be storing selection in V3 documents as well selectedCases: ICodapV2DataContextSelectedCase[] } } @@ -190,6 +197,7 @@ export interface ICodapV2GameContextStorage extends ICodapV2DataContextStorage { // cfm-shared/0b5715a7dab0a92ef332c8407bf51c53cc3ae710/file.json // It has gameState in the GameContext // It restores the gameState in v2 +// NOTE: The plan is to reimplement Markov and other "legacy" data games plugins to use current API // // An example with Next Gen MW games: // cfm-shared/0b65185859c238170055bde1fef60830e52bd63d49bec96e0b1db84c65ea3356/file.json @@ -237,7 +245,7 @@ export interface ICodapV2BaseComponentStorage { name?: string userSetTitle?: boolean // in a document saved by build 0441 this property didn't exist - // TODO_V2_IMPORT: this property seems to be ignored by the import code + // TODO_V2_IMPORT_CARRY_OVER: this property seems to be ignored by the import code // The v3 models do support it, but from what I can tell each component // importer needs to read this property from componentStorage and then // set it on the tile snapshot they pass to insertTile @@ -284,7 +292,7 @@ export interface ICodapV2TableStorage extends ICodapV2BaseComponentStorage { // a context _links_?: { context: IGuidLink<"DG.DataContextRecord"> - // TODO_V2_IMPORT collapsedNodes is not imported + // TODO_V2_IMPORT_CARRY_OVER collapsedNodes is not imported // it appears 1,518 times in cfm-shared // none of those times are empty arrays collapsedNodes?: IGuidLink<"DG.Case"> | IGuidLink<"DG.Case">[] @@ -296,21 +304,21 @@ export interface ICodapV2TableStorage extends ICodapV2BaseComponentStorage { width?: number }> title?: string - // TODO_V2_IMPORT isActive is not imported + // TODO_V2_IMPORT_STORE isActive is not imported // it occurs in close to 11,0000 files in cfm-shared // these are both in table and case card // it might not be optional isActive?: boolean - // TODO_V2_IMPORT rowHeights is not imported + // TODO_V2_IMPORT_DEFINE_AND_IMPLEMENT rowHeights is not imported // it occurs more than 20,000 times in cfm-shared // more than 4,200 of those have non-empty arrays // it might not be optional rowHeights?: ICodapV2RowHeight[] - // TODO_V2_IMPORT horizontalScrollOffset is not imported + // TODO_V2_IMPORT_CARRY_OVER horizontalScrollOffset is not imported // it occurs more than 20,000 times in cfm-shared // more than 20,000 of those times it has a value other than 0 horizontalScrollOffset?: number - // TODO_V2_IMPORT isIndexHidden is not imported + // TODO_V2_IMPORT_DEFINE_AND_IMPLEMENT isIndexHidden is not imported // it occurs more than 20,000 times in cfm-shared // it is true 4,346 times isIndexHidden?: boolean