fix: Time Comparison Feature Reverts Metric Labels to Metric Keys in Table Charts (#32665)

Co-authored-by: Fardin Mustaque <fardinmustaque@Fardins-Mac-mini.local>

superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx

@@ -605,6 +605,8 @@ export default function TableChart<D extends DataRecord = DataRecord>(
       // Calculate the number of placeholder columns needed before the current header
       const startPosition = value[0];
       const colSpan = value.length;
+      // Retrieve the originalLabel from the first column in this group
+      const originalLabel = columnsMeta[value[0]]?.originalLabel || key;
 
       // Add placeholder <th> for columns before this header
       for (let i = currentColumnIndex; i < startPosition; i += 1) {
@@ -620,7 +622,7 @@ export default function TableChart<D extends DataRecord = DataRecord>(
       // Add the current header <th>
       headers.push(
         <th key={`header-${key}`} colSpan={colSpan} style={{ borderBottom: 0 }}>
-          {key}
+          {originalLabel}
           <span
             css={css`
               float: right;
@@ -975,7 +977,7 @@ export default function TableChart<D extends DataRecord = DataRecord>(
         ),
         Footer: totals ? (
           i === 0 ? (
-            <th>
+            <th key={`footer-summary-${i}`}>
               <div
                 css={css`
                   display: flex;
@@ -997,7 +999,7 @@ export default function TableChart<D extends DataRecord = DataRecord>(
               </div>
             </th>
           ) : (
-            <td style={sharedStyle}>
+            <td key={`footer-total-${i}`} style={sharedStyle}>
               <strong>{formatColumnValue(column, totals[key])[1]}</strong>
             </td>
           )

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

@@ -347,6 +347,7 @@ const processComparisonColumns = (
       } = props;
       const savedFormat = columnFormats?.[col.key];
       const savedCurrency = currencyFormats?.[col.key];
+      const originalLabel = col.label;
       if (
         (col.isMetric || col.isPercentMetric) &&
         !col.key.includes(comparisonSuffix) &&
@@ -355,6 +356,7 @@ const processComparisonColumns = (
         return [
           {
             ...col,
+            originalLabel,
             label: t('Main'),
             key: `${t('Main')} ${col.key}`,
             config: getComparisonColConfig(t('Main'), col.key, columnConfig),
@@ -368,6 +370,7 @@ const processComparisonColumns = (
           },
           {
             ...col,
+            originalLabel,
             label: `#`,
             key: `# ${col.key}`,
             config: getComparisonColConfig(`#`, col.key, columnConfig),
@@ -381,6 +384,7 @@ const processComparisonColumns = (
           },
           {
             ...col,
+            originalLabel,
             label: `△`,
             key: `△ ${col.key}`,
             config: getComparisonColConfig(`△`, col.key, columnConfig),
@@ -394,6 +398,7 @@ const processComparisonColumns = (
           },
           {
             ...col,
+            originalLabel,
             label: `%`,
             key: `% ${col.key}`,
             config: getComparisonColConfig(`%`, col.key, columnConfig),

superset-frontend/plugins/plugin-chart-table/src/types.ts

@@ -56,6 +56,8 @@ export interface DataColumnMeta {
   key: string;
   // `label` is verbose column name used for rendering
   label: string;
+  // `originalLabel` preserves the original label when time comparison transforms the labels
+  originalLabel?: string;
   dataType: GenericDataType;
   formatter?:
     | TimeFormatter

superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx

@@ -175,6 +175,75 @@ describe('plugin-chart-table', () => {
         ?.formatter?.(0.123456);
       expect(formattedPercentMetric).toBe('0.123');
     });
+
+    it('should set originalLabel for comparison columns when time_compare and comparison_type are set', () => {
+      const transformedProps = transformProps(testData.comparison);
+
+      // Check if comparison columns are processed
+      const comparisonColumns = transformedProps.columns.filter(
+        col =>
+          col.label === 'Main' ||
+          col.label === '#' ||
+          col.label === '△' ||
+          col.label === '%',
+      );
+
+      expect(comparisonColumns.length).toBeGreaterThan(0);
+      expect(comparisonColumns.some(col => col.label === 'Main')).toBe(true);
+      expect(comparisonColumns.some(col => col.label === '#')).toBe(true);
+      expect(comparisonColumns.some(col => col.label === '△')).toBe(true);
+      expect(comparisonColumns.some(col => col.label === '%')).toBe(true);
+
+      // Verify originalLabel for metric_1 comparison columns
+      const mainMetric1 = transformedProps.columns.find(
+        col => col.key === 'Main metric_1',
+      );
+      expect(mainMetric1).toBeDefined();
+      expect(mainMetric1?.originalLabel).toBe('metric_1');
+
+      const hashMetric1 = transformedProps.columns.find(
+        col => col.key === '# metric_1',
+      );
+      expect(hashMetric1).toBeDefined();
+      expect(hashMetric1?.originalLabel).toBe('metric_1');
+
+      const deltaMetric1 = transformedProps.columns.find(
+        col => col.key === '△ metric_1',
+      );
+      expect(deltaMetric1).toBeDefined();
+      expect(deltaMetric1?.originalLabel).toBe('metric_1');
+
+      const percentMetric1 = transformedProps.columns.find(
+        col => col.key === '% metric_1',
+      );
+      expect(percentMetric1).toBeDefined();
+      expect(percentMetric1?.originalLabel).toBe('metric_1');
+
+      // Verify originalLabel for metric_2 comparison columns
+      const mainMetric2 = transformedProps.columns.find(
+        col => col.key === 'Main metric_2',
+      );
+      expect(mainMetric2).toBeDefined();
+      expect(mainMetric2?.originalLabel).toBe('metric_2');
+
+      const hashMetric2 = transformedProps.columns.find(
+        col => col.key === '# metric_2',
+      );
+      expect(hashMetric2).toBeDefined();
+      expect(hashMetric2?.originalLabel).toBe('metric_2');
+
+      const deltaMetric2 = transformedProps.columns.find(
+        col => col.key === '△ metric_2',
+      );
+      expect(deltaMetric2).toBeDefined();
+      expect(deltaMetric2?.originalLabel).toBe('metric_2');
+
+      const percentMetric2 = transformedProps.columns.find(
+        col => col.key === '% metric_2',
+      );
+      expect(percentMetric2).toBeDefined();
+      expect(percentMetric2?.originalLabel).toBe('metric_2');
+    });
   });
 
   describe('TableChart', () => {
@@ -400,6 +469,17 @@ describe('plugin-chart-table', () => {
       );
       expect(getComputedStyle(screen.getByText('N/A')).background).toBe('');
     });
+    it('should display originalLabel in grouped headers', () => {
+      render(
+        <ThemeProvider theme={supersetTheme}>
+          <TableChart {...transformProps(testData.comparison)} sticky={false} />
+        </ThemeProvider>,
+      );
+
+      const groupHeaders = screen.getAllByRole('columnheader');
+      expect(groupHeaders[0]).toHaveTextContent('metric_1');
+      expect(groupHeaders[1]).toHaveTextContent('metric_2');
+    });
   });
 
   it('render cell bars properly, and only when it is toggled on in both regular and percent metrics', () => {