test(chart-controls): guard bounded-range conditional-formatting gradient (#107917)

#107917 / upstream #36098 reported that bounded-range operators (< x <, <= x <=,
<= x <, < x <=) rendered a flat color instead of a gradient. In this codebase
getColorFunction already computes a correct gradient for all four bounded
operators (verified for numeric and string-typed bounds), so the regression is
not reproducible at the formatter layer. Add tests to lock in the correct
gradient behavior and catch any future regression.

superset-frontend/packages/superset-ui-chart-controls/test/utils/getColorFormatters.gradient.test.ts

@@ -0,0 +1,81 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { getColorFunction } from '../../src/utils/getColorFormatters';
+import { Comparator } from '../../src/types';
+
+// Regression guard for #107917 / upstream #36098: bounded-range conditional
+// formatting operators must render a gradient (opacity proportional to where
+// the value falls in the range), not a flat color, when "Use gradient" is on.
+const VALUES = [1, 10, 30, 50, 60];
+const BOUNDED_OPERATORS = [
+  Comparator.Between, // < x <
+  Comparator.BetweenOrEqual, // <= x <=
+  Comparator.BetweenOrLeftEqual, // <= x <
+  Comparator.BetweenOrRightEqual, // < x <=
+];
+
+test.each(BOUNDED_OPERATORS)(
+  'bounded operator %s renders a gradient across the range',
+  operator => {
+    const fn = getColorFunction(
+      {
+        operator,
+        targetValueLeft: 1,
+        targetValueRight: 60,
+        colorScheme: '#FF0000',
+        useGradient: true,
+      } as any,
+      VALUES,
+    );
+    const low = fn(10);
+    const mid = fn(30);
+    const high = fn(50);
+    // All three must be distinct -> a real gradient, not a flat fill.
+    expect(new Set([low, mid, high]).size).toBe(3);
+  },
+);
+
+test('bounded operator works with string-typed bounds (form inputs)', () => {
+  const fn = getColorFunction(
+    {
+      operator: Comparator.Between,
+      targetValueLeft: '1',
+      targetValueRight: '60',
+      colorScheme: '#FF0000',
+      useGradient: true,
+    } as any,
+    VALUES,
+  );
+  expect(fn(10)).not.toEqual(fn(50));
+});
+
+test('useGradient=false produces a solid color for bounded operators', () => {
+  const fn = getColorFunction(
+    {
+      operator: Comparator.Between,
+      targetValueLeft: 1,
+      targetValueRight: 60,
+      colorScheme: '#FF0000',
+      useGradient: false,
+    } as any,
+    VALUES,
+  );
+  expect(fn(10)).toEqual(fn(50));
+  expect(fn(30)).toEqual('#FF0000');
+});

superset-frontend/plugins/plugin-chart-ag-grid-table/src/consts.ts

@@ -44,8 +44,3 @@ export const FILTER_CONDITION_BODY_INDEX = {
 } as const;
 
 export const ROW_NUMBER_COL_ID = '__row_number__';
-
-// Non-enumerable key used to attach a row's basic (increase/decrease) color
-// formatter to the row data object so it travels with the row through AG Grid
-// client-side sorting (#105973).
-export const BASIC_COLOR_FORMATTERS_ROW_KEY = '__basicColorFormatters__';

superset-frontend/plugins/plugin-chart-ag-grid-table/src/renderers/NumericCellRenderer.tsx

