Notify feature owners for accuracy emails

internals/reminders.py

@@ -32,6 +32,7 @@
 from internals import slo
 from internals.core_enums import (
     STAGE_TYPES_BY_FIELD_MAPPING)
+from internals.user_models import UserPref
 import settings
 
 
@@ -54,11 +55,16 @@ def get_current_milestone_info(anchor_channel: str):
 
 
 def choose_email_recipients(
-    feature: FeatureEntry, is_escalated: bool) -> list[str]:
+    feature: FeatureEntry, is_escalated: bool, is_accuracy_email: bool) -> list[str]:
   """Choose which recipients will receive the email notification."""
-  # Only feature owners are notified for a non-escalated notification.
-  if not is_escalated:
-    return feature.owner_emails
+
+  # Only feature owners are notified for accuracy or non-escalated notification emails, if not bounced.
+  if is_accuracy_email or not is_escalated:
+    user_prefs = UserPref.get_prefs_for_emails(feature.owner_emails)
+    receivers = list(set([up.email for up in user_prefs
+                  if not up.bounced]))
+    if receivers:
+      return receivers
 
   # Escalated notification. Add extended recipients.
   ws_group_emails = [STAGING_EMAIL]
@@ -77,7 +83,8 @@ def build_email_tasks(
     subject_format: str,
     body_template_path: str,
     current_milestone_info: dict,
-    escalation_check: Callable
+    escalation_check: Callable,
+    is_accuracy_email: Callable
     ) -> list[dict[str, Any]]:
   email_tasks: list[dict[str, Any]] = []
   beta_date = datetime.fromisoformat(current_milestone_info['earliest_beta'])
@@ -106,7 +113,7 @@ def build_email_tasks(
         subject = subject.replace(EMAIL_SUBJECT_PREFIX, 'Escalation request')
       else:
         subject = f'ESCALATED: {subject}'
-    recipients = choose_email_recipients(fe, is_escalated)
+    recipients = choose_email_recipients(fe, is_escalated, is_accuracy_email())
     for recipient in recipients:
       email_tasks.append({
         'to': recipient,
@@ -134,7 +141,7 @@ def get_template_data(self, **kwargs):
     email_tasks = build_email_tasks(
         features_to_notify, self.SUBJECT_FORMAT,
         self.EMAIL_TEMPLATE_PATH, current_milestone_info,
-        self.should_escalate_notification)
+        self.should_escalate_notification, self.is_accuracy_email)
     notifier.send_emails(email_tasks)
 
     recipients_str = ''
@@ -215,6 +222,10 @@ def should_escalate_notification(self, feature: FeatureEntry) -> bool:
     """Determine if the notification should be escalated to more users."""
     return False
 
+  # Subclasses should override if needed.
+  def is_accuracy_email(self) -> bool:
+    return False
+
   # Subclasses should override if processing is needed after notifications sent.
   def changes_after_sending_notifications(
       self, features_notified: list[tuple[FeatureEntry, int]]) -> None:
@@ -261,6 +272,9 @@ def should_escalate_notification(self, feature: FeatureEntry) -> bool:
     """Escalate notification if 2 previous emails have had no response."""
     return feature.outstanding_notifications >= 2
 
+  def is_accuracy_email(self) -> bool:
+    return True
+
   def changes_after_sending_notifications(
       self, notified_features: list[tuple[FeatureEntry, int]]) -> None:
     """Updates the count of any outstanding notifications."""

internals/reminders_test.py

@@ -23,6 +23,7 @@
 from internals.core_models import FeatureEntry, MilestoneSet, Stage
 from internals.review_models import Gate, Vote
 from internals import reminders
+from internals.user_models import UserPref
 
 from google.cloud import ndb  # type: ignore
 
@@ -110,27 +111,56 @@ def setUp(self):
 
     self.feature_template.put()
 
+    self.owner_user_pref = UserPref(
+        email='[email protected]',
+        notify_as_starrer=False)
+    self.owner_user_pref.put()
+    owner_user_pref_1 = UserPref(
+        email='[email protected]',
+        notify_as_starrer=False)
+    owner_user_pref_1.put()
+    owner_user_pref_2 = UserPref(
+        email='[email protected]',
+        notify_as_starrer=False)
+    owner_user_pref_2.put()
+
     self.maxDiff = None
 
   def tearDown(self) -> None:
-    kinds: list[ndb.Model] = [FeatureEntry, Stage]
+    kinds: list[ndb.Model] = [FeatureEntry, Stage, UserPref]
     for kind in kinds:
       for entity in kind.query():
         entity.key.delete()
 
   def test_choose_email_recipients__normal(self):
-    """Normal reminders go to feature participants."""
+    """Normal reminders go to feature owners."""
     actual = reminders.choose_email_recipients(
-        self.feature_template, False)
+        self.feature_template, False, False)
     expected = ['[email protected]',
                 ]
     self.assertEqual(set(actual), set(expected))
 
+  def test_choose_email_recipients__owners_bounced(self):
+    """Normal reminders go to feature participants when owners' emails
+    are bounced."""
+    self.owner_user_pref.bounced = True
+    self.owner_user_pref.put()
+
+    actual = reminders.choose_email_recipients(
+        self.feature_template, False, False)
+    expected = ['[email protected]',
+                '[email protected]',
+                '[email protected]',
+                '[email protected]',
+                '[email protected]',
+                ]
+    self.assertEqual(set(actual), set(expected))
+
   @mock.patch('settings.PROD', True)
   def test_choose_email_recipients__escalated(self):
     """Escalated reminders go to feature participants and lists."""
     actual = reminders.choose_email_recipients(
-        self.feature_template, True)
+        self.feature_template, True, False)
     expected = ['[email protected]',
                 '[email protected]',
                 '[email protected]',
@@ -140,15 +170,50 @@ def test_choose_email_recipients__escalated(self):
                 ]
     self.assertEqual(set(actual), set(expected))
 
