Implement storage and api for gate survey questions (#4709)

* initial

* added new file, updated openapi, POST to PATCH

* progress

* complete

* complete

* correct security to privacy

* remove maxDiff

* correct security to privacy

api/converters.py

@@ -20,7 +20,7 @@
 from google.cloud import ndb  # type: ignore
 
 import settings
-from internals import approval_defs, slo
+from internals import approval_defs, slo, self_certify
 from internals.core_enums import *
 from internals.core_models import (
   FeatureEntry,
@@ -667,6 +667,9 @@ def gate_value_to_json_dict(gate: Gate) -> dict[str, Any]:
       slo_resolve_remaining = slo.remaining_days(
           gate.requested_on, slo_resolve) + (gate.needs_work_elapsed or 0)
 
+  self_certify_eligible = self_certify.is_eligible(gate)
+  survey_answers = to_dict(gate.survey_answers) if gate.survey_answers else None
+
   return {
       'id': gate.key.integer_id(),
       'feature_id': gate.feature_id,
@@ -689,4 +692,6 @@ def gate_value_to_json_dict(gate: Gate) -> dict[str, Any]:
       'slo_resolve_remaining': slo_resolve_remaining,
       # YYYY-MM-DD HH:MM:SS or None
       'needs_work_started_on': needs_work_started_on,
+      'self_certify_eligible': self_certify_eligible,
+      'survey_answers': survey_answers,
       }

api/converters_test.py

@@ -20,7 +20,7 @@
 from api import converters
 from internals.core_enums import *
 from internals.core_models import FeatureEntry, MilestoneSet, Stage
-from internals.review_models import Vote, Gate
+from internals.review_models import Vote, Gate, SurveyAnswers
 from internals import approval_defs
 
 
@@ -546,18 +546,22 @@ def test_minimal(self):
       'slo_resolve_took': None,
       'slo_resolve_remaining': None,
       'needs_work_started_on': None,
+      'self_certify_eligible': False,
+      'survey_answers': None,
       }
     self.assertEqual(expected, actual)
 
   @mock.patch('internals.slo.now_utc')
   def test_maxmimal(self, mock_now):
     """If a Gate has all fields set, we can convert it to JSON."""
     gate = Gate(
-        feature_id=1, stage_id=2, gate_type=34, state=4,
+        feature_id=1, stage_id=2, gate_type=GATE_PRIVACY_ORIGIN_TRIAL, state=4,
         requested_on=datetime(2022, 12, 14, 1, 2, 3), # Wednesday
         assignee_emails=['[email protected]', '[email protected]'],
         next_action=datetime(2022, 12, 25),
         additional_review=True)
+    gate.survey_answers = SurveyAnswers(
+        is_language_polyfill=True, launch_or_contact='[email protected]')
     gate.put()
     # The review was due on Wednesday 2022-12-21.
     mock_now.return_value = datetime(2022, 12, 23, 1, 2, 3)  # Thursday after.
@@ -568,7 +572,7 @@ def test_maxmimal(self, mock_now):
       'id': gate.key.integer_id(),
       'feature_id': 1,
       'stage_id': 2,
-      'gate_type': 34,
+      'gate_type': GATE_PRIVACY_ORIGIN_TRIAL,
       'team_name': appr_def.team_name,
       'gate_name': appr_def.name,
       'escalation_email': '[email protected]',
@@ -585,6 +589,13 @@ def test_maxmimal(self, mock_now):
       'slo_resolve_took': None,
       'slo_resolve_remaining': None,
       'needs_work_started_on': None,
+      'self_certify_eligible': True,
+      'survey_answers': {
+          'is_api_polyfill': False,
+          'is_language_polyfill': True,
+          'is_same_origin_css': False,
+          'launch_or_contact': '[email protected]',
+        },
       }
     self.assertEqual(expected, actual)
 

api/reviews_api.py

@@ -16,14 +16,14 @@
 import logging
 from typing import Any, Tuple
 
