From 61319fd759b336992259a4e84f1459a134d55df0 Mon Sep 17 00:00:00 2001 From: Mayur Date: Fri, 7 Oct 2022 12:49:14 +0530 Subject: [PATCH] feat(sqllab): save query parameters in database (#21682) --- .../src/SqlLab/actions/sqlLab.js | 49 ++++++++++++------- .../src/SqlLab/actions/sqlLab.test.js | 30 ++++++++---- .../src/SqlLab/components/SaveQuery/index.tsx | 30 +++++------- .../src/SqlLab/components/SqlEditor/index.jsx | 8 +-- .../components/SqlEditorTabHeader/index.tsx | 2 +- superset-frontend/src/SqlLab/fixtures.ts | 12 ++--- .../src/SqlLab/reducers/sqlLab.js | 4 +- ...eb4c9d4a4ef_parameters_in_saved_queries.py | 46 +++++++++++++++++ superset/models/sql_lab.py | 1 + superset/queries/saved_queries/api.py | 10 +++- .../queries/saved_queries/api_tests.py | 1 + 11 files changed, 132 insertions(+), 61 deletions(-) create mode 100644 superset/migrations/versions/2022-10-03_17-34_deb4c9d4a4ef_parameters_in_saved_queries.py diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 5e5c530c28e2a..155e0d08c869a 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -111,8 +111,8 @@ const ERR_MSG_CANT_LOAD_QUERY = t("The query couldn't be loaded"); const queryClientMapping = { id: 'remoteId', db_id: 'dbId', - client_id: 'id', label: 'name', + template_parameters: 'templateParams', }; const queryServerMapping = invert(queryClientMapping); @@ -120,8 +120,8 @@ const queryServerMapping = invert(queryClientMapping); const fieldConverter = mapping => obj => mapKeys(obj, (value, key) => (key in mapping ? mapping[key] : key)); -const convertQueryToServer = fieldConverter(queryServerMapping); -const convertQueryToClient = fieldConverter(queryClientMapping); +export const convertQueryToServer = fieldConverter(queryServerMapping); +export const convertQueryToClient = fieldConverter(queryClientMapping); export function getUpToDateQuery(rootState, queryEditor, key) { const { @@ -903,17 +903,23 @@ export function queryEditorSetAutorun(queryEditor, autorun) { }; } -export function queryEditorSetTitle(queryEditor, name) { +export function queryEditorSetTitle(queryEditor, name, id) { return function (dispatch) { const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) ? SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), + endpoint: encodeURI(`/tabstateview/${id}`), postPayload: { label: name }, }) : Promise.resolve(); return sync - .then(() => dispatch({ type: QUERY_EDITOR_SET_TITLE, queryEditor, name })) + .then(() => + dispatch({ + type: QUERY_EDITOR_SET_TITLE, + queryEditor: { ...queryEditor, id }, + name, + }), + ) .catch(() => dispatch( addDangerToast( @@ -926,21 +932,26 @@ export function queryEditorSetTitle(queryEditor, name) { }; } -export function saveQuery(query) { +export function saveQuery(query, clientId) { + const { id, ...payload } = convertQueryToServer(query); + return dispatch => SupersetClient.post({ - endpoint: '/savedqueryviewapi/api/create', - postPayload: convertQueryToServer(query), - stringify: false, + endpoint: '/api/v1/saved_query/', + jsonPayload: convertQueryToServer(payload), }) .then(result => { - const savedQuery = convertQueryToClient(result.json.item); + const savedQuery = convertQueryToClient({ + id: result.json.id, + ...result.json.result, + }); dispatch({ type: QUERY_EDITOR_SAVED, query, + clientId, result: savedQuery, }); - dispatch(queryEditorSetTitle(query, query.name)); + dispatch(queryEditorSetTitle(query, query.name, clientId)); return savedQuery; }) .catch(() => @@ -966,16 +977,17 @@ export const addSavedQueryToTabState = }); }; -export function updateSavedQuery(query) { +export function updateSavedQuery(query, clientId) { + const { id, ...payload } = convertQueryToServer(query); + return dispatch => SupersetClient.put({ - endpoint: `/savedqueryviewapi/api/update/${query.remoteId}`, - postPayload: convertQueryToServer(query), - stringify: false, + endpoint: `/api/v1/saved_query/${query.remoteId}`, + jsonPayload: convertQueryToServer(payload), }) .then(() => { dispatch(addSuccessToast(t('Your query was updated'))); - dispatch(queryEditorSetTitle(query, query.name)); + dispatch(queryEditorSetTitle(query, query.name, clientId)); }) .catch(e => { const message = t('Your query could not be updated'); @@ -1350,11 +1362,12 @@ export function popStoredQuery(urlId) { export function popSavedQuery(saveQueryId) { return function (dispatch) { return SupersetClient.get({ - endpoint: `/savedqueryviewapi/api/get/${saveQueryId}`, + endpoint: `/api/v1/saved_query/${saveQueryId}`, }) .then(({ json }) => { const queryEditorProps = { ...convertQueryToClient(json.result), + dbId: json.result?.database?.id, loaded: true, autorun: false, }; diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js index c0f3c8cd4bd3c..b216f17927285 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js @@ -24,7 +24,12 @@ import thunk from 'redux-thunk'; import shortid from 'shortid'; import * as featureFlags from 'src/featureFlags'; import * as actions from 'src/SqlLab/actions/sqlLab'; -import { defaultQueryEditor, query, initialState } from 'src/SqlLab/fixtures'; +import { + defaultQueryEditor, + query, + initialState, + queryId, +} from 'src/SqlLab/fixtures'; const middlewares = [thunk]; const mockStore = configureMockStore(middlewares); @@ -59,11 +64,11 @@ describe('async actions', () => { fetchMock.post(runQueryEndpoint, `{ "data": ${mockBigNumber} }`); describe('saveQuery', () => { - const saveQueryEndpoint = 'glob:*/savedqueryviewapi/api/create'; + const saveQueryEndpoint = 'glob:*/api/v1/saved_query/'; fetchMock.post(saveQueryEndpoint, { results: { json: {} } }); const makeRequest = () => { - const request = actions.saveQuery(query); + const request = actions.saveQuery(query, queryId); return request(dispatch, () => initialState); }; @@ -71,18 +76,20 @@ describe('async actions', () => { expect.assertions(1); const store = mockStore(initialState); - return store.dispatch(actions.saveQuery(query)).then(() => { + return store.dispatch(actions.saveQuery(query, queryId)).then(() => { expect(fetchMock.calls(saveQueryEndpoint)).toHaveLength(1); }); }); it('posts the correct query object', () => { const store = mockStore(initialState); - return store.dispatch(actions.saveQuery(query)).then(() => { + return store.dispatch(actions.saveQuery(query, queryId)).then(() => { const call = fetchMock.calls(saveQueryEndpoint)[0]; - const formData = call[1].body; - Object.keys(query).forEach(key => { - expect(formData.get(key)).toBeDefined(); + const formData = JSON.parse(call[1].body); + const mappedQueryToServer = actions.convertQueryToServer(query); + + Object.keys(mappedQueryToServer).forEach(key => { + expect(formData[key]).toBeDefined(); }); }); }); @@ -370,12 +377,13 @@ describe('async actions', () => { queryEditor: { name: 'Copy of Dummy query editor', dbId: 1, - schema: null, + schema: query.schema, autorun: true, sql: 'SELECT * FROM something', queryLimit: undefined, maxRow: undefined, id: 'abcd', + templateParams: undefined, }, }, ]; @@ -635,7 +643,9 @@ describe('async actions', () => { }, ]; return store - .dispatch(actions.queryEditorSetTitle(queryEditor, name)) + .dispatch( + actions.queryEditorSetTitle(queryEditor, name, queryEditor.id), + ) .then(() => { expect(store.getActions()).toEqual(expectedActions); expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1); diff --git a/superset-frontend/src/SqlLab/components/SaveQuery/index.tsx b/superset-frontend/src/SqlLab/components/SaveQuery/index.tsx index eab37eceb375f..769b1b4606d4a 100644 --- a/superset-frontend/src/SqlLab/components/SaveQuery/index.tsx +++ b/superset-frontend/src/SqlLab/components/SaveQuery/index.tsx @@ -36,8 +36,8 @@ import { QueryEditor } from 'src/SqlLab/types'; interface SaveQueryProps { queryEditorId: string; columns: ISaveableDatasource['columns']; - onSave: (arg0: QueryPayload) => void; - onUpdate: (arg0: QueryPayload) => void; + onSave: (arg0: QueryPayload, id: string) => void; + onUpdate: (arg0: QueryPayload, id: string) => void; saveQueryWarning: string | null; database: Record; } @@ -46,19 +46,8 @@ type QueryPayload = { name: string; description?: string; id?: string; -} & Pick< - QueryEditor, - | 'autorun' - | 'dbId' - | 'schema' - | 'sql' - | 'selectedText' - | 'remoteId' - | 'latestQueryId' - | 'queryLimit' - | 'tableOptions' - | 'schemaOptions' ->; + remoteId?: number; +} & Pick; const Styles = styled.span` span[role='img'] { @@ -93,11 +82,11 @@ export default function SaveQuery({ 'selectedText', 'sql', 'tableOptions', + 'templateParams', ]); const query = useMemo( () => ({ ...queryEditor, - columns, }), [queryEditor, columns], ); @@ -120,10 +109,13 @@ export default function SaveQuery({ ); const queryPayload = () => ({ - ...query, name: label, description, dbId: query.dbId ?? 0, + sql: query.sql, + schema: query.schema, + templateParams: query.templateParams, + remoteId: query?.remoteId || undefined, }); useEffect(() => { @@ -133,12 +125,12 @@ export default function SaveQuery({ const close = () => setShowSave(false); const onSaveWrapper = () => { - onSave(queryPayload()); + onSave(queryPayload(), query.id); close(); }; const onUpdateWrapper = () => { - onUpdate(queryPayload()); + onUpdate(queryPayload(), query.id); close(); }; diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx index 141a39fa512db..a4c31aaa1abb2 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx +++ b/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx @@ -499,8 +499,8 @@ const SqlEditor = ({ ); }; - const onSaveQuery = async query => { - const savedQuery = await dispatch(saveQuery(query)); + const onSaveQuery = async (query, clientId) => { + const savedQuery = await dispatch(saveQuery(query, clientId)); dispatch(addSavedQueryToTabState(queryEditor, savedQuery)); }; @@ -580,7 +580,9 @@ const SqlEditor = ({ queryEditorId={queryEditor.id} columns={latestQuery?.results?.columns || []} onSave={onSaveQuery} - onUpdate={query => dispatch(updateSavedQuery(query))} + onUpdate={(query, remoteId, id) => + dispatch(updateSavedQuery(query, remoteId, id)) + } saveQueryWarning={saveQueryWarning} database={database} /> diff --git a/superset-frontend/src/SqlLab/components/SqlEditorTabHeader/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditorTabHeader/index.tsx index dce2f2700e6e8..debacbb0d336d 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditorTabHeader/index.tsx +++ b/superset-frontend/src/SqlLab/components/SqlEditorTabHeader/index.tsx @@ -75,7 +75,7 @@ const SqlEditorTabHeader: React.FC = ({ queryEditor }) => { function renameTab() { const newTitle = prompt(t('Enter a new title for the tab')); if (newTitle) { - actions.queryEditorSetTitle(qe, newTitle); + actions.queryEditorSetTitle(qe, newTitle, qe.id); } } diff --git a/superset-frontend/src/SqlLab/fixtures.ts b/superset-frontend/src/SqlLab/fixtures.ts index 72c1c0d50f344..bb38fe6873a82 100644 --- a/superset-frontend/src/SqlLab/fixtures.ts +++ b/superset-frontend/src/SqlLab/fixtures.ts @@ -682,17 +682,15 @@ export const initialState = { }; export const query = { - id: 'clientId2353', + name: 'test query', dbId: 1, sql: 'SELECT * FROM something', - sqlEditorId: defaultQueryEditor.id, - tab: 'unimportant', - tempTable: null, - runAsync: false, - ctas: false, - cached: false, + description: 'test description', + schema: 'test schema', }; +export const queryId = 'clientId2353'; + export const testQuery: ISaveableDatasource = { name: 'unimportant', dbId: 1, diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.js b/superset-frontend/src/SqlLab/reducers/sqlLab.js index 164691c666381..1414235965b61 100644 --- a/superset-frontend/src/SqlLab/reducers/sqlLab.js +++ b/superset-frontend/src/SqlLab/reducers/sqlLab.js @@ -56,8 +56,8 @@ export default function sqlLabReducer(state = {}, action) { return addToArr(newState, 'queryEditors', action.queryEditor); }, [actions.QUERY_EDITOR_SAVED]() { - const { query, result } = action; - const existing = state.queryEditors.find(qe => qe.id === query.id); + const { query, result, clientId } = action; + const existing = state.queryEditors.find(qe => qe.id === clientId); return alterInArr( state, 'queryEditors', diff --git a/superset/migrations/versions/2022-10-03_17-34_deb4c9d4a4ef_parameters_in_saved_queries.py b/superset/migrations/versions/2022-10-03_17-34_deb4c9d4a4ef_parameters_in_saved_queries.py new file mode 100644 index 0000000000000..af3f6157a8658 --- /dev/null +++ b/superset/migrations/versions/2022-10-03_17-34_deb4c9d4a4ef_parameters_in_saved_queries.py @@ -0,0 +1,46 @@ +# 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. +"""parameters in saved queries + +Revision ID: deb4c9d4a4ef +Revises: 291f024254b5 +Create Date: 2022-10-03 17:34:00.721559 + +""" + +# revision identifiers, used by Alembic. +revision = "deb4c9d4a4ef" +down_revision = "291f024254b5" + +import sqlalchemy as sa +from alembic import op + + +def upgrade(): + op.add_column( + "saved_query", + sa.Column( + "template_parameters", + sa.TEXT(), + nullable=True, + ), + ) + + +def downgrade(): + with op.batch_alter_table("saved_query") as batch_op: + batch_op.drop_column("template_parameters") diff --git a/superset/models/sql_lab.py b/superset/models/sql_lab.py index f75973ad1794f..babea35baf39b 100644 --- a/superset/models/sql_lab.py +++ b/superset/models/sql_lab.py @@ -351,6 +351,7 @@ class SavedQuery(Model, AuditMixinNullable, ExtraJSONMixin, ImportExportMixin): label = Column(String(256)) description = Column(Text) sql = Column(Text) + template_parameters = Column(Text) user = relationship( security_manager.user_model, backref=backref("saved_queries", cascade="all, delete-orphan"), diff --git a/superset/queries/saved_queries/api.py b/superset/queries/saved_queries/api.py index 1f2088d7598ad..91c9158ffd028 100644 --- a/superset/queries/saved_queries/api.py +++ b/superset/queries/saved_queries/api.py @@ -93,6 +93,7 @@ class SavedQueryRestApi(BaseSupersetModelRestApi): "schema", "sql", "sql_tables", + "template_parameters", ] list_columns = [ "changed_on_delta_humanized", @@ -113,7 +114,14 @@ class SavedQueryRestApi(BaseSupersetModelRestApi): "last_run_delta_humanized", "extra", ] - add_columns = ["db_id", "description", "label", "schema", "sql"] + add_columns = [ + "db_id", + "description", + "label", + "schema", + "sql", + "template_parameters", + ] edit_columns = add_columns order_columns = [ "schema", diff --git a/tests/integration_tests/queries/saved_queries/api_tests.py b/tests/integration_tests/queries/saved_queries/api_tests.py index 437225f6c519d..4a615a816f578 100644 --- a/tests/integration_tests/queries/saved_queries/api_tests.py +++ b/tests/integration_tests/queries/saved_queries/api_tests.py @@ -524,6 +524,7 @@ def test_get_saved_query(self): "sql_tables": [{"catalog": None, "schema": None, "table": "table1"}], "schema": "schema1", "label": "label1", + "template_parameters": None, } data = json.loads(rv.data.decode("utf-8")) self.assertIn("changed_on_delta_humanized", data["result"])