Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: switch form_data between different datasource #20867

Merged
merged 1 commit into from
Jul 26, 2022
Merged

fix: switch form_data between different datasource #20867

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
261 changes: 151 additions & 110 deletions superset-frontend/src/explore/controlUtils/standardizedFormData.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,12 @@ const adhocColumn: AdhocColumn = {
optionName: 'country',
sqlExpression: 'country',
};

const adhocMetricSQL: AdhocMetricSQL = {
expressionType: 'SQL',
label: 'count',
optionName: 'count',
sqlExpression: 'count(*)',
};

const adhocMetricSimple: AdhocMetricSimple = {
expressionType: 'SIMPLE',
column: {
Expand All @@ -61,6 +59,114 @@ const adhocMetricSimple: AdhocMetricSimple = {
optionName: 'count',
};

const tableVizFormData = {
Copy link
Member Author

Choose a reason for hiding this comment

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

Constants just move to the top of file, no change.

datasource: '30__table',
viz_type: 'table',
granularity_sqla: 'ds',
time_grain_sqla: TimeGranularity.DAY,
time_range: 'No filter',
query_mode: 'aggregate',
groupby: ['name', 'gender', adhocColumn],
metrics: ['count', 'avg(sales)', adhocMetricSimple, adhocMetricSQL],
all_columns: [],
percent_metrics: [],
adhoc_filters: [],
order_by_cols: [],
row_limit: 10000,
server_page_length: 10,
order_desc: true,
table_timestamp_format: 'smart_date',
show_cell_bars: true,
color_pn: true,
url_params: {
form_data_key:
'p3No_sqDW7k-kMTzlBPAPd9vwp1IXTf6stbyzjlrPPa0ninvdYUUiMC6F1iKit3Y',
dataset_id: '30',
},
};
const tableVizStore = {
form_data: tableVizFormData,
controls: {
datasource: {
value: '30__table',
},
viz_type: {
value: 'table',
},
slice_id: {},
cache_timeout: {},
url_params: {
value: {
form_data_key:
'p3No_sqDW7k-kMTzlBPAPd9vwp1IXTf6stbyzjlrPPa0ninvdYUUiMC6F1iKit3Y',
dataset_id: '30',
},
},
granularity_sqla: {
value: 'ds',
},
time_grain_sqla: {
value: 'P1D',
},
time_range: {
value: 'No filter',
},
query_mode: {
value: 'aggregate',
},
groupby: {
value: ['name', 'gender', adhocColumn],
},
metrics: {
value: ['count', 'avg(sales)', adhocMetricSimple, adhocMetricSQL],
},
all_columns: {
value: [],
},
percent_metrics: {
value: [],
},
adhoc_filters: {
value: [],
},
timeseries_limit_metric: {},
order_by_cols: {
value: [],
},
server_pagination: {},
row_limit: {
value: 10000,
},
server_page_length: {
value: 10,
},
include_time: {},
order_desc: {
value: true,
},
show_totals: {},
emit_filter: {},
table_timestamp_format: {
value: 'smart_date',
},
page_length: {},
include_search: {},
show_cell_bars: {
value: true,
},
align_pn: {},
color_pn: {
value: true,
},
column_config: {},
conditional_formatting: {},
},
datasource: {
type: 'table',
columns: [],
},
};

describe('should collect control values and create SFD', () => {
const sharedKey = [...sharedMetricsKey, ...sharedColumnsKey];
const sharedControlsFormData = {
Expand Down Expand Up @@ -271,114 +377,6 @@ describe('should collect control values and create SFD', () => {
});

describe('should transform form_data between table and bigNumberTotal', () => {
const tableVizFormData = {
datasource: '30__table',
viz_type: 'table',
granularity_sqla: 'ds',
time_grain_sqla: TimeGranularity.DAY,
time_range: 'No filter',
query_mode: 'aggregate',
groupby: ['name', 'gender', adhocColumn],
metrics: ['count', 'avg(sales)', adhocMetricSimple, adhocMetricSQL],
all_columns: [],
percent_metrics: [],
adhoc_filters: [],
order_by_cols: [],
row_limit: 10000,
server_page_length: 10,
order_desc: true,
table_timestamp_format: 'smart_date',
show_cell_bars: true,
color_pn: true,
url_params: {
form_data_key:
'p3No_sqDW7k-kMTzlBPAPd9vwp1IXTf6stbyzjlrPPa0ninvdYUUiMC6F1iKit3Y',
dataset_id: '30',
},
};
const tableVizStore = {
form_data: tableVizFormData,
controls: {
datasource: {
value: '30__table',
},
viz_type: {
value: 'table',
},
slice_id: {},
cache_timeout: {},
url_params: {
value: {
form_data_key:
'p3No_sqDW7k-kMTzlBPAPd9vwp1IXTf6stbyzjlrPPa0ninvdYUUiMC6F1iKit3Y',
dataset_id: '30',
},
},
granularity_sqla: {
value: 'ds',
},
time_grain_sqla: {
value: 'P1D',
},
time_range: {
value: 'No filter',
},
query_mode: {
value: 'aggregate',
},
groupby: {
value: ['name', 'gender', adhocColumn],
},
metrics: {
value: ['count', 'avg(sales)', adhocMetricSimple, adhocMetricSQL],
},
all_columns: {
value: [],
},
percent_metrics: {
value: [],
},
adhoc_filters: {
value: [],
},
timeseries_limit_metric: {},
order_by_cols: {
value: [],
},
server_pagination: {},
row_limit: {
value: 10000,
},
server_page_length: {
value: 10,
},
include_time: {},
order_desc: {
value: true,
},
show_totals: {},
emit_filter: {},
table_timestamp_format: {
value: 'smart_date',
},
page_length: {},
include_search: {},
show_cell_bars: {
value: true,
},
align_pn: {},
color_pn: {
value: true,
},
column_config: {},
conditional_formatting: {},
},
datasource: {
type: 'table',
columns: [],
},
};

beforeAll(() => {
getChartControlPanelRegistry().registerValue(
'big_number_total',
Expand Down Expand Up @@ -445,3 +443,46 @@ describe('should transform form_data between table and bigNumberTotal', () => {
expect(tblFormData.time_range).toBe('2021 : 2022');
});
});

describe('initial SFD between different datasource', () => {
beforeAll(() => {
getChartControlPanelRegistry().registerValue(
'big_number_total',
new BigNumberTotalChartPlugin().controlPanel,
);
getChartControlPanelRegistry().registerValue(
'table',
new TableChartPlugin().controlPanel,
);
});

test('initial SFD between different datasource', () => {
const sfd = new StandardizedFormData(tableVizFormData);
// table -> big number
const { formData: bntFormData, controlsState: bntControlsState } =
sfd.transform('big_number_total', tableVizStore);
const sfd2 = new StandardizedFormData(bntFormData);
// big number -> table
const { formData: tblFormData } = sfd2.transform('table', {
...tableVizStore,
form_data: bntFormData,
controls: bntControlsState,
});

expect(
tblFormData.standardizedFormData.memorizedFormData.map(
(mfd: [string, QueryFormData][]) => mfd[0],
),
).toEqual(['table', 'big_number_total']);
const newDatasourceFormData = { ...tblFormData, datasource: '20__table' };
const newDatasourceSFD = new StandardizedFormData(newDatasourceFormData);
expect(
newDatasourceSFD
.serialize()
.memorizedFormData.map(([vizType]) => vizType),
).toEqual(['table']);
expect(newDatasourceSFD.get('table')).not.toHaveProperty(
'standardizedFormData',
);
});
});
38 changes: 26 additions & 12 deletions superset-frontend/src/explore/controlUtils/standardizedFormData.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import { omit } from 'lodash';
import {
ensureIsArray,
getChartControlPanelRegistry,
Expand Down Expand Up @@ -91,20 +92,33 @@ export class StandardizedFormData {
* */
const formData = Object.freeze(sourceFormData);

// generates an ordered map, the key is viz_type and the value is form_data. the last item is current viz
const memorizedFormData: Map<string, QueryFormData> = Array.isArray(
formData?.standardizedFormData?.memorizedFormData,
)
? new Map(formData.standardizedFormData.memorizedFormData)
: new Map();
// generates an ordered map, the key is viz_type and the value is form_data. the last item is current viz.
const mfd = formData?.standardizedFormData?.memorizedFormData;
const vizType = formData.viz_type;
if (memorizedFormData.has(vizType)) {
memorizedFormData.delete(vizType);
let memorizedFormData = new Map<string, QueryFormData>();
let controls: StandardizedControls;
if (
Array.isArray(mfd) &&
mfd.length > 0 &&
formData.datasource === mfd.slice(-1)[0][1]?.datasource
) {
memorizedFormData = new Map(
formData.standardizedFormData.memorizedFormData,
);
if (memorizedFormData.has(vizType)) {
memorizedFormData.delete(vizType);
}
memorizedFormData.set(vizType, formData);
controls = StandardizedFormData.getStandardizedControls(formData);
} else {
// reset the `memorizedFormData` if a request between different datasource.
const restFormData = omit(
formData,
'standardizedFormData',
) as QueryFormData;
memorizedFormData.set(vizType, restFormData);
controls = StandardizedFormData.getStandardizedControls(restFormData);
}
memorizedFormData.set(vizType, formData);

// calculate sharedControls
const controls = StandardizedFormData.getStandardizedControls(formData);

this.sfd = {
controls,
Expand Down