gabotorresruiz commented on code in PR #44494:
URL: https://github.com/apache/superset/pull/44494#discussion_r4066302066
##########
superset/commands/chart/update.py:
##########
@@ -166,9 +166,14 @@ def validate(self) -> None: # noqa: C901
if not self._model:
raise ChartNotFoundError()
- # Check and update editorship; when only updating query context we
relax
- # editorship so report workers can save context. We still require chart
- # access so users cannot rewrite query context for charts they cannot
access.
+ # Both a regular update and a query-context-only update require edit
+ # rights on the chart. A query-context-only update is the refreshed
+ # execution payload that Explore recomputes and saves in the
+ # background when a chart is opened; it is treated the same as the
+ # normal update path. Scheduled reports render the chart under an
Review Comment:
Not accurate: `ExecutorType.EDITOR` resolves editorship against the
`ReportSchedule`, not the chart. `get_executor` reads `model.editors`
(`superset/tasks/utils.py:92`) and `resolve_executor_user` passes the report
schedule as that `model` (`superset/commands/report/execute.py:135`). A user
who can only view a chart may still create a report on it
(`superset/commands/report/base.py:132`), so the executor is often not a chart
editor, and a `FixedExecutor` service account never is. Details in the review
comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
