This is an automated email from the ASF dual-hosted git repository. michaelsmolina pushed a commit to branch 3.1 in repository https://gitbox.apache.org/repos/asf/superset.git
commit ee8be59fdc5c1e66488455edbfb168ba6177c11b Author: JUST.in DO IT <[email protected]> AuthorDate: Wed Mar 6 08:59:56 2024 -0800 fix(sqllab): Close already removed tab (#27391) (cherry picked from commit 5107cc0fd9134886d7a8eefd51fb242e520a542e) --- superset-frontend/src/SqlLab/actions/sqlLab.js | 19 ++++--- .../src/hooks/apiResources/queryApi.ts | 5 +- superset/views/sql_lab/views.py | 20 ++++++-- tests/integration_tests/sql_lab/api_tests.py | 59 ++++++++++++++++++++++ 4 files changed, 90 insertions(+), 13 deletions(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 97db64fb89..cbab24a21e 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -742,15 +742,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 b7bf7f5b5d..4099422b54 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 00ba4b8a5f..4b96d0d6c2 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("/<int:tab_state_id>", 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("/<int:tab_state_id>", 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("<int:tab_state_id>", 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("<int:tab_state_id>/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 da050c2363..597f961346 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)
