Add cc field to features (#2252)

* Add cc field to features Closes #2240. Similar implementation as editors. Users who are cc'd on features can see unlisted features. internals/core_models.py: - Add new cc_recipients field. Defaults to empty list if field is not there. internals/notifier.py - Adds reason for being notified as a person in the cc_recipients field internals/search.py - Add the shorthand query for `cc:me` internals/search_queries.py - Add the ability to search the cc_recipients field by adding it to QUERIABLE_FIELDS static/elements/chromedash-guide-metadata.js - Add the ability to view the value on the metadata display - Maps cc_recipients to individual mailto links ccRecipient - Note: ccRecipient is camelCase static/elements/form-field-specs.js - Declare a new form field cc_recipients static/elements/form-definition.js - Add the cc_recipients field to the metadata form field * cc_recipients changes to persist on create/update These changes allow changes to cc_recipients to persist on feature create or update * Check if cc_recipients empty cc_recipients can be missing from the object due to del_none in core_models.py Also fomat_for_template currently returns cc_recipients at a different level and not inside the chrome object. As a result, need to change the chromedash-guide-metadata file to look for the data at the right place. * Use short hand CC instead of carbon copy
2022-09-21 13:15:22 -04:00 · 2022-09-21 13:15:22 -04:00 · 229cae9d31
--- a/internals/core_models.py
+++ b/internals/core_models.py
@ -258,6 +258,7 @@ class Feature(DictModel):
      }
      d['tags'] = d.pop('search_tags', [])
      d['editors'] = d.pop('editors', [])
+      d['cc_recipients'] = d.pop('cc_recipients', [])
      d['creator'] = d.pop('creator', None)
      d['browsers'] = {
        'chrome': {
@ -375,6 +376,7 @@ class Feature(DictModel):
    #d['id'] = self.key().id
    d['owner'] = ', '.join(self.owner)
    d['editors'] = ', '.join(self.editors)
+    d['cc_recipients'] = ', '.join(self.cc_recipients)
    d['explainer_links'] = '\r\n'.join(self.explainer_links)
    d['spec_mentors'] = ', '.join(self.spec_mentors)
    d['standard_maturity'] = self.standard_maturity or UNKNOWN_STD
@ -923,6 +925,7 @@ class Feature(DictModel):
  comments = ndb.StringProperty()
  owner = ndb.StringProperty(repeated=True)
  editors = ndb.StringProperty(repeated=True)
+  cc_recipients = ndb.StringProperty(repeated=True)
  footprint = ndb.IntegerProperty()  # Deprecated

  # Tracability to intent discussion threads
--- a/internals/notifier.py
+++ b/internals/notifier.py
@ -160,6 +160,10 @@ def make_email_tasks(feature, is_update=False, changes=[]):
    addr_reasons, feature.editors,
    'You are listed as an editor of this feature'
  )
+  accumulate_reasons(
+    addr_reasons, feature.cc_recipients,
+    'You are CC\'d on this feature'
+  )
  accumulate_reasons(
      addr_reasons, feature_watchers,
      'You are watching all feature changes')
--- a/internals/notifier_test.py
+++ b/internals/notifier_test.py
@ -39,6 +39,7 @@ class EmailFormattingTest(testing_config.CustomTestCase):
        name='feature one', summary='sum', owner=['feature_owner@example.com'],
        ot_milestone_desktop_start=100,
        editors=['feature_editor@example.com', 'owner_1@example.com'],
+        cc_recipients=['cc@example.com'],
        category=1, visibility=1, standardization=1, web_dev_views=1,
        impl_status_chrome=1, created_by=ndb.User(
            email='creator1@gmail.com', _auth_domain='gmail.com'),
@ -220,9 +221,9 @@ class EmailFormattingTest(testing_config.CustomTestCase):
    mock_f_e_b.return_value = 'mock body html'
    actual_tasks = notifier.make_email_tasks(
        self.feature_1, is_update=False, changes=[])
-    self.assertEqual(4, len(actual_tasks))
-    (feature_editor_task, feature_owner_task, component_owner_task,
-     watcher_task) = actual_tasks
+    self.assertEqual(5, len(actual_tasks))
+    (feature_cc_task, feature_editor_task, feature_owner_task,
+     component_owner_task, watcher_task) = actual_tasks

    # Notification to feature owner.
    self.assertEqual('feature_owner@example.com', feature_owner_task['to'])
@ -238,6 +239,13 @@ class EmailFormattingTest(testing_config.CustomTestCase):
      feature_editor_task['html'])
    self.assertEqual('feature_editor@example.com', feature_editor_task['to'])

