Implement a menu to add xfn gates. (#3539)

* Implement a menu to add xfn gates.

* Added unit tests

api/reviews_api.py

@@ -17,13 +17,15 @@ import datetime
 import logging
 import re
 from typing import Any, Optional, Tuple
+from google.cloud import ndb
 
 from api import converters
 from framework import basehandlers
 from framework import permissions
 from framework.users import User
 from internals import approval_defs, notifier_helpers
-from internals.core_models import FeatureEntry
+from internals.core_enums import *
+from internals.core_models import FeatureEntry, Stage
 from internals.review_models import Gate, Vote
 
 
@@ -159,3 +161,60 @@ class GatesAPI(basehandlers.APIHandler):
       is_approver = permissions.can_review_gate(reviewer, fe, gate, approvers)
       if not is_approver:
         self.abort(400, 'Assignee is not a reviewer')
+
+
+class XfnGatesAPI(basehandlers.APIHandler):
+
+  def do_get(self, **kwargs):
+    """Reject unneeded GET requests without triggering Error Reporting."""
+    self.abort(405, valid_methods=['POST'])
+
+  def do_post(self, **kwargs) -> dict[str, str]:
+    """Add a full set of cross-functional gates to a stage."""
+    feature_id: int = kwargs['feature_id']
+    fe: FeatureEntry | None = self.get_specified_feature(feature_id=feature_id)
+    if fe is None:
+      self.abort(404, msg=f'Feature {feature_id} not found')
+    stage_id: int = kwargs['stage_id']
+    stage: Stage | None = Stage.get_by_id(stage_id)
+    if stage is None:
+      self.abort(404, msg=f'Stage {stage_id} not found')
+
+    user: User = self.get_current_user(required=True)
+    is_editor = fe and permissions.can_edit_feature(user, fe.key.integer_id())
+    is_approver = approval_defs.fields_approvable_by(user)
+    if not is_editor and not is_approver:
+      self.abort(403, msg='User lacks permission to create gates')
+
+    count = self.create_xfn_gates(feature_id, stage_id)
+    return {'message': f'Created {count} gates'}
+
+  def get_needed_gate_types(self) -> list[int]:
+    """Return a list of gate types normally used to ship a new feature."""
+    needed_gate_tuples = STAGES_AND_GATES_BY_FEATURE_TYPE[
+        FEATURE_TYPE_INCUBATE_ID]
+    for stage_type, gate_types in needed_gate_tuples:
+      if stage_type == STAGE_BLINK_SHIPPING:
+        return gate_types
+    raise ValueError('Could not find expected list of gate types')
+
+  def create_xfn_gates(self, feature_id, stage_id) -> int:
+    """Create all new incubation gates on a PSA stage"""
+    logging.info('Creating xfn gates')
+    existing_gates = Gate.query(
+        Gate.feature_id == feature_id, Gate.stage_id == stage_id).fetch()
+    existing_gate_types = set([eg.gate_type for eg in existing_gates])
+    logging.info('Found existing: %r', existing_gate_types)
+    new_gates = []
+    for gate_type in self.get_needed_gate_types():
+      if gate_type not in existing_gate_types:
+        logging.info(f'Creating gate type {gate_type}')
+        gate = Gate(
+            feature_id=feature_id, stage_id=stage_id, gate_type=gate_type,
+            state=Gate.PREPARING)
+        new_gates.append(gate)
+
+    ndb.put_multi(new_gates)
+    num_new = len(new_gates)
+    logging.info(f'Created {num_new} gates')
+    return num_new

api/reviews_api_test.py

@@ -21,7 +21,8 @@ import werkzeug.exceptions  # Flask HTTP stuff.
 from google.cloud import ndb  # type: ignore
 
 from api import reviews_api
-from internals import core_enums
+from internals import approval_defs
+from internals.core_enums import *
 from internals import core_models
 from internals.review_models import Gate, Vote
 
@@ -29,6 +30,10 @@ test_app = flask.Flask(__name__)
 
 NOW = datetime.datetime.now()
 
+ALL_SHIPPING_GATE_TYPES = [
+    GATE_PRIVACY_SHIP, GATE_SECURITY_SHIP, GATE_ENTERPRISE_SHIP,
+    GATE_DEBUGGABILITY_SHIP, GATE_TESTING_SHIP, GATE_API_SHIP]
+
 
 class VotesAPITest(testing_config.CustomTestCase):
 
@@ -355,3 +360,104 @@ class GatesAPITest(testing_config.CustomTestCase):
         'gates': [],
     }
     self.assertEqual(actual, expected)
