fix(plugin-chart-table): ignore duplicate percent metrics (#994)

apache · Nov 26, 2021 · 69a5c90 · 69a5c90
1 parent 2625698
commit 69a5c90
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 17 deletions.
diff --git a/...t-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/buildQuery.ts b/...t-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/buildQuery.ts
@@ -73,7 +73,7 @@ const buildQuery: BuildQuery<TableChartFormData> = (formData: TableChartFormData
       }
       // add postprocessing for percent metrics only when in aggregation mode
       if (percentMetrics && percentMetrics.length > 0) {
-        const percentMetricLabels = percentMetrics.map(getMetricLabel);
+        const percentMetricLabels = removeDuplicates(percentMetrics.map(getMetricLabel));
         metrics = removeDuplicates(metrics.concat(percentMetrics), getMetricLabel);
         postProcessing = [
           {
@@ -126,7 +126,7 @@ const buildQuery: BuildQuery<TableChartFormData> = (formData: TableChartFormData
 
 // Use this closure to cache changing of external filters, if we have server pagination we need reset page to 0, after
 // external filter changed
-const cachedBuildQuery = (): BuildQuery<TableChartFormData> => {
+export const cachedBuildQuery = (): BuildQuery<TableChartFormData> => {
   let cachedChanges: any = {};
   const setCachedChanges = (newChanges: any) => {
     cachedChanges = { ...cachedChanges, ...newChanges };
@@ -146,4 +146,4 @@ const cachedBuildQuery = (): BuildQuery<TableChartFormData> => {
     );
 };
 
-export default cachedBuildQuery;
+export default cachedBuildQuery();
diff --git a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/index.ts b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/index.ts
@@ -16,11 +16,11 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { t, ChartMetadata, ChartPlugin, BuildQueryFunction } from '@superset-ui/core';
+import { t, ChartMetadata, ChartPlugin } from '@superset-ui/core';
 import transformProps from './transformProps';
 import thumbnail from './images/thumbnail.png';
 import controlPanel from './controlPanel';
-import cachedBuildQuery from './buildQuery';
+import buildQuery from './buildQuery';
 import { TableChartFormData, TableChartProps } from './types';
 
 // must export something for the module to be exist in dev mode
@@ -41,7 +41,7 @@ export default class TableChartPlugin extends ChartPlugin<TableChartFormData, Ta
       metadata,
       transformProps,
       controlPanel,
-      buildQuery: cachedBuildQuery() as BuildQueryFunction<TableChartFormData>,
+      buildQuery,
     });
   }
 }
diff --git a/...tend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/test/buildQuery.test.ts b/...tend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/test/buildQuery.test.ts
@@ -17,7 +17,7 @@
  * under the License.
  */
 import { QueryMode } from '@superset-ui/core';
-import buildQueryCached from '../src/buildQuery';
+import buildQuery from '../src/buildQuery';
 import { TableChartFormData } from '../src/types';
 
 const basicFormData: TableChartFormData = {
@@ -27,27 +27,27 @@ const basicFormData: TableChartFormData = {
 
 describe('plugin-chart-table', () => {
   describe('buildQuery', () => {
-    it('should add post-processing in aggregate mode', () => {
-      const query = buildQueryCached()({
+    it('should add post-processing and ignore duplicate metrics', () => {
+      const query = buildQuery({
         ...basicFormData,
         query_mode: QueryMode.aggregate,
-        metrics: ['aaa'],
-        percent_metrics: ['pct_abc'],
+        metrics: ['aaa', 'aaa'],
+        percent_metrics: ['bbb', 'bbb'],
       }).queries[0];
-      expect(query.metrics).toEqual(['aaa', 'pct_abc']);
+      expect(query.metrics).toEqual(['aaa', 'bbb']);
       expect(query.post_processing).toEqual([
         {
           operation: 'contribution',
           options: {
-            columns: ['pct_abc'],
-            rename_columns: ['%pct_abc'],
+            columns: ['bbb'],
+            rename_columns: ['%bbb'],
           },
         },
       ]);
     });
 
-    it('should not add post-processing when there is not percent metric', () => {
-      const query = buildQueryCached()({
+    it('should not add post-processing when there is no percent metric', () => {
+      const query = buildQuery({
         ...basicFormData,
         query_mode: QueryMode.aggregate,
         metrics: ['aaa'],
@@ -58,7 +58,7 @@ describe('plugin-chart-table', () => {
     });
 
     it('should not add post-processing in raw records mode', () => {
-      const query = buildQueryCached()({
+      const query = buildQuery({
         ...basicFormData,
         query_mode: QueryMode.raw,
         metrics: ['aaa'],