fix(plugin-chart-ag-grid-table): render Show summary totals row on ag-grid 36 (#42115)

superset-frontend/plugins/plugin-chart-ag-grid-table/src/styles/index.tsx

@@ -361,6 +361,18 @@ export const StyledChartContainer = styled.div<{
       font-size: ${theme.fontSizeSM}px;
     }
 
+    /*
+     * AG Grid 34+ adds the row-entrance-animation class 'ag-opacity-zero'
+     * (opacity: 0) to a newly inserted row and removes it on the next frame to
+     * fade the row in. Under AG Grid 36 that class is never removed from the
+     * pinned bottom row, so the "Show summary" totals row renders fully (correct
+     * values in the DOM) but stays permanently transparent. Force pinned rows
+     * opaque so the summary row is visible.
+     */
+    .ag-row-pinned {
+      opacity: 1 !important;
+    }
+
     .ag-spanned-row {
       font-size: ${theme.fontSizeSM}px;
       font-weight: ${theme.fontWeightStrong};

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

@@ -628,7 +628,10 @@ const transformProps = (
                     percentDifferenceNum,
                     col.colorScheme || comparisonColorScheme,
                   );
-                item[col.column] = {
+                // Key by the metric column key (not the raw rule column) so the
+                // renderer's `col.metricName` lookup resolves it, identical to
+                // the comparison-color path below.
+                item[origCol.key] = {
                   mainArrow: arrow,
                   arrowColor,
                   backgroundColor,
@@ -717,9 +720,32 @@ const transformProps = (
   const passedData = isUsingTimeComparison ? comparisonData || [] : data;
   const passedColumns = isUsingTimeComparison ? comparisonColumns : columns;
 
-  const basicColorFormatters =
+  // Increase/decrease formatters from the "Comparison color" toggle, keyed by
+  // metric column key.
+  const comparisonColorFormatters =
     comparisonColorEnabled && getBasicColorFormatter(baseQuery?.data, columns);
 
+  // Custom conditional-formatting rules using the Green (increase) / Red
+  // (decrease) color scheme on a time-comparison table. These were computed but
+  // never consumed by the AG Grid renderer, so the colors/arrows never showed
+  // (the classic plugin-chart-table does consume them). Route them through the
+  // same increase/decrease path, keyed by metric column key (see above).
+  const basicColorColumnFormatters = getBasicColorFormatterForColumn(
+    baseQuery?.data,
+    columns,
+    conditionalFormatting,
+  );
+
+  // Merge both per-row into a single map so the existing row-attached formatter
+  // drives the renderer for either source.
+  const basicColorFormatters =
+    comparisonColorFormatters || basicColorColumnFormatters
+      ? (baseQuery?.data ?? []).map((_row, index) => ({
+          ...(comparisonColorFormatters || [])[index],
+          ...(basicColorColumnFormatters || [])[index],
+        }))
+      : comparisonColorFormatters;
+
   // 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
@@ -737,14 +763,20 @@ const transformProps = (
       });
     });
   }
-  const columnColorFormatters =
-    getColorFormatters(conditionalFormatting, passedData, theme) ?? [];
 
-  const basicColorColumnFormatters = getBasicColorFormatterForColumn(
-    baseQuery?.data,
-    columns,
-    conditionalFormatting,
-  );
+  // Green/Red custom rules are rendered via the increase/decrease path above, so
+  // exclude them here: getColorFormatters treats the scheme name as a hex color
+  // and would emit an invalid `'<scheme>FF'` background otherwise.
+  const columnColorFormatters =
+    getColorFormatters(
+      (conditionalFormatting || []).filter(
+        (config: ConditionalFormattingConfig) =>
+          config.colorScheme !== ColorSchemeEnum.Green &&
+          config.colorScheme !== ColorSchemeEnum.Red,
+      ),
+      passedData,
+      theme,
+    ) ?? [];
 
   const hasPageLength = isPositiveNumber(pageLength);
 

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

@@ -85,11 +85,19 @@ const getCellStyle = (params: CellStyleParams) => {
     col?.metricName &&
     node?.rowPinned !== 'bottom'
   ) {
-    backgroundColor = getRowBasicColorFormatter(
+    const basicBackgroundColor = getRowBasicColorFormatter(
       node,
       rowIndex,
       basicColorFormatters,
     )?.[col.metricName]?.backgroundColor;
+    // Only override when this column actually has an increase/decrease
+    // formatter. Green/Red conditional-format rules only target some metrics,
+    // so a column carrying only a standard conditional-format rule would
+    // otherwise have its background clobbered with `undefined` when Green/Red
+    // rules exist on other columns.
+    if (basicBackgroundColor) {
+      backgroundColor = basicBackgroundColor;
+    }
   }
 
   const textAlign =

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

@@ -264,3 +264,53 @@ test('uses description from column even when verboseMap renames the column', ()
   expect(columnMeta!.label).toBe('Custom Label');
   expect(columnMeta!.description).toBe('Original column description');
 });
+
+test('excludes Green/Red color-scheme rules from columnColorFormatters', () => {
+  // Green/Red rules are rendered via the increase/decrease path, so they must
+  // not reach getColorFormatters, which would treat the scheme name as a hex
+  // color and emit an invalid `'GreenFF'` background.
+  const props = createMockChartProps({
+    rawFormData: {
+      viz_type: 'table',
+      datasource: '1__table',
+      query_mode: QueryMode.Aggregate,
+      metrics: ['metric_a', 'metric_b'],
+      percent_metrics: [],
+      column_config: {},
+      table_timestamp_format: '',
+      granularity_sqla: 'day',
+      time_range: 'No filter',
+      conditional_formatting: [
+        {
+          column: 'metric_a',
+          operator: '>',
+          targetValue: 0,
+          colorScheme: 'Green',
+        },
+        {
+          column: 'metric_b',
+          operator: '>',
+          targetValue: 0,
+          colorScheme: '#FF0000',
+        },
+      ],
+    } as unknown as TableChartProps['rawFormData'],
+    queriesData: [
+      {
+        data: [{ metric_a: 10, metric_b: 20 }],
+        colnames: ['metric_a', 'metric_b'],
+        coltypes: [GenericDataType.Numeric, GenericDataType.Numeric],
+        rowcount: 1,
+        applied_filters: [],
+        rejected_filters: [],
+      },
+    ] as unknown as TableChartProps['queriesData'],
+  });
+
+  const result = transformProps(props);
+  const formattedColumns = result.columnColorFormatters.map(f => f.column);
+  // The standard (#FF0000) rule is kept...
+  expect(formattedColumns).toContain('metric_b');
+  // ...but the Green rule is excluded.
+  expect(formattedColumns).not.toContain('metric_a');
+});

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

@@ -0,0 +1,108 @@
+/**
+ * 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 getCellStyle from '../../src/utils/getCellStyle';
+
+// A standard conditional-formatting rule that paints metric_a red.
+const standardCfFormatter = {
+  column: 'metric_a',
+  getColorFromValue: () => '#ff0000',
+  objectFormatting: undefined,
+  toTextColor: false,
+};
+
+function buildParams(overrides: Record<string, unknown> = {}) {
+  return {
+    value: 100,
+    valueFormatted: '100',
+    colDef: { field: 'metric_a' },
+    rowIndex: 0,
+    node: { rowPinned: undefined, data: {} },
+    col: {
+      key: 'metric_a',
+      metricName: 'metric_a',
+      isNumeric: true,
+      config: {},
+    },
+    cellSurfaceColor: '#ffffff',
+    hoverCellSurfaceColor: '#eeeeee',
+    hasColumnColorFormatters: false,
+    columnColorFormatters: [],
+    hasBasicColorFormatters: false,
+    basicColorFormatters: undefined,
+    ...overrides,
+  } as unknown as Parameters<typeof getCellStyle>[0];
+}
+
+test('applies a standard conditional-format background', () => {
+  const style = getCellStyle(
+    buildParams({
+      hasColumnColorFormatters: true,
+      columnColorFormatters: [standardCfFormatter],
+    }),
+  );
+  expect(style.backgroundColor).toBe('#ff0000');
+});
+
+test('preserves the standard CF background when the column has no increase/decrease formatter', () => {
+  // hasBasicColorFormatters is enabled (e.g. a Green/Red rule exists on another
+  // column), but this column has no basic formatter entry. The standard
+  // conditional-format background must not be clobbered with undefined.
+  const style = getCellStyle(
+    buildParams({
+      hasColumnColorFormatters: true,
+      columnColorFormatters: [standardCfFormatter],
+      hasBasicColorFormatters: true,
+      basicColorFormatters: [{}], // row 0 has no formatter for metric_a
+    }),
+  );
+  expect(style.backgroundColor).toBe('#ff0000');
+});
+
+test('applies the increase/decrease background when the column has one', () => {
+  const style = getCellStyle(
+    buildParams({
+      hasColumnColorFormatters: true,
+      columnColorFormatters: [standardCfFormatter],
+      hasBasicColorFormatters: true,
+      basicColorFormatters: [
+        {
+          metric_a: {
+            backgroundColor: '#00ff00',
+            mainArrow: '↑',
+            arrowColor: 'green',
+          },
+        },
+      ],
+    }),
+  );
+  expect(style.backgroundColor).toBe('#00ff00');
+});
+
+test('does not apply basic formatting to the pinned summary row', () => {
+  const style = getCellStyle(
+    buildParams({
+      node: { rowPinned: 'bottom', data: {} },
+      hasBasicColorFormatters: true,
+      basicColorFormatters: [
+        { metric_a: { backgroundColor: '#00ff00', mainArrow: '↑' } },
+      ],
+    }),
+  );
+  expect(style.backgroundColor).toBe('');
+});