+    # Notification to user CC'd to feature changes.
+    self.assertEqual('new feature: feature one', feature_cc_task['subject'])
+    self.assertIn('mock body html', feature_cc_task['html'])
+    self.assertIn('<li>You are CC\'d on this feature</li>',
+      feature_cc_task['html'])
+    self.assertEqual('cc@example.com', feature_cc_task['to'])
+
    # Notification to component owner.
    self.assertEqual('new feature: feature one', component_owner_task['subject'])
    self.assertIn('mock body html', component_owner_task['html'])
@ -263,9 +271,9 @@ class EmailFormattingTest(testing_config.CustomTestCase):
    mock_f_e_b.return_value = 'mock body html'
    actual_tasks = notifier.make_email_tasks(
        self.feature_1, True, self.changes)
-    self.assertEqual(4, len(actual_tasks))
-    (feature_editor_task, feature_owner_task, component_owner_task,
-     watcher_task) = actual_tasks
+    self.assertEqual(5, len(actual_tasks))
+    (feature_cc_task, feature_editor_task, feature_owner_task,
+     component_owner_task, watcher_task) = actual_tasks

    # Notification to feature owner.
    self.assertEqual('feature_owner@example.com', feature_owner_task['to'])
@ -283,6 +291,14 @@ class EmailFormattingTest(testing_config.CustomTestCase):
      feature_editor_task['html'])
    self.assertEqual('feature_editor@example.com', feature_editor_task['to'])

+    # Notification to user CC'd on feature changes.
+    self.assertEqual('updated feature: feature one',
+      feature_cc_task['subject'])
+    self.assertIn('mock body html', feature_cc_task['html'])
+    self.assertIn('<li>You are CC\'d on this feature</li>',
+      feature_cc_task['html'])
+    self.assertEqual('cc@example.com', feature_cc_task['to'])
+
    # Notification to component owner.
    self.assertEqual('updated feature: feature one',
      component_owner_task['subject'])
@ -311,9 +327,9 @@ class EmailFormattingTest(testing_config.CustomTestCase):
        'starrer_1@example.com', self.feature_1.key.integer_id())
    actual_tasks = notifier.make_email_tasks(
        self.feature_1, True, self.changes)
-    self.assertEqual(5, len(actual_tasks))
-    (feature_editor_task, feature_owner_task, component_owner_task,
-     starrer_task, watcher_task) = actual_tasks
+    self.assertEqual(6, len(actual_tasks))
+    (feature_cc_task, feature_editor_task, feature_owner_task,
+     component_owner_task, starrer_task, watcher_task) = actual_tasks

    # Notification to feature owner.
    self.assertEqual('feature_owner@example.com', feature_owner_task['to'])
@ -331,6 +347,14 @@ class EmailFormattingTest(testing_config.CustomTestCase):
      feature_editor_task['html'])
    self.assertEqual('feature_editor@example.com', feature_editor_task['to'])

+    # Notification to user CC'd on feature changes.
+    self.assertEqual('updated feature: feature one',
+      feature_cc_task['subject'])
+    self.assertIn('mock body html', feature_cc_task['html'])
+    self.assertIn('<li>You are CC\'d on this feature</li>',
+      feature_cc_task['html'])
+    self.assertEqual('cc@example.com', feature_cc_task['to'])
+
    # Notification to component owner.
    self.assertEqual('updated feature: feature one',
      component_owner_task['subject'])
--- a/internals/search.py
+++ b/internals/search.py
@ -155,6 +155,8 @@ def process_query_term(field_name, op_str, val_str):
    return process_access_me_query('editors')
  if query_term == 'can_edit:me':
    return process_access_me_query('can_edit')
+  if query_term == 'cc:me':
+    return process_access_me_query('cc_recipients')

  if val_str.startswith('"') and val_str.endswith('"'):
    val_str = val_str[1:-1]
