From 152e799abea5ccf5d34c617338c482a200dc433c Mon Sep 17 00:00:00 2001 From: Antonio Rivero <38889534+Antonio-RiveroMartnez@users.noreply.github.com> Date: Mon, 6 Nov 2023 18:51:28 +0100 Subject: [PATCH] fix(charts): Time grain is None when dataset uses Jinja (#25842) --- .../src/query/normalizeTimeColumn.ts | 5 +---- .../test/query/normalizeTimeColumn.test.ts | 8 +++++--- .../test/MixedTimeseries/buildQuery.test.ts | 1 + .../test/Timeseries/buildQuery.test.ts | 4 ++-- .../src/plugin/buildQuery.ts | 6 +----- .../test/plugin/buildQuery.test.ts | 13 +++++++++++++ 6 files changed, 23 insertions(+), 14 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/src/query/normalizeTimeColumn.ts b/superset-frontend/packages/superset-ui-core/src/query/normalizeTimeColumn.ts index f7ea0d0e6006..5fb9590c685a 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/normalizeTimeColumn.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/normalizeTimeColumn.ts @@ -70,10 +70,7 @@ export function normalizeTimeColumn( }; } - const newQueryObject = omit(queryObject, [ - 'extras.time_grain_sqla', - 'is_timeseries', - ]); + const newQueryObject = omit(queryObject, ['is_timeseries']); newQueryObject.columns = mutatedColumns; return newQueryObject; diff --git a/superset-frontend/packages/superset-ui-core/test/query/normalizeTimeColumn.test.ts b/superset-frontend/packages/superset-ui-core/test/query/normalizeTimeColumn.test.ts index 22189b90551c..2f6bdd0fb507 100644 --- a/superset-frontend/packages/superset-ui-core/test/query/normalizeTimeColumn.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/query/normalizeTimeColumn.test.ts @@ -92,7 +92,9 @@ describe('GENERIC_CHART_AXES is disabled', () => { datasource: '5__table', viz_type: 'table', granularity: 'time_column', - extras: {}, + extras: { + time_grain_sqla: 'P1Y', + }, time_range: '1 year ago : 2013', orderby: [['count(*)', true]], columns: [ @@ -182,7 +184,7 @@ describe('GENERIC_CHART_AXES is enabled', () => { datasource: '5__table', viz_type: 'table', granularity: 'time_column', - extras: { where: '', having: '' }, + extras: { where: '', having: '', time_grain_sqla: 'P1Y' }, time_range: '1 year ago : 2013', orderby: [['count(*)', true]], columns: [ @@ -240,7 +242,7 @@ describe('GENERIC_CHART_AXES is enabled', () => { datasource: '5__table', viz_type: 'table', granularity: 'time_column', - extras: { where: '', having: '' }, + extras: { where: '', having: '', time_grain_sqla: 'P1Y' }, time_range: '1 year ago : 2013', orderby: [['count(*)', true]], columns: [ diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts index 15d9e2cbb6e1..7aef4f582a32 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts @@ -307,6 +307,7 @@ test('should convert a queryObject with x-axis although FF is disabled', () => { extras: { having: '', where: "(foo in ('a', 'b'))", + time_grain_sqla: 'P1W', }, applied_time_extras: {}, columns: [ diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/buildQuery.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/buildQuery.test.ts index 611f021af16f..8887342933ca 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/buildQuery.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/buildQuery.test.ts @@ -120,7 +120,7 @@ describe('GENERIC_CHART_AXES is enabled', () => { expect.objectContaining({ granularity: 'time_column', time_range: '1 year ago : 2013', - extras: { having: '', where: '' }, + extras: { having: '', where: '', time_grain_sqla: 'P1Y' }, columns: [ { columnType: 'BASE_AXIS', @@ -209,7 +209,7 @@ describe('GENERIC_CHART_AXES is disabled', () => { expect.objectContaining({ granularity: 'time_column', time_range: '1 year ago : 2013', - extras: { having: '', where: '' }, + extras: { having: '', where: '', time_grain_sqla: 'P1Y' }, columns: [ { columnType: 'BASE_AXIS', diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/buildQuery.ts b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/buildQuery.ts index 55a6cefd083a..4cd314e06fdc 100644 --- a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/buildQuery.ts @@ -16,8 +16,6 @@ * specific language governing permissions and limitations * under the License. */ -import omit from 'lodash/omit'; - import { AdhocColumn, buildQueryContext, @@ -72,9 +70,7 @@ export default function buildQuery(formData: PivotTableQueryFormData) { } return [ { - ...(hasGenericChartAxes - ? omit(baseQueryObject, ['extras.time_grain_sqla']) - : baseQueryObject), + ...baseQueryObject, orderby: orderBy, columns, }, diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/test/plugin/buildQuery.test.ts b/superset-frontend/plugins/plugin-chart-pivot-table/test/plugin/buildQuery.test.ts index 0e29de919652..bff468232ff6 100644 --- a/superset-frontend/plugins/plugin-chart-pivot-table/test/plugin/buildQuery.test.ts +++ b/superset-frontend/plugins/plugin-chart-pivot-table/test/plugin/buildQuery.test.ts @@ -121,3 +121,16 @@ test('should fallback to formData.time_grain_sqla if extra_form_data.time_grain_ expressionType: 'SQL', }); }); + +test('should not omit extras.time_grain_sqla from queryContext so dashboards apply them', () => { + 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.extras?.time_grain_sqla).toEqual(TimeGranularity.QUARTER); +});