Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new notification when OT approvals are obtained #4685

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions api/reviews_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,12 @@ def do_post(self, **kwargs) -> dict[str, str]:
new_gate_state = approval_defs.set_vote(feature_id, None, new_state,
user.email(), gate_id)

# Send any notifications necessary if the gate is newly approved.
recently_approved = (old_gate_state not in (Vote.APPROVED, Vote.NA) and
new_gate_state in (Vote.APPROVED, Vote.NA))
# Notify that trial extension has been approved.
if gate.gate_type == GATE_API_EXTEND_ORIGIN_TRIAL and recently_approved:
if recently_approved:
stage = Stage.get_by_id(gate.stage_id)
notifier_helpers.send_trial_extension_approved_notification(
fe, stage, gate_id)
notifier_helpers.notify_approvals(fe, stage, gate)

if new_state in (Vote.REVIEW_REQUESTED, Vote.NA_REQUESTED):
old_assignees = gate.assignee_emails[:]
Expand Down
6 changes: 2 additions & 4 deletions internals/detect_intent.py
Original file line number Diff line number Diff line change
Expand Up @@ -376,10 +376,8 @@ def create_approvals(self, feature: FeatureEntry, stage: Stage, gate: Gate,
gate.key.integer_id())
recently_approved = (old_gate_state not in (Vote.APPROVED, Vote.NA) and
new_gate_state in (Vote.APPROVED, Vote.NA))
if (gate.gate_type == core_enums.GATE_API_EXTEND_ORIGIN_TRIAL and
recently_approved):
notifier_helpers.send_trial_extension_approved_notification(
feature, stage, gate.key.integer_id())
if recently_approved:
notifier_helpers.notify_approvals(feature, stage, gate)

# Case 2: Create a review request and set gate state for any
# discussion that does not already have any approval values
Expand Down
29 changes: 29 additions & 0 deletions internals/notifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,35 @@ def build_email(self, stage: StageDict, contacts: list[str]) -> dict:
'html': body,
}


class OTCreationApprovedHandler(basehandlers.FlaskHandler):
"""Notify about an origin trial having received reviews and approvals and is
ready to request creation."""
Comment on lines +683 to +684
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""Notify about an origin trial having received reviews and approvals and is
ready to request creation."""
"""Notify about an origin trial having received reviews and approvals and being
ready for the user to request creation."""


IS_INTERNAL_HANDLER = True
EMAIL_TEMPLATE_PATH = 'origintrials/ot-creation-approved-email.html'

def process_post_data(self, **kwargs):
self.require_task_header()
feature = self.get_param('feature', required=True)
contacts = feature['owner_emails'] or []

send_emails([self.build_email(contacts)])
return {'message': 'OK'}

def build_email(self, feature: dict[str, Any], contacts: list[str]) -> dict:
body_data = {
'chromestatus_url': f'https://chromestatus.com/feature/{feature["id"]}'
}
body = render_template(self.EMAIL_TEMPLATE_PATH, **body_data)
return {
'to': contacts,
'subject': 'You can now submit your origin trial creation request',
'reply_to': None,
'html': body,
}


class OTCreationProcessedHandler(basehandlers.FlaskHandler):
"""Notify about an origin trial creation request being processed,
but activation is at a later date.
Expand Down
24 changes: 24 additions & 0 deletions internals/notifier_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,21 @@ def send_ot_creation_notification(stage: Stage):
'/tasks/email-ot-creation-request',params)


def notify_approvals(feature: 'FeatureEntry', stage: Stage, gate: Gate) -> None:
"""Send notifications when important approvals have been obtained"""
if gate.gate_type == core_enums.GATE_API_EXTEND_ORIGIN_TRIAL:
send_trial_extension_approved_notification(
feature, stage, gate.key.integer_id())
if gate.gate_type == core_enums.GATE_API_ORIGIN_TRIAL:
# OT creation notifications should only be sent when all gates are
# approved or N/A.
all_ot_gates: list[Gate] = Gate.query(
Gate.stage_id == stage.key.integer_id()).fetch()
if all(g.state in (Vote.APPROVED, Vote.NA) for g in all_ot_gates):
send_trial_creation_approved_notification(
feature, stage)


def send_trial_extension_approved_notification(
fe: 'FeatureEntry', stage: Stage, gate_id: int) -> None:
"""Notify that a trial extension is ready to be finalized."""
Expand All @@ -228,6 +243,15 @@ def send_trial_extension_approved_notification(
cloud_tasks_helpers.enqueue_task('/tasks/email-ot-extension-approved', params)


def send_trial_creation_approved_notification(
fe: 'FeatureEntry', stage: Stage) -> None:
"""Notify that a trial extension is ready to be finalized."""
params = {
'feature': converters.feature_entry_to_json_verbose(fe),
}
cloud_tasks_helpers.enqueue_task('/tasks/email-ot-creation-approved', params)


def send_trial_extended_notification(ot_stage: Stage, extension_stage: Stage):
"""Notify about a successful automatic trial extension."""
params = {
Expand Down
66 changes: 66 additions & 0 deletions internals/notifier_helpers_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,3 +142,69 @@ def test_notify_subscribers_of_new_comments(self, mock_task_helpers):
self.feature_1, self.gate_1, '[email protected]', 'fake comments')

mock_task_helpers.assert_called_once()


class NotifierHelpersTest(testing_config.CustomTestCase):
def setUp(self):
self.feature_1 = FeatureEntry(
name='feature a', summary='sum', category=1,
owner_emails=['[email protected]',
'[email protected]'])
self.feature_1.put()

feature_id = self.feature_1.key.integer_id()
self.ot_stage = Stage(
id=123,
feature_id=feature_id,
stage_type=150)
self.extension_stage = Stage(
id=456,
feature_id=feature_id,
stage_type=151)
self.ot_stage.put()
self.extension_stage.put()

self.gate_1 = Gate(id=123, feature_id=feature_id, stage_id=123,
gate_type=2, state=Vote.APPROVED)
self.gate_1.put()

self.gate_2 = Gate(id=123, feature_id=feature_id, stage_id=123,
gate_type=2, state=Vote.NA)
self.gate_2.put()

self.gate_3 = Gate(id=123, feature_id=feature_id, stage_id=456,
gate_type=3, state=Vote.APPROVED)
self.gate_3.put()

def tearDown(self):
for kind in [FeatureEntry, Stage, Gate]:
for entity in kind.query():
entity.key.delete()

@mock.patch(
'internals.notifier_helpers.send_trial_creation_approved_notification')
def test_notify_approvals__creation(self, mock_sender):
"""OT creation approval notification is sent when all gates are approved."""
notifier_helpers.notify_approvals(
self.feature_1, self.ot_stage,self.gate_1)
mock_sender.assert_called_once()

@mock.patch(
'internals.notifier_helpers.send_trial_creation_approved_notification')
def test_notify_approvals__creation_gates_unapproved(self, mock_sender):
"""OT creation approval notification is only sent if all gates are
approved."""
# A separate gate related to the OT stage is not approved.
self.gate_2.state = Vote.DENIED
self.gate_2.put()
notifier_helpers.notify_approvals(
self.feature_1, self.ot_stage, self.gate_1)
mock_sender.assert_not_called()

@mock.patch(
'internals.notifier_helpers.send_trial_extension_approved_notification')
def test_notify_approvals__extension(self, mock_sender):
"""OT extension approved notification sent when gate is approved."""
notifier_helpers.notify_approvals(
self.feature_1, self.extension_stage, self.gate_3)
mock_sender.assert_called_once()
25 changes: 25 additions & 0 deletions internals/notifier_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1213,6 +1213,31 @@ def test_make_activated_email(self):
TESTDATA['test_make_activated_email.html'])


class OTCreationApprovedHandlerTest(testing_config.CustomTestCase):
def setUp(self):
self.contacts = ['[email protected]',
'[email protected]',
'[email protected]']
self.feature_1 = FeatureEntry(
id=1, name='feature one', summary='sum', category=1, feature_type=0)
self.feature_1.put()

def tearDown(self):
self.feature_1.key.delete()

def test_make_creation_processed_email(self):
with test_app.app_context():
handler = notifier.OTCreationApprovedHandler()
fe_dict = converters.feature_entry_to_json_verbose(self.feature_1)
email_task = handler.build_email(fe_dict, self.contacts)
# TESTDATA.make_golden(email_task['html'], 'test_make_creation_approved_email.html')
self.assertEqual(
email_task['subject'],
'You can now submit your origin trial creation request')
self.assertEqual(email_task['html'],
TESTDATA['test_make_creation_approved_email.html'])


class OTCreationProcessedHandlerTest(testing_config.CustomTestCase):
def setUp(self):
self.contacts = ['[email protected]',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@

<div style="background: #eee; padding: 0 16px 16px">
<div style="color: #666; padding: 8px 0 0 16px;"></div>

<div style="padding: 8px; background: white; border-top: 4px solid #888; border-color: #d32f2f;">
<section id="details" style="margin: 16px; padding: 16px; background: white; border: 2px solid #ccc; border-radius:8px;">
<p>
All prerequisite reviews have been completed and you can now submit a
request to create your origin trial. Click the "Request Trial Creation"
button on the Origin Trial section of <a href="https://chromestatus.com/feature/1"
>your feature's detail page</a>.
</p>
<p>
<strong>Note:</strong> Only feature owners signed in with a
"chromium.org" or "google.com" domain email address will have access to
the trial creation form, and at least 1 Googler contact is required
during form submission to continue the trial creation process. If you
have any questions or need assistance, reach out to <a
href="mailto:[email protected]"
>[email protected]</a>.
<br><br>
Thanks,<br>
Origin Trials team
</p>
</section>
</div>
</div>

2 changes: 2 additions & 0 deletions main.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,8 @@ class Route:
Route('/tasks/email-ot-activation-failed',
notifier.OTActivationFailedHandler),
Route('/tasks/email-ot-creation-request', notifier.OTCreationRequestHandler),
Route('/tasks/email-ot-creation-approved',
notifier.OTCreationApprovedHandler),
Route('/tasks/email-ot-extended', notifier.OTExtendedHandler),
Route('/tasks/email-ot-extension-approved',
notifier.OTExtensionApprovedHandler),
Expand Down
28 changes: 28 additions & 0 deletions templates/origintrials/ot-creation-approved-email.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
{% import 'email-styles.html' as styles %}
<div style="{{styles.body}}">
<div style="{{styles.branding}}">{{APP_TITLE}}</div>

<div style="{{styles.content}} {{styles.content_alert}}">
<section id="details" style="{{styles.section}}">
<p>
All prerequisite reviews have been completed and you can now submit a
request to create your origin trial. Click the "Request Trial Creation"
button on the Origin Trial section of <a href="{{chromestatus_url}}"
>your feature's detail page</a>.
</p>
<p>
<strong>Note:</strong> Only feature owners signed in with a
"chromium.org" or "google.com" domain email address will have access to
the trial creation form, and at least 1 Googler contact is required
during form submission to continue the trial creation process. If you
have any questions or need assistance, reach out to <a
href="mailto:[email protected]"
>[email protected]</a>.
<br><br>
Thanks,<br>
Origin Trials team
</p>
</section>
</div>
</div>

Loading