From c09f8f6f7665e503a376926700fa815add6892ca Mon Sep 17 00:00:00 2001 From: ethan-l-geotab <165913720+ethan-l-geotab@users.noreply.github.com> Date: Fri, 30 May 2025 18:57:33 -0400 Subject: [PATCH] fix(sqllab): save datasets with template parameters (#33195) --- .../src/SqlLab/actions/sqlLab.js | 4 +- .../SaveDatasetModal.test.tsx | 85 +++++++++++++++++++ .../components/SaveDatasetModal/index.tsx | 48 +++++++++-- .../src/utils/datasourceUtils.js | 1 + superset/datasets/schemas.py | 1 + 5 files changed, 130 insertions(+), 9 deletions(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 8b54319a568..35b497468a3 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -1294,7 +1294,8 @@ export function createDatasourceFailed(err) { export function createDatasource(vizOptions) { return dispatch => { dispatch(createDatasourceStarted()); - const { dbId, catalog, schema, datasourceName, sql } = vizOptions; + const { dbId, catalog, schema, datasourceName, sql, templateParams } = + vizOptions; return SupersetClient.post({ endpoint: '/api/v1/dataset/', headers: { 'Content-Type': 'application/json' }, @@ -1306,6 +1307,7 @@ export function createDatasource(vizOptions) { table_name: datasourceName, is_managed_externally: false, external_url: null, + template_params: templateParams, }), }) .then(({ json }) => { diff --git a/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx b/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx index b1ebe60016e..7d6effb54f5 100644 --- a/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx +++ b/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx @@ -29,6 +29,7 @@ import fetchMock from 'fetch-mock'; import { SaveDatasetModal } from 'src/SqlLab/components/SaveDatasetModal'; import { createDatasource } from 'src/SqlLab/actions/sqlLab'; import { user, testQuery, mockdatasets } from 'src/SqlLab/fixtures'; +import { FeatureFlag } from '@superset-ui/core'; const mockedProps = { visible: true, @@ -250,4 +251,88 @@ describe('SaveDatasetModal', () => { templateParams: undefined, }); }); + + it('does not renders a checkbox button when template processing is disabled', () => { + render(, { useRedux: true }); + expect(screen.queryByRole('checkbox')).not.toBeInTheDocument(); + }); + + it('renders a checkbox button when template processing is enabled', () => { + // @ts-ignore + global.featureFlags = { + [FeatureFlag.EnableTemplateProcessing]: true, + }; + render(, { useRedux: true }); + expect(screen.getByRole('checkbox')).toBeInTheDocument(); + }); + + it('correctly includes template parameters when template processing is enabled', () => { + // @ts-ignore + global.featureFlags = { + [FeatureFlag.EnableTemplateProcessing]: true, + }; + const propsWithTemplateParam = { + ...mockedProps, + datasource: { + ...testQuery, + templateParams: JSON.stringify({ my_param: 12 }), + }, + }; + render(, { + useRedux: true, + }); + const inputFieldText = screen.getByDisplayValue(/unimportant/i); + fireEvent.change(inputFieldText, { target: { value: 'my dataset' } }); + + userEvent.click(screen.getByRole('checkbox')); + + const saveConfirmationBtn = screen.getByRole('button', { + name: /save/i, + }); + userEvent.click(saveConfirmationBtn); + + expect(createDatasource).toHaveBeenCalledWith({ + datasourceName: 'my dataset', + dbId: 1, + catalog: null, + schema: 'main', + sql: 'SELECT *', + templateParams: JSON.stringify({ my_param: 12 }), + }); + }); + + it('correctly excludes template parameters when template processing is enabled', () => { + // @ts-ignore + global.featureFlags = { + [FeatureFlag.EnableTemplateProcessing]: true, + }; + const propsWithTemplateParam = { + ...mockedProps, + datasource: { + ...testQuery, + templateParams: JSON.stringify({ my_param: 12 }), + }, + }; + render(, { + useRedux: true, + }); + const inputFieldText = screen.getByDisplayValue(/unimportant/i); + fireEvent.change(inputFieldText, { target: { value: 'my dataset' } }); + + userEvent.click(screen.getByRole('checkbox')); + + const saveConfirmationBtn = screen.getByRole('button', { + name: /save/i, + }); + userEvent.click(saveConfirmationBtn); + + expect(createDatasource).toHaveBeenCalledWith({ + datasourceName: 'my dataset', + dbId: 1, + catalog: null, + schema: 'main', + sql: 'SELECT *', + templateParams: undefined, + }); + }); }); diff --git a/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx b/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx index 1138b00125a..079566e50ee 100644 --- a/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx +++ b/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx @@ -24,6 +24,7 @@ import { AsyncSelect } from 'src/components'; import { Input } from 'src/components/Input'; import StyledModal from 'src/components/Modal'; import Button from 'src/components/Button'; +import Checkbox from 'src/components/Checkbox'; import { styled, t, @@ -33,6 +34,8 @@ import { QueryResponse, QueryFormData, VizType, + FeatureFlag, + isFeatureEnabled, } from '@superset-ui/core'; import { useSelector, useDispatch } from 'react-redux'; import dayjs from 'dayjs'; @@ -185,6 +188,8 @@ export const SaveDatasetModal = ({ const user = useSelector(state => state.user); const dispatch = useDispatch<(dispatch: any) => Promise>(); + const [includeTemplateParameters, setIncludeTemplateParameters] = + useState(false); const createWindow = (url: string) => { if (openWindow) { @@ -285,14 +290,21 @@ export const SaveDatasetModal = ({ // Remove the special filters entry from the templateParams // before saving the dataset. let templateParams; - if (typeof datasource?.templateParams === 'string') { - const p = JSON.parse(datasource.templateParams); - /* eslint-disable-next-line no-underscore-dangle */ - if (p._filters) { + if ( + typeof datasource?.templateParams === 'string' && + includeTemplateParameters + ) { + try { + const p = JSON.parse(datasource.templateParams); /* eslint-disable-next-line no-underscore-dangle */ - delete p._filters; - // eslint-disable-next-line no-param-reassign + if (p._filters) { + /* eslint-disable-next-line no-underscore-dangle */ + delete p._filters; + } templateParams = JSON.stringify(p); + } catch (e) { + // malformed templateParams, do not include it + templateParams = undefined; } } @@ -362,7 +374,27 @@ export const SaveDatasetModal = ({ title={t('Save or Overwrite Dataset')} onHide={onHide} footer={ - <> +
+ {isFeatureEnabled(FeatureFlag.EnableTemplateProcessing) && ( +
+ + setIncludeTemplateParameters(checked ?? false) + } + /> + + {t('Include Template Parameters')} + +
+ )} {newOrOverwrite === DatasetRadioState.SaveNew && (
} > diff --git a/superset-frontend/src/utils/datasourceUtils.js b/superset-frontend/src/utils/datasourceUtils.js index ef984044d13..144a3ff88b6 100644 --- a/superset-frontend/src/utils/datasourceUtils.js +++ b/superset-frontend/src/utils/datasourceUtils.js @@ -23,4 +23,5 @@ export const getDatasourceAsSaveableDataset = source => ({ sql: source?.sql || '', catalog: source?.catalog, schema: source?.schema, + templateParams: source?.templateParams, }); diff --git a/superset/datasets/schemas.py b/superset/datasets/schemas.py index c68d38879fa..54ce4481737 100644 --- a/superset/datasets/schemas.py +++ b/superset/datasets/schemas.py @@ -129,6 +129,7 @@ class DatasetPostSchema(Schema): external_url = fields.String(allow_none=True) normalize_columns = fields.Boolean(load_default=False) always_filter_main_dttm = fields.Boolean(load_default=False) + template_params = fields.String(allow_none=True) class DatasetPutSchema(Schema):