Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

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

Merged
merged 2 commits into from
Mar 8, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7a585af
fix(plugin-chart-table): ignore duplicate percent metrics
ktmud Mar 8, 2021
3ac5dab
Add test
ktmud Mar 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions plugins/plugin-chart-table/src/buildQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
{
Expand Down Expand Up @@ -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 };
Expand All @@ -146,4 +146,4 @@ const cachedBuildQuery = (): BuildQuery<TableChartFormData> => {
);
};

export default cachedBuildQuery;
export default cachedBuildQuery();
Copy link
Contributor Author

@ktmud ktmud Mar 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also changed how the default buildQuery function is exported.

cc @simcha90

6 changes: 3 additions & 3 deletions plugins/plugin-chart-table/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -41,7 +41,7 @@ export default class TableChartPlugin extends ChartPlugin<TableChartFormData, Ta
metadata,
transformProps,
controlPanel,
buildQuery: cachedBuildQuery() as BuildQueryFunction<TableChartFormData>,
buildQuery,
});
}
}
22 changes: 11 additions & 11 deletions plugins/plugin-chart-table/test/buildQuery.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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'],
Expand All @@ -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'],
Expand Down