fix(pivot-table): add bottom border to last grouped row

superset-frontend/plugins/plugin-chart-pivot-table/package.json

@@ -31,6 +31,11 @@
     "@ant-design/icons": "^5.2.6",
     "@superset-ui/chart-controls": "*",
     "@superset-ui/core": "*",
+    "@testing-library/dom": "^8.20.1",
+    "@testing-library/jest-dom": "*",
+    "@testing-library/react": "^12.1.5",
+    "@testing-library/react-hooks": "*",
+    "@testing-library/user-event": "*",
     "lodash": "^4.17.11",
     "prop-types": "*",
     "react": "^17.0.2",

superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/Styles.js

@@ -115,6 +115,10 @@ export const Styles = styled.div`
       vertical-align: baseline;
     }
 
+    table.pvtTable tbody tr th.pvtRowLabel.pvtRowLabelLast {
+      border-bottom: 1px solid ${theme.colorSplit};
+    }
+
     .pvtTotal,
     .pvtGrandTotal {
       font-weight: ${theme.fontWeightStrong};

superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.jsx

@@ -609,6 +609,7 @@ export class TableRenderer extends Component {
       rowTotalCallbacks,
       namesMapping,
       allowRenderHtml,
+      visibleRowCount,
     } = pivotSettings;
 
     const {
@@ -650,14 +651,20 @@ export class TableRenderer extends Component {
           ? this.toggleRowKey(flatRowKey)
           : null;
 
+        const isLastRow = rowIdx + rowSpan === visibleRowCount;
+        let cellClassName = valueCellClassName;
+        if (isLastRow) {
+          cellClassName += ' pvtRowLabelLast';
+        }
+
         const headerCellFormattedValue =
           dateFormatters && dateFormatters[rowAttrs[i]]
             ? dateFormatters[rowAttrs[i]](r)
             : r;
         return (
           <th
             key={`rowKeyLabel-${i}`}
-            className={valueCellClassName}
+            className={cellClassName}
             rowSpan={rowSpan}
             colSpan={colSpan}
             role="columnheader button"
@@ -916,6 +923,7 @@ export class TableRenderer extends Component {
       maxColVisible: Math.max(...visibleColKeys.map(k => k.length)),
       rowAttrSpans: this.calcAttrSpans(visibleRowKeys, rowAttrs.length),
       colAttrSpans: this.calcAttrSpans(visibleColKeys, colAttrs.length),
+      visibleRowCount: visibleRowKeys.length + (colTotals ? 1 : 0),
       allowRenderHtml,
       ...this.cachedBasePivotSettings,
     };

superset-frontend/plugins/plugin-chart-pivot-table/test/TableRenderer.test.tsx

@@ -0,0 +1,91 @@
+/**
+ * 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 '@testing-library/jest-dom';
+import { render } from '@testing-library/react';
+import PivotTableChart from '../src/PivotTableChart';
+import transformProps from '../src/plugin/transformProps';
+import testData from './testData';
+import { ProviderWrapper } from './testHelpers';
+
+test('applies pvtRowLabelLast class to last data row when colTotals is disabled', () => {
+  const transformedProps = {
+    ...transformProps(testData.withoutColTotals),
+    margin: 32,
+    legacy_order_by: null,
+    order_desc: false,
+  };
+  const { container } = render(
+    ProviderWrapper({
+      children: <PivotTableChart {...transformedProps} />,
+    }),
+  );
+
+  const tableBody = container.querySelector('tbody');
+  const dataRows = Array.from(tableBody?.querySelectorAll('tr') ?? []).filter(
+    row => !row.classList.contains('pvtRowTotals'),
+  );
+
+  // Get the last data row
+  const lastDataRow = dataRows[dataRows.length - 1];
+  expect(lastDataRow).toBeInTheDocument();
+
+  // Check if any cell in the last data row has pvtRowLabelLast class
+  const lastRowCells = lastDataRow.querySelectorAll('th.pvtRowLabel');
+  const hasLastClass = Array.from(lastRowCells).some(cell =>
+    cell.classList.contains('pvtRowLabelLast'),
+  );
+
+  expect(hasLastClass).toBe(true);
+});
+
+test('does not apply pvtRowLabelLast class to last data row when colTotals is enabled', () => {
+  const transformedProps = {
+    ...transformProps(testData.withColTotals),
+    margin: 32,
+    legacy_order_by: null,
+    order_desc: false,
+  };
+  const { container } = render(
+    ProviderWrapper({
+      children: <PivotTableChart {...transformedProps} />,
+    }),
+  );
+
+  const tableBody = container.querySelector('tbody');
+  const dataRows = Array.from(tableBody?.querySelectorAll('tr') ?? []).filter(
+    row => !row.classList.contains('pvtRowTotals'),
+  );
+
+  // Get the last data row (before totals row)
+  const lastDataRow = dataRows[dataRows.length - 1];
+  expect(lastDataRow).toBeInTheDocument();
+
+  // Check if any cell in the last data row has pvtRowLabelLast class
+  const lastRowCells = lastDataRow.querySelectorAll('th.pvtRowLabel');
+  const hasLastClass = Array.from(lastRowCells).some(cell =>
+    cell.classList.contains('pvtRowLabelLast'),
+  );
+
+  // Should NOT have the class because totals row will have the border
+  expect(hasLastClass).toBe(false);
+
+  // Verify totals row exists
+  const totalsRow = container.querySelector('tr.pvtRowTotals');
+  expect(totalsRow).toBeInTheDocument();
+});

superset-frontend/plugins/plugin-chart-pivot-table/test/testData.ts

@@ -0,0 +1,156 @@
+/**
+ * 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 {
+  ChartDataResponseResult,
+  ChartProps,
+  DatasourceType,
+  VizType,
+  QueryFormData,
+} from '@superset-ui/core';
+import { supersetTheme } from '@apache-superset/core/ui';
+import { GenericDataType } from '@apache-superset/core/api/core';
+
+const basicFormData: QueryFormData = {
+  datasource: '1__table',
+  viz_type: VizType.PivotTable,
+  groupbyRows: ['country'],
+  groupbyColumns: ['city'],
+  metrics: ['SUM(sales)'],
+  metricsLayout: 'COLUMNS',
+  rowOrder: 'key_a_to_z',
+  colOrder: 'key_a_to_z',
+  aggregateFunction: 'Sum',
+  rowSubTotals: false,
+  colTotals: true,
+  colSubTotals: false,
+  rowTotals: true,
+  valueFormat: 'SMART_NUMBER',
+  dateFormat: 'smart_date',
+  transposePivot: false,
+  combineMetric: false,
+  rowSubtotalPosition: false,
+  colSubtotalPosition: false,
+};
+
+const basicChartProps = {
+  width: 800,
+  height: 600,
+  datasource: {
+    id: 1,
+    name: 'test_dataset',
+    type: DatasourceType.Table,
+    columns: [],
+    metrics: [],
+    columnFormats: {},
+    verboseMap: {},
+  },
+  hooks: {},
+  initialValues: {},
+  queriesData: [
+    {
+      data: {
+        columns: [],
+        records: [],
+      },
+    },
+  ],
+  formData: basicFormData,
+  theme: supersetTheme,
+};
+
+const basicQueryResult: ChartDataResponseResult = {
+  annotation_data: null,
+  cache_key: null,
+  cached_dttm: null,
+  cache_timeout: null,
+  data: [],
+  colnames: [],
+  coltypes: [],
+  error: null,
+  is_cached: false,
+  query: 'SELECT ...',
+  rowcount: 100,
+  sql_rowcount: 100,
+  stacktrace: null,
+  status: 'success',
+  from_dttm: null,
+  to_dttm: null,
+};
+
+// Shared test data
+const pivotData = [
+  { country: 'France', city: 'Paris', 'SUM(sales)': 1000 },
+  { country: 'Germany', city: 'Berlin', 'SUM(sales)': 2000 },
+  { country: 'Spain', city: 'Madrid', 'SUM(sales)': 1500 },
+  { country: 'Italy', city: 'Rome', 'SUM(sales)': 3000 },
+  { country: 'UK', city: 'London', 'SUM(sales)': 2500 },
+];
+
+// Shared query result structure
+const basicQueriesData = [
+  {
+    ...basicQueryResult,
+    colnames: ['country', 'city', 'SUM(sales)'],
+    coltypes: [
+      GenericDataType.String,
+      GenericDataType.String,
+      GenericDataType.Numeric,
+    ],
+    data: pivotData,
+  },
+];
+
+/**
+ * Pivot table data with colTotals enabled
+ */
+const withColTotals = {
+  ...new ChartProps({
+    ...basicChartProps,
+    formData: {
+      ...basicFormData,
+      colTotals: true,
+      rowTotals: true,
+      rowSubTotals: false,
+      colSubTotals: false,
+    },
+  }),
+  queriesData: basicQueriesData,
+};
+
+/**
+ * Pivot table data without colTotals
+ */
+const withoutColTotals = {
+  ...new ChartProps({
+    ...basicChartProps,
+    formData: {
+      ...basicFormData,
+      colTotals: false,
+      rowTotals: false,
+      rowSubTotals: false,
+      colSubTotals: false,
+    },
+  }),
+  queriesData: basicQueriesData,
+};
+
+export default {
+  withColTotals,
+  withoutColTotals,
+};

superset-frontend/plugins/plugin-chart-pivot-table/test/testHelpers.tsx

@@ -0,0 +1,37 @@
+/**
+ * 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 {
+  supersetTheme,
+  ThemeProvider,
+  EmotionCacheProvider,
+  createEmotionCache,
+} from '@apache-superset/core/ui';
+
+const emotionCache = createEmotionCache({
+  key: 'test',
+});
+
+export function ProviderWrapper(props: any) {
+  const { children, theme = supersetTheme } = props;
+  return (
+    <EmotionCacheProvider value={emotionCache}>
+      <ThemeProvider theme={theme}>{children}</ThemeProvider>
+    </EmotionCacheProvider>
+  );
+}

superset-frontend/plugins/plugin-chart-pivot-table/test/tsconfig.json

@@ -0,0 +1,9 @@
+{
+  "compilerOptions": {
+    "composite": false,
+    "emitDeclarationOnly": false,
+    "rootDir": "../../../"
+  },
+  "extends": "../../../tsconfig.json",
+  "include": ["**/*", "../types/**/*", "../../../types/**/*"]
+}