--- a/internals/search_queries.py
+++ b/internals/search_queries.py
@ -130,6 +130,7 @@ QUERIABLE_FIELDS = {
    'creator': core_models.Feature.creator,
    'browsers.chrome.owners': core_models.Feature.owner,
    'editors': core_models.Feature.editors,
+    'cc_recipients': core_models.Feature.cc_recipients,
    'intent_to_implement_url': core_models.Feature.intent_to_implement_url,
    'intent_to_ship_url': core_models.Feature.intent_to_ship_url,
    'ready_for_trial_url': core_models.Feature.ready_for_trial_url,
--- a/internals/search_test.py
+++ b/internals/search_test.py
@ -31,6 +31,7 @@ class SearchFunctionsTest(testing_config.CustomTestCase):
        standardization=1, web_dev_views=1, impl_status_chrome=3)
    self.feature_1.owner = ['owner@example.com']
    self.feature_1.editors = ['editor@example.com']
+    self.feature_1.cc_recipients = ['cc@example.com']
    self.feature_1.put()
    self.feature_2 = core_models.Feature(
        name='feature 2', summary='sum', category=2, visibility=1,
@ -172,6 +173,19 @@ class SearchFunctionsTest(testing_config.CustomTestCase):
    self.assertEqual(len(actual), 1)
    self.assertEqual(actual[0], self.feature_1.key.integer_id())

+  def test_process_access_me_query__cc_recipients_none(self):
+    """We can return a list of features the user is CC'd on."""
+    testing_config.sign_in('visitor@example.com', 111)
+    actual = search.process_access_me_query('cc_recipients')
+    self.assertEqual(actual, [])
+
+  def test_process_access_me_query__cc_recipients_some(self):
+    """We can return a list of features the user is CC'd on."""
+    testing_config.sign_in('cc@example.com', 111)
+    actual = search.process_access_me_query('cc_recipients')
+    self.assertEqual(len(actual), 1)
+    self.assertEqual(actual[0], self.feature_1.key.integer_id())
+
  @mock.patch('internals.review_models.Approval.get_approvals')
  @mock.patch('internals.approval_defs.fields_approvable_by')
  def test_process_recent_reviews_query__none(
--- a/pages/guide.py
+++ b/pages/guide.py
@ -34,6 +34,7 @@ class FeatureCreateHandler(basehandlers.FlaskHandler):
  def process_post_data(self):
    owners = self.split_emails('owner')
    editors = self.split_emails('editors')
+    cc_recipients = self.split_emails('cc_recipients')

    blink_components = (
        self.split_input('blink_components', delim=',') or
@ -53,6 +54,7 @@ class FeatureCreateHandler(basehandlers.FlaskHandler):
        summary=self.form.get('summary'),
        owner=owners,
        editors=editors,
+        cc_recipients=cc_recipients,
        creator=signed_in_user.email(),
        accurate_as_of=datetime.now(),
        impl_status_chrome=core_enums.NO_ACTIVE_DEV,
@ -267,6 +269,9 @@ class FeatureEditHandler(basehandlers.FlaskHandler):
    if self.touched('editors'):
      feature.editors = self.split_emails('editors')

+    if self.touched('cc_recipients'):
+      feature.cc_recipients = self.split_emails('cc_recipients')
+
    if self.touched('doc_links'):
      feature.doc_links = self.parse_links('doc_links')

--- a/static/elements/chromedash-guide-metadata.js
+++ b/static/elements/chromedash-guide-metadata.js
@ -130,6 +130,18 @@ export class ChromedashGuideMetadata extends LitElement {
              </td>
            </tr>

+            <tr>
+              <th>CC</th>
+              <td>
+                ${this.feature.cc_recipients ?
+                  this.feature.cc_recipients.map((ccRecipient)=> html`
+                  <a href="mailto:${ccRecipient}">${ccRecipient}</a>
+                  `): html`
+                  None
+                `}
+              </td>
+            </tr>
+
            <tr>
              <th>Category</th>
              <td>${this.feature.category}</td>
--- a/static/elements/chromedash-guide-metadata_test.js
+++ b/static/elements/chromedash-guide-metadata_test.js
@ -11,6 +11,7 @@ describe('chromedash-guide-metadata', () => {
    feature_type: 'fake feature type',
    intent_stage: 'fake intent stage',
    new_crbug_url: 'fake crbug link',
+    cc_recipients: ['fake chrome cc one', 'fake chrome cc two'],
    browsers: {
      chrome: {
        blink_components: ['Blink'],
@ -50,6 +51,9 @@ describe('chromedash-guide-metadata', () => {
    // feature owners are listed
    assert.include(metadataDiv.innerHTML, 'href="mailto:fake chrome owner one"');
    assert.include(metadataDiv.innerHTML, 'href="mailto:fake chrome owner two"');
+    // cc recipients are listed
+    assert.include(metadataDiv.innerHTML, 'href="mailto:fake chrome cc one"');
+    assert.include(metadataDiv.innerHTML, 'href="mailto:fake chrome cc two"');
    // feature category is listed
    assert.include(metadataDiv.innerHTML, 'fake category');
    // feature feature type is listed
--- a/static/elements/form-definition.js
+++ b/static/elements/form-definition.js
@ -5,8 +5,8 @@ import {
 } from './form-field-enums';


-const COMMA_SEPARATED_FIELDS = ['owner', 'editors', 'spec_mentors',
-  'search_tags', 'devrel', 'i2e_lgtms', 'i2s_lgtms'];
+const COMMA_SEPARATED_FIELDS = ['owner', 'editors', 'cc_recipients',
+  'spec_mentors', 'search_tags', 'devrel', 'i2e_lgtms', 'i2s_lgtms'];

 const LINE_SEPARATED_FIELDS = ['explainer_links', 'doc_links', 'sample_links'];

@ -87,13 +87,13 @@ export function formatFeatureForEdit(feature) {

 export const NEW_FEATURE_FORM_FIELDS = [
  'name', 'summary', 'unlisted', 'owner',
-  'editors', 'blink_components', 'category',
+  'editors', 'cc_recipients', 'blink_components', 'category',
 ];
 // Note: feature_type is done with custom HTML in chromedash-guide-new-page

 export const METADATA_FORM_FIELDS = [
  'name', 'summary', 'unlisted', 'owner',
-  'editors', 'category',
+  'editors', 'cc_recipients', 'category',
  'feature_type', 'intent_stage',
  'search_tags',
  // Implemention
@ -103,7 +103,7 @@ export const METADATA_FORM_FIELDS = [
 ];

 export const VERIFY_ACCURACY_FORM_FIELDS = [
-  'summary', 'owner', 'editors', 'impl_status_chrome', 'intent_stage',
+  'summary', 'owner', 'editors', 'cc_recipients', 'impl_status_chrome', 'intent_stage',
  'dt_milestone_android_start', 'dt_milestone_desktop_start',
  'dt_milestone_ios_start', 'ot_milestone_android_start',
  'ot_milestone_android_end', 'ot_milestone_desktop_start',
@ -116,7 +116,7 @@ export const VERIFY_ACCURACY_FORM_FIELDS = [
 const FLAT_METADATA_FIELDS = [
  // Standardizaton
  'name', 'summary', 'unlisted', 'owner',
-  'editors', 'category',
+  'editors', 'cc_recipients', 'category',
  'feature_type', 'intent_stage',
  'search_tags',
  // Implementtion
@ -292,9 +292,9 @@ const MOST_PREPARETOSHIP = [
 const ANY_SHIP = ['launch_bug_url', 'finch_url', 'comments'];

 const EXISTING_PROTOTYPE = [
-  'owner', 'editors', 'blink_components', 'motivation', 'explainer_links',
-  'spec_link', 'standard_maturity', 'api_spec', 'bug_url', 'launch_bug_url',
-  'intent_to_implement_url', 'comments',
+  'owner', 'editors', 'cc_recipients', 'blink_components', 'motivation',
+  'explainer_links', 'spec_link', 'standard_maturity', 'api_spec', 'bug_url',
+  'launch_bug_url', 'intent_to_implement_url', 'comments',
 ];

 const EXISTING_ORIGINTRIAL = [
--- a/static/elements/form-field-specs.js
+++ b/static/elements/form-field-specs.js
@ -160,6 +160,17 @@ export const ALL_FIELDS = {
        Comma separated list of full email addresses. These users will be allowed to edit this feature, but will not be listed as feature owners.`,
  },

+  'cc_recipients': {
+    type: 'input',
+    attrs: MULTI_EMAIL_FIELD_ATTRS,
+    required: false,
+    label: 'CC',
+    help_text: html`
+        Comma separated list of full email addresses. These users will be
+        notified of any changes to the feature, but do not gain permission to
+        edit.`,
+  },
+
  'unlisted': {
    type: 'checkbox',
    label: 'Unlisted',