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:29 UTC
(superset) branch 3.1 updated (4972fbe751 -> c294b9d63c)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a change to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
from 4972fbe751 fix: Results section in Explore shows an infinite spinner (#27366)
new efd305eb8d fix(docker): Remove race condition when building image (#26205)
new 4b5de7d203 fix: improve explore REST api validations (#27395)
new e84b2a3ba1 fix(API): Updating assets via the API should preserve ownership configuration (#27364)
new ee8be59fdc fix(sqllab): Close already removed tab (#27391)
new c294b9d63c fix: missing shared color in mixed timeseries (#27403)
The 5 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
Dockerfile | 14 ++-
.../src/MixedTimeseries/transformProps.ts | 5 +-
superset-frontend/src/SqlLab/actions/sqlLab.js | 19 ++--
.../src/hooks/apiResources/queryApi.ts | 5 +-
superset/commands/base.py | 22 +++-
superset/commands/chart/update.py | 5 +-
superset/commands/dashboard/update.py | 9 +-
superset/commands/dataset/update.py | 5 +-
superset/commands/explore/get.py | 11 +-
superset/commands/report/update.py | 7 +-
superset/commands/utils.py | 21 +++-
superset/views/datasource/views.py | 4 +-
superset/views/sql_lab/views.py | 20 +++-
tests/integration_tests/charts/api_tests.py | 53 ++++++++-
tests/integration_tests/dashboards/api_tests.py | 37 +++++++
tests/integration_tests/datasets/api_tests.py | 55 ++++++++++
tests/integration_tests/explore/api_tests.py | 20 +++-
tests/integration_tests/reports/api_tests.py | 87 +++++++++++++++
tests/integration_tests/sql_lab/api_tests.py | 59 +++++++++++
tests/unit_tests/commands/test_utils.py | 118 +++++++++++++++++++++
20 files changed, 532 insertions(+), 44 deletions(-)
create mode 100644 tests/unit_tests/commands/test_utils.py
(superset) 04/05: fix(sqllab): Close already removed tab (#27391)
Posted by mi...@apache.org.
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)
(superset) 02/05: fix: improve explore REST api validations (#27395)
Posted by mi...@apache.org.
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 4b5de7d203e4b5bb1bfdc00412a125203a054b6e
Author: Daniel Vaz Gaspar <da...@gmail.com>
AuthorDate: Tue Mar 5 17:44:51 2024 +0000
fix: improve explore REST api validations (#27395)
(cherry picked from commit a3d2e0bf447fad5d4495eea97529118b562f4e3c)
---
superset/commands/explore/get.py | 11 +++++++++--
tests/integration_tests/explore/api_tests.py | 20 +++++++++++++++++++-
2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/superset/commands/explore/get.py b/superset/commands/explore/get.py
index bb8f5a85e9..6c52763d24 100644
--- a/superset/commands/explore/get.py
+++ b/superset/commands/explore/get.py
@@ -38,6 +38,7 @@ from superset.daos.exceptions import DatasourceNotFound
from superset.exceptions import SupersetException
from superset.explore.exceptions import WrongEndpointError
from superset.explore.permalink.exceptions import ExplorePermalinkGetFailedError
+from superset.extensions import security_manager
from superset.utils import core as utils
from superset.views.utils import (
get_datasource_info,
@@ -62,7 +63,6 @@ class GetExploreCommand(BaseCommand, ABC):
# pylint: disable=too-many-locals,too-many-branches,too-many-statements
def run(self) -> Optional[dict[str, Any]]:
initial_form_data = {}
-
if self._permalink_key is not None:
command = GetExplorePermalinkCommand(self._permalink_key)
permalink_value = command.run()
@@ -111,12 +111,19 @@ class GetExploreCommand(BaseCommand, ABC):
self._datasource_type = SqlaTable.type
datasource: Optional[BaseDatasource] = None
+
if self._datasource_id is not None:
with contextlib.suppress(DatasourceNotFound):
datasource = DatasourceDAO.get_datasource(
db.session, cast(str, self._datasource_type), self._datasource_id
)
- datasource_name = datasource.name if datasource else _("[Missing Dataset]")
+
+ datasource_name = _("[Missing Dataset]")
+
+ if datasource:
+ datasource_name = datasource.name
+ security_manager.can_access_datasource(datasource)
+
viz_type = form_data.get("viz_type")
if not viz_type and datasource and datasource.default_endpoint:
raise WrongEndpointError(redirect=datasource.default_endpoint)
diff --git a/tests/integration_tests/explore/api_tests.py b/tests/integration_tests/explore/api_tests.py
index e37200e310..7594522af5 100644
--- a/tests/integration_tests/explore/api_tests.py
+++ b/tests/integration_tests/explore/api_tests.py
@@ -199,7 +199,7 @@ def test_get_from_permalink_unknown_key(test_client, login_as_admin):
@patch("superset.security.SupersetSecurityManager.can_access_datasource")
-def test_get_dataset_access_denied(
+def test_get_dataset_access_denied_with_form_data_key(
mock_can_access_datasource, test_client, login_as_admin, dataset
):
message = "Dataset access denied"
@@ -216,6 +216,24 @@ def test_get_dataset_access_denied(
assert data["message"] == message
+@patch("superset.security.SupersetSecurityManager.can_access_datasource")
+def test_get_dataset_access_denied(
+ mock_can_access_datasource, test_client, login_as_admin, dataset
+):
+ message = "Dataset access denied"
+ mock_can_access_datasource.side_effect = DatasetAccessDeniedError(
+ message=message, datasource_id=dataset.id, datasource_type=dataset.type
+ )
+ resp = test_client.get(
+ f"api/v1/explore/?datasource_id={dataset.id}&datasource_type={dataset.type}"
+ )
+ data = json.loads(resp.data.decode("utf-8"))
+ assert resp.status_code == 403
+ assert data["datasource_id"] == dataset.id
+ assert data["datasource_type"] == dataset.type
+ assert data["message"] == message
+
+
@patch("superset.daos.datasource.DatasourceDAO.get_datasource")
def test_wrong_endpoint(mock_get_datasource, test_client, login_as_admin, dataset):
dataset.default_endpoint = "another_endpoint"
(superset) 03/05: fix(API): Updating assets via the API should preserve ownership configuration (#27364)
Posted by mi...@apache.org.
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 e84b2a3ba1a39e582407bc1b4cfc673bf5e473c3
Author: Vitor Avila <96...@users.noreply.github.com>
AuthorDate: Wed Mar 6 13:40:41 2024 -0300
fix(API): Updating assets via the API should preserve ownership configuration (#27364)
(cherry picked from commit 66bf70172f2cbd24b17b503588f2edbed0a63247)
---
superset/commands/base.py | 20 +++-
superset/commands/chart/update.py | 5 +-
superset/commands/dashboard/update.py | 9 +-
superset/commands/dataset/update.py | 5 +-
superset/commands/report/update.py | 7 +-
superset/commands/utils.py | 21 ++++-
superset/views/datasource/views.py | 4 +-
tests/integration_tests/charts/api_tests.py | 53 ++++++++++-
tests/integration_tests/dashboards/api_tests.py | 37 ++++++++
tests/integration_tests/datasets/api_tests.py | 55 +++++++++++
tests/integration_tests/reports/api_tests.py | 87 +++++++++++++++++
tests/unit_tests/commands/test_utils.py | 118 ++++++++++++++++++++++++
12 files changed, 404 insertions(+), 17 deletions(-)
diff --git a/superset/commands/base.py b/superset/commands/base.py
index caca50755d..8b330d0669 100644
--- a/superset/commands/base.py
+++ b/superset/commands/base.py
@@ -19,7 +19,7 @@ from typing import Any, Optional
from flask_appbuilder.security.sqla.models import User
-from superset.commands.utils import populate_owners
+from superset.commands.utils import compute_owner_list, populate_owner_list
class BaseCommand(ABC):
@@ -55,7 +55,7 @@ class CreateMixin: # pylint: disable=too-few-public-methods
:raises OwnersNotFoundValidationError: if at least one owner can't be resolved
:returns: Final list of owners
"""
- return populate_owners(owner_ids, default_to_user=True)
+ return populate_owner_list(owner_ids, default_to_user=True)
class UpdateMixin: # pylint: disable=too-few-public-methods
@@ -69,4 +69,18 @@ class UpdateMixin: # pylint: disable=too-few-public-methods
:raises OwnersNotFoundValidationError: if at least one owner can't be resolved
:returns: Final list of owners
"""
- return populate_owners(owner_ids, default_to_user=False)
+ return populate_owner_list(owner_ids, default_to_user=False)
+
+ @staticmethod
+ def compute_owners(
+ current_owners: Optional[list[User]],
+ new_owners: Optional[list[int]],
+ ) -> list[User]:
+ """
+ Handle list of owners for update events.
+
+ :param current_owners: list of current owners
+ :param new_owners: list of new owners specified in the update payload
+ :returns: Final list of owners
+ """
+ return compute_owner_list(current_owners, new_owners)
diff --git a/superset/commands/chart/update.py b/superset/commands/chart/update.py
index 40b36ebcc5..178344634e 100644
--- a/superset/commands/chart/update.py
+++ b/superset/commands/chart/update.py
@@ -90,7 +90,10 @@ class UpdateChartCommand(UpdateMixin, BaseCommand):
if not is_query_context_update(self._properties):
try:
security_manager.raise_for_ownership(self._model)
- owners = self.populate_owners(owner_ids)
+ owners = self.compute_owners(
+ self._model.owners,
+ owner_ids,
+ )
self._properties["owners"] = owners
except SupersetSecurityException as ex:
raise ChartForbiddenError() from ex
diff --git a/superset/commands/dashboard/update.py b/superset/commands/dashboard/update.py
index 22dcad4b2c..b2b11e5f6e 100644
--- a/superset/commands/dashboard/update.py
+++ b/superset/commands/dashboard/update.py
@@ -66,7 +66,7 @@ class UpdateDashboardCommand(UpdateMixin, BaseCommand):
def validate(self) -> None:
exceptions: list[ValidationError] = []
- owners_ids: Optional[list[int]] = self._properties.get("owners")
+ owner_ids: Optional[list[int]] = self._properties.get("owners")
roles_ids: Optional[list[int]] = self._properties.get("roles")
slug: Optional[str] = self._properties.get("slug")
@@ -85,10 +85,11 @@ class UpdateDashboardCommand(UpdateMixin, BaseCommand):
exceptions.append(DashboardSlugExistsValidationError())
# Validate/Populate owner
- if owners_ids is None:
- owners_ids = [owner.id for owner in self._model.owners]
try:
- owners = self.populate_owners(owners_ids)
+ owners = self.compute_owners(
+ self._model.owners,
+ owner_ids,
+ )
self._properties["owners"] = owners
except ValidationError as ex:
exceptions.append(ex)
diff --git a/superset/commands/dataset/update.py b/superset/commands/dataset/update.py
index 8a72c24fd5..5c0d87b230 100644
--- a/superset/commands/dataset/update.py
+++ b/superset/commands/dataset/update.py
@@ -100,7 +100,10 @@ class UpdateDatasetCommand(UpdateMixin, BaseCommand):
exceptions.append(DatabaseChangeValidationError())
# Validate/Populate owner
try:
- owners = self.populate_owners(owner_ids)
+ owners = self.compute_owners(
+ self._model.owners,
+ owner_ids,
+ )
self._properties["owners"] = owners
except ValidationError as ex:
exceptions.append(ex)
diff --git a/superset/commands/report/update.py b/superset/commands/report/update.py
index a33ba6b59a..cb63ec5011 100644
--- a/superset/commands/report/update.py
+++ b/superset/commands/report/update.py
@@ -118,10 +118,11 @@ class UpdateReportScheduleCommand(UpdateMixin, BaseReportScheduleCommand):
raise ReportScheduleForbiddenError() from ex
# Validate/Populate owner
- if owner_ids is None:
- owner_ids = [owner.id for owner in self._model.owners]
try:
- owners = self.populate_owners(owner_ids)
+ owners = self.compute_owners(
+ self._model.owners,
+ owner_ids,
+ )
self._properties["owners"] = owners
except ValidationError as ex:
exceptions.append(ex)
diff --git a/superset/commands/utils.py b/superset/commands/utils.py
index 8cfeab3c11..d2a1e38ea0 100644
--- a/superset/commands/utils.py
+++ b/superset/commands/utils.py
@@ -36,7 +36,7 @@ if TYPE_CHECKING:
from superset.connectors.sqla.models import BaseDatasource
-def populate_owners(
+def populate_owner_list(
owner_ids: list[int] | None,
default_to_user: bool,
) -> list[User]:
@@ -63,6 +63,25 @@ def populate_owners(
return owners
+def compute_owner_list(
+ current_owners: list[User] | None,
+ new_owners: list[int] | None,
+) -> list[User]:
+ """
+ Helper function for update commands, to properly handle the owners list.
+ Preserve the previous configuration unless included in the update payload.
+
+ :param current_owners: list of current owners
+ :param new_owners: list of new owners specified in the update payload
+ :returns: Final list of owners
+ """
+ current_owners = current_owners or []
+ owners_ids = (
+ [owner.id for owner in current_owners] if new_owners is None else new_owners
+ )
+ return populate_owner_list(owners_ids, default_to_user=False)
+
+
def populate_roles(role_ids: list[int] | None = None) -> list[Role]:
"""
Helper function for commands, will fetch all roles from roles id's
diff --git a/superset/views/datasource/views.py b/superset/views/datasource/views.py
index a4c158a11f..08f2c73855 100644
--- a/superset/views/datasource/views.py
+++ b/superset/views/datasource/views.py
@@ -32,7 +32,7 @@ from superset.commands.dataset.exceptions import (
DatasetForbiddenError,
DatasetNotFoundError,
)
-from superset.commands.utils import populate_owners
+from superset.commands.utils import populate_owner_list
from superset.connectors.sqla.models import SqlaTable
from superset.connectors.sqla.utils import get_physical_table_metadata
from superset.daos.datasource import DatasourceDAO
@@ -95,7 +95,7 @@ class Datasource(BaseSupersetView):
except SupersetSecurityException as ex:
raise DatasetForbiddenError() from ex
- datasource_dict["owners"] = populate_owners(
+ datasource_dict["owners"] = populate_owner_list(
datasource_dict["owners"], default_to_user=False
)
diff --git a/tests/integration_tests/charts/api_tests.py b/tests/integration_tests/charts/api_tests.py
index 69888104fa..9dec56fedd 100644
--- a/tests/integration_tests/charts/api_tests.py
+++ b/tests/integration_tests/charts/api_tests.py
@@ -735,10 +735,59 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin):
db.session.delete(model)
db.session.commit()
+ @pytest.mark.usefixtures("add_dashboard_to_chart")
+ def test_update_chart_preserve_ownership(self):
+ """
+ Chart API: Test update chart preserves owner list (if un-changed)
+ """
+ chart_data = {
+ "slice_name": "title1_changed",
+ }
+ admin = self.get_user("admin")
+ self.login(username="admin")
+ uri = f"api/v1/chart/{self.chart.id}"
+ rv = self.put_assert_metric(uri, chart_data, "put")
+ self.assertEqual(rv.status_code, 200)
+ self.assertEqual([admin], self.chart.owners)
+
+ @pytest.mark.usefixtures("add_dashboard_to_chart")
+ def test_update_chart_clear_owner_list(self):
+ """
+ Chart API: Test update chart admin can clear owner list
+ """
+ chart_data = {"slice_name": "title1_changed", "owners": []}
+ admin = self.get_user("admin")
+ self.login(username="admin")
+ uri = f"api/v1/chart/{self.chart.id}"
+ rv = self.put_assert_metric(uri, chart_data, "put")
+ self.assertEqual(rv.status_code, 200)
+ self.assertEqual([], self.chart.owners)
+
+ def test_update_chart_populate_owner(self):
+ """
+ Chart API: Test update admin can update chart with
+ no owners to a different owner
+ """
+ gamma = self.get_user("gamma")
+ admin = self.get_user("admin")
+ chart_id = self.insert_chart("title", [], 1).id
+ model = db.session.query(Slice).get(chart_id)
+ self.assertEqual(model.owners, [])
+ chart_data = {"owners": [gamma.id]}
+ self.login(username="admin")
+ uri = f"api/v1/chart/{chart_id}"
+ rv = self.put_assert_metric(uri, chart_data, "put")
+ self.assertEqual(rv.status_code, 200)
+ model_updated = db.session.query(Slice).get(chart_id)
+ self.assertNotIn(admin, model_updated.owners)
+ self.assertIn(gamma, model_updated.owners)
+ db.session.delete(model_updated)
+ db.session.commit()
+
@pytest.mark.usefixtures("add_dashboard_to_chart")
def test_update_chart_new_dashboards(self):
"""
- Chart API: Test update set new owner to current user
+ Chart API: Test update chart associating it with new dashboard
"""
chart_data = {
"slice_name": "title1_changed",
@@ -754,7 +803,7 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin):
@pytest.mark.usefixtures("add_dashboard_to_chart")
def test_not_update_chart_none_dashboards(self):
"""
- Chart API: Test update set new owner to current user
+ Chart API: Test update chart without changing dashboards configuration
"""
chart_data = {"slice_name": "title1_changed_again"}
self.login(username="admin")
diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py
index a5c44f9f08..6023b5b525 100644
--- a/tests/integration_tests/dashboards/api_tests.py
+++ b/tests/integration_tests/dashboards/api_tests.py
@@ -1551,6 +1551,43 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi
db.session.delete(model)
db.session.commit()
+ def test_update_dashboard_clear_owner_list(self):
+ """
+ Dashboard API: Test update admin can clear up owners list
+ """
+ admin = self.get_user("admin")
+ dashboard_id = self.insert_dashboard("title1", "slug1", [admin.id]).id
+ self.login(username="admin")
+ uri = f"api/v1/dashboard/{dashboard_id}"
+ dashboard_data = {"owners": []}
+ rv = self.client.put(uri, json=dashboard_data)
+ self.assertEqual(rv.status_code, 200)
+ model = db.session.query(Dashboard).get(dashboard_id)
+ self.assertEqual([], model.owners)
+ db.session.delete(model)
+ db.session.commit()
+
+ def test_update_dashboard_populate_owner(self):
+ """
+ Dashboard API: Test update admin can update dashboard with
+ no owners to a different owner
+ """
+ self.login(username="admin")
+ gamma = self.get_user("gamma")
+ dashboard = self.insert_dashboard(
+ "title1",
+ "slug1",
+ [],
+ )
+ uri = f"api/v1/dashboard/{dashboard.id}"
+ dashboard_data = {"owners": [gamma.id]}
+ rv = self.client.put(uri, json=dashboard_data)
+ self.assertEqual(rv.status_code, 200)
+ model = db.session.query(Dashboard).get(dashboard.id)
+ self.assertEqual([gamma], model.owners)
+ db.session.delete(model)
+ db.session.commit()
+
def test_update_dashboard_slug_formatting(self):
"""
Dashboard API: Test update slug formatting
diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py
index d969895489..9b264a7eb6 100644
--- a/tests/integration_tests/datasets/api_tests.py
+++ b/tests/integration_tests/datasets/api_tests.py
@@ -885,6 +885,59 @@ class TestDatasetApi(SupersetTestCase):
assert rv.status_code == 422
assert data == {"message": "Dataset could not be created."}
+ def test_update_dataset_preserve_ownership(self):
+ """
+ Dataset API: Test update dataset preserves owner list (if un-changed)
+ """
+
+ dataset = self.insert_default_dataset()
+ current_owners = dataset.owners
+ self.login(username="admin")
+ dataset_data = {"description": "new description"}
+ uri = f"api/v1/dataset/{dataset.id}"
+ rv = self.put_assert_metric(uri, dataset_data, "put")
+ assert rv.status_code == 200
+ model = db.session.query(SqlaTable).get(dataset.id)
+ assert model.owners == current_owners
+
+ db.session.delete(dataset)
+ db.session.commit()
+
+ def test_update_dataset_clear_owner_list(self):
+ """
+ Dataset API: Test update dataset admin can clear ownership config
+ """
+
+ dataset = self.insert_default_dataset()
+ self.login(username="admin")
+ dataset_data = {"owners": []}
+ uri = f"api/v1/dataset/{dataset.id}"
+ rv = self.put_assert_metric(uri, dataset_data, "put")
+ assert rv.status_code == 200
+ model = db.session.query(SqlaTable).get(dataset.id)
+ assert model.owners == []
+
+ db.session.delete(dataset)
+ db.session.commit()
+
+ def test_update_dataset_populate_owner(self):
+ """
+ Dataset API: Test update admin can update dataset with
+ no owners to a different owner
+ """
+ self.login(username="admin")
+ gamma = self.get_user("gamma")
+ dataset = self.insert_dataset("ab_permission", [], get_main_database())
+ dataset_data = {"owners": [gamma.id]}
+ uri = f"api/v1/dataset/{dataset.id}"
+ rv = self.put_assert_metric(uri, dataset_data, "put")
+ assert rv.status_code == 200
+ model = db.session.query(SqlaTable).get(dataset.id)
+ assert model.owners == [gamma]
+
+ db.session.delete(dataset)
+ db.session.commit()
+
def test_update_dataset_item(self):
"""
Dataset API: Test update dataset item
@@ -893,6 +946,7 @@ class TestDatasetApi(SupersetTestCase):
return
dataset = self.insert_default_dataset()
+ current_owners = dataset.owners
self.login(username="admin")
dataset_data = {"description": "changed_description"}
uri = f"api/v1/dataset/{dataset.id}"
@@ -900,6 +954,7 @@ class TestDatasetApi(SupersetTestCase):
assert rv.status_code == 200
model = db.session.query(SqlaTable).get(dataset.id)
assert model.description == dataset_data["description"]
+ assert model.owners == current_owners
db.session.delete(dataset)
db.session.commit()
diff --git a/tests/integration_tests/reports/api_tests.py b/tests/integration_tests/reports/api_tests.py
index 12d00d59f3..1b74d11f5a 100644
--- a/tests/integration_tests/reports/api_tests.py
+++ b/tests/integration_tests/reports/api_tests.py
@@ -1458,6 +1458,93 @@ class TestReportSchedulesApi(SupersetTestCase):
rv = self.put_assert_metric(uri, report_schedule_data, "put")
self.assertEqual(rv.status_code, 403)
+ @pytest.mark.usefixtures("create_report_schedules")
+ def test_update_report_preserve_ownership(self):
+ """
+ ReportSchedule API: Test update report preserves owner list (if un-changed)
+ """
+ self.login(username="admin")
+ existing_report = (
+ db.session.query(ReportSchedule)
+ .filter(ReportSchedule.name == "name1")
+ .one_or_none()
+ )
+ current_owners = existing_report.owners
+ report_schedule_data = {
+ "description": "Updated description",
+ }
+ uri = f"api/v1/report/{existing_report.id}"
+ rv = self.put_assert_metric(uri, report_schedule_data, "put")
+ updated_report = (
+ db.session.query(ReportSchedule)
+ .filter(ReportSchedule.name == "name1")
+ .one_or_none()
+ )
+ assert updated_report.owners == current_owners
+
+ @pytest.mark.usefixtures("create_report_schedules")
+ def test_update_report_clear_owner_list(self):
+ """
+ ReportSchedule API: Test update report admin can clear ownership config
+ """
+ self.login(username="admin")
+ existing_report = (
+ db.session.query(ReportSchedule)
+ .filter(ReportSchedule.name == "name1")
+ .one_or_none()
+ )
+ report_schedule_data = {
+ "owners": [],
+ }
+ uri = f"api/v1/report/{existing_report.id}"
+ rv = self.put_assert_metric(uri, report_schedule_data, "put")
+ updated_report = (
+ db.session.query(ReportSchedule)
+ .filter(ReportSchedule.name == "name1")
+ .one_or_none()
+ )
+ assert updated_report.owners == []
+
+ @pytest.mark.usefixtures("create_report_schedules")
+ def test_update_report_populate_owner(self):
+ """
+ ReportSchedule API: Test update admin can update report with
+ no owners to a different owner
+ """
+ gamma = self.get_user("gamma")
+ self.login(username="admin")
+
+ # Modify an existing report to make remove all owners
+ existing_report = (
+ db.session.query(ReportSchedule)
+ .filter(ReportSchedule.name == "name1")
+ .one_or_none()
+ )
+ report_update_data = {
+ "owners": [],
+ }
+ uri = f"api/v1/report/{existing_report.id}"
+ rv = self.put_assert_metric(uri, report_update_data, "put")
+ updated_report = (
+ db.session.query(ReportSchedule)
+ .filter(ReportSchedule.name == "name1")
+ .one_or_none()
+ )
+ assert updated_report.owners == []
+
+ # Populate the field
+ report_update_data = {
+ "owners": [gamma.id],
+ }
+ uri = f"api/v1/report/{updated_report.id}"
+ rv = self.put_assert_metric(uri, report_update_data, "put")
+ updated_report = (
+ db.session.query(ReportSchedule)
+ .filter(ReportSchedule.name == "name1")
+ .one_or_none()
+ )
+ assert updated_report.owners == [gamma]
+
@pytest.mark.usefixtures("create_report_schedules")
def test_delete_report_schedule(self):
"""
diff --git a/tests/unit_tests/commands/test_utils.py b/tests/unit_tests/commands/test_utils.py
new file mode 100644
index 0000000000..cb99ac37bf
--- /dev/null
+++ b/tests/unit_tests/commands/test_utils.py
@@ -0,0 +1,118 @@
+# 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.
+
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from superset.commands.utils import compute_owner_list, populate_owner_list, User
+
+
+@patch("superset.commands.utils.g")
+def test_populate_owner_list_default_to_user(mock_user):
+ owner_list = populate_owner_list([], True)
+ assert owner_list == [mock_user.user]
+
+
+@patch("superset.commands.utils.g")
+def test_populate_owner_list_default_to_user_handle_none(mock_user):
+ owner_list = populate_owner_list(None, True)
+ assert owner_list == [mock_user.user]
+
+
+@patch("superset.commands.utils.g")
+@patch("superset.commands.utils.security_manager")
+@patch("superset.commands.utils.get_user_id")
+def test_populate_owner_list_admin_user(mock_user_id, mock_sm, mock_g):
+ test_user = User(id=1, first_name="First", last_name="Last")
+ mock_g.user = User(id=4, first_name="Admin", last_name="User")
+ mock_user_id.return_value = 4
+ mock_sm.is_admin = MagicMock(return_value=True)
+ mock_sm.get_user_by_id = MagicMock(return_value=test_user)
+
+ owner_list = populate_owner_list([1], False)
+ assert owner_list == [test_user]
+
+
+@patch("superset.commands.utils.g")
+@patch("superset.commands.utils.security_manager")
+@patch("superset.commands.utils.get_user_id")
+def test_populate_owner_list_admin_user_empty_list(mock_user_id, mock_sm, mock_g):
+ mock_g.user = User(id=4, first_name="Admin", last_name="User")
+ mock_user_id.return_value = 4
+ mock_sm.is_admin = MagicMock(return_value=True)
+ owner_list = populate_owner_list([], False)
+ assert owner_list == []
+
+
+@patch("superset.commands.utils.g")
+@patch("superset.commands.utils.security_manager")
+@patch("superset.commands.utils.get_user_id")
+def test_populate_owner_list_non_admin(mock_user_id, mock_sm, mock_g):
+ test_user = User(id=1, first_name="First", last_name="Last")
+ mock_g.user = User(id=4, first_name="Non", last_name="admin")
+ mock_user_id.return_value = 4
+ mock_sm.is_admin = MagicMock(return_value=False)
+ mock_sm.get_user_by_id = MagicMock(return_value=test_user)
+
+ owner_list = populate_owner_list([1], False)
+ assert owner_list == [mock_g.user, test_user]
+
+
+@patch("superset.commands.utils.populate_owner_list")
+def test_compute_owner_list_new_owners(mock_populate_owner_list):
+ current_owners = [User(id=1), User(id=2), User(id=3)]
+ new_owners = [4, 5, 6]
+
+ compute_owner_list(current_owners, new_owners)
+ mock_populate_owner_list.assert_called_once_with(new_owners, default_to_user=False)
+
+
+@patch("superset.commands.utils.populate_owner_list")
+def test_compute_owner_list_no_new_owners(mock_populate_owner_list):
+ current_owners = [User(id=1), User(id=2), User(id=3)]
+ new_owners = None
+
+ compute_owner_list(current_owners, new_owners)
+ mock_populate_owner_list.assert_called_once_with([1, 2, 3], default_to_user=False)
+
+
+@patch("superset.commands.utils.populate_owner_list")
+def test_compute_owner_list_new_owner_empty_list(mock_populate_owner_list):
+ current_owners = [User(id=1), User(id=2), User(id=3)]
+ new_owners = []
+
+ compute_owner_list(current_owners, new_owners)
+ mock_populate_owner_list.assert_called_once_with(new_owners, default_to_user=False)
+
+
+@patch("superset.commands.utils.populate_owner_list")
+def test_compute_owner_list_no_owners(mock_populate_owner_list):
+ current_owners = []
+ new_owners = [4, 5, 6]
+
+ compute_owner_list(current_owners, new_owners)
+ mock_populate_owner_list.assert_called_once_with(new_owners, default_to_user=False)
+
+
+@patch("superset.commands.utils.populate_owner_list")
+def test_compute_owner_list_no_owners_handle_none(mock_populate_owner_list):
+ current_owners = None
+ new_owners = [4, 5, 6]
+
+ compute_owner_list(current_owners, new_owners)
+ mock_populate_owner_list.assert_called_once_with(new_owners, default_to_user=False)
(superset) 05/05: fix: missing shared color in mixed timeseries (#27403)
Posted by mi...@apache.org.
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 c294b9d63c8af7a997534721eafd8d849758e5ea
Author: JUST.in DO IT <ju...@airbnb.com>
AuthorDate: Thu Mar 7 10:04:49 2024 -0800
fix: missing shared color in mixed timeseries (#27403)
(cherry picked from commit 9ced2552dbeeaf60217b385d4c40cbaf4372c787)
---
.../plugin-chart-echarts/src/MixedTimeseries/transformProps.ts | 5 +++--
superset/commands/base.py | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
index ce6969213a..8eecb6de47 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
@@ -406,8 +406,9 @@ export default function transformProps(
rawSeriesB.forEach(entry => {
const entryName = String(entry.name || '');
- const seriesName = `${inverted[entryName] || entryName} (1)`;
- const colorScaleKey = getOriginalSeries(seriesName, array);
+ const seriesEntry = inverted[entryName] || entryName;
+ const seriesName = `${seriesEntry} (1)`;
+ const colorScaleKey = getOriginalSeries(seriesEntry, array);
const seriesFormatter = getFormatter(
customFormattersSecondary,
diff --git a/superset/commands/base.py b/superset/commands/base.py
index 8b330d0669..d2efcda4fe 100644
--- a/superset/commands/base.py
+++ b/superset/commands/base.py
@@ -58,7 +58,7 @@ class CreateMixin: # pylint: disable=too-few-public-methods
return populate_owner_list(owner_ids, default_to_user=True)
-class UpdateMixin: # pylint: disable=too-few-public-methods
+class UpdateMixin:
@staticmethod
def populate_owners(owner_ids: Optional[list[int]] = None) -> list[User]:
"""
(superset) 01/05: fix(docker): Remove race condition when building image (#26205)
Posted by mi...@apache.org.
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 efd305eb8d38e0ade68d152944352e2d11657f48
Author: Aleksey Karpov <86...@users.noreply.github.com>
AuthorDate: Thu Dec 7 15:54:59 2023 +0300
fix(docker): Remove race condition when building image (#26205)
(cherry picked from commit f68dd8293f9c7e798756e90c154d8473d0d1cb49)
---
Dockerfile | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/Dockerfile b/Dockerfile
index b9714d6c69..fc3e667037 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -61,9 +61,7 @@ ENV LANG=C.UTF-8 \
SUPERSET_HOME="/app/superset_home" \
SUPERSET_PORT=8088
-RUN --mount=target=/var/lib/apt/lists,type=cache \
- --mount=target=/var/cache/apt,type=cache \
- mkdir -p ${PYTHONPATH} superset/static superset-frontend apache_superset.egg-info requirements \
+RUN mkdir -p ${PYTHONPATH} superset/static superset-frontend apache_superset.egg-info requirements \
&& useradd --user-group -d ${SUPERSET_HOME} -m --no-log-init --shell /bin/bash superset \
&& apt-get update -qq && apt-get install -yqq --no-install-recommends \
build-essential \
@@ -75,7 +73,8 @@ RUN --mount=target=/var/lib/apt/lists,type=cache \
libecpg-dev \
libldap2-dev \
&& touch superset/static/version_info.json \
- && chown -R superset:superset ./*
+ && chown -R superset:superset ./* \
+ && rm -rf /var/lib/apt/lists/*
COPY --chown=superset:superset setup.py MANIFEST.in README.md ./
# setup.py uses the version information in package.json
@@ -112,9 +111,8 @@ ARG GECKODRIVER_VERSION=v0.33.0 \
USER root
-RUN --mount=target=/var/lib/apt/lists,type=cache \
- --mount=target=/var/cache/apt,type=cache \
- apt-get install -yqq --no-install-recommends \
+RUN apt-get update -qq \
+ && apt-get install -yqq --no-install-recommends \
libnss3 \
libdbus-glib-1-2 \
libgtk-3-0 \
@@ -127,7 +125,7 @@ RUN --mount=target=/var/lib/apt/lists,type=cache \
# Install Firefox
&& wget -q https://download-installer.cdn.mozilla.net/pub/firefox/releases/${FIREFOX_VERSION}/linux-x86_64/en-US/firefox-${FIREFOX_VERSION}.tar.bz2 -O - | tar xfj - -C /opt \
&& ln -s /opt/firefox/firefox /usr/local/bin/firefox \
- && apt-get autoremove -yqq --purge wget && rm -rf /var/[log,tmp]/* /tmp/*
+ && apt-get autoremove -yqq --purge wget && rm -rf /var/[log,tmp]/* /tmp/* /var/lib/apt/lists/*
# Cache everything for dev purposes...
RUN --mount=type=bind,target=./requirements/base.txt,src=./requirements/base.txt \
--mount=type=bind,target=./requirements/docker.txt,src=./requirements/docker.txt \