feat(sqllab): save query parameters in database (apache#21682)

superset-frontend/src/SqlLab/actions/sqlLab.js

@@ -111,17 +111,17 @@ 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);
 
 // uses a mapping like those above to convert object key names to another style
 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,
         };

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,30 +64,32 @@ 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);
     };
 
     it('posts to the correct url', () => {
       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);

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<string, any>;
 }
@@ -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<QueryEditor, 'dbId' | 'schema' | 'sql'>;
 
 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();
   };
 

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}
             />

superset-frontend/src/SqlLab/components/SqlEditorTabHeader/index.tsx

@@ -75,7 +75,7 @@ const SqlEditorTabHeader: React.FC<Props> = ({ 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);
     }
   }
 

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,

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',

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")

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"),