-from chromestatus_openapi.models import (GetVotesResponse, GetGateResponse, PostGateRequest, SuccessMessage)
+from chromestatus_openapi.models import (GetVotesResponse, GetGateResponse, PatchGateRequest, SuccessMessage)
 from google.cloud import ndb
 from google.cloud.ndb.tasklets import Future  # for type checking only
 
 from api import converters
 from framework import basehandlers, permissions
 from framework.users import User
-from internals import approval_defs, notifier_helpers
+from internals import approval_defs, notifier_helpers, self_certify
 from internals.core_enums import *
 from internals.core_models import FeatureEntry, Stage
 from internals.review_models import Gate, Vote, Amendment, Activity
@@ -148,23 +148,42 @@ def do_get(self, **kwargs) -> dict[str, Any]:
         'gates': dicts,
         }).to_dict()
 
+  # TODO(jrobbins): phase out do_post here because it should have been patch.
   def do_post(self, **kwargs) -> dict[str, str]:
+    return self.do_patch(**kwargs)
+
+  def do_patch(self, **kwargs) -> dict[str, str]:
     """Set the assignees for a gate."""
     user, fe, gate, feature_id, gate_id = get_user_feature_and_gate(
         self, kwargs)
-    request = PostGateRequest.from_dict(self.request.json)
-    assignees = request.assignees
+    request = PatchGateRequest.from_dict(self.request.json)
+    changes: list[str] = []
+    new_assignees = request.assignees
+    new_answers = request.survey_answers
+
+    logging.info('@@@ request is %r', request)
 
     self.require_permissions(user, fe, gate)
-    self.validate_assignees(assignees, fe, gate)
-    old_assignees = gate.assignee_emails
-    gate.assignee_emails = assignees
-    gate.put()
-    notifier_helpers.notify_assignees(
-        fe, gate, user.email(), old_assignees, assignees)
+
+    if new_assignees is not None:
+      self.validate_assignees(new_assignees, fe, gate)
+      old_assignees = gate.assignee_emails
+      gate.assignee_emails = new_assignees
+      notifier_helpers.notify_assignees(
+          fe, gate, user.email(), old_assignees, new_assignees)
+      changes.append('assignees')
+
+    if new_answers is not None:
+      self_certify.update_survey_answers(gate, new_answers)
+      # Note: these changes don't generate notifications or activities.
+      changes.append('answers')
+
+    if changes:
+      gate.put()
 
     # Callers don't use the JSON response for this API call.
-    return SuccessMessage(message='Done').to_dict()
+    message = 'Changed: %s' % (', '.join(changes) or None)
+    return SuccessMessage(message=message).to_dict()
 
   def require_permissions(self, user, feature, gate):
     """Abort the request if the user lacks permission to set assignees."""

api/reviews_api_test.py

@@ -338,6 +338,7 @@ def test_do_get__success(self, mock_get_approvers):
                 "assignee_emails": [],
                 "next_action": None,
                 "additional_review": False,
+                'self_certify_eligible': False,
                 'slo_initial_response': 5,
                 'slo_initial_response_took': None,
                 'slo_initial_response_remaining': None,
@@ -346,6 +347,7 @@ def test_do_get__success(self, mock_get_approvers):
                 'slo_resolve_remaining': None,
                 'needs_work_started_on': None,
                 'possible_assignee_emails': ['[email protected]'],
+                'survey_answers': None,
             },
         ],
         }

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

@@ -539,8 +539,11 @@ export class ChromeStatusClient {
     return this.doGet('/gates/pending');
   }
 
