diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 9cfd76e2e..16bf3f530 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -743,15 +743,18 @@ export function removeQueryEditor(queryEditor) { return sync .then(() => dispatch({ type: REMOVE_QUERY_EDITOR, queryEditor })) - .catch(() => - dispatch( - addDangerToast( - t( - 'An error occurred while removing tab. Please contact your administrator.', + .catch(({ status }) => { + if (status !== 404) { + return dispatch( + addDangerToast( + t( + 'An error occurred while removing tab. Please contact your administrator.', + ), ), - ), - ), - ); + ); + } + return dispatch({ type: REMOVE_QUERY_EDITOR, queryEditor }); + }); }; } diff --git a/superset-frontend/src/hooks/apiResources/queryApi.ts b/superset-frontend/src/hooks/apiResources/queryApi.ts index b7bf7f5b5..4099422b5 100644 --- a/superset-frontend/src/hooks/apiResources/queryApi.ts +++ b/superset-frontend/src/hooks/apiResources/queryApi.ts @@ -64,7 +64,10 @@ export const supersetClientQuery: BaseQueryFn< })) .catch(response => getClientErrorObject(response).then(errorObj => ({ - error: errorObj, + error: { + error: errorObj?.message || errorObj?.error || response.statusText, + status: response.status, + }, })), ); diff --git a/superset/views/sql_lab/views.py b/superset/views/sql_lab/views.py index 5c122ea88..8c21eea69 100644 --- a/superset/views/sql_lab/views.py +++ b/superset/views/sql_lab/views.py @@ -112,7 +112,10 @@ class TabStateView(BaseSupersetView): @has_access_api @expose("/", methods=("DELETE",)) def delete(self, tab_state_id: int) -> FlaskResponse: - if _get_owner_id(tab_state_id) != get_user_id(): + owner_id = _get_owner_id(tab_state_id) + if owner_id is None: + return Response(status=404) + if owner_id != get_user_id(): return Response(status=403) db.session.query(TabState).filter(TabState.id == tab_state_id).delete( @@ -127,7 +130,10 @@ class TabStateView(BaseSupersetView): @has_access_api @expose("/", methods=("GET",)) def get(self, tab_state_id: int) -> FlaskResponse: - if _get_owner_id(tab_state_id) != get_user_id(): + owner_id = _get_owner_id(tab_state_id) + if owner_id is None: + return Response(status=404) + if owner_id != get_user_id(): return Response(status=403) tab_state = db.session.query(TabState).filter_by(id=tab_state_id).first() @@ -157,7 +163,10 @@ class TabStateView(BaseSupersetView): @has_access_api @expose("", methods=("PUT",)) def put(self, tab_state_id: int) -> FlaskResponse: - if _get_owner_id(tab_state_id) != get_user_id(): + owner_id = _get_owner_id(tab_state_id) + if owner_id is None: + return Response(status=404) + if owner_id != get_user_id(): return Response(status=403) fields = {k: json.loads(v) for k, v in request.form.to_dict().items()} @@ -172,7 +181,10 @@ class TabStateView(BaseSupersetView): @has_access_api @expose("/migrate_query", methods=("POST",)) def migrate_query(self, tab_state_id: int) -> FlaskResponse: - if _get_owner_id(tab_state_id) != get_user_id(): + owner_id = _get_owner_id(tab_state_id) + if owner_id is None: + return Response(status=404) + if owner_id != get_user_id(): return Response(status=403) client_id = json.loads(request.form["queryId"]) diff --git a/tests/integration_tests/sql_lab/api_tests.py b/tests/integration_tests/sql_lab/api_tests.py index da050c236..597f96134 100644 --- a/tests/integration_tests/sql_lab/api_tests.py +++ b/tests/integration_tests/sql_lab/api_tests.py @@ -137,6 +137,65 @@ class TestSqlLabApi(SupersetTestCase): result = resp["result"] self.assertEqual(len(result["queries"]), 1) + @mock.patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", + {"SQLLAB_BACKEND_PERSISTENCE": True}, + clear=True, + ) + def test_deleted_tab(self): + username = "admin" + self.login(username) + data = { + "queryEditor": json.dumps( + { + "title": "Untitled Query 2", + "dbId": 1, + "schema": None, + "autorun": False, + "sql": "SELECT ...", + "queryLimit": 1000, + } + ) + } + resp = self.get_json_resp("/tabstateview/", data=data) + tab_state_id = resp["id"] + resp = self.client.delete("/tabstateview/" + str(tab_state_id)) + assert resp.status_code == 200 + resp = self.client.get("/tabstateview/" + str(tab_state_id)) + assert resp.status_code == 404 + resp = self.client.put( + "/tabstateview/" + str(tab_state_id), + json=data, + ) + assert resp.status_code == 404 + + @mock.patch.dict( + "superset.extensions.feature_flag_manager._feature_flags", + {"SQLLAB_BACKEND_PERSISTENCE": True}, + clear=True, + ) + def test_delete_tab_already_removed(self): + username = "admin" + self.login(username) + data = { + "queryEditor": json.dumps( + { + "title": "Untitled Query 3", + "dbId": 1, + "schema": None, + "autorun": False, + "sql": "SELECT ...", + "queryLimit": 1000, + } + ) + } + resp = self.get_json_resp("/tabstateview/", data=data) + tab_state_id = resp["id"] + resp = self.client.delete("/tabstateview/" + str(tab_state_id)) + assert resp.status_code == 200 + resp = self.client.delete("/tabstateview/" + str(tab_state_id)) + assert resp.status_code == 404 + def test_get_access_denied(self): new_role = Role(name="Dummy Role", permissions=[]) db.session.add(new_role)