Send reminders to verify accuracy of data before important milestones (#2078)

* accuracy notifications cron job * send emails in staging environment * Update templates/accuracy_notice_email.html Co-authored-by: Joe Medley <jmedley@google.com> * changes suggested by @jrobbins * remove unnecessary type conversion * typo fix Co-authored-by: Joe Medley <jmedley@google.com>
2022-08-08 11:33:18 -04:00 · 2022-08-08 11:33:18 -04:00 · 8625cf3c61
--- a/cron.yaml
+++ b/cron.yaml
@ -17,3 +17,6 @@ cron:
 - description: Writes string creator field from created_by user field.
  url: /cron/write_creator
  schedule: 1 of month 03:00
+- description: Send reminders to verify the accuracy of feature data.
+  url: /cron/send_accuracy_notifications
+  schedule: every monday 09:00
--- a/internals/models.py
+++ b/internals/models.py
@ -611,6 +611,7 @@ class Feature(DictModel):
        'by': d.pop('updated_by', None),
        'when': d.pop('updated', None),
      }
+      d['accurate_as_of'] = d.pop('accurate_as_of', None)
      d['standards'] = {
        'spec': d.pop('spec_link', None),
        'status': {
@ -1171,7 +1172,8 @@ class Feature(DictModel):
    # Diff values to see what properties have changed.
    changed_props = []
    for prop_name, prop in list(self._properties.items()):
-      if prop_name in ('created_by', 'updated_by', 'updated', 'created'):
+      if prop_name in (
+          'created_by', 'updated_by', 'updated', 'created'):
        continue
      new_val = getattr(self, prop_name, None)
      old_val = getattr(self, '_old_' + prop_name, None)
@ -1180,6 +1182,11 @@ class Feature(DictModel):
          continue
        new_val = convert_enum_int_to_string(prop_name, new_val)
        old_val = convert_enum_int_to_string(prop_name, old_val)
+        # Convert any dateime props to string.
+        if isinstance(new_val, datetime.datetime):
+          new_val = str(new_val)
+          if old_val is not None:
+            old_val = str(old_val)
        changed_props.append({
            'prop_name': prop_name, 'old_val': old_val, 'new_val': new_val})

@ -1207,6 +1214,7 @@ class Feature(DictModel):
  # Metadata.
  created = ndb.DateTimeProperty(auto_now_add=True)
  updated = ndb.DateTimeProperty(auto_now=True)
+  accurate_as_of = ndb.DateTimeProperty(auto_now=False)
  updated_by = ndb.UserProperty()
  created_by = ndb.UserProperty()

--- a/internals/notifier.py
+++ b/internals/notifier.py
@ -15,9 +15,9 @@

 __author__ = 'ericbidelman@chromium.org (Eric Bidelman)'

+from datetime import datetime, timedelta
 import collections
 import logging
-import datetime
 import json
 import os
 import urllib
@ -254,6 +254,99 @@ class FeatureStar(models.DictModel):
    return user_prefs


+class FeatureAccuracyHandler(basehandlers.FlaskHandler):
+  SEND_NOTIFICATIONS = False
+
+  CHROME_RELEASE_SCHEDULE_URL = (
+'https://chromiumdash.appspot.com/fetch_milestone_schedule')
+  ACCURACY_AS_OF_WEEKS = 4
+  MILESTONE_FIELDS = (
+    'dt_milestone_android_start',
+    'dt_milestone_desktop_start',
+    'dt_milestone_ios_start',
+    'dt_milestone_webview_start',
+    'ot_milestone_android_start',
+    'ot_milestone_desktop_start',
+    'ot_milestone_webview_start',
+    'shipped_android_milestone',
+    'shipped_ios_milestone',
+    'shipped_milestone',
+    'shipped_webview_milestone'
+  )
+  TEMPLATE_PATH = 'accuracy_notice_email.html'
+   
+  def get_template_data(self):
+    """Sends notifications to users requesting feature updates for accuracy."""
+
+    # Do not send notifications until client-side changes are made.
+    # TODO(danielrsmith): This check should be removed when the new page to
+    # verify data accuracy is implemented.
+    if not self.SEND_NOTIFICATIONS:
+      return {'message': '0 emails sent or logged.'}
+
+    features_to_notify = self._determine_features_to_notify()
+    email_tasks = self._build_email_tasks(features_to_notify)
+    send_emails(email_tasks)
+    return {'message': f'{len(email_tasks)} emails sent or logged.'}
+    
+  def _determine_features_to_notify(self):
+    # 'current' milestone is the next stable milestone that hasn't landed.
+    # We send notifications to any feature planned for beta or stable launch
+    # in the next 8 weeks. Beta for (current + 2) starts in roughly 8 weeks.
+    # So we check if any features have launches in these 3 milestones
+    # (current, current + 1, and current + 2).
+    try:
+      resp = requests.get(f'{self.CHROME_RELEASE_SCHEDULE_URL}?mstone=current')
+    except requests.RequestException as e:
+      raise e
+    mstone_info = json.loads(resp.text)
+    mstone = int(mstone_info['mstones'][0]['mstone'])
+    
+    features = models.Feature.query(models.Feature.deleted == False).fetch(None)
+    features_to_notify = []
+    now = datetime.now()
+    accuracy_as_of_delta = timedelta(weeks=self.ACCURACY_AS_OF_WEEKS)
+    for feature in features:
+      # If the data has been recently verified as accurate, no need for email.
+      if (feature.accurate_as_of is not None and
+          feature.accurate_as_of + accuracy_as_of_delta < now):
+        continue
+  
+      # Check each milestone field and see if it corresponds with
+      # the next 3 milestones. Use the closest milestone for the email.
+      closest_mstone = None
+      for field in self.MILESTONE_FIELDS:
+        launch_mstone = getattr(feature, field)
+        if (launch_mstone is not None and
+            launch_mstone >= mstone and launch_mstone <= mstone + 2):
+          if closest_mstone is None:
+            closest_mstone = launch_mstone
+          else:
+            closest_mstone = min(closest_mstone, launch_mstone)
+      if closest_mstone is not None:
+        features_to_notify.append((feature, closest_mstone))
+    return features_to_notify
+
+  def _build_email_tasks(self, features_to_notify):
+    email_tasks = []
+    for feature, mstone in features_to_notify:
+      body_data = {
+        'feature': feature.format_for_template(),
+        'site_url': settings.SITE_URL,
+        'milestone': mstone,
+      }
+      html = render_to_string(self.TEMPLATE_PATH, body_data)
+      subject = f'[Action requested] Update {feature.name}'
+      for owner in feature.owner:
+        email_tasks.append({
+          'to': owner,
+          'subject': subject,
+          'reply_to': None,
+          'html': html
+        })
+    return email_tasks
+
+
 class FeatureChangeHandler(basehandlers.FlaskHandler):
  """This task handles a feature creation or update by making email tasks."""

@ -275,21 +368,7 @@ class FeatureChangeHandler(basehandlers.FlaskHandler):
    if feature and (is_update and len(changes) or not is_update):
      email_tasks = make_email_tasks(
          feature, is_update=is_update, changes=changes)
-      logging.info('Processing %d email tasks', len(email_tasks))
-      for one_email_dict in email_tasks:
-        if settings.SEND_EMAIL:
-          cloud_tasks_helpers.enqueue_task(
-              '/tasks/outbound-email', one_email_dict)
-        else:
-          logging.info(
-              'Would send the following email:\n'
-              'To: %s\n'
-              'Subject: %s\n'
-              'Reply-To: %s\n'
-              'Body:\n%s',
-              one_email_dict['to'], one_email_dict['subject'],
-              one_email_dict['reply_to'],
-              one_email_dict['html'][:settings.MAX_LOG_LINE])
+      send_emails(email_tasks)

    return {'message': 'Done'}

@ -359,6 +438,30 @@ def get_thread_id(feature, approval_field):
  return thread_id


+def send_emails(email_tasks):
+  """Process a list of email tasks (send or log)."""
+  logging.info('Processing %d email tasks', len(email_tasks))
+  for task in email_tasks:
+    if settings.SEND_EMAIL:
+      cloud_tasks_helpers.enqueue_task(
+          '/tasks/outbound-email', task)
+    else:
+      logging.info(
+          'Would send the following email:\n'
+          'To: %s\n'
+          'From: %s\n'
+          'References: %s\n'
+          'Reply-To: %s\n'
+          'Subject: %s\n'
+          'Body:\n%s',
+          task.get('to', None),
+          task.get('from_user', None),
+          task.get('references', None),
+          task.get('reply_to', None),
+          task.get('subject', None),
+          task.get('html', "")[:settings.MAX_LOG_LINE])
+
+
 def post_comment_to_mailing_list(
    feature, approval_field_id, author_addr, comment_content):
  """Post a message to the intent thread."""
@ -376,25 +479,11 @@ def post_comment_to_mailing_list(
  html = render_to_string(
      'review-comment-email.html', {'comment_content': comment_content})

-  one_email_task = {
+  email_task = {
      'to': to_addr,
      'from_user': from_user,
      'references': references,
      'subject': subject,
      'html': html,
      }
-
-  if settings.SEND_EMAIL:
-    cloud_tasks_helpers.enqueue_task(
-        '/tasks/outbound-email', one_email_task)
-  else:
-    logging.info(
-        'Would send the following email:\n'
-        'To: %s\n'
-        'From: %s\n'
-        'References: %s\n'
-        'Subject: %s\n'
-        'Body:\n%s',
-        one_email_task['to'], one_email_task['from_user'],
-        one_email_task['references'], one_email_task['subject'],
-        one_email_task['html'][:settings.MAX_LOG_LINE])
+  send_emails([email_task])
--- a/internals/notifier_test.py
+++ b/internals/notifier_test.py
@ -34,6 +34,7 @@ class EmailFormattingTest(testing_config.CustomTestCase):
  def setUp(self):
    self.feature_1 = models.Feature(
        name='feature one', summary='sum', owner=['feature_owner@example.com'],
+        ot_milestone_desktop_start=100,
        editors=['feature_editor@example.com', 'owner_1@example.com'],
        category=1, visibility=1, standardization=1, web_dev_views=1,
        impl_status_chrome=1, created_by=ndb.User(
@ -471,6 +472,14 @@ class FeatureStarTest(testing_config.CustomTestCase):
        [app_user_1.email, app_user_2.email],
        [au.email for au in actual])

+  @mock.patch('requests.get')
+  def test_determine_features_to_notify(self, mock_get):
+    mock_get.return_value = '{"mstones":[{"mstone": "100"}]}'
+    accuracy_notifier = notifier.FeatureAccuracyHandler()
+    result = accuracy_notifier.get_template_data()
+    expected = {'message': '0 emails sent or logged.'}
+    self.assertEqual(result, expected)
+

 class FunctionsTest(testing_config.CustomTestCase):

--- a/main.py
+++ b/main.py
@ -191,6 +191,7 @@ internals_routes = [
  ('/cron/update_blink_components', fetchmetrics.BlinkComponentHandler),
  ('/cron/export_backup', data_backup.BackupExportHandler),
  ('/cron/write_creator', write_creator.UpdateCreatorHandler),
+  ('/cron/send_accuracy_notifications', notifier.FeatureAccuracyHandler),

  ('/tasks/email-subscribers', notifier.FeatureChangeHandler),

--- a/pages/guide.py
+++ b/pages/guide.py
@ -13,7 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.

-import datetime
+from datetime import datetime
 import flask
 import json
 import logging
@ -142,6 +142,7 @@ class FeatureNew(basehandlers.FlaskHandler):
        owner=owners,
        editors=editors,
        creator=signed_in_user.email(),
+        accurate_as_of=datetime.now(),
        impl_status_chrome=models.NO_ACTIVE_DEV,
        standardization=models.EDITORS_DRAFT,
        unlisted=self.form.get('unlisted') == 'on',
--- a/settings.py
+++ b/settings.py
@ -59,7 +59,7 @@ if DEV_MODE or UNIT_TEST_MODE:
 else:
  APP_ID = os.environ['GOOGLE_CLOUD_PROJECT']

-SITE_URL = 'http://%s.appspot.com/' % APP_ID
+SITE_URL = 'https://%s.appspot.com/' % APP_ID
 CLOUD_TASKS_REGION = 'us-central1'

 GOOGLE_SIGN_IN_CLIENT_ID = (
@ -92,7 +92,7 @@ elif APP_ID == 'cr-status':
  APP_TITLE = 'Chrome Platform Status'
  SEND_EMAIL = True
  SEND_ALL_EMAIL_TO = None  # Deliver it to the intended users
-  SITE_URL = 'http://chromestatus.com/'
+  SITE_URL = 'https://chromestatus.com/'
  GOOGLE_SIGN_IN_CLIENT_ID = (
      '999517574127-7ueh2a17bv1ave9thlgtap19pt5qjp4g.'
      'apps.googleusercontent.com')
--- a/templates/accuracy_notice_email.html
+++ b/templates/accuracy_notice_email.html
@ -0,0 +1,31 @@
+<p>
+  You are receiving this notification because you are listed as an owner of the following ChromeStatus feature entry:
+</p>
+</br>
+<p>
+  <strong>{{feature.name}}</strong>
+</p> 
+</br>
+<p>
+  Your feature is slated to launch soon for m{{milestone}}:
+</p>
+</br>
+<p>
+  {% include "estimated-milestones-table.html" %}
+</p>
+</br>
+<p>
+  Your feature entry is an important resource for
+  cross functional teams that help drive adoption of new features and
+  enterprise IT admins who might be affected by web platform changes.
+</p>
+</br>
+<p>
+  We need to know whether your plans are changing or staying
+  the same. <strong>Please click the link below to update and confirm key
+  fields of your feature entry.</strong>
+</p>
+</br>
+<b><a href="{{site_url}}guide/verify_accuracy/{{feature.id}}">
+  {{site_url}}guide/verify_accuracy/{{feature.id}}
+</a></b>