-  updateGate(featureId, gateId, assignees) {
-    return this.doPost(`/features/${featureId}/gates/${gateId}`, {assignees});
+  updateGate(featureId, gateId, assignees, survey_answers) {
+    return this.doPatch(`/features/${featureId}/gates/${gateId}`, {
+      assignees,
+      survey_answers,
+    });
   }
 
   getComments(featureId, gateId) {

gen/js/chromestatus-openapi/.openapi-generator/FILES

@@ -62,8 +62,8 @@ src/models/OriginTrialsInfo.ts
 src/models/OutstandingReview.ts
 src/models/OwnersAndSubscribersOfComponent.ts
 src/models/PatchCommentRequest.ts
+src/models/PatchGateRequest.ts
 src/models/PermissionsResponse.ts
-src/models/PostGateRequest.ts
 src/models/PostIntentRequest.ts
 src/models/PostSettingsRequest.ts
 src/models/PostStarsRequest.ts
@@ -79,6 +79,7 @@ src/models/SpecMentor.ts
 src/models/StageField.ts
 src/models/StringFieldInfoValue.ts
 src/models/SuccessMessage.ts
+src/models/SurveyAnswers.ts
 src/models/TokenRefreshResponse.ts
 src/models/UserPermissions.ts
 src/models/Vote.ts

gen/js/chromestatus-openapi/src/apis/DefaultApi.ts

@@ -41,8 +41,8 @@ import type {
   GetVotesResponse,
   MessageResponse,
   PatchCommentRequest,
+  PatchGateRequest,
   PermissionsResponse,
-  PostGateRequest,
   PostIntentRequest,
   PostSettingsRequest,
   PostVoteRequest,
@@ -107,10 +107,10 @@ import {
     MessageResponseToJSON,
     PatchCommentRequestFromJSON,
     PatchCommentRequestToJSON,
+    PatchGateRequestFromJSON,
+    PatchGateRequestToJSON,
     PermissionsResponseFromJSON,
     PermissionsResponseToJSON,
-    PostGateRequestFromJSON,
-    PostGateRequestToJSON,
     PostIntentRequestFromJSON,
     PostIntentRequestToJSON,
     PostSettingsRequestFromJSON,
@@ -267,7 +267,7 @@ export interface RemoveUserFromComponentRequest {
 export interface SetAssigneesForGateRequest {
     featureId: number;
     gateId: number;
-    postGateRequest: PostGateRequest;
+    patchGateRequest: PatchGateRequest;
 }
 
 export interface SetStarOperationRequest {
@@ -897,7 +897,7 @@ export interface DefaultApiInterface {
      * @summary Set the assignees for a gate.
      * @param {number} featureId The ID of the feature to retrieve votes for.
      * @param {number} gateId The ID of the gate to retrieve votes for.
-     * @param {PostGateRequest} postGateRequest 
+     * @param {PatchGateRequest} patchGateRequest 
      * @param {*} [options] Override http request option.
      * @throws {RequiredError}
      * @memberof DefaultApiInterface
@@ -2348,10 +2348,10 @@ export class DefaultApi extends runtime.BaseAPI implements DefaultApiInterface {
             );
         }
 
-        if (requestParameters['postGateRequest'] == null) {
+        if (requestParameters['patchGateRequest'] == null) {
             throw new runtime.RequiredError(
-                'postGateRequest',
-                'Required parameter "postGateRequest" was null or undefined when calling setAssigneesForGate().'
+                'patchGateRequest',
+                'Required parameter "patchGateRequest" was null or undefined when calling setAssigneesForGate().'
             );
         }
 
@@ -2363,10 +2363,10 @@ export class DefaultApi extends runtime.BaseAPI implements DefaultApiInterface {
 
         const response = await this.request({
             path: `/features/{feature_id}/gates/{gate_id}`.replace(`{${"feature_id"}}`, encodeURIComponent(String(requestParameters['featureId']))).replace(`{${"gate_id"}}`, encodeURIComponent(String(requestParameters['gateId']))),
-            method: 'POST',
+            method: 'PATCH',
             headers: headerParameters,
             query: queryParameters,
-            body: PostGateRequestToJSON(requestParameters['postGateRequest']),
+            body: PatchGateRequestToJSON(requestParameters['patchGateRequest']),
         }, initOverrides);
 
         return new runtime.JSONApiResponse(response, (jsonValue) => SuccessMessageFromJSON(jsonValue));

gen/js/chromestatus-openapi/src/models/Gate.ts

@@ -13,6 +13,13 @@
  */
 
 import { mapValues } from '../runtime';
+import type { SurveyAnswers } from './SurveyAnswers';
+import {
+    SurveyAnswersFromJSON,
+    SurveyAnswersFromJSONTyped,
+    SurveyAnswersToJSON,
+} from './SurveyAnswers';
+
 /**
  * 
  * @export
@@ -145,6 +152,18 @@ export interface Gate {
      * @memberof Gate
      */
     possible_assignee_emails?: Array<string>;
+    /**
+     * 
+     * @type {boolean}
+     * @memberof Gate
+     */
+    self_certify_eligible?: boolean;
+    /**
+     * 
+     * @type {SurveyAnswers}
+     * @memberof Gate
+     */
+    survey_answers?: SurveyAnswers;
 }
 
 /**
@@ -185,6 +204,8 @@ export function GateFromJSONTyped(json: any, ignoreDiscriminator: boolean): Gate
         'slo_resolve_remaining': json['slo_resolve_remaining'] == null ? undefined : json['slo_resolve_remaining'],
         'needs_work_started_on': json['needs_work_started_on'] == null ? undefined : (new Date(json['needs_work_started_on'])),
         'possible_assignee_emails': json['possible_assignee_emails'] == null ? undefined : json['possible_assignee_emails'],
+        'self_certify_eligible': json['self_certify_eligible'] == null ? undefined : json['self_certify_eligible'],
+        'survey_answers': json['survey_answers'] == null ? undefined : SurveyAnswersFromJSON(json['survey_answers']),
     };
 }
 
@@ -215,6 +236,8 @@ export function GateToJSON(value?: Gate | null): any {
         'slo_resolve_remaining': value['slo_resolve_remaining'],
         'needs_work_started_on': value['needs_work_started_on'] == null ? undefined : ((value['needs_work_started_on']).toISOString()),
         'possible_assignee_emails': value['possible_assignee_emails'],
+        'self_certify_eligible': value['self_certify_eligible'],
+        'survey_answers': SurveyAnswersToJSON(value['survey_answers']),
     };
 }
 

gen/js/chromestatus-openapi/src/models/PostGateRequest.ts → gen/js/chromestatus-openapi/src/models/PatchGateRequest.ts

@@ -13,48 +13,63 @@
  */
 
 import { mapValues } from '../runtime';
+import type { SurveyAnswers } from './SurveyAnswers';
+import {
+    SurveyAnswersFromJSON,
+    SurveyAnswersFromJSONTyped,
+    SurveyAnswersToJSON,
+} from './SurveyAnswers';
+
 /**
  * 
  * @export
- * @interface PostGateRequest
+ * @interface PatchGateRequest
  */
-export interface PostGateRequest {
+export interface PatchGateRequest {
     /**
      * 
      * @type {Array<string>}
-     * @memberof PostGateRequest
+     * @memberof PatchGateRequest
      */
     assignees?: Array<string>;
+    /**
+     * 
+     * @type {SurveyAnswers}
+     * @memberof PatchGateRequest
+     */
+    survey_answers?: SurveyAnswers;
 }
 
 /**
- * Check if a given object implements the PostGateRequest interface.
+ * Check if a given object implements the PatchGateRequest interface.
  */
-export function instanceOfPostGateRequest(value: object): value is PostGateRequest {
+export function instanceOfPatchGateRequest(value: object): value is PatchGateRequest {
     return true;
 }
 
-export function PostGateRequestFromJSON(json: any): PostGateRequest {
-    return PostGateRequestFromJSONTyped(json, false);
+export function PatchGateRequestFromJSON(json: any): PatchGateRequest {
+    return PatchGateRequestFromJSONTyped(json, false);
 }
 
-export function PostGateRequestFromJSONTyped(json: any, ignoreDiscriminator: boolean): PostGateRequest {
+export function PatchGateRequestFromJSONTyped(json: any, ignoreDiscriminator: boolean): PatchGateRequest {
     if (json == null) {
         return json;
     }
     return {
 
         'assignees': json['assignees'] == null ? undefined : json['assignees'],
+        'survey_answers': json['survey_answers'] == null ? undefined : SurveyAnswersFromJSON(json['survey_answers']),
     };
 }
 
-export function PostGateRequestToJSON(value?: PostGateRequest | null): any {
+export function PatchGateRequestToJSON(value?: PatchGateRequest | null): any {
     if (value == null) {
         return value;
     }
     return {
 
         'assignees': value['assignees'],
+        'survey_answers': SurveyAnswersToJSON(value['survey_answers']),
     };
 }