You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2024/03/07 18:15:33 UTC
(superset) 04/05: fix(sqllab): Close already removed tab (#27391)
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 <ju...@airbnb.com>
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)