Skip to content

Commit

Permalink
fix: Dashboard time grain in Pivot Table (#24665)
Browse files Browse the repository at this point in the history
  • Loading branch information
@michael-s-molina
michael-s-molina authored Jul 13, 2023
1 parent a156816 commit 6e59f11
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 59 deletions.
9 changes: 6 additions & 3 deletions superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/buildQuery.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ import {
import { PivotTableQueryFormData } from '../types';

export default function buildQuery(formData: PivotTableQueryFormData) {
const { groupbyColumns = [], groupbyRows = [] } = formData;
const { groupbyColumns = [], groupbyRows = [], extra_form_data } = formData;
const time_grain_sqla =
extra_form_data?.time_grain_sqla || formData.time_grain_sqla;

// TODO: add deduping of AdhocColumns
const columns = Array.from(
new Set([
Expand All @@ -40,7 +43,7 @@ export default function buildQuery(formData: PivotTableQueryFormData) {
).map(col => {
if (
isPhysicalColumn(col) &&
formData.time_grain_sqla &&
time_grain_sqla &&
hasGenericChartAxes &&
/* Charts created before `GENERIC_CHART_AXES` is enabled have a different
* form data, with `granularity_sqla` set instead.
Expand All @@ -49,7 +52,7 @@ export default function buildQuery(formData: PivotTableQueryFormData) {
formData.granularity_sqla === col)
) {
return {
timeGrain: formData.time_grain_sqla,
timeGrain: time_grain_sqla,
columnType: 'BASE_AXIS',
sqlExpression: col,
label: col,
Expand Down
146 changes: 90 additions & 56 deletions superset-frontend/plugins/plugin-chart-pivot-table/test/plugin/buildQuery.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,65 +22,99 @@ import * as supersetCoreModule from '@superset-ui/core';
import buildQuery from '../../src/plugin/buildQuery';
import { PivotTableQueryFormData } from '../../src/types';

describe('PivotTableChart buildQuery', () => {
const formData: PivotTableQueryFormData = {
groupbyRows: ['row1', 'row2'],
groupbyColumns: ['col1', 'col2'],
metrics: ['metric1', 'metric2'],
tableRenderer: 'Table With Subtotal',
colOrder: 'key_a_to_z',
rowOrder: 'key_a_to_z',
aggregateFunction: 'Sum',
transposePivot: true,
rowSubtotalPosition: true,
colSubtotalPosition: true,
colTotals: true,
rowTotals: true,
valueFormat: 'SMART_NUMBER',
datasource: '5__table',
viz_type: 'my_chart',
width: 800,
height: 600,
combineMetric: false,
verboseMap: {},
columnFormats: {},
currencyFormats: {},
metricColorFormatters: [],
dateFormatters: {},
setDataMask: () => {},
legacy_order_by: 'count',
order_desc: true,
margin: 0,
const formData: PivotTableQueryFormData = {
groupbyRows: ['row1', 'row2'],
groupbyColumns: ['col1', 'col2'],
metrics: ['metric1', 'metric2'],
tableRenderer: 'Table With Subtotal',
colOrder: 'key_a_to_z',
rowOrder: 'key_a_to_z',
aggregateFunction: 'Sum',
transposePivot: true,
rowSubtotalPosition: true,
colSubtotalPosition: true,
colTotals: true,
rowTotals: true,
valueFormat: 'SMART_NUMBER',
datasource: '5__table',
viz_type: 'my_chart',
width: 800,
height: 600,
combineMetric: false,
verboseMap: {},
columnFormats: {},
currencyFormats: {},
metricColorFormatters: [],
dateFormatters: {},
setDataMask: () => {},
legacy_order_by: 'count',
order_desc: true,
margin: 0,
time_grain_sqla: TimeGranularity.MONTH,
temporal_columns_lookup: { col1: true },
};

test('should build groupby with series in form data', () => {
const queryContext = buildQuery(formData);
const [query] = queryContext.queries;
expect(query.columns).toEqual(['col1', 'col2', 'row1', 'row2']);
});

test('should work with old charts after GENERIC_CHART_AXES is enabled', () => {
Object.defineProperty(supersetCoreModule, 'hasGenericChartAxes', {
value: true,
});
const modifiedFormData = {
...formData,
time_grain_sqla: TimeGranularity.MONTH,
granularity_sqla: 'col1',
};
const queryContext = buildQuery(modifiedFormData);
const [query] = queryContext.queries;
expect(query.columns).toEqual([
{
timeGrain: 'P1M',
columnType: 'BASE_AXIS',
sqlExpression: 'col1',
label: 'col1',
expressionType: 'SQL',
},
'col2',
'row1',
'row2',
]);
});

it('should build groupby with series in form data', () => {
const queryContext = buildQuery(formData);
const [query] = queryContext.queries;
expect(query.columns).toEqual(['col1', 'col2', 'row1', 'row2']);
test('should prefer extra_form_data.time_grain_sqla over formData.time_grain_sqla', () => {
Object.defineProperty(supersetCoreModule, 'hasGenericChartAxes', {
value: true,
});
const modifiedFormData = {
...formData,
extra_form_data: { time_grain_sqla: TimeGranularity.QUARTER },
};
const queryContext = buildQuery(modifiedFormData);
const [query] = queryContext.queries;
expect(query.columns?.[0]).toEqual({
timeGrain: TimeGranularity.QUARTER,
columnType: 'BASE_AXIS',
sqlExpression: 'col1',
label: 'col1',
expressionType: 'SQL',
});
});

it('should work with old charts after GENERIC_CHART_AXES is enabled', () => {
Object.defineProperty(supersetCoreModule, 'hasGenericChartAxes', {
value: true,
});
const modifiedFormData = {
...formData,
time_grain_sqla: TimeGranularity.MONTH,
granularity_sqla: 'col1',
};
const queryContext = buildQuery(modifiedFormData);
const [query] = queryContext.queries;
expect(query.columns).toEqual([
{
timeGrain: 'P1M',
columnType: 'BASE_AXIS',
sqlExpression: 'col1',
label: 'col1',
expressionType: 'SQL',
},
'col2',
'row1',
'row2',
]);
test('should fallback to formData.time_grain_sqla if extra_form_data.time_grain_sqla is not set', () => {
Object.defineProperty(supersetCoreModule, 'hasGenericChartAxes', {
value: true,
});
const queryContext = buildQuery(formData);
const [query] = queryContext.queries;
expect(query.columns?.[0]).toEqual({
timeGrain: formData.time_grain_sqla,
columnType: 'BASE_AXIS',
sqlExpression: 'col1',
label: 'col1',
expressionType: 'SQL',
});
});

0 comments on commit 6e59f11

Please sign in to comment.