+
+
+class XfnGatesAPITest(testing_config.CustomTestCase):
+
+  def setUp(self):
+    self.feature_1 = core_models.FeatureEntry(
+        name='feature one', summary='sum', category=1,
+        owner_emails=['owner1@example.com'])
+    self.feature_1.put()
+    self.feature_id = self.feature_1.key.integer_id()
+
+    self.stage_1 = core_models.Stage(
+        feature_id=self.feature_id, stage_type=STAGE_BLINK_SHIPPING)
+    self.stage_1.put()
+    self.stage_id = self.stage_1.key.integer_id()
+
+    self.gate_1 = Gate(id=1, feature_id=self.feature_id, stage_id=self.stage_id,
+        gate_type=GATE_API_SHIP, state=Vote.NA)
+    self.gate_1.put()
+    self.gate_1_id = self.gate_1.key.integer_id()
+
+    self.handler = reviews_api.XfnGatesAPI()
+    self.request_path = '/api/v0/features/%d/stages/%d/addXfnGates' % (
+        self.feature_id, self.stage_id)
+
+  def tearDown(self):
+    self.feature_1.key.delete()
+    kinds: list[ndb.Model] = [Gate, Vote]
+    for kind in kinds:
+      for entity in kind.query():
+        entity.key.delete()
+
+  def test_get(self):
+    """We reject all GETs to this endpoint."""
+    with test_app.test_request_context(self.request_path):
+      with self.assertRaises(werkzeug.exceptions.MethodNotAllowed):
+        self.handler.do_get()
+
+  def test_do_post__not_found(self):
+    """Handler rejects bad requests."""
+    with test_app.test_request_context(self.request_path):
+      with self.assertRaises(werkzeug.exceptions.NotFound):
+        self.handler.do_post(
+            feature_id=self.feature_id + 1, stage_id=self.stage_id)
+
+    with test_app.test_request_context(self.request_path):
+      with self.assertRaises(werkzeug.exceptions.NotFound):
+        self.handler.do_post(
+            feature_id=self.feature_id, stage_id=self.stage_id + 1)
+
+  def test_do_post__not_allowed(self):
+    """Handler rejects users who lack permission."""
+    testing_config.sign_out()
+    with test_app.test_request_context(self.request_path):
+      with self.assertRaises(werkzeug.exceptions.Forbidden):
+        self.handler.do_post(
+            feature_id=self.feature_id, stage_id=self.stage_id)
+
+    testing_config.sign_in('other@example.com', 999)
+    with test_app.test_request_context(self.request_path):
+      with self.assertRaises(werkzeug.exceptions.Forbidden):
+        self.handler.do_post(
+            feature_id=self.feature_id, stage_id=self.stage_id)
+
+  @mock.patch('api.reviews_api.XfnGatesAPI.create_xfn_gates')
+  def test_do_post__editors_allowed(self, mock_create):
+    """Handler accepts users who can edit the feature."""
+    testing_config.sign_in('owner1@example.com', 123567890)
+    mock_create.return_value = 111
+    with test_app.test_request_context(self.request_path):
+      actual = self.handler.do_post(
+          feature_id=self.feature_id, stage_id=self.stage_id)
+
+    mock_create.assert_called_once_with(self.feature_id, self.stage_id)
+    self.assertEqual(actual, {'message': 'Created 111 gates'})
+
+  @mock.patch('api.reviews_api.XfnGatesAPI.create_xfn_gates')
+  def test_do_post__reviewers_allowed(self, mock_create):
+    """Handler accepts users who can review any gate."""
+    testing_config.sign_in(approval_defs.ENTERPRISE_APPROVERS[0], 123567890)
+    mock_create.return_value = 222
+    with test_app.test_request_context(self.request_path):
+      actual = self.handler.do_post(
+          feature_id=self.feature_id, stage_id=self.stage_id)
+
+    mock_create.assert_called_once_with(self.feature_id, self.stage_id)
+    self.assertEqual(actual, {'message': 'Created 222 gates'})
+
+  def test_get_needed_gate_types(self):
+    """We always assume that we are adding all gates for STAGE_BLINK_SHIPPING."""
+    actual = self.handler.get_needed_gate_types()
+    self.assertEqual(actual,ALL_SHIPPING_GATE_TYPES)
+
+  def test_create_xfn_gates__normal(self):
+    """We can create the missing gates from STAGE_BLINK_SHIPPING."""
+    actual = self.handler.create_xfn_gates(self.feature_id, self.stage_id)
+
+    self.assertEqual(actual, 5)
+    actual_gates_dict = Gate.get_feature_gates(self.feature_id)
+    self.assertCountEqual(
+        actual_gates_dict.keys(), ALL_SHIPPING_GATE_TYPES)

client-src/elements/chromedash-feature-detail.js

@@ -24,6 +24,7 @@ import {
   PLATFORMS_DISPLAYNAME,
   STAGE_SPECIFIC_FIELDS,
   OT_MILESTONE_END_FIELDS,
+  STAGE_PSA_SHIPPING,
   ENTERPRISE_FEATURE_CATEGORIES_DISPLAYNAME,
   ROLLOUT_IMPACT_DISPLAYNAME} from './form-field-enums';
 import '@polymer/iron-icon';
@@ -124,10 +125,14 @@ class ChromedashFeatureDetail extends LitElement {
         padding: 8px 16px;
       }
 
-      sl-details sl-button {
+      sl-details sl-button,
+      sl-details sl-dropdown {
         float: right;
         margin-right: 4px;
       }
+      sl-details sl-dropdown sl-icon-button {
+        font-size: 1.4rem;
+      }
 
       sl-details sl-button[variant="default"]::part(base) {
         color: var(--sl-color-primary-600);
@@ -187,6 +192,15 @@ class ChromedashFeatureDetail extends LitElement {
     `];
   }
 
+  _fireEvent(eventName, detail) {
+    const event = new CustomEvent(eventName, {
+      bubbles: true,
+      composed: true,
+      detail,
+    });
+    this.dispatchEvent(event);
+  }
+
   connectedCallback() {
     super.connectedCallback();
     this.intializeGateColumn();
@@ -244,6 +258,18 @@ class ChromedashFeatureDetail extends LitElement {
     });
   }
 
+  handleAddXfnGates(feStage) {
+    const prompt = (
+      'Would you like to add gates for Privacy, Security, etc.? \n\n' +
+          'This is needed if the API Owners ask you to add them, ' +
+          'or if you send an "Intent to Ship" rather than a PSA.');
+    if (confirm(prompt)) {
+      window.csClient.addXfnGates(feStage.feature_id, feStage.id).then(() => {
+        this._fireEvent('refetch-needed', {});
+      });
+    }
+  }
+
   renderControls() {
     const editAllButton = html`
       <sl-button variant="text"
@@ -591,12 +617,14 @@ class ChromedashFeatureDetail extends LitElement {
     const isActive = this.feature.active_stage_id === feStage.id;
 
     // Show any buttons that should be displayed at the top of the detail card.
+    const stageMenu = this.renderStageMenu(feStage);
     const addExtensionButton = this.renderExtensionButton(feStage);
     const editButton = this.renderEditButton(feStage, processStage);
     const trialButton = this.renderOriginTrialButton(feStage);
 
     const content = html`
       <p class="description">
+        ${stageMenu}
         ${trialButton}
         ${editButton}
         ${addExtensionButton}
@@ -707,6 +735,33 @@ class ChromedashFeatureDetail extends LitElement {
     return nothing;
   }
 
+  offerAddXfnGates(feStage) {
+    const stageGates = this.gates.filter(g => g.stage_id == feStage.id);
+    return (feStage.stage_type == STAGE_PSA_SHIPPING &&
+            stageGates.length < 6);
+  }
+
+  renderStageMenu(feStage) {
+    const items = [];
+    if (this.offerAddXfnGates(feStage)) {
+      items.push(html`
+        <sl-menu-item @click=${() => this.handleAddXfnGates(feStage)}>
+           Add cross-functional gates
+        </sl-menu-item>
+      `);
+    }
+
+    if (items.length === 0) return nothing;
+
+    return html`
+    <sl-dropdown>
+      <sl-icon-button library="material" name="more_vert_24px" label="Stage menu"
+         slot="trigger"></sl-icon-button>
+      <sl-menu>${items}</sl-menu>
+    </sl-dropdown>
+    `;
+  }
+
   renderAddStageButton() {
     if (!this.canEdit) {
       return nothing;

client-src/js-src/cs-client.js

@@ -338,6 +338,11 @@ class ChromeStatusClient {
     return this.doPatch(`/features/${featureId}/stages/${stageId}`, body);
   }
 
+  async addXfnGates(featureId, stageId) {
+    return this.doPost(
+        `/features/${featureId}/stages/${stageId}/addXfnGates`);
+  }
+
   // Processes API
   async getFeatureProcess(featureId) {
     return this.doGet(`/features/${featureId}/process`);

main.py

@@ -129,6 +129,9 @@ api_routes: list[Route] = [
             stages_api.StagesAPI),
     Route(f'{API_BASE}/features/<int:feature_id>/stages/<int:stage_id>',
             stages_api.StagesAPI),
+    Route(
+        f'{API_BASE}/features/<int:feature_id>/stages/<int:stage_id>/addXfnGates',
+        reviews_api.XfnGatesAPI),
 
     Route(f'{API_BASE}/blinkcomponents',
         blink_components_api.BlinkComponentsAPI),

static/shoelace/assets/material-icons/more_vert_24px.svg

@@ -0,0 +1 @@
+<svg xmlns="http://www.w3.org/2000/svg" height="24" viewBox="0 -960 960 960" width="24"><path d="M480-160q-33 0-56.5-23.5T400-240q0-33 23.5-56.5T480-320q33 0 56.5 23.5T560-240q0 33-23.5 56.5T480-160Zm0-240q-33 0-56.5-23.5T400-480q0-33 23.5-56.5T480-560q33 0 56.5 23.5T560-480q0 33-23.5 56.5T480-400Zm0-240q-33 0-56.5-23.5T400-720q0-33 23.5-56.5T480-800q33 0 56.5 23.5T560-720q0 33-23.5 56.5T480-640Z"/></svg>