Skip to content

Commit

Permalink
refactor(standardized form data): refine interface and improve code s…
Browse files Browse the repository at this point in the history
…mells (apache#20518)
  • Loading branch information
zhaoyongjie authored and akshatsri committed Jul 19, 2022
1 parent aaf09f3 commit 0af966c
Show file tree
Hide file tree
Showing 34 changed files with 579 additions and 223 deletions.
28 changes: 17 additions & 11 deletions superset-frontend/packages/superset-ui-chart-controls/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,30 +344,36 @@ export interface ControlPanelSectionConfig {
controlSetRows: ControlSetRow[];
}

export interface StandardizedState {
export interface StandardizedControls {
metrics: QueryFormMetric[];
columns: QueryFormColumn[];
}

export interface StandardizedFormDataInterface {
standardizedState: StandardizedState;
// Controls not used in the current viz
controls: StandardizedControls;
// Transformation history
memorizedFormData: Map<string, QueryFormData>;
}

export type QueryStandardizedFormData = QueryFormData & {
standardizedFormData: StandardizedFormDataInterface;
};

export const isStandardizedFormData = (
formData: QueryFormData,
): formData is QueryStandardizedFormData =>
formData?.standardizedFormData?.controls &&
formData?.standardizedFormData?.memorizedFormData &&
Array.isArray(formData.standardizedFormData.controls.metrics) &&
Array.isArray(formData.standardizedFormData.controls.columns);

export interface ControlPanelConfig {
controlPanelSections: (ControlPanelSectionConfig | null)[];
controlOverrides?: ControlOverrides;
sectionOverrides?: SectionOverrides;
onInit?: (state: ControlStateMapping) => void;
denormalizeFormData?: (
formData: QueryFormData & {
standardizedFormData: StandardizedFormDataInterface;
},
) => QueryFormData;
updateStandardizedState?: (
prevState: StandardizedState,
currState: StandardizedState,
) => StandardizedState;
formDataOverrides?: (formData: QueryFormData) => QueryFormData;
}

export type ControlOverrides = {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/**
* 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 { makeSingleton, QueryFormData } from '@superset-ui/core';
import { isStandardizedFormData, StandardizedControls } from '../types';

class StandardizedControlsManager {
controls: StandardizedControls;

constructor() {
this.controls = {
metrics: [],
columns: [],
};
}

setStandardizedControls(formData: QueryFormData) {
if (isStandardizedFormData(formData)) {
const { controls } = formData.standardizedFormData;
this.controls = {
metrics: controls.metrics,
columns: controls.columns,
};
}
}

shiftMetric() {
return this.controls.metrics.shift();
}

popAllMetrics() {
return this.controls.metrics.splice(0, this.controls.metrics.length);
}

popAllColumns() {
return this.controls.columns.splice(0, this.controls.columns.length);
}

clear() {
this.controls = {
metrics: [],
columns: [],
};
}
}

export const getStandardizedControls = makeSingleton(
StandardizedControlsManager,
);
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ export * from './getColorFormatters';
export { default as mainMetric } from './mainMetric';
export { default as columnChoices } from './columnChoices';
export * from './defineSavedMetrics';
export * from './getStandardizedControls';
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/**
* 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 { QueryFormData } from '@superset-ui/core';
import { getStandardizedControls } from '../../src';

const formData: QueryFormData = {
datasource: '30__table',
viz_type: 'table',
standardizedFormData: {
controls: {
metrics: ['count(*)', 'sum(sales)'],
columns: ['gender', 'gender'],
},
memorizedFormData: [],
},
};

test('without standardizedFormData', () => {
getStandardizedControls().setStandardizedControls({
datasource: '30__table',
viz_type: 'table',
});
expect(getStandardizedControls().controls).toEqual({
metrics: [],
columns: [],
});
});

test('getStandardizedControls', () => {
expect(getStandardizedControls().controls).toEqual({
metrics: [],
columns: [],
});
getStandardizedControls().setStandardizedControls(formData);
expect(getStandardizedControls().controls).toEqual({
metrics: ['count(*)', 'sum(sales)'],
columns: ['gender', 'gender'],
});
expect(getStandardizedControls().shiftMetric()).toEqual('count(*)');
expect(getStandardizedControls().controls).toEqual({
metrics: ['sum(sales)'],
columns: ['gender', 'gender'],
});
expect(getStandardizedControls().popAllMetrics()).toEqual(['sum(sales)']);
expect(getStandardizedControls().controls).toEqual({
metrics: [],
columns: ['gender', 'gender'],
});
expect(getStandardizedControls().popAllColumns()).toEqual([
'gender',
'gender',
]);
expect(getStandardizedControls().controls).toEqual({
metrics: [],
columns: [],
});

getStandardizedControls().setStandardizedControls(formData);
getStandardizedControls().clear();
expect(getStandardizedControls().controls).toEqual({
metrics: [],
columns: [],
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
*/

import { GenericDataType } from './QueryResponse';
import { QueryFormColumn } from './QueryFormData';

export interface AdhocColumn {
hasCustomLabel?: boolean;
Expand Down Expand Up @@ -53,12 +54,18 @@ export interface Column {

export default {};

export function isPhysicalColumn(
column?: AdhocColumn | PhysicalColumn,
): column is PhysicalColumn {
export function isPhysicalColumn(column?: any): column is PhysicalColumn {
return typeof column === 'string';
}

export function isAdhocColumn(column?: AdhocColumn | PhysicalColumn) {
return (column as AdhocColumn)?.sqlExpression !== undefined;
export function isAdhocColumn(column?: any): column is AdhocColumn {
return (
typeof column !== 'string' &&
column?.sqlExpression !== undefined &&
column?.expressionType === 'SQL'
);
}

export function isQueryFormColumn(column: any): column is QueryFormColumn {
return isPhysicalColumn(column) || isAdhocColumn(column);
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import { Maybe } from '../../types';
import { Maybe, QueryFormMetric } from '../../types';
import { Column } from './Column';

export type Aggregate =
Expand Down Expand Up @@ -74,8 +74,22 @@ export interface Metric {

export default {};

export function isAdhocMetricSimple(
metric: AdhocMetric,
): metric is AdhocMetricSimple {
return metric.expressionType === 'SIMPLE';
export function isSavedMetric(metric: any): metric is SavedMetric {
return typeof metric === 'string';
}

export function isAdhocMetricSimple(metric: any): metric is AdhocMetricSimple {
return typeof metric !== 'string' && metric?.expressionType === 'SIMPLE';
}

export function isAdhocMetricSQL(metric: any): metric is AdhocMetricSQL {
return typeof metric !== 'string' && metric?.expressionType === 'SQL';
}

export function isQueryFormMetric(metric: any): metric is QueryFormMetric {
return (
isSavedMetric(metric) ||
isAdhocMetricSimple(metric) ||
isAdhocMetricSQL(metric)
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,4 @@ export function isDruidFormData(
return 'granularity' in formData;
}

export function isSavedMetric(metric: QueryFormMetric): metric is SavedMetric {
return typeof metric === 'string';
}

export default {};
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/**
* 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 {
isAdhocColumn,
isPhysicalColumn,
isQueryFormColumn,
} from '@superset-ui/core';

const adhocColumn = {
expressionType: 'SQL',
label: 'country',
optionName: 'country',
sqlExpression: 'country',
};

test('isPhysicalColumn returns true', () => {
expect(isPhysicalColumn('gender')).toEqual(true);
});

test('isPhysicalColumn returns false', () => {
expect(isPhysicalColumn(adhocColumn)).toEqual(false);
});

test('isAdhocColumn returns true', () => {
expect(isAdhocColumn(adhocColumn)).toEqual(true);
});

test('isAdhocColumn returns false', () => {
expect(isAdhocColumn('hello')).toEqual(false);
expect(isAdhocColumn({})).toEqual(false);
expect(
isAdhocColumn({
expressionType: 'SQL',
label: 'country',
optionName: 'country',
}),
).toEqual(false);
});

test('isQueryFormColumn returns true', () => {
expect(isQueryFormColumn('gender')).toEqual(true);
expect(isQueryFormColumn(adhocColumn)).toEqual(true);
});

test('isQueryFormColumn returns false', () => {
expect(isQueryFormColumn({})).toEqual(false);
});
Loading

0 comments on commit 0af966c

Please sign in to comment.