+  def test_choose_email_recipients__normal_accuracy_email(self):
+    """Normal accuracy emails go to feature owners."""
+    actual = reminders.choose_email_recipients(
+        self.feature_template, False, True)
+    expected = ['[email protected]',
+                ]
+    self.assertEqual(set(actual), set(expected))
+
+  def test_choose_email_recipients__normal_accuracy_email_when_owners_bounced(self):
+    """Normal accuracy emails go to feature participants when owners' emails
+    are bounced."""
+    self.owner_user_pref.bounced = True
+    self.owner_user_pref.put()
+
+    actual = reminders.choose_email_recipients(
+        self.feature_template, False, True)
+    expected = ['[email protected]',
+                '[email protected]',
+                '[email protected]',
+                '[email protected]',
+                '[email protected]',
+                ]
+    self.assertEqual(set(actual), set(expected))
+
+  @mock.patch('settings.PROD', True)
+  def test_choose_email_recipients_escalated_accuracy_email(self):
+    """Escalated accuracy emails go to feature owners."""
+    actual = reminders.choose_email_recipients(
+        self.feature_template, True, True)
+    expected = ['[email protected]',
+                ]
+    self.assertEqual(set(actual), set(expected))
+
   def test_build_email_tasks_feature_accuracy(self):
     with test_app.app_context():
       handler = reminders.FeatureAccuracyHandler()
       actual = reminders.build_email_tasks(
-          [(self.feature_template, 100)],
-          'Action requested - Verify %s',
-          handler.EMAIL_TEMPLATE_PATH,
-          self.current_milestone_info,
-          handler.should_escalate_notification)
+        [(self.feature_template, 100)],
+        'Action requested - Verify %s',
+        handler.EMAIL_TEMPLATE_PATH,
+        self.current_milestone_info,
+        handler.should_escalate_notification,
+        handler.is_accuracy_email,
+      )
 
     self.assertEqual(1, len(actual))
     task = actual[0]
