Browse Source

fix(hc): Handle emtpy string values for UserOption more gracefully (#54345)

Zach Collins 1 year ago
parent
commit
8dd27791af
2 changed files with 24 additions and 2 deletions
  1. 1 2
      src/sentry/tasks/weekly_reports.py
  2. 23 0
      tests/sentry/tasks/test_weekly_reports.py

+ 1 - 2
src/sentry/tasks/weekly_reports.py

@@ -657,8 +657,7 @@ def deliver_reports(ctx, dry_run=False, target_user=None, email_override=None):
         }
         }
 
 
         for user_id in user_set:
         for user_id in user_set:
-            # We manually pick out user.options and use PickledObjectField to deserialize it. We get a list of organizations the user has unsubscribed from user reports
-            option = options_by_user_id.get(user_id, [])
+            option = list(options_by_user_id.get(user_id, []))
             user_subscribed_to_organization_reports = ctx.organization.id not in option
             user_subscribed_to_organization_reports = ctx.organization.id not in option
             if user_subscribed_to_organization_reports:
             if user_subscribed_to_organization_reports:
                 send_email(ctx, user_id, dry_run=dry_run)
                 send_email(ctx, user_id, dry_run=dry_run)

+ 23 - 0
tests/sentry/tasks/test_weekly_reports.py

@@ -13,6 +13,7 @@ from freezegun import freeze_time
 
 
 from sentry.constants import DataCategory
 from sentry.constants import DataCategory
 from sentry.models import GroupHistoryStatus, GroupStatus, OrganizationMember, Project, UserOption
 from sentry.models import GroupHistoryStatus, GroupStatus, OrganizationMember, Project, UserOption
+from sentry.services.hybrid_cloud.user_option import user_option_service
 from sentry.silo import SiloMode, unguarded_write
 from sentry.silo import SiloMode, unguarded_write
 from sentry.tasks.weekly_reports import (
 from sentry.tasks.weekly_reports import (
     ONE_DAY,
     ONE_DAY,
@@ -66,6 +67,28 @@ class WeeklyReportsTest(OutcomesSnubaTest, SnubaTestCase):
             message = mail.outbox[0]
             message = mail.outbox[0]
             assert self.organization.name in message.subject
             assert self.organization.name in message.subject
 
 
+    @freeze_time(before_now(days=2).replace(hour=0, minute=0, second=0, microsecond=0))
+    def test_with_empty_string_user_option(self):
+        now = datetime.now().replace(tzinfo=pytz.utc)
+
+        project = self.create_project(
+            organization=self.organization, teams=[self.team], date_added=now - timedelta(days=90)
+        )
+        self.store_event(data={"timestamp": iso_format(before_now(days=1))}, project_id=project.id)
+        member_set = set(project.teams.first().member_set.all())
+        for member in member_set:
+            # some users have an empty string value set for this key, presumably cleared.
+            user_option_service.set_option(
+                user_id=member.user_id, key="reports:disabled-organizations", value=""
+            )
+
+        with self.tasks():
+            schedule_organizations(timestamp=to_timestamp(now))
+            assert len(mail.outbox) == len(member_set) == 1
+
+            message = mail.outbox[0]
+            assert self.organization.name in message.subject
+
     @with_feature("organizations:customer-domains")
     @with_feature("organizations:customer-domains")
     @with_feature("organizations:weekly-email-refresh")
     @with_feature("organizations:weekly-email-refresh")
     @freeze_time(before_now(days=2).replace(hour=0, minute=0, second=0, microsecond=0))
     @freeze_time(before_now(days=2).replace(hour=0, minute=0, second=0, microsecond=0))