gabotorresruiz commented on code in PR #44169:
URL: https://github.com/apache/superset/pull/44169#discussion_r4030072061
##########
tests/unit_tests/commands/chart/update_test.py:
##########
@@ -280,6 +280,42 @@ def
test_update_chart_rejects_repointing_to_non_table_datasource(
get_datasource_by_id.assert_not_called()
+def test_update_chart_accepts_semantic_view_datasource(
+ mocker: MockerFixture,
+) -> None:
+ """Repointing a chart at a SIP-182 semantic view must be accepted: the
+ view is a first-class resolvable datasource (Slice resolves it through
+ the type-guarded ``semantic_view`` relationship), so the non-table guard
+ must explicitly allow it (apache/superset#44167)."""
+ from superset.semantic_layers.models import SemanticView
+
+ find_by_id =
mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
+ find_by_id.return_value = mocker.MagicMock(id=1, tags=[], dashboards=[])
Review Comment:
Blocker, but a small one and not your fault: this is the only `find_by_id`
mock in the file that does not set `is_managed_externally`. The `master` merge
added `raise_if_managed_externally(self._model, ChartForbiddenError)` right
after the editorship check in `UpdateChartCommand.validate`, and a bare
`MagicMock` attribute is truthy, so `validate()` raises `ChartForbiddenError`
before it ever reaches the datasource guard this test is about.
I ran `tests/unit_tests/commands/chart/` against `refs/pull/44169/merge`: `1
failed, 69 passed`, then `70 passed` with this applied.
```suggestion
find_by_id.return_value = mocker.MagicMock(
is_managed_externally=False, id=1, tags=[], dashboards=[]
)
```
--
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]
