From 20cc4f8c778ab72903d471d89f99fe7ea104448e Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 7 Apr 2020 15:41:28 +0100 Subject: [PATCH 1/4] [dashboards] Fix, update dashboard owners not propagating to charts owners --- superset/dashboards/commands/create.py | 3 ++- superset/dashboards/commands/update.py | 3 ++- superset/dashboards/dao.py | 9 +++++++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/superset/dashboards/commands/create.py b/superset/dashboards/commands/create.py index 38658be3b2bb2..a94bd79dfbcfb 100644 --- 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 e: logger.exception(e.exception) raise DashboardCreateFailedError() diff --git a/superset/dashboards/commands/update.py b/superset/dashboards/commands/update.py index 8f23dafa0df4b..da5384cc11e26 100644 --- 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 e: logger.exception(e.exception) raise DashboardUpdateFailedError() diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py index 050b507b6369f..41d5f680be952 100644 --- 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 = [o for o 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 [] From 1a7b2ca5faebde9d6911f5bcecd0f4600845e92d Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Wed, 8 Apr 2020 11:47:03 +0100 Subject: [PATCH 2/4] Add tests --- tests/dashboards/api_tests.py | 39 +++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/dashboards/api_tests.py b/tests/dashboards/api_tests.py index fcd8d11ef2814..fc15835b1c26f 100644 --- a/tests/dashboards/api_tests.py +++ b/tests/dashboards/api_tests.py @@ -567,6 +567,45 @@ 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) + + 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 From 7d846f9016c2ee7924d0a97c83957bb55c195123 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Wed, 8 Apr 2020 13:56:33 +0100 Subject: [PATCH 3/4] Fix tests --- tests/dashboards/api_tests.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/dashboards/api_tests.py b/tests/dashboards/api_tests.py index fc15835b1c26f..881e897d51742 100644 --- a/tests/dashboards/api_tests.py +++ b/tests/dashboards/api_tests.py @@ -600,7 +600,11 @@ def test_update_dashboard_chart_owners(self): 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) From d03ec4135db47e945a13fd084c44ad18e24f0b8e Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Wed, 8 Apr 2020 16:15:59 +0100 Subject: [PATCH 4/4] better naming --- superset/dashboards/dao.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py index 41d5f680be952..60ec3a61abe2b 100644 --- a/superset/dashboards/dao.py +++ b/superset/dashboards/dao.py @@ -49,7 +49,7 @@ def validate_update_slug_uniqueness(dashboard_id: int, slug: Optional[str]) -> b @staticmethod def update_charts_owners(model: Dashboard, commit: bool = True) -> Dashboard: - owners = [o for o in model.owners] + owners = [owner for owner in model.owners] for slc in model.slices: slc.owners = list(set(owners) | set(slc.owners)) if commit: