Skip to content

Commit

Permalink
Notify feature owners for accuracy emails (#4528)
Browse files Browse the repository at this point in the history
* Send feature accuracy emails to owners only

* nit
KyleJu authored Nov 14, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent f9f5571 commit 60555f9
Showing 2 changed files with 129 additions and 34 deletions.
28 changes: 21 additions & 7 deletions internals/reminders.py
Original file line number Diff line number Diff line change
@@ -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."""
135 changes: 108 additions & 27 deletions internals/reminders_test.py
Original file line number Diff line number Diff line change
@@ -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}

0 comments on commit 60555f9

Please sign in to comment.