[dashboards] Fix, update dashboard owners not propagating to charts o… (apache#9484)

dpgaspar · web-flow · commit bb80ceaccc6b · 2020-04-09T09:37:37.000+01:00
* [dashboards] Fix, update dashboard owners not propagating to charts owners

* Add tests

* Fix tests

* better naming
diff --git a/superset/dashboards/commands/create.py b/superset/dashboards/commands/create.py
@@ -42,7 +42,8 @@ def __init__(self, user: User, data: Dict):
     def run(self) -> Model:
         self.validate()
         try:
-            dashboard = DashboardDAO.create(self._properties)
+            dashboard = DashboardDAO.create(self._properties, commit=False)
+            dashboard = DashboardDAO.update_charts_owners(dashboard, commit=True)
         except DAOCreateFailedError as ex:
             logger.exception(ex.exception)
             raise DashboardCreateFailedError()
diff --git a/superset/dashboards/commands/update.py b/superset/dashboards/commands/update.py
@@ -49,7 +49,8 @@ def __init__(self, user: User, model_id: int, data: Dict):
     def run(self) -> Model:
         self.validate()
         try:
-            dashboard = DashboardDAO.update(self._model, self._properties)
+            dashboard = DashboardDAO.update(self._model, self._properties, commit=False)
+            dashboard = DashboardDAO.update_charts_owners(dashboard, commit=True)
         except DAOUpdateFailedError as ex:
             logger.exception(ex.exception)
             raise DashboardUpdateFailedError()
diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py
@@ -47,6 +47,15 @@ def validate_update_slug_uniqueness(dashboard_id: int, slug: Optional[str]) -> b
             return not db.session.query(dashboard_query.exists()).scalar()
         return True
 
+    @staticmethod
+    def update_charts_owners(model: Dashboard, commit: bool = True) -> Dashboard:
+        owners = [owner for owner in model.owners]
+        for slc in model.slices:
+            slc.owners = list(set(owners) | set(slc.owners))
+        if commit:
+            db.session.commit()
+        return model
+
     @staticmethod
     def bulk_delete(models: Optional[List[Dashboard]], commit: bool = True) -> None:
         item_ids = [model.id for model in models] if models else []
diff --git a/tests/dashboards/api_tests.py b/tests/dashboards/api_tests.py
@@ -567,6 +567,49 @@ def test_update_dashboard(self):
         db.session.delete(model)
         db.session.commit()
 
+    def test_update_dashboard_chart_owners(self):
+        """
+            Dashboard API: Test update chart owners
+        """
+        user_alpha1 = self.create_user(
+            "alpha1", "password", "Alpha", email="alpha1@superset.org"
+        )
+        user_alpha2 = self.create_user(
+            "alpha2", "password", "Alpha", email="alpha2@superset.org"
+        )
+        admin = self.get_user("admin")
+        slices = []
+        slices.append(
+            db.session.query(Slice).filter_by(slice_name="Girl Name Cloud").first()
+        )
+        slices.append(db.session.query(Slice).filter_by(slice_name="Trends").first())
+        slices.append(db.session.query(Slice).filter_by(slice_name="Boys").first())
+
+        dashboard = self.insert_dashboard("title1", "slug1", [admin.id], slices=slices,)
+        self.login(username="admin")
+        uri = f"api/v1/dashboard/{dashboard.id}"
+        dashboard_data = {"owners": [user_alpha1.id, user_alpha2.id]}
+        rv = self.client.put(uri, json=dashboard_data)
+        self.assertEqual(rv.status_code, 200)
+
+        # verify slices owners include alpha1 and alpha2 users
+        slices_ids = [slice.id for slice in slices]
+        # Refetch Slices
+        slices = db.session.query(Slice).filter(Slice.id.in_(slices_ids)).all()
+        for slice in slices:
+            self.assertIn(user_alpha1, slice.owners)
+            self.assertIn(user_alpha2, slice.owners)
+            self.assertIn(admin, slice.owners)
+            # Revert owners on slice
+            slice.owners = []
+            db.session.commit()
+
+        # Rollback changes
+        db.session.delete(dashboard)
+        db.session.delete(user_alpha1)
+        db.session.delete(user_alpha2)
+        db.session.commit()
+
     def test_update_partial_dashboard(self):
         """
             Dashboard API: Test update partial