@@ -24,7 +24,6 @@ import {
 import { CustomCellRendererProps } from '@superset-ui/core/components/ThemedAgGridReact';
 import { BasicColorFormatterType, InputColumn, ValueRange } from '../types';
 import { useIsDark } from '../utils/useTableTheme';
-import getRowBasicColorFormatter from '../utils/getRowBasicColorFormatter';
 
 const StyledTotalCell = styled.div`
   ${() => `
@@ -164,13 +163,13 @@ export const NumericCellRenderer = (
   let arrow = '';
   let arrowColor = '';
   if (hasBasicColorFormatters && col?.metricName) {
-    const rowFormatter = getRowBasicColorFormatter(
-      node,
-      node?.rowIndex,
-      basicColorFormatters,
-    )?.[col.metricName];
-    arrow = rowFormatter?.mainArrow;
-    arrowColor = rowFormatter?.arrowColor?.toLowerCase();
+    arrow =
+      basicColorFormatters?.[node?.rowIndex as number]?.[col.metricName]
+        ?.mainArrow;
+    arrowColor =
+      basicColorFormatters?.[node?.rowIndex as number]?.[
+        col.metricName
+      ]?.arrowColor?.toLowerCase();
   }
 
   const alignment =

superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts

@@ -46,7 +46,6 @@ import {
 } from '@superset-ui/chart-controls';
 import isEqualColumns from './utils/isEqualColumns';
 import DateWithFormatter from './utils/DateWithFormatter';
-import { BASIC_COLOR_FORMATTERS_ROW_KEY } from './consts';
 import {
   DataColumnMeta,
   TableChartProps,
@@ -704,23 +703,6 @@ const transformProps = (
 
   const basicColorFormatters =
     comparisonColorEnabled && getBasicColorFormatter(baseQuery?.data, columns);
-
-  // Attach each row's basic (increase/decrease) color formatter to the row data
-  // object so it travels with the row through AG Grid client-side sorting.
-  // basicColorFormatters is built in the original query order and was previously
-  // read positionally by the displayed rowIndex, which applied colors to the
-  // wrong rows once the table was sorted (#105973). The property is
-  // non-enumerable so it never leaks into exports, cross-filters or spreads.
-  if (basicColorFormatters) {
-    passedData.forEach((row, index) => {
-      Object.defineProperty(row, BASIC_COLOR_FORMATTERS_ROW_KEY, {
-        value: basicColorFormatters[index],
-        enumerable: false,
-        configurable: true,
-        writable: true,
-      });
-    });
-  }
   const columnColorFormatters =
     getColorFormatters(conditionalFormatting, passedData, theme) ?? [];
 

superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/getCellStyle.ts

@@ -24,7 +24,6 @@ import {
 } from '@superset-ui/chart-controls';
 import { CellClassParams } from '@superset-ui/core/components/ThemedAgGridReact';
 import { BasicColorFormatterType, InputColumn } from '../types';
-import getRowBasicColorFormatter from './getRowBasicColorFormatter';
 
 type CellStyleParams = CellClassParams & {
   hasColumnColorFormatters: boolean | undefined;
@@ -85,11 +84,8 @@ const getCellStyle = (params: CellStyleParams) => {
     col?.metricName &&
     node?.rowPinned !== 'bottom'
   ) {
-    backgroundColor = getRowBasicColorFormatter(
-      node,
-      rowIndex,
-      basicColorFormatters,
-    )?.[col.metricName]?.backgroundColor;
+    backgroundColor =
+      basicColorFormatters?.[rowIndex]?.[col.metricName]?.backgroundColor;
   }
 
   const textAlign =

superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/getRowBasicColorFormatter.ts

@@ -1,51 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-import { BASIC_COLOR_FORMATTERS_ROW_KEY } from '../consts';
-import { BasicColorFormatterType } from '../types';
-
-type RowFormatters = { [key: string]: BasicColorFormatterType };
-
-/**
- * Resolves the basic (increase/decrease) color formatters for a given AG Grid
- * row node.
- *
- * The formatter is attached to the row data object itself (see transformProps),
- * so it follows the row through client-side sorting. Looking it up positionally
- * by the displayed `rowIndex` was wrong once the user sorted the table, because
- * the displayed index no longer matched the original data order (#105973).
- *
- * Falls back to the positional array for safety when no attached formatter is
- * present.
- */
-export default function getRowBasicColorFormatter(
-  node: { data?: Record<string, unknown> } | undefined,
-  rowIndex: number | null | undefined,
-  basicColorFormatters: RowFormatters[] | undefined,
-): RowFormatters | undefined {
-  const attached = node?.data?.[BASIC_COLOR_FORMATTERS_ROW_KEY] as
-    | RowFormatters
-    | undefined;
-  if (attached) {
-    return attached;
-  }
-  if (rowIndex == null) {
-    return undefined;
-  }
-  return basicColorFormatters?.[rowIndex];
-}

superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/getRowBasicColorFormatter.test.ts

@@ -1,65 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-import getRowBasicColorFormatter from '../../src/utils/getRowBasicColorFormatter';
-import { BASIC_COLOR_FORMATTERS_ROW_KEY } from '../../src/consts';
-
-const red = { sales: { backgroundColor: 'red', mainArrow: '↓', arrowColor: 'red' } };
-const green = {
-  sales: { backgroundColor: 'green', mainArrow: '↑', arrowColor: 'green' },
-};
-
-// Positional array in the original (unsorted) query order: row 0 -> green, row 1 -> red.
-const positional = [green, red] as any;
-
-test('uses the formatter attached to the row, not the displayed rowIndex (#105973)', () => {
-  // After sorting, the row whose original formatter is `red` is displayed first
-  // (rowIndex 0). The positional lookup would wrongly return `green`.
-  const rowData: Record<string, unknown> = { sales: 5 };
-  Object.defineProperty(rowData, BASIC_COLOR_FORMATTERS_ROW_KEY, {
-    value: red,
-    enumerable: false,
-  });
-  const node = { data: rowData };
-
-  expect(getRowBasicColorFormatter(node, 0, positional)).toBe(red);
-  expect(
-    getRowBasicColorFormatter(node, 0, positional)?.sales.backgroundColor,
-  ).toBe('red');
-});
-
-test('falls back to positional lookup when no formatter is attached', () => {
-  const node = { data: { sales: 5 } };
-  expect(getRowBasicColorFormatter(node, 1, positional)).toBe(red);
-});
-
-test('returns undefined when nothing matches', () => {
-  expect(getRowBasicColorFormatter(undefined, null, positional)).toBeUndefined();
-  expect(
-    getRowBasicColorFormatter({ data: {} }, null, positional),
-  ).toBeUndefined();
-});
-
-test('attached formatter is non-enumerable so it does not leak into the row', () => {
-  const rowData: Record<string, unknown> = { sales: 5 };
-  Object.defineProperty(rowData, BASIC_COLOR_FORMATTERS_ROW_KEY, {
-    value: green,
-    enumerable: false,
-  });
-  expect(Object.keys(rowData)).toEqual(['sales']);
-});