mirror of https://github.com/apache/superset.git
fix(explore): retain chart ownership on query context update (#16419)
This commit is contained in:
parent
6a5568764e
commit
35864748f2
|
@ -84,13 +84,17 @@ class UpdateChartCommand(UpdateMixin, BaseCommand):
|
||||||
if not self._model:
|
if not self._model:
|
||||||
raise ChartNotFoundError()
|
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
|
# ownership so the update can be performed by report workers
|
||||||
if not is_query_context_update(self._properties):
|
if not is_query_context_update(self._properties):
|
||||||
try:
|
try:
|
||||||
check_ownership(self._model)
|
check_ownership(self._model)
|
||||||
|
owners = self.populate_owners(self._actor, owner_ids)
|
||||||
|
self._properties["owners"] = owners
|
||||||
except SupersetSecurityException as ex:
|
except SupersetSecurityException as ex:
|
||||||
raise ChartForbiddenError() from ex
|
raise ChartForbiddenError() from ex
|
||||||
|
except ValidationError as ex:
|
||||||
|
exceptions.append(ex)
|
||||||
|
|
||||||
# Validate/Populate datasource
|
# Validate/Populate datasource
|
||||||
if datasource_id is not None:
|
if datasource_id is not None:
|
||||||
|
@ -107,12 +111,6 @@ class UpdateChartCommand(UpdateMixin, BaseCommand):
|
||||||
exceptions.append(DashboardsNotFoundValidationError())
|
exceptions.append(DashboardsNotFoundValidationError())
|
||||||
self._properties["dashboards"] = dashboards
|
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:
|
if exceptions:
|
||||||
exception = ChartInvalidError()
|
exception = ChartInvalidError()
|
||||||
exception.add_list(exceptions)
|
exception.add_list(exceptions)
|
||||||
|
|
|
@ -321,7 +321,7 @@ class TestChartsUpdateCommand(SupersetTestCase):
|
||||||
json_obj = {
|
json_obj = {
|
||||||
"description": "test for update",
|
"description": "test for update",
|
||||||
"cache_timeout": None,
|
"cache_timeout": None,
|
||||||
"owners": [1],
|
"owners": [actor.id],
|
||||||
}
|
}
|
||||||
command = UpdateChartCommand(actor, model_id, json_obj)
|
command = UpdateChartCommand(actor, model_id, json_obj)
|
||||||
last_saved_before = db.session.query(Slice).get(pk).last_saved_at
|
last_saved_before = db.session.query(Slice).get(pk).last_saved_at
|
||||||
|
@ -329,3 +329,31 @@ class TestChartsUpdateCommand(SupersetTestCase):
|
||||||
chart = db.session.query(Slice).get(pk)
|
chart = db.session.query(Slice).get(pk)
|
||||||
assert chart.last_saved_at != last_saved_before
|
assert chart.last_saved_at != last_saved_before
|
||||||
assert chart.last_saved_by == actor
|
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
|
||||||
|
|
Loading…
Reference in New Issue