fix(explore): retain chart ownership on query context update (#16419)
(cherry picked from commit 35864748f2d7dde53ebbe707bb7a79ef5cf8765d)
diff --git a/superset/charts/commands/update.py b/superset/charts/commands/update.py
index 449c67d..c63d0bc 100644
--- a/superset/charts/commands/update.py
+++ b/superset/charts/commands/update.py
@@ -64,7 +64,7 @@
chart = ChartDAO.update(self._model, self._properties)
except DAOUpdateFailedError as ex:
logger.exception(ex.exception)
- raise ChartUpdateFailedError()
+ raise ChartUpdateFailedError() from ex
return chart
def validate(self) -> None:
@@ -84,13 +84,17 @@
if not self._model:
raise ChartNotFoundError()
- # Check ownership; when only updating query context we ignore
+ # Check and update ownership; when only updating query context we ignore
# ownership so the update can be performed by report workers
if not is_query_context_update(self._properties):
try:
check_ownership(self._model)
- except SupersetSecurityException:
- raise ChartForbiddenError()
+ owners = self.populate_owners(self._actor, owner_ids)
+ self._properties["owners"] = owners
+ except SupersetSecurityException as ex:
+ raise ChartForbiddenError() from ex
+ except ValidationError as ex:
+ exceptions.append(ex)
# Validate/Populate datasource
if datasource_id is not None:
@@ -107,12 +111,6 @@
exceptions.append(DashboardsNotFoundValidationError())
self._properties["dashboards"] = dashboards
- # Validate/Populate owner
- try:
- owners = self.populate_owners(self._actor, owner_ids)
- self._properties["owners"] = owners
- except ValidationError as ex:
- exceptions.append(ex)
if exceptions:
exception = ChartInvalidError()
exception.add_list(exceptions)
diff --git a/tests/integration_tests/charts/commands_tests.py b/tests/integration_tests/charts/commands_tests.py
index 238a54e..7e718f8 100644
--- a/tests/integration_tests/charts/commands_tests.py
+++ b/tests/integration_tests/charts/commands_tests.py
@@ -321,7 +321,7 @@
json_obj = {
"description": "test for update",
"cache_timeout": None,
- "owners": [1],
+ "owners": [actor.id],
}
command = UpdateChartCommand(actor, model_id, json_obj)
last_saved_before = db.session.query(Slice).get(pk).last_saved_at
@@ -329,3 +329,31 @@
chart = db.session.query(Slice).get(pk)
assert chart.last_saved_at != last_saved_before
assert chart.last_saved_by == actor
+
+ @patch("superset.views.base.g")
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_query_context_update_command(self, mock_sm_g, mock_g):
+ """"
+ Test that a user can generate the chart query context
+ payloadwithout affecting owners
+ """
+ chart = db.session.query(Slice).all()[0]
+ pk = chart.id
+ admin = security_manager.find_user(username="admin")
+ chart.owners = [admin]
+ db.session.commit()
+
+ actor = security_manager.find_user(username="alpha")
+ mock_g.user = mock_sm_g.user = actor
+ query_context = json.dumps({"foo": "bar"})
+ json_obj = {
+ "query_context_generation": True,
+ "query_context": query_context,
+ }
+ command = UpdateChartCommand(actor, pk, json_obj)
+ command.run()
+ chart = db.session.query(Slice).get(pk)
+ assert chart.query_context == query_context
+ assert len(chart.owners) == 1
+ assert chart.owners[0] == admin
