From fe06c249ebd6803f01362c24490e01adcd8118ad Mon Sep 17 00:00:00 2001 From: sinhashubham95 Date: Fri, 21 Oct 2022 21:31:52 +0530 Subject: [PATCH 1/5] fixed code for bulk delete of embedded dashboards --- superset/dashboards/dao.py | 1 + .../integration_tests/dashboards/api_tests.py | 41 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py index 36db7e341a114..9f3d5178d7bb6 100644 --- a/superset/dashboards/dao.py +++ b/superset/dashboards/dao.py @@ -160,6 +160,7 @@ def bulk_delete(models: Optional[List[Dashboard]], commit: bool = True) -> None: for model in models: model.slices = [] model.owners = [] + model.embedded = [] db.session.merge(model) # bulk delete itself try: diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index 363ba9232051b..0e4b6628c96fd 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -803,6 +803,47 @@ def test_delete_bulk_dashboards(self): model = db.session.query(Dashboard).get(dashboard_id) self.assertEqual(model, None) + def test_delete_bulk_embedded_dashboards(self): + """ + Dashboard API: Test delete bulk embedded + """ + admin_id = self.get_user("admin").id + dashboard_count = 4 + dashboard_ids = list() + for dashboard_name_index in range(dashboard_count): + dashboard_ids.append( + self.insert_dashboard( + f"title{dashboard_name_index}", + f"slug{dashboard_name_index}", + [admin_id], + ).id + ) + self.login(username="admin") + for dashboard_id in range(dashboard_ids): + # post succeeds and returns value + allowed_domains = ["test.example", "embedded.example"] + resp = self.post_assert_metric( + f"api/v1/dashboard/{dashboard_id}/embedded", + {"allowed_domains": allowed_domains}, + "set_embedded", + ) + self.assertEqual(resp.status_code, 200) + result = json.loads(resp.data.decode("utf-8"))["result"] + self.assertIsNotNone(result["uuid"]) + self.assertNotEqual(result["uuid"], "") + self.assertEqual(result["allowed_domains"], allowed_domains) + self.login(username="admin") + argument = dashboard_ids + uri = f"api/v1/dashboard/?q={prison.dumps(argument)}" + rv = self.delete_assert_metric(uri, "bulk_delete") + self.assertEqual(rv.status_code, 200) + response = json.loads(rv.data.decode("utf-8")) + expected_response = {"message": f"Deleted {dashboard_count} dashboards"} + self.assertEqual(response, expected_response) + for dashboard_id in dashboard_ids: + model = db.session.query(Dashboard).get(dashboard_id) + self.assertEqual(model, None) + def test_delete_bulk_dashboards_bad_request(self): """ Dashboard API: Test delete bulk bad request From 224784e4de9cc7fa01bc9eb69d33440e26b37ac2 Mon Sep 17 00:00:00 2001 From: sinhashubham95 Date: Fri, 21 Oct 2022 21:54:15 +0530 Subject: [PATCH 2/5] fixed test case --- tests/integration_tests/dashboards/api_tests.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index 0e4b6628c96fd..3ab2c66a5c83c 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -819,11 +819,11 @@ def test_delete_bulk_embedded_dashboards(self): ).id ) self.login(username="admin") - for dashboard_id in range(dashboard_ids): + for dashboard_name_index in range(dashboard_count): # post succeeds and returns value allowed_domains = ["test.example", "embedded.example"] resp = self.post_assert_metric( - f"api/v1/dashboard/{dashboard_id}/embedded", + f"api/v1/dashboard/slug{dashboard_name_index}/embedded", {"allowed_domains": allowed_domains}, "set_embedded", ) From 7fd6abef519b7b81531db039d176b6a6689bbe7e Mon Sep 17 00:00:00 2001 From: sinhashubham95 Date: Sat, 22 Oct 2022 13:04:20 +0530 Subject: [PATCH 3/5] fixed test case based on suggestion --- tests/integration_tests/dashboards/api_tests.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index 3ab2c66a5c83c..0dab4ec150b13 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -807,7 +807,7 @@ def test_delete_bulk_embedded_dashboards(self): """ Dashboard API: Test delete bulk embedded """ - admin_id = self.get_user("admin").id + user = self.get_user("admin") dashboard_count = 4 dashboard_ids = list() for dashboard_name_index in range(dashboard_count): @@ -815,10 +815,10 @@ def test_delete_bulk_embedded_dashboards(self): self.insert_dashboard( f"title{dashboard_name_index}", f"slug{dashboard_name_index}", - [admin_id], + [user.id], ).id ) - self.login(username="admin") + self.login(username=user.username) for dashboard_name_index in range(dashboard_count): # post succeeds and returns value allowed_domains = ["test.example", "embedded.example"] @@ -832,7 +832,6 @@ def test_delete_bulk_embedded_dashboards(self): self.assertIsNotNone(result["uuid"]) self.assertNotEqual(result["uuid"], "") self.assertEqual(result["allowed_domains"], allowed_domains) - self.login(username="admin") argument = dashboard_ids uri = f"api/v1/dashboard/?q={prison.dumps(argument)}" rv = self.delete_assert_metric(uri, "bulk_delete") From 37bedcbe3b97cfeda39fbd4802132ebf6b36b7cb Mon Sep 17 00:00:00 2001 From: sinhashubham95 Date: Sat, 22 Oct 2022 15:20:16 +0530 Subject: [PATCH 4/5] updated for testing --- superset/dashboards/dao.py | 1 - tests/integration_tests/dashboards/api_tests.py | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py index 9f3d5178d7bb6..36db7e341a114 100644 --- a/superset/dashboards/dao.py +++ b/superset/dashboards/dao.py @@ -160,7 +160,6 @@ def bulk_delete(models: Optional[List[Dashboard]], commit: bool = True) -> None: for model in models: model.slices = [] model.owners = [] - model.embedded = [] db.session.merge(model) # bulk delete itself try: diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index 0dab4ec150b13..35f59f363923c 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -814,16 +814,16 @@ def test_delete_bulk_embedded_dashboards(self): dashboard_ids.append( self.insert_dashboard( f"title{dashboard_name_index}", - f"slug{dashboard_name_index}", + None, [user.id], ).id ) self.login(username=user.username) - for dashboard_name_index in range(dashboard_count): + for dashboard_id in dashboard_ids: # post succeeds and returns value allowed_domains = ["test.example", "embedded.example"] resp = self.post_assert_metric( - f"api/v1/dashboard/slug{dashboard_name_index}/embedded", + f"api/v1/dashboard/{dashboard_id}/embedded", {"allowed_domains": allowed_domains}, "set_embedded", ) From 88172e2864c889726aa98ece15e1e23c1ca1b16b Mon Sep 17 00:00:00 2001 From: sinhashubham95 Date: Sat, 22 Oct 2022 15:36:47 +0530 Subject: [PATCH 5/5] fixed bulk deleting embedded dashboards --- superset/dashboards/dao.py | 1 + 1 file changed, 1 insertion(+) diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py index 36db7e341a114..9f3d5178d7bb6 100644 --- a/superset/dashboards/dao.py +++ b/superset/dashboards/dao.py @@ -160,6 +160,7 @@ def bulk_delete(models: Optional[List[Dashboard]], commit: bool = True) -> None: for model in models: model.slices = [] model.owners = [] + model.embedded = [] db.session.merge(model) # bulk delete itself try: