From 841332777035e5a88f0abe92685c079ec2002c73 Mon Sep 17 00:00:00 2001 From: Joe Li Date: Mon, 17 Oct 2022 15:11:08 -0700 Subject: [PATCH] Revert "feat(sqllab): save query parameters in database (#21682)" This reverts commit 61319fd759b336992259a4e84f1459a134d55df0. --- .../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, 61 insertions(+), 132 deletions(-) delete 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 155e0d08c869..5e5c530c28e2 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)); -export const convertQueryToServer = fieldConverter(queryServerMapping); -export const convertQueryToClient = fieldConverter(queryClientMapping); +const convertQueryToServer = fieldConverter(queryServerMapping); +const convertQueryToClient = fieldConverter(queryClientMapping); export function getUpToDateQuery(rootState, queryEditor, key) { const { @@ -903,23 +903,17 @@ export function queryEditorSetAutorun(queryEditor, autorun) { }; } -export function queryEditorSetTitle(queryEditor, name, id) { +export function queryEditorSetTitle(queryEditor, name) { return function (dispatch) { const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) ? SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${id}`), + endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), postPayload: { label: name }, }) : Promise.resolve(); return sync - .then(() => - dispatch({ - type: QUERY_EDITOR_SET_TITLE, - queryEditor: { ...queryEditor, id }, - name, - }), - ) + .then(() => dispatch({ type: QUERY_EDITOR_SET_TITLE, queryEditor, name })) .catch(() => dispatch( addDangerToast( @@ -932,26 +926,21 @@ export function queryEditorSetTitle(queryEditor, name, id) { }; } -export function saveQuery(query, clientId) { - const { id, ...payload } = convertQueryToServer(query); - +export function saveQuery(query) { return dispatch => SupersetClient.post({ - endpoint: '/api/v1/saved_query/', - jsonPayload: convertQueryToServer(payload), + endpoint: '/savedqueryviewapi/api/create', + postPayload: convertQueryToServer(query), + stringify: false, }) .then(result => { - const savedQuery = convertQueryToClient({ - id: result.json.id, - ...result.json.result, - }); + const savedQuery = convertQueryToClient(result.json.item); dispatch({ type: QUERY_EDITOR_SAVED, query, - clientId, result: savedQuery, }); - dispatch(queryEditorSetTitle(query, query.name, clientId)); + dispatch(queryEditorSetTitle(query, query.name)); return savedQuery; }) .catch(() => @@ -977,17 +966,16 @@ export const addSavedQueryToTabState = }); }; -export function updateSavedQuery(query, clientId) { - const { id, ...payload } = convertQueryToServer(query); - +export function updateSavedQuery(query) { return dispatch => SupersetClient.put({ - endpoint: `/api/v1/saved_query/${query.remoteId}`, - jsonPayload: convertQueryToServer(payload), + endpoint: `/savedqueryviewapi/api/update/${query.remoteId}`, + postPayload: convertQueryToServer(query), + stringify: false, }) .then(() => { dispatch(addSuccessToast(t('Your query was updated'))); - dispatch(queryEditorSetTitle(query, query.name, clientId)); + dispatch(queryEditorSetTitle(query, query.name)); }) .catch(e => { const message = t('Your query could not be updated'); @@ -1362,12 +1350,11 @@ export function popStoredQuery(urlId) { export function popSavedQuery(saveQueryId) { return function (dispatch) { return SupersetClient.get({ - endpoint: `/api/v1/saved_query/${saveQueryId}`, + endpoint: `/savedqueryviewapi/api/get/${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 b216f1792728..c0f3c8cd4bd3 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js @@ -24,12 +24,7 @@ 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, - queryId, -} from 'src/SqlLab/fixtures'; +import { defaultQueryEditor, query, initialState } from 'src/SqlLab/fixtures'; const middlewares = [thunk]; const mockStore = configureMockStore(middlewares); @@ -64,11 +59,11 @@ describe('async actions', () => { fetchMock.post(runQueryEndpoint, `{ "data": ${mockBigNumber} }`); describe('saveQuery', () => { - const saveQueryEndpoint = 'glob:*/api/v1/saved_query/'; + const saveQueryEndpoint = 'glob:*/savedqueryviewapi/api/create'; fetchMock.post(saveQueryEndpoint, { results: { json: {} } }); const makeRequest = () => { - const request = actions.saveQuery(query, queryId); + const request = actions.saveQuery(query); return request(dispatch, () => initialState); }; @@ -76,20 +71,18 @@ describe('async actions', () => { expect.assertions(1); const store = mockStore(initialState); - return store.dispatch(actions.saveQuery(query, queryId)).then(() => { + return store.dispatch(actions.saveQuery(query)).then(() => { expect(fetchMock.calls(saveQueryEndpoint)).toHaveLength(1); }); }); it('posts the correct query object', () => { const store = mockStore(initialState); - return store.dispatch(actions.saveQuery(query, queryId)).then(() => { + return store.dispatch(actions.saveQuery(query)).then(() => { const call = fetchMock.calls(saveQueryEndpoint)[0]; - const formData = JSON.parse(call[1].body); - const mappedQueryToServer = actions.convertQueryToServer(query); - - Object.keys(mappedQueryToServer).forEach(key => { - expect(formData[key]).toBeDefined(); + const formData = call[1].body; + Object.keys(query).forEach(key => { + expect(formData.get(key)).toBeDefined(); }); }); }); @@ -377,13 +370,12 @@ describe('async actions', () => { queryEditor: { name: 'Copy of Dummy query editor', dbId: 1, - schema: query.schema, + schema: null, autorun: true, sql: 'SELECT * FROM something', queryLimit: undefined, maxRow: undefined, id: 'abcd', - templateParams: undefined, }, }, ]; @@ -643,9 +635,7 @@ describe('async actions', () => { }, ]; return store - .dispatch( - actions.queryEditorSetTitle(queryEditor, name, queryEditor.id), - ) + .dispatch(actions.queryEditorSetTitle(queryEditor, name)) .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 769b1b4606d4..eab37eceb375 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, id: string) => void; - onUpdate: (arg0: QueryPayload, id: string) => void; + onSave: (arg0: QueryPayload) => void; + onUpdate: (arg0: QueryPayload) => void; saveQueryWarning: string | null; database: Record; } @@ -46,8 +46,19 @@ type QueryPayload = { name: string; description?: string; id?: string; - remoteId?: number; -} & Pick; +} & Pick< + QueryEditor, + | 'autorun' + | 'dbId' + | 'schema' + | 'sql' + | 'selectedText' + | 'remoteId' + | 'latestQueryId' + | 'queryLimit' + | 'tableOptions' + | 'schemaOptions' +>; const Styles = styled.span` span[role='img'] { @@ -82,11 +93,11 @@ export default function SaveQuery({ 'selectedText', 'sql', 'tableOptions', - 'templateParams', ]); const query = useMemo( () => ({ ...queryEditor, + columns, }), [queryEditor, columns], ); @@ -109,13 +120,10 @@ 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(() => { @@ -125,12 +133,12 @@ export default function SaveQuery({ const close = () => setShowSave(false); const onSaveWrapper = () => { - onSave(queryPayload(), query.id); + onSave(queryPayload()); close(); }; const onUpdateWrapper = () => { - onUpdate(queryPayload(), query.id); + onUpdate(queryPayload()); close(); }; diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx index a4c31aaa1abb..141a39fa512d 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, clientId) => { - const savedQuery = await dispatch(saveQuery(query, clientId)); + const onSaveQuery = async query => { + const savedQuery = await dispatch(saveQuery(query)); dispatch(addSavedQueryToTabState(queryEditor, savedQuery)); }; @@ -580,9 +580,7 @@ const SqlEditor = ({ queryEditorId={queryEditor.id} columns={latestQuery?.results?.columns || []} onSave={onSaveQuery} - onUpdate={(query, remoteId, id) => - dispatch(updateSavedQuery(query, remoteId, id)) - } + onUpdate={query => dispatch(updateSavedQuery(query))} 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 debacbb0d336..dce2f2700e6e 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, qe.id); + actions.queryEditorSetTitle(qe, newTitle); } } diff --git a/superset-frontend/src/SqlLab/fixtures.ts b/superset-frontend/src/SqlLab/fixtures.ts index bb38fe6873a8..72c1c0d50f34 100644 --- a/superset-frontend/src/SqlLab/fixtures.ts +++ b/superset-frontend/src/SqlLab/fixtures.ts @@ -682,15 +682,17 @@ export const initialState = { }; export const query = { - name: 'test query', + id: 'clientId2353', dbId: 1, sql: 'SELECT * FROM something', - description: 'test description', - schema: 'test schema', + sqlEditorId: defaultQueryEditor.id, + tab: 'unimportant', + tempTable: null, + runAsync: false, + ctas: false, + cached: false, }; -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 1414235965b6..164691c66638 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, clientId } = action; - const existing = state.queryEditors.find(qe => qe.id === clientId); + const { query, result } = action; + const existing = state.queryEditors.find(qe => qe.id === query.id); 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 deleted file mode 100644 index af3f6157a865..000000000000 --- a/superset/migrations/versions/2022-10-03_17-34_deb4c9d4a4ef_parameters_in_saved_queries.py +++ /dev/null @@ -1,46 +0,0 @@ -# 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 babea35baf39..f75973ad1794 100644 --- a/superset/models/sql_lab.py +++ b/superset/models/sql_lab.py @@ -351,7 +351,6 @@ 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 91c9158ffd02..1f2088d7598a 100644 --- a/superset/queries/saved_queries/api.py +++ b/superset/queries/saved_queries/api.py @@ -93,7 +93,6 @@ class SavedQueryRestApi(BaseSupersetModelRestApi): "schema", "sql", "sql_tables", - "template_parameters", ] list_columns = [ "changed_on_delta_humanized", @@ -114,14 +113,7 @@ class SavedQueryRestApi(BaseSupersetModelRestApi): "last_run_delta_humanized", "extra", ] - add_columns = [ - "db_id", - "description", - "label", - "schema", - "sql", - "template_parameters", - ] + add_columns = ["db_id", "description", "label", "schema", "sql"] 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 4a615a816f57..437225f6c519 100644 --- a/tests/integration_tests/queries/saved_queries/api_tests.py +++ b/tests/integration_tests/queries/saved_queries/api_tests.py @@ -524,7 +524,6 @@ 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"])