fix: series with different current and previous breakdowns are throwi…

…ng errors (#24092) Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Thomas Obermüller <[email protected]>
PostHog · Jul 31, 2024 · 930455e · 930455e
1 parent 8ff3732
commit 930455e
Show file tree

Hide file tree

Showing 7 changed files with 26 additions and 16 deletions.
diff --git a/...snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--light.png b/...snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--light.png
diff --git a/frontend/src/scenes/insights/views/InsightsTable/InsightsTable.tsx b/frontend/src/scenes/insights/views/InsightsTable/InsightsTable.tsx
@@ -267,8 +267,6 @@ export function InsightsTable({
         columns.push(...valueColumns)
     }
 
-    const totalItems = indexedResults.length
-
     return (
         <LemonTable
             id={isInDashboardContext ? queryBasedInsight.short_id : undefined}
@@ -286,11 +284,8 @@ export function InsightsTable({
             useURLForSorting={insightMode !== ItemMode.Edit}
             rowRibbonColor={
                 isLegend
-                    ? (item) => {
-                          const isPrevious = !!item.compare && item.compare_label === 'previous'
-                          const adjustedIndex = isPrevious ? item.seriesIndex - totalItems / 2 : item.seriesIndex
-                          return getTrendLikeSeriesColor(adjustedIndex, isPrevious)
-                      }
+                    ? (item) =>
+                          getTrendLikeSeriesColor(item.colorIndex, !!item.compare && item.compare_label === 'previous')
                     : undefined
             }
             firstColumnSticky

diff --git a/frontend/src/scenes/insights/views/InsightsTable/columns/SeriesColumn.tsx b/frontend/src/scenes/insights/views/InsightsTable/columns/SeriesColumn.tsx
@@ -25,14 +25,12 @@ export function SeriesColumnItem({
 }: SeriesColumnItemProps): JSX.Element {
     const showCountedByTag = !!indexedResults.find(({ action }) => action?.math && action.math !== 'total')
 
-    const totalItems = indexedResults.length
     const isPrevious = !!item.compare && item.compare_label === 'previous'
-    const adjustedIndex = isPrevious ? item.seriesIndex - totalItems / 2 : item.seriesIndex
 
     return (
         <div className="series-name-wrapper-col space-x-1">
             <InsightLabel
-                seriesColor={getTrendLikeSeriesColor(adjustedIndex, isPrevious)}
+                seriesColor={getTrendLikeSeriesColor(item.colorIndex, isPrevious)}
                 action={item.action}
                 fallbackName={item.breakdown_value === '' ? 'None' : item.label}
                 hasMultipleSeries={hasMultipleSeries}

diff --git a/frontend/src/scenes/insights/views/LineGraph/LineGraph.tsx b/frontend/src/scenes/insights/views/LineGraph/LineGraph.tsx
@@ -313,13 +313,12 @@ export function LineGraph_({
         }
     }, [])
 
-    function processDataset(dataset: ChartDataset<any>, totalDatasets: number): ChartDataset<any> {
+    function processDataset(dataset: ChartDataset<any>): ChartDataset<any> {
         const isPrevious = !!dataset.compare && dataset.compare_label === 'previous'
-        const adjustedIndex = isPrevious ? dataset.seriesIndex - totalDatasets / 2 : dataset.seriesIndex
 
         const mainColor = dataset?.status
             ? getBarColorFromStatus(dataset.status)
-            : getTrendLikeSeriesColor(adjustedIndex, isPrevious && !isArea)
+            : getTrendLikeSeriesColor(dataset?.colorIndex ?? dataset.seriesIndex, isPrevious && !isArea)
         const hoverColor = dataset?.status ? getBarColorFromStatus(dataset.status, true) : mainColor
         const areaBackgroundColor = hexToRGBA(mainColor, 0.5)
         const areaIncompletePattern = createPinstripePattern(areaBackgroundColor, isDarkModeOn)
@@ -391,7 +390,7 @@ export function LineGraph_({
             }
         }
 
-        datasets = datasets.map((dataset) => processDataset(dataset, datasets.length))
+        datasets = datasets.map(processDataset)
 
         const seriesNonZeroMax = Math.max(...datasets.flatMap((d) => d.data).filter((n) => !!n && n !== LOG_ZERO))
         const seriesNonZeroMin = Math.min(...datasets.flatMap((d) => d.data).filter((n) => !!n && n !== LOG_ZERO))

diff --git a/frontend/src/scenes/trends/trendsDataLogic.test.ts b/frontend/src/scenes/trends/trendsDataLogic.test.ts
@@ -60,7 +60,7 @@ describe('trendsDataLogic', () => {
                 await expectLogic(logic, () => {
                     builtDataNodeLogic.actions.loadDataSuccess(insight)
                 }).toMatchValues({
-                    indexedResults: [{ ...trendResult.result[0], id: 0, seriesIndex: 0 }],
+                    indexedResults: [{ ...trendResult.result[0], id: 0, seriesIndex: 0, colorIndex: 0 }],
                 })
             })
 

diff --git a/frontend/src/scenes/trends/trendsDataLogic.ts b/frontend/src/scenes/trends/trendsDataLogic.ts
@@ -158,7 +158,21 @@ export const trendsDataLogic = kea<trendsDataLogicType>([
                             defaultLifecyclesOrder.indexOf(String(a.status))
                     )
                 }
-                return indexedResults.map((result, index) => ({ ...result, id: index }))
+
+                /** Unique series in the results, determined by `item.label` and `item.action.order`. */
+                const uniqSeries = Array.from(
+                    // :TRICKY: Stickiness insights don't have an `action` object, despite
+                    // types here not reflecting that.
+                    new Set(indexedResults.map((item) => `${item.label}_${item.action?.order}`))
+                )
+
+                // Give current and previous versions of the same dataset the same colorIndex
+                return indexedResults.map((item, index) => {
+                    const colorIndex = uniqSeries.findIndex(
+                        (identifier) => identifier === `${item.label}_${item.action?.order}`
+                    )
+                    return { ...item, colorIndex: colorIndex, id: index }
+                })
             },
         ],
 

diff --git a/frontend/src/scenes/trends/types.ts b/frontend/src/scenes/trends/types.ts
@@ -16,6 +16,10 @@ export interface IndexedTrendResult extends TrendResult {
      * specific order (e.g. for pie chart). The series index is used e.g. to
      * get series color correctly. */
     seriesIndex: number
+    /** An index computed in trendsDataLogic that is used to generate colors. This index is the same for current and previous
+     * series with the same label. The previous series is given a slightly lighter version of the same color.
+     * */
+    colorIndex: number
 }
 
 export interface TrendActors {