Browse Source

fix(weekly-report): wrap filter call in list() (#42276)

- previously, the weekly report was using `filter` as part of the
`key_errors` calculation. filter returns an iterable, which will be
empty after it is iterated through.
- we fix this by adding a wrapping `list` call
- update one test to send an email to two organization members, and
check the length of key_errors. this test fails on master.
Josh Ferge 2 years ago
parent
commit
9992685932
2 changed files with 34 additions and 26 deletions
  1. 12 10
      src/sentry/tasks/weekly_reports.py
  2. 22 16
      tests/sentry/tasks/test_weekly_reports.py

+ 12 - 10
src/sentry/tasks/weekly_reports.py

@@ -382,16 +382,18 @@ def fetch_key_error_groups(ctx):
     for project_ctx in ctx.projects.values():
         # note Snuba might have groups that have since been deleted
         # we should just ignore those
-        project_ctx.key_errors = filter(
-            lambda x: x[0] is not None,
-            [
-                (
-                    group_id_to_group.get(group_id),
-                    group_id_to_group_history.get(group_id, None),
-                    count,
-                )
-                for group_id, count in project_ctx.key_errors
-            ],
+        project_ctx.key_errors = list(
+            filter(
+                lambda x: x[0] is not None,
+                [
+                    (
+                        group_id_to_group.get(group_id),
+                        group_id_to_group_history.get(group_id, None),
+                        count,
+                    )
+                    for group_id, count in project_ctx.key_errors
+                ],
+            )
         )
 
 

+ 22 - 16
tests/sentry/tasks/test_weekly_reports.py

@@ -144,6 +144,10 @@ class WeeklyReportsTest(OutcomesSnubaTest, SnubaTestCase):
         two_days_ago = now - timedelta(days=2)
         three_days_ago = now - timedelta(days=3)
 
+        self.create_member(
+            teams=[self.team], user=self.create_user(), organization=self.organization
+        )
+
         event1 = self.store_event(
             data={
                 "event_id": "a" * 32,
@@ -202,22 +206,24 @@ class WeeklyReportsTest(OutcomesSnubaTest, SnubaTestCase):
 
         prepare_organization_report(to_timestamp(now), ONE_DAY * 7, self.organization.id)
 
-        message_params = message_builder.call_args.kwargs
-        context = message_params["context"]
-
-        assert message_params["template"] == "sentry/emails/reports/body.txt"
-        assert message_params["html_template"] == "sentry/emails/reports/body.html"
-
-        assert context["organization"] == self.organization
-        assert context["issue_summary"] == {
-            "all_issue_count": 2,
-            "existing_issue_count": 0,
-            "new_issue_count": 2,
-            "reopened_issue_count": 0,
-        }
-        assert context["trends"]["total_error_count"] == 2
-        assert context["trends"]["total_transaction_count"] == 10
-        assert "Weekly Report for" in message_params["subject"]
+        for call_args in message_builder.call_args_list:
+            message_params = call_args.kwargs
+            context = message_params["context"]
+
+            assert message_params["template"] == "sentry/emails/reports/body.txt"
+            assert message_params["html_template"] == "sentry/emails/reports/body.html"
+
+            assert context["organization"] == self.organization
+            assert context["issue_summary"] == {
+                "all_issue_count": 2,
+                "existing_issue_count": 0,
+                "new_issue_count": 2,
+                "reopened_issue_count": 0,
+            }
+            assert len(context["key_errors"]) == 2
+            assert context["trends"]["total_error_count"] == 2
+            assert context["trends"]["total_transaction_count"] == 10
+            assert "Weekly Report for" in message_params["subject"]
 
     @mock.patch("sentry.tasks.weekly_reports.MessageBuilder")
     def test_message_builder_advanced(self, message_builder):