@@ -163,11 +228,13 @@ def test_build_email_tasks_feature_accuracy__enterprise(self):
     with test_app.app_context():
       handler = reminders.FeatureAccuracyHandler()
       actual = reminders.build_email_tasks(
-          [(self.feature_template, 110)],
-          'Action requested - Verify %s',
-          handler.EMAIL_TEMPLATE_PATH,
-          self.current_milestone_info,
-          handler.should_escalate_notification)
+        [(self.feature_template, 110)],
+        'Action requested - Verify %s',
+        handler.EMAIL_TEMPLATE_PATH,
+        self.current_milestone_info,
+        handler.should_escalate_notification,
+        handler.is_accuracy_email,
+      )
 
     self.assertEqual(1, len(actual))
     task = actual[0]
@@ -186,13 +253,15 @@ def test_build_email_tasks_feature_accuracy__escalated(self):
     with test_app.app_context():
       handler = reminders.FeatureAccuracyHandler()
       actual = reminders.build_email_tasks(
-          [(self.feature_template, 100)],
-          'Action requested - Verify %s',
-          handler.EMAIL_TEMPLATE_PATH,
-          self.current_milestone_info,
-          handler.should_escalate_notification)
+        [(self.feature_template, 100)],
+        'Action requested - Verify %s',
+        handler.EMAIL_TEMPLATE_PATH,
+        self.current_milestone_info,
+        handler.should_escalate_notification,
+        handler.is_accuracy_email,
+      )
 
-    self.assertEqual(5, len(actual))
+    self.assertEqual(1, len(actual))
     task = actual[0]
     self.assertEqual(
         'Escalation request - Verify feature one', task['subject'])
@@ -205,9 +274,13 @@ def test_build_email_tasks_prepublication(self):
     with test_app.app_context():
       handler = reminders.PrepublicationHandler()
       actual = reminders.build_email_tasks(
-          [(self.feature_template, 100)], 'Action requested - Verify %s',
-          handler.EMAIL_TEMPLATE_PATH,
-          self.current_milestone_info, handler.should_escalate_notification)
+        [(self.feature_template, 100)],
+        'Action requested - Verify %s',
+        handler.EMAIL_TEMPLATE_PATH,
+        self.current_milestone_info,
+        handler.should_escalate_notification,
+        handler.is_accuracy_email,
+      )
     self.assertEqual(1, len(actual))
     task = actual[0]
     self.assertEqual('[email protected]', task['to'])
@@ -222,13 +295,24 @@ class FeatureAccuracyHandlerTest(testing_config.CustomTestCase):
   def setUp(self):
     self.feature_1, self.feature_2, self.feature_3 = make_test_features()
     self.handler = reminders.FeatureAccuracyHandler()
+    self.owner_user_pref_1 = UserPref(
+        email='[email protected]',
+        notify_as_starrer=False)
+    self.owner_user_pref_1.put()
+    self.owner_user_pref_2 = UserPref(
+        email='[email protected]',
+        notify_as_starrer=False)
+    self.owner_user_pref_2.put()
 
   def tearDown(self):
     self.feature_1.key.delete()
     self.feature_2.key.delete()
     self.feature_3.key.delete()
     for stage in Stage.query():
       stage.key.delete()
+    self.owner_user_pref_1.key.delete()
+    self.owner_user_pref_2.key.delete()
+
 
   @mock.patch('requests.get')
   def test_determine_features_to_notify__no_features(self, mock_get):
@@ -281,11 +365,8 @@ def test_determine_features_to_notify__escalated(self, mock_get):
     with test_app.app_context():
       result = self.handler.get_template_data()
     # More email tasks should be created to notify extended contributors.
-    expected_message = ('5 email(s) sent or logged.\n'
+    expected_message = ('2 email(s) sent or logged.\n'
                         'Recipients:\n'
-                        '[email protected]\n'
-                        '[email protected]\n'
-                        '[email protected]\n'
                         '[email protected]\n'
                         '[email protected]')
     expected = {'message': expected_message}