sadpandajoe commented on code in PR #45538:
URL: https://github.com/apache/superset/pull/45538#discussion_r4392085113


##########
tests/integration_tests/dashboards/api_tests.py:
##########
@@ -6152,6 +6153,398 @@ def test_copy_dashboard_layout_without_current_charts(
                 db.session.delete(chart)
             db.session.commit()
 
+    @contextmanager
+    def _layout_access_fixture(self) -> Iterator[dict[str, int]]:
+        """A dashboard Gamma edits, whose member chart Gamma cannot read,
+        plus a chart Gamma owns, an unrelated chart Gamma cannot read, a
+        chart Gamma reads through a granted dataset but does not edit, and
+        an archived member."""
+        from superset.connectors.sqla.models import SqlaTable
+        from superset.models.helpers import skip_visibility_filter
+
+        admin: User = self.get_user(ADMIN_USERNAME)
+        gamma: User = self.get_user(GAMMA_USERNAME)
+        datasets: list[SqlaTable] = (
+            db.session.query(SqlaTable).order_by(SqlaTable.id).limit(2).all()
+        )
+        dataset_id: int = datasets[0].id
+        granted: SqlaTable = datasets[1]
+        self.grant_role_access_to_table(granted, "Gamma")
+        readable: Slice = self.insert_chart(

Review Comment:
   Every fixture chart leaves `viewers` empty, so `can_access_chart` always 
falls through to the dataset check here. An implementation that let dataset 
access decide for any new chart would still pass every case, even though a 
chart with an explicit viewer list that excludes the caller is unreadable to 
them even on a granted dataset. Should there be a case with a chart on the 
granted dataset whose `viewers` is restricted to someone other than Gamma, 
asserting a 422 that names it and an unchanged member set?



-- 
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]

Reply via email to