From c131f025f88073fce2a002466b70d088e8902a9d Mon Sep 17 00:00:00 2001 From: AAfghahi <48933336+AAfghahi@users.noreply.github.com> Date: Mon, 6 Jun 2022 10:13:22 -0400 Subject: [PATCH] chore: add event logger to reports/alerts CRUD (#20249) * add event logger to report CRUD * added assert metric to tests * added unique name to action * testing * with inline log context --- superset/reports/api.py | 35 ++++++++- tests/integration_tests/reports/api_tests.py | 76 ++++++++++---------- 2 files changed, 73 insertions(+), 38 deletions(-) diff --git a/superset/reports/api.py b/superset/reports/api.py index 4694e22874..046fe90595 100644 --- a/superset/reports/api.py +++ b/superset/reports/api.py @@ -29,6 +29,7 @@ from superset.charts.filters import ChartFilter from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.dashboards.filters import DashboardAccessFilter from superset.databases.filters import DatabaseFilter +from superset.extensions import event_logger from superset.models.reports import ReportSchedule from superset.reports.commands.bulk_delete import BulkDeleteReportScheduleCommand from superset.reports.commands.create import CreateReportScheduleCommand @@ -227,8 +228,12 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi): @expose("/", methods=["DELETE"]) @protect() @safe - @statsd_metrics @permission_name("delete") + @statsd_metrics + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.delete", + log_to_statsd=False, + ) def delete(self, pk: int) -> Response: """Delete a Report Schedule --- @@ -281,7 +286,9 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi): @statsd_metrics @permission_name("post") @requires_json - def post(self) -> Response: + def post( + self, + ) -> Response: """Creates a new Report Schedule --- post: @@ -319,6 +326,16 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi): """ try: item = self.add_model_schema.load(request.json) + # normally this would be covered by a decorator, however + # due to this model being formatted incorrectly the data + # needed some manipulation. + event_logger.log_with_context( + action="ReportScheduleRestApi.post", + dashboard_id=request.json.get("dashboard", None), + chart_id=request.json.get("chart", None), + report_format=request.json.get("report_format", None), + active=request.json.get("active", None), + ) # This validates custom Schema with custom validations except ValidationError as error: return self.response_400(message=error.messages) @@ -390,6 +407,16 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi): """ try: item = self.edit_model_schema.load(request.json) + # normally this would be covered by a decorator, however + # due to this model being formatted incorrectly the data + # needed some manipulation. + event_logger.log_with_context( + action="ReportScheduleRestApi.put", + dashboard_id=request.json.get("dashboard", None), + chart_id=request.json.get("chart", None), + report_format=request.json.get("report_format", None), + active=request.json.get("active", None), + ) # This validates custom Schema with custom validations except ValidationError as error: return self.response_400(message=error.messages) @@ -416,6 +443,10 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi): @safe @statsd_metrics @rison(get_delete_ids_schema) + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.bulk_delete", + log_to_statsd=False, + ) def bulk_delete(self, **kwargs: Any) -> Response: """Delete bulk Report Schedule layers --- diff --git a/tests/integration_tests/reports/api_tests.py b/tests/integration_tests/reports/api_tests.py index b111a19110..2ca41610a2 100644 --- a/tests/integration_tests/reports/api_tests.py +++ b/tests/integration_tests/reports/api_tests.py @@ -471,7 +471,7 @@ class TestReportSchedulesApi(SupersetTestCase): "database": example_db.id, } uri = "api/v1/report/" - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") data = json.loads(rv.data.decode("utf-8")) assert rv.status_code == 201 created_model = db.session.query(ReportSchedule).get(data.get("id")) @@ -507,7 +507,7 @@ class TestReportSchedulesApi(SupersetTestCase): "database": example_db.id, } uri = "api/v1/report/" - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") assert rv.status_code == 422 data = json.loads(rv.data.decode("utf-8")) assert data == {"message": {"name": ['An alert named "name3" already exists']}} @@ -554,7 +554,7 @@ class TestReportSchedulesApi(SupersetTestCase): "database": example_db.id, } uri = "api/v1/report/" - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") assert rv.status_code == 400 # Test that report can be created with null grace period @@ -579,7 +579,7 @@ class TestReportSchedulesApi(SupersetTestCase): "database": example_db.id, } uri = "api/v1/report/" - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") assert rv.status_code == 201 # Test that grace period and working timeout cannot be < 1 @@ -604,7 +604,7 @@ class TestReportSchedulesApi(SupersetTestCase): "database": example_db.id, } uri = "api/v1/report/" - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") assert rv.status_code == 400 report_schedule_data = { @@ -629,7 +629,7 @@ class TestReportSchedulesApi(SupersetTestCase): "database": example_db.id, } uri = "api/v1/report/" - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") assert rv.status_code == 400 # Test that report can be created with null dashboard @@ -655,7 +655,7 @@ class TestReportSchedulesApi(SupersetTestCase): "database": example_db.id, } uri = "api/v1/report/" - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") assert rv.status_code == 201 # Test that report can be created with null chart @@ -681,7 +681,7 @@ class TestReportSchedulesApi(SupersetTestCase): "database": example_db.id, } uri = "api/v1/report/" - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") assert rv.status_code == 201 # Test that report cannot be created with null timezone @@ -706,7 +706,7 @@ class TestReportSchedulesApi(SupersetTestCase): "dashboard": dashboard.id, "database": example_db.id, } - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") assert rv.status_code == 400 data = json.loads(rv.data.decode("utf-8")) assert data == {"message": {"timezone": ["Field may not be null."]}} @@ -733,7 +733,7 @@ class TestReportSchedulesApi(SupersetTestCase): "dashboard": dashboard.id, "database": example_db.id, } - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") assert rv.status_code == 400 data = json.loads(rv.data.decode("utf-8")) assert data == { @@ -765,7 +765,7 @@ class TestReportSchedulesApi(SupersetTestCase): "database": example_db.id, } uri = "api/v1/report/" - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") data = json.loads(rv.data.decode("utf-8")) assert data["result"]["timezone"] == "America/Los_Angeles" assert rv.status_code == 201 @@ -791,7 +791,7 @@ class TestReportSchedulesApi(SupersetTestCase): "chart": 0, } uri = "api/v1/report/" - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") data = json.loads(rv.data.decode("utf-8")) assert rv.status_code == 422 assert ( @@ -818,7 +818,7 @@ class TestReportSchedulesApi(SupersetTestCase): "crontab": "0 9 * * *", } uri = "api/v1/report/" - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") data = json.loads(rv.data.decode("utf-8")) assert rv.status_code == 422 assert ( @@ -847,7 +847,7 @@ class TestReportSchedulesApi(SupersetTestCase): "chart": chart.id, } uri = "api/v1/report/" - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") data = json.loads(rv.data.decode("utf-8")) assert rv.status_code == 201 @@ -863,7 +863,7 @@ class TestReportSchedulesApi(SupersetTestCase): "chart": chart.id, } uri = "api/v1/report/" - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") data = json.loads(rv.data.decode("utf-8")) assert rv.status_code == 409 assert data == { @@ -905,7 +905,7 @@ class TestReportSchedulesApi(SupersetTestCase): "dashboard": dashboard.id, } uri = "api/v1/report/" - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") data = json.loads(rv.data.decode("utf-8")) assert rv.status_code == 201 @@ -921,7 +921,7 @@ class TestReportSchedulesApi(SupersetTestCase): "dashboard": dashboard.id, } uri = "api/v1/report/" - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") data = json.loads(rv.data.decode("utf-8")) assert rv.status_code == 409 assert data == { @@ -964,7 +964,7 @@ class TestReportSchedulesApi(SupersetTestCase): "database": example_db.id, } uri = "api/v1/report/" - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") assert rv.status_code == 422 data = json.loads(rv.data.decode("utf-8")) assert data == {"message": {"chart": "Choose a chart or dashboard not both"}} @@ -987,7 +987,7 @@ class TestReportSchedulesApi(SupersetTestCase): "chart": chart.id, } uri = "api/v1/report/" - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") assert rv.status_code == 422 data = json.loads(rv.data.decode("utf-8")) assert data == {"message": {"database": "Database is required for alerts"}} @@ -1014,7 +1014,7 @@ class TestReportSchedulesApi(SupersetTestCase): "database": database_max_id + 1, } uri = "api/v1/report/" - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") assert rv.status_code == 422 data = json.loads(rv.data.decode("utf-8")) assert data == { @@ -1036,7 +1036,7 @@ class TestReportSchedulesApi(SupersetTestCase): "database": examples_db.id, } uri = "api/v1/report/" - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") assert rv.status_code == 422 data = json.loads(rv.data.decode("utf-8")) assert data == {"message": {"dashboard": "Dashboard does not exist"}} @@ -1110,7 +1110,7 @@ class TestReportSchedulesApi(SupersetTestCase): "database": example_db.id, } uri = "api/v1/report/" - rv = self.client.post(uri, json=report_schedule_data) + rv = self.post_assert_metric(uri, report_schedule_data, "post") response = json.loads(rv.data.decode("utf-8")) assert response == { "message": {"creation_method": ["Invalid enum value BAD_CREATION_METHOD"]} @@ -1148,7 +1148,7 @@ class TestReportSchedulesApi(SupersetTestCase): uri = f"api/v1/report/{report_schedule.id}" - rv = self.client.put(uri, json=report_schedule_data) + rv = self.put_assert_metric(uri, report_schedule_data, "put") assert rv.status_code == 200 updated_model = db.session.query(ReportSchedule).get(report_schedule.id) assert updated_model is not None @@ -1173,7 +1173,7 @@ class TestReportSchedulesApi(SupersetTestCase): self.login(username="admin") report_schedule_data = {"active": False} uri = f"api/v1/report/{report_schedule.id}" - rv = self.client.put(uri, json=report_schedule_data) + rv = self.put_assert_metric(uri, report_schedule_data, "put") assert rv.status_code == 200 report_schedule = ( db.session.query(ReportSchedule) @@ -1196,7 +1196,7 @@ class TestReportSchedulesApi(SupersetTestCase): self.login(username="admin") report_schedule_data = {"name": "name3", "description": "changed_description"} uri = f"api/v1/report/{report_schedule.id}" - rv = self.client.put(uri, json=report_schedule_data) + rv = self.put_assert_metric(uri, report_schedule_data, "put") data = json.loads(rv.data.decode("utf-8")) assert rv.status_code == 422 assert data == {"message": {"name": ['An alert named "name3" already exists']}} @@ -1238,7 +1238,7 @@ class TestReportSchedulesApi(SupersetTestCase): "database": example_db.id, } uri = f"api/v1/report/{report_schedule.id}" - rv = self.client.put(uri, json=report_schedule_data) + rv = self.put_assert_metric(uri, report_schedule_data, "put") assert rv.status_code == 422 data = json.loads(rv.data.decode("utf-8")) assert data == {"message": {"chart": "Choose a chart or dashboard not both"}} @@ -1272,7 +1272,7 @@ class TestReportSchedulesApi(SupersetTestCase): "database": database_max_id + 1, } uri = f"api/v1/report/{report_schedule.id}" - rv = self.client.put(uri, json=report_schedule_data) + rv = self.put_assert_metric(uri, report_schedule_data, "put") assert rv.status_code == 422 data = json.loads(rv.data.decode("utf-8")) assert data == { @@ -1293,7 +1293,7 @@ class TestReportSchedulesApi(SupersetTestCase): "database": examples_db.id, } uri = f"api/v1/report/{report_schedule.id}" - rv = self.client.put(uri, json=report_schedule_data) + rv = self.put_assert_metric(uri, report_schedule_data, "put") assert rv.status_code == 422 data = json.loads(rv.data.decode("utf-8")) assert data == {"message": {"dashboard": "Dashboard does not exist"}} @@ -1330,7 +1330,7 @@ class TestReportSchedulesApi(SupersetTestCase): ) self.login(username="admin") uri = f"api/v1/report/{report_schedule.id}" - rv = self.client.delete(uri) + rv = self.delete_assert_metric(uri, "delete") assert rv.status_code == 200 deleted_report_schedule = db.session.query(ReportSchedule).get( report_schedule.id @@ -1357,7 +1357,7 @@ class TestReportSchedulesApi(SupersetTestCase): max_id = db.session.query(func.max(ReportSchedule.id)).scalar() self.login(username="admin") uri = f"api/v1/report/{max_id + 1}" - rv = self.client.delete(uri) + rv = self.delete_assert_metric(uri, "delete") assert rv.status_code == 404 @pytest.mark.usefixtures("create_report_schedules") @@ -1374,7 +1374,7 @@ class TestReportSchedulesApi(SupersetTestCase): self.login(username="alpha2", password="password") uri = f"api/v1/report/{report_schedule.id}" - rv = self.client.delete(uri) + rv = self.delete_assert_metric(uri, "delete") self.assertEqual(rv.status_code, 403) @pytest.mark.usefixtures("create_report_schedules") @@ -1390,7 +1390,7 @@ class TestReportSchedulesApi(SupersetTestCase): ] self.login(username="admin") uri = f"api/v1/report/?q={prison.dumps(report_schedules_ids)}" - rv = self.client.delete(uri) + rv = self.delete_assert_metric(uri, "bulk_delete") assert rv.status_code == 200 deleted_report_schedules = query_report_schedules.all() assert deleted_report_schedules == [] @@ -1413,7 +1413,7 @@ class TestReportSchedulesApi(SupersetTestCase): report_schedules_ids.append(max_id + 1) self.login(username="admin") uri = f"api/v1/report/?q={prison.dumps(report_schedules_ids)}" - rv = self.client.delete(uri) + rv = self.delete_assert_metric(uri, "bulk_delete") assert rv.status_code == 404 @pytest.mark.usefixtures("create_report_schedules") @@ -1431,7 +1431,7 @@ class TestReportSchedulesApi(SupersetTestCase): self.login(username="alpha2", password="password") uri = f"api/v1/report/?q={prison.dumps(report_schedules_ids)}" - rv = self.client.delete(uri) + rv = self.delete_assert_metric(uri, "bulk_delete") self.assertEqual(rv.status_code, 403) @pytest.mark.usefixtures("create_report_schedules") @@ -1562,7 +1562,9 @@ class TestReportSchedulesApi(SupersetTestCase): "database": example_db.id, "extra": {"dashboard_tab_ids": ["INVALID-TAB-ID-1", "TABS-IpViLohnyP"]}, } - response = self.client.post("api/v1/report/", json=report_schedule_data) + response = self.post_assert_metric( + "api/v1/report/", report_schedule_data, "post" + ) assert response.status_code == 422 assert response.json == { "message": {"extra": ["Invalid tab IDs selected: ['INVALID-TAB-ID-1']"]} @@ -1597,7 +1599,9 @@ class TestReportSchedulesApi(SupersetTestCase): "database": example_db.id, "extra": {"dashboard_tab_ids": ["TABS-IpViLohnyP"]}, } - response = self.client.post("api/v1/report/", json=report_schedule_data) + response = self.post_assert_metric( + "api/v1/report/", report_schedule_data, "post" + ) assert response.status_code == 201 assert json.loads( db.session.query(ReportSchedule)