sha174n commented on PR #44494: URL: https://github.com/apache/superset/pull/44494#issuecomment-5767856610
@gabotorresruiz you read `get_executor` correctly, thanks for the detailed trace. Went with your first option in d6284aa157. - Restored `raise_for_access(chart=...)` on the query-context-only path and added an explicit `is_guest_user()` deny ahead of it. That keeps the cell that actually moves (guest principals, which `is_editor` already excluded on the regular update path) without changing anything for the report executor. - The report backfill is untouched, so CSV/Excel on charts with `query_context IS NULL` self-heal as before. - Added `test_csv_report_query_context_backfill_allows_non_chart_editor_executor` in `tests/integration_tests/reports/commands_tests.py`: a CSV report whose executor edits the report but not the chart. I confirmed it fails if the gate is swapped back to `raise_for_editorship` and passes as landed. - Unit tests now cover the guest deny; the other two keep the previous contract. On the separate point: since the contract went back to what it was, `test_query_context_update_requires_chart_access` and the skipped `test_query_context_update_command` are accurate again, so I left them alone. Shout if you would rather I reworded them anyway. -- 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]
