feat: generate and save colorMapping for cells that do not have it (#3495)

Author: ChitlangeSahas

package.json

@@ -173,6 +173,7 @@
     "codemirror": "^5.58.2",
     "connected-react-router": "^6.8.0",
     "d3-dsv": "^3.0.1",
+    "deep-object-diff": "^1.1.0",
     "fix-date": "^1.1.6",
     "history": "^4.7.2",
     "honeybadger-js": "^1.0.2",

src/visualization/types/Graph/view.tsx

@@ -28,7 +28,7 @@ import {DEFAULT_LINE_COLORS} from 'src/shared/constants/graphColorPalettes'
 import {INVALID_DATA_COPY} from 'src/visualization/constants'
 
 // Types
-import {XYViewProperties} from 'src/types'
+import {AppState, ResourceType, View, XYViewProperties} from 'src/types'
 import {VisualizationProps} from 'src/visualization'
 
 // Utils
@@ -54,6 +54,10 @@ import {addAnnotationLayer} from 'src/visualization/utils/annotationUtils'
 import {getColorMappingObjects} from 'src/visualization/utils/colorMappingUtils'
 import {isFlagEnabled} from '../../../shared/utils/featureFlag'
 
+// Selectors
+import {getByID} from 'src/resources/selectors'
+import {updateViewAndVariables} from 'src/views/actions/thunks'
+
 interface Props extends VisualizationProps {
   properties: XYViewProperties
 }
@@ -73,6 +77,13 @@ const XYPlot: FC<Props> = ({
   const tooltipOrientationThreshold = properties.legendOrientationThreshold
   const staticLegend = useStaticLegend(properties)
   const dispatch = useDispatch()
+  const view = useSelector((state: AppState) =>
+    getByID<View<XYViewProperties>>(state, ResourceType.Views, cellID)
+  )
+  const {activeTimeMachineID} = useSelector(
+    (state: AppState) => state.timeMachines
+  )
+  const isVeoOpen = activeTimeMachineID === 'veo'
 
   // these two values are set in the dashboard, and used whether or not this view
   // is in a dashboard or in configuration/single cell popout mode
@@ -186,11 +197,22 @@ const XYPlot: FC<Props> = ({
 
   if (isFlagEnabled('graphColorMapping')) {
     const [, fillColumnMap] = createGroupIDColumn(result.table, groupKey)
-    const {colorMappingForGiraffe} = getColorMappingObjects(
-      fillColumnMap,
-      properties
-    )
+    const {
+      colorMappingForGiraffe,
+      colorMappingForIDPE,
+      needsToSaveToIDPE,
+    } = getColorMappingObjects(fillColumnMap, properties)
     colorMapping = colorMappingForGiraffe
+
+    // when the view is in a dashboard cell, and there is a need to save to IDPE, save it.
+    // when VEO is open, prevent from saving because it causes state issues. It will be handled in the timemachine code separately.
+    if (needsToSaveToIDPE && view?.dashboardID && !isVeoOpen) {
+      const newView = {...view}
+      newView.properties.colorMapping = colorMappingForIDPE
+
+      // save to IDPE
+      dispatch(updateViewAndVariables(view.dashboardID, newView))
+    }
   }
 
   const config: Config = {

src/visualization/utils/colorMappingUtils.test.ts

@@ -134,9 +134,9 @@ const dataFromIDPE_sameColorMapping = {
       },
     ],
     colorMapping: {
-      'co-airSensors-TLM0100-mean-': 0,
-      'co-airSensors-TLM0101-mean-': 1,
-      'co-airSensors-TLM0102-mean-': 2,
+      'co-airSensors-TLM0100-mean-': '#31C0F6',
+      'co-airSensors-TLM0101-mean-': '#A500A5',
+      'co-airSensors-TLM0102-mean-': '#FF7E27',
     },
   },
 }
@@ -264,9 +264,9 @@ const expectedData = {
     },
     mappingForIDPE: {
       colorMapping: {
-        'co-airSensors-TLM0100-mean-': 0,
-        'co-airSensors-TLM0101-mean-': 1,
-        'co-airSensors-TLM0102-mean-': 2,
+        'co-airSensors-TLM0100-mean-': '#31C0F6',
+        'co-airSensors-TLM0101-mean-': '#A500A5',
+        'co-airSensors-TLM0102-mean-': '#FF7E27',
       },
     },
   },
@@ -280,7 +280,7 @@ const expectedData = {
           _measurement: 'airSensors',
           sensor_id: 'TLM0100',
           result: 'mean',
-          color: '#31C0F6',
+          color: 'rgb(49, 192, 246)',
         },
         {
           _start: `${MOCK_START}`,
@@ -289,7 +289,7 @@ const expectedData = {
           _measurement: 'airSensors',
           sensor_id: 'TLM0101',
           result: 'mean',
-          color: '#A500A5',
+          color: 'rgb(106, 103, 205)',
         },
         {
           _start: `${MOCK_START}`,
@@ -298,7 +298,7 @@ const expectedData = {
           _measurement: 'airSensors',
           sensor_id: 'TLM0102',
           result: 'mean',
-          color: '#FF7E27',
+          color: 'rgb(161, 53, 158)',
         },
         {
           _start: `${MOCK_START}`,
@@ -307,7 +307,7 @@ const expectedData = {
           _measurement: 'airSensors',
           sensor_id: 'TLM0103',
           result: 'mean',
-          color: '#31C0F6',
+          color: 'rgb(209, 70, 101)',
         },
         {
           _start: `${MOCK_START}`,
@@ -316,7 +316,7 @@ const expectedData = {
           _measurement: 'airSensors',
           sensor_id: 'TLM0104',
           result: 'mean',
-          color: '#A500A5',
+          color: 'rgb(255, 126, 39)',
         },
       ],
       columnKeys: [
@@ -330,11 +330,11 @@ const expectedData = {
     },
     mappingForIDPE: {
       colorMapping: {
-        'co-airSensors-TLM0100-mean-': 0,
-        'co-airSensors-TLM0101-mean-': 1,
-        'co-airSensors-TLM0102-mean-': 2,
-        'co-airSensors-TLM0103-mean-': 0,
-        'co-airSensors-TLM0104-mean-': 1,
+        'co-airSensors-TLM0100-mean-': 'rgb(49, 192, 246)',
+        'co-airSensors-TLM0101-mean-': 'rgb(106, 103, 205)',
+        'co-airSensors-TLM0102-mean-': 'rgb(161, 53, 158)',
+        'co-airSensors-TLM0103-mean-': 'rgb(209, 70, 101)',
+        'co-airSensors-TLM0104-mean-': 'rgb(255, 126, 39)',
       },
     },
   },

src/visualization/utils/colorMappingUtils.ts

@@ -1,36 +1,29 @@
 import {XYViewProperties} from 'src/types'
+import {addedDiff, deletedDiff} from 'deep-object-diff'
+import {getNominalColorScale} from '@influxdata/giraffe'
 
 /**
- * Evaluates deeper equality for two map objects based on their key:value pair
+ * Evaluates whether mappings need to be updated
  * @param map1
  * @param map2
  */
-const areMappingsSame = (map1, map2) => {
-  if (!map1 || !map2) {
-    return false
+const compareIDPEandLocalMappings = (map1, map2) => {
+  // SPECIAL CASE: handle null case when colorMapping from IDPE is `null`
+  if (!map1) {
+    return {isMappingReusable: false, additions: map2}
   }
 
-  // Create arrays of property names
-  const aProps = Object.getOwnPropertyNames(map1)
-  const bProps = Object.getOwnPropertyNames(map2)
-
-  // If number of properties is different,
-  // objects are not equivalent
-  if (aProps.length !== bProps.length) {
-    return false
+  function isEmpty(obj) {
+    return Object.keys(obj).length === 0
   }
 
-  for (let i = 0; i < aProps.length; i++) {
-    const propName = aProps[i]
+  const additions = addedDiff(map1, map2)
+  const deletions = deletedDiff(map1, map2)
 
-    // If values of same property are not equal,
-    // objects are not equivalent
-    if (map1[propName] !== map2[propName]) {
-      return false
-    }
-  }
+  // if no additions or no deletions we are good to go
+  const isMappingReusable = isEmpty(additions) && isEmpty(deletions)
 
-  return true
+  return {isMappingReusable, additions, deletions}
 }
 
 /**
@@ -44,26 +37,29 @@ export const getColorMappingObjects = (
   columnGroupMap,
   properties: XYViewProperties
 ) => {
-  const seriesToColorIndexMap = generateSeriesToColorIndexMap(
+  const seriesToColorHexMap = generateSeriesToColorHex(
     columnGroupMap,
     properties
   )
 
+  const {isMappingReusable} = compareIDPEandLocalMappings(
+    properties.colorMapping,
+    seriesToColorHexMap
+  )
+
   // if the mappings from the IDPE and the *required* one's for the current view are the same, we don't need to generate new mappings
-  if (areMappingsSame(properties.colorMapping, seriesToColorIndexMap)) {
+  if (isMappingReusable) {
     const columnKeys = columnGroupMap.columnKeys
     const mappings = {...columnGroupMap}
+    const needsToSaveToIDPE = false
 
     mappings.mappings.forEach(graphLine => {
       const seriesID = getSeriesId(graphLine, columnKeys)
 
-      const colors = properties.colors
-
       // this is needed for giraffe
-      graphLine.color = colors[properties.colorMapping[seriesID]].hex
+      graphLine.color = properties.colorMapping[seriesID]
     })
 
-    const needsToSaveToIDPE = false
     return {
       colorMappingForGiraffe: {columnKeys, ...mappings},
       needsToSaveToIDPE,
@@ -76,18 +72,33 @@ export const getColorMappingObjects = (
     mappings.mappings.forEach(graphLine => {
       const seriesID = getSeriesId(graphLine, columnKeys)
 
-      const colors = properties.colors
-
       // this is needed for giraffe
-      graphLine.color = colors[seriesToColorIndexMap[seriesID]].hex
+      graphLine.color = seriesToColorHexMap[seriesID]
     })
 
     const newColorMappingForGiraffe = {
       ...mappings,
     }
 
+    const {additions, deletions} = compareIDPEandLocalMappings(
+      properties.colorMapping,
+      seriesToColorHexMap
+    )
+
+    const colorMappingForIDPE = {...properties.colorMapping}
+
+    // perform additions
+    for (const add in additions) {
+      colorMappingForIDPE[add] = seriesToColorHexMap[add]
+    }
+
+    // perform deletions
+    for (const minus in deletions) {
+      delete colorMappingForIDPE[minus]
+    }
+
     return {
-      colorMappingForIDPE: seriesToColorIndexMap,
+      colorMappingForIDPE: colorMappingForIDPE,
       colorMappingForGiraffe: newColorMappingForGiraffe,
       needsToSaveToIDPE,
     }
@@ -136,19 +147,21 @@ const getSeriesId = (graphLine, columnKeys) => {
  * This function generates a map that maps the series ID to the color Index.
  * @param columnGroupMap - generated using the createGroupIDColumn function
  * @param properties - XYViewProperties
- * @returns - a map that contains the series ID and it's color index (value bounded by the properties.colors array)
+ * @returns - a map that contains the series ID and it's color hex value
  */
 
-export const generateSeriesToColorIndexMap = (
+const generateSeriesToColorHex = (
   columnGroupMap,
   properties: XYViewProperties
 ) => {
-  const seriesToColorIndexMap = {}
+  const seriesToColorHex = {}
   const cgMap = {...columnGroupMap}
   cgMap.mappings.forEach((graphLine, colorIndex) => {
     const id = getSeriesId(graphLine, columnGroupMap.columnKeys)
-    seriesToColorIndexMap[id] = colorIndex % properties.colors.length
+    const colors = properties.colors.map(value => value.hex)
+    const fillScale = getNominalColorScale(columnGroupMap, colors)
+    seriesToColorHex[id] = fillScale(colorIndex)
   })
 
-  return {...seriesToColorIndexMap}
+  return {...seriesToColorHex}
 }

yarn.lock

@@ -2571,6 +2571,13 @@ aws4@^1.8.0:
   resolved "https://registry.yarnpkg.com/aws4/-/aws4-1.11.0.tgz#d61f46d83b2519250e2784daf5b09479a8b41c59"
   integrity sha512-xh1Rl34h6Fi1DC2WWKfxUTVqRsNnr6LsKz2+hfwDxQJWmrx8+c7ylaqBMcHfl1U1r2dsifOvKX3LQuLNZ+XSvA==
 
+axios@^0.19.0:
+  version "0.19.2"
+  resolved "https://registry.yarnpkg.com/axios/-/axios-0.19.2.tgz#3ea36c5d8818d0d5f8a8a97a6d36b86cdc00cb27"
+  integrity sha512-fjgm5MvRHLhx+osE2xoekY70AhARk3a6hkN+3Io1jc00jtquGvxYlKlsFUhmUET0V5te6CcZI7lcv2Ym61mjHA==
+  dependencies:
+    follow-redirects "1.5.10"
+
 babel-jest@^27.4.4:
   version "27.4.4"
   resolved "https://registry.yarnpkg.com/babel-jest/-/babel-jest-27.4.4.tgz#a012441f8a155df909839543a09510ab3477aa11"
@@ -4100,7 +4107,7 @@ debug@2.6.9, debug@^2.2.0, debug@^2.3.3:
   dependencies:
     ms "2.0.0"
 
-debug@3.1.0:
+debug@3.1.0, debug@=3.1.0:
   version "3.1.0"
   resolved "https://registry.yarnpkg.com/debug/-/debug-3.1.0.tgz#5bb5a0672628b64149566ba16819e61518c67261"
   integrity sha512-OX8XqP7/1a9cqkxYw2yXss15f26NKWBpDXQd0/uK/KPqdQhxbPa994hnzjcE2VqQpDslf55723cKPUOGSmMY3g==
@@ -4158,6 +4165,11 @@ deep-is@~0.1.3:
   resolved "https://registry.yarnpkg.com/deep-is/-/deep-is-0.1.4.tgz#a6f2dce612fadd2ef1f519b73551f17e85199831"
   integrity sha512-oIPzksmTg4/MriiaYGO+okXDT7ztn/w3Eptv/+gSIdMdKsJo0u4CfYNFJPy+4SKMuCqGw2wxnA+URMg3t8a/bQ==
 
+deep-object-diff@^1.1.0:
+  version "1.1.0"
+  resolved "https://registry.yarnpkg.com/deep-object-diff/-/deep-object-diff-1.1.0.tgz#d6fabf476c2ed1751fc94d5ca693d2ed8c18bc5a"
+  integrity sha512-b+QLs5vHgS+IoSNcUE4n9HP2NwcHj7aqnJWsjPtuG75Rh5TOaGt0OjAYInh77d5T16V5cRDC+Pw/6ZZZiETBGw==
+
 deepmerge@^4.2.2:
   version "4.2.2"
   resolved "https://registry.yarnpkg.com/deepmerge/-/deepmerge-4.2.2.tgz#44d2ea3679b8f4d4ffba33f03d865fc1e7bf4955"
@@ -5238,6 +5250,13 @@ flush-write-stream@^1.0.0:
     inherits "^2.0.3"
     readable-stream "^2.3.6"
 
+follow-redirects@1.5.10:
+  version "1.5.10"
+  resolved "https://registry.yarnpkg.com/follow-redirects/-/follow-redirects-1.5.10.tgz#7b7a9f9aea2fdff36786a94ff643ed07f4ff5e2a"
+  integrity sha512-0V5l4Cizzvqt5D44aTXbFZz+FtyXV1vrDN6qrelxtfYQKW0KO0W2T/hkE8xvGa/540LkZlkaUjO4ailYTFtHVQ==
+  dependencies:
+    debug "=3.1.0"
+
 follow-redirects@^1.0.0:
   version "1.14.6"
   resolved "https://registry.yarnpkg.com/follow-redirects/-/follow-redirects-1.14.6.tgz#8cfb281bbc035b3c067d6cd975b0f6ade6e855cd"