OT requested clean up/changes (#4509)

* OT process cleanup * remove unused test file * test name fix * update tests to specifically test template render * make OT documentation field required only on request form --------- Co-authored-by: DanielRyanSmith <danielrsmith@google.com>
2024-11-07 19:10:25 +00:00 · 2024-11-07 19:10:25 +00:00 · 92c888d621
--- a/client-src/elements/form-definition.ts
+++ b/client-src/elements/form-definition.ts
@ -349,6 +349,7 @@ const FLAT_ORIGIN_TRIAL_FIELDS: MetadataFields = {
        'ongoing_constraints',
        // TODO(jrobbins): display r4dt_url instead when deprecating.
        'i2e_lgtms',
+        'ot_documentation_url',
        'intent_to_experiment_url',
        'origin_trial_feedback_url',
      ],
@ -563,7 +564,7 @@ export const ORIGIN_TRIAL_CREATION_FIELDS: MetadataFields = {
        'ot_creation__milestone_desktop_first',
        'ot_creation__milestone_desktop_last',
        'ot_creation__intent_to_experiment_url',
-        'ot_documentation_url',
+        'ot_creation__ot_documentation_url',
        'ot_feedback_submission_url',
        'ot_chromium_trial_name',
        'ot_is_deprecation_trial',
--- a/client-src/elements/form-field-enums.ts
+++ b/client-src/elements/form-field-enums.ts
@ -377,6 +377,7 @@ export const STAGE_SPECIFIC_FIELDS = new Set<string>([
  'ot_request_note',
  'ot_webfeature_use_counter',
  'ot_documentation_url',
+  'ot_creation__ot_documentation_url',
  'ot_is_deprecation_trial',
  'ot_has_third_party_support',
  'ot_is_critical_trial',
@ -415,6 +416,7 @@ export const STAGE_FIELD_NAME_MAPPING: Record<string, string> = {
  ot_milestone_webview_end: 'webview_last',
  ot_creation__milestone_desktop_first: 'desktop_first',
  ot_creation__milestone_desktop_last: 'desktop_last',
+  ot_creation__ot_documentation_url: 'ot_documentation_url',
  ot_extension__milestone_desktop_last: 'desktop_last',
  dt_milestone_desktop_start: 'desktop_first',
  dt_milestone_android_start: 'android_first',
--- a/client-src/elements/form-field-specs.ts
+++ b/client-src/elements/form-field-specs.ts
@ -1434,6 +1434,14 @@ export const ALL_FIELDS: Record<string, Field> = {
  },

  ot_documentation_url: {
+    type: 'input',
+    attrs: URL_FIELD_ATTRS,
+    label: 'Documentation link',
+    help_text: html` Link to more information to help developers use the trial's
+    feature (e.g. blog post, Github explainer, etc.).`,
+  },
+
+  ot_creation__ot_documentation_url: {
    type: 'input',
    attrs: URL_FIELD_ATTRS,
    required: true,
--- a/pages/intentpreview_test.py
+++ b/pages/intentpreview_test.py
@ -14,6 +14,7 @@

 import testing_config  # Must be imported before the module under test.

+from flask import render_template
 from unittest import mock

 import os
@ -248,7 +249,8 @@ class IntentEmailPreviewTemplateTest(testing_config.CustomTestCase):
  def setUp(self):
    super(IntentEmailPreviewTemplateTest, self).setUp()
    self.feature_1 = FeatureEntry(
-        name='feature one', summary='sum', owner_emails=['user1@google.com'],
+        id=234, name='feature one', summary='sum',
+        owner_emails=['user1@google.com'], feature_type=0,
        category=1, intent_stage=core_enums.INTENT_IMPLEMENT)
    # Hardcode the key for the template test
    self.feature_1.key = ndb.Key('FeatureEntry', 234)
@ -256,8 +258,21 @@ class IntentEmailPreviewTemplateTest(testing_config.CustomTestCase):
    self.feature_1.wpt_descr = 'We love WPT!'
    self.feature_1.put()

+    self.stage_1 = Stage(id=100, feature_id=234, stage_type=150,
+                         ot_display_name="Test 123")
+    self.stage_1.put()
+    self.gate_1 = Gate(id=101, feature_id=234, stage_id=100,
+                       gate_type=3, state=Vote.NA)
+    self.gate_1.put()
+
+    self.stage_2 = Stage(id=200, feature_id=234, stage_type=110)
+    self.stage_2.put()
+    self.gate_2 = Gate(id=201, feature_id=234, stage_id=100,
+                       gate_type=1, state=Vote.NA)
+    self.gate_2.put()
    self.request_path = '/admin/features/launch/%d/%d?intent' % (
        core_enums.INTENT_SHIP, self.feature_1.key.integer_id())
+    self.intent_preview_path = 'blink/intent_to_implement.html'
    self.handler = self.HANDLER_CLASS()
    self.feature_id = self.feature_1.key.integer_id()

@ -276,7 +291,9 @@ class IntentEmailPreviewTemplateTest(testing_config.CustomTestCase):
    self.maxDiff = None

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

  def test_html_rendering(self):
@ -297,3 +314,39 @@ class IntentEmailPreviewTemplateTest(testing_config.CustomTestCase):
    # TESTDATA.make_golden(template_text, 'test_html_rendering.html')
    self.assertMultiLineEqual(
      TESTDATA['test_html_rendering.html'], template_text)
+
+  def test_template_rendering_prototype(self):
+    """We can render the prototype template with valid html."""
+    with test_app.test_request_context(self.request_path):
+      actual_data = self.handler.get_template_data(
+          feature_id=self.feature_id,
+          intent_stage=core_enums.INTENT_IMPLEMENT,
+          gate_id=self.gate_2.key.integer_id())
+      actual_data.update(self.handler.get_common_data())
+      actual_data['nonce'] = 'fake nonce'
+      actual_data['xsrf_token'] = ''
+      actual_data['xsrf_token_expires'] = 0
+
+      body = render_template(self.intent_preview_path, **actual_data)
+      testing_config.sign_out()
+    # TESTDATA.make_golden(body, 'test_html_prototype_rendering.html')
+    self.assertMultiLineEqual(
+      TESTDATA['test_html_prototype_rendering.html'], body)
+
+  def test_template_rendering_origin_trial(self):
+    """We can render the origin trial intent template."""
+    with test_app.test_request_context(self.request_path):
+      actual_data = self.handler.get_template_data(
+          feature_id=self.feature_id,
+          intent_stage=core_enums.INTENT_ORIGIN_TRIAL,
+          gate_id=self.gate_1.key.integer_id())
+      actual_data.update(self.handler.get_common_data())
+      actual_data['nonce'] = 'fake nonce'
+      actual_data['xsrf_token'] = ''
+      actual_data['xsrf_token_expires'] = 0
+
+      body = render_template(self.intent_preview_path, **actual_data)
+      testing_config.sign_out()
+    # TESTDATA.make_golden(body, 'test_html_ot_rendering.html')
+    self.assertMultiLineEqual(
+      TESTDATA['test_html_ot_rendering.html'], body)
--- a/pages/testdata/intentpreview_test/test_html_ot_rendering.html
+++ b/pages/testdata/intentpreview_test/test_html_ot_rendering.html
@ -0,0 +1,169 @@
+  <h4>Contact emails</h4>
+  
+  
+    <a href="mailto:user1@google.com">user1@google.com</a>
+  
+
+  
+    <br><br><h4>Explainer</h4>
+    None
+    
+  
+
+  <br><br><h4>Specification</h4>
+  None
+
+  
+
+  <br><br><h4>Summary</h4>
+  <p class="preformatted">sum</p>
+
+  <br><br><h4>Blink component</h4>
+  
+    <a href="https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink" target="_blank" rel="noopener">Blink</a>
+  
+
+  
+
+  
+
+  <br><br><h4>TAG review</h4>
+  None
+
+  
+    <br><br><h4>TAG review status</h4>
+    Pending
+  
+
+  
+    
+      <br><br><h4>Origin Trial Name</h4>
+      Test 123
+
+    
+  
+  <br><br><h4>Risks</h4>
+  <div style="margin-left: 4em;">
+    <br><br><h4>Interoperability and Compatibility</h4>
+    <p class="preformatted">None</p>
+
+    <br><br><i>Gecko</i>: No signal
+    
+    
+
+    <br><br><i>WebKit</i>: No signal
+    
+    
+
+    <br><br><i>Web developers</i>: No signals
+    
+    
+
+    <br><br><i>Other signals</i>:
+    
+
+    
+
+    
+
+    
+
+    <br><br><h4>WebView application risks</h4>
+    <p style="font-style: italic">
+      Does this intent deprecate or change behavior of existing APIs,
+      such that it has potentially high risk for Android WebView-based
+      applications?</p>
+    <p class="preformatted">None</p>
+
+  </div> <!-- end risks -->
+
+
+
+  <br><br><h4>Goals for experimentation</h4>
+  <p class="preformatted"></p>
+
+  
+
+  
+    
+  
+
+  <br><br><h4>Ongoing technical constraints</h4>
+  <p class="preformatted">None</p>
+
+
+
+<br><br><h4>Debuggability</h4>
+<p class="preformatted">None</p>
+
+
+  <br><br><h4>Will this feature be supported on all six Blink platforms
+      (Windows, Mac, Linux, ChromeOS, Android, and Android WebView)?</h4>
+  No
+  
+
+
+<br><br><h4>Is this feature fully tested by <a href="https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md">web-platform-tests</a>?</h4>
+Yes
+
+  <p class="preformatted">We love WPT!</p>
+
+
+
+
+<br><br><h4>Flag name on about://flags</h4>
+None
+
+<br><br><h4>Finch feature name</h4>
+None
+
+
+  
+    <br><br><h4>Non-finch justification</h4>
+    None
+  
+
+
+
+
+<br><br><h4>Requires code in //chrome?</h4>
+False
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+<br><br><h4>Estimated milestones</h4>
+
+
+  <p>No milestones specified</p>
+
+
+
+
+
+
+<br><br><h4>Link to entry on the Local testing</h4>
+<a href="http://localhost/feature/234?gate=101">http://localhost/feature/234?gate=101</a>
+
+
+
+
+  <br><br><div><small>
+    This intent message was generated by
+    <a href="https://chromestatus.com">Chrome Platform Status</a>.
+  </small></div>
--- a/pages/testdata/intentpreview_test/test_html_prototype_rendering.html
+++ b/pages/testdata/intentpreview_test/test_html_prototype_rendering.html
@ -0,0 +1,169 @@
+  <h4>Contact emails</h4>
+  
+  
+    <a href="mailto:user1@google.com">user1@google.com</a>
+  
+
+  
+    <br><br><h4>Explainer</h4>
+    None
+    
+  
+
+  <br><br><h4>Specification</h4>
+  None
+
+  
+
+  <br><br><h4>Summary</h4>
+  <p class="preformatted">sum</p>
+
+  <br><br><h4>Blink component</h4>
+  
+    <a href="https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink" target="_blank" rel="noopener">Blink</a>
+  
+
+  
+
+  
+
+  <br><br><h4>TAG review</h4>
+  None
+
+  
+    <br><br><h4>TAG review status</h4>
+    Pending
+  
+
+  
+    
+      <br><br><h4>Origin Trial Name</h4>
+      Test 123
+
+    
+  
+  <br><br><h4>Risks</h4>
+  <div style="margin-left: 4em;">
+    <br><br><h4>Interoperability and Compatibility</h4>
+    <p class="preformatted">None</p>
+
+    <br><br><i>Gecko</i>: No signal
+    
+    
+
+    <br><br><i>WebKit</i>: No signal
+    
+    
+
+    <br><br><i>Web developers</i>: No signals
+    
+    
+
+    <br><br><i>Other signals</i>:
+    
+
+    
+
+    
+
+    
+
+    <br><br><h4>WebView application risks</h4>
+    <p style="font-style: italic">
+      Does this intent deprecate or change behavior of existing APIs,
+      such that it has potentially high risk for Android WebView-based
+      applications?</p>
+    <p class="preformatted">None</p>
+
+  </div> <!-- end risks -->
+
+
+
+  <br><br><h4>Goals for experimentation</h4>
+  <p class="preformatted"></p>
+
+  
+
+  
+    
+  
+
+  <br><br><h4>Ongoing technical constraints</h4>
+  <p class="preformatted">None</p>
+
+
+
+<br><br><h4>Debuggability</h4>
+<p class="preformatted">None</p>
+
+
+  <br><br><h4>Will this feature be supported on all six Blink platforms
+      (Windows, Mac, Linux, ChromeOS, Android, and Android WebView)?</h4>
+  No
+  
+
+
+<br><br><h4>Is this feature fully tested by <a href="https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md">web-platform-tests</a>?</h4>
+Yes
+
+  <p class="preformatted">We love WPT!</p>
+
+
+
+
+<br><br><h4>Flag name on about://flags</h4>
+None
+
+<br><br><h4>Finch feature name</h4>
+None
+
+
+  
+    <br><br><h4>Non-finch justification</h4>
+    None
+  
+
+
+
+
+<br><br><h4>Requires code in //chrome?</h4>
+False
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+<br><br><h4>Estimated milestones</h4>
+
+
+  <p>No milestones specified</p>
+
+
+
+
+
+
+<br><br><h4>Link to entry on the Local testing</h4>
+<a href="http://localhost/feature/234?gate=201">http://localhost/feature/234?gate=201</a>
+
+
+
+
+  <br><br><div><small>
+    This intent message was generated by
+    <a href="https://chromestatus.com">Chrome Platform Status</a>.
+  </small></div>
--- a/pages/testdata/intentpreview_test/test_html_rendering.html
+++ b/pages/testdata/intentpreview_test/test_html_rendering.html
@ -208,6 +208,12 @@ limitations under the License.
  

  
+    
+      <br><br><h4>Origin Trial Name</h4>
+      Test 123
+
+    
+  
  <br><br><h4>Risks</h4>
  <div style="margin-left: 4em;">
    <br><br><h4>Interoperability and Compatibility</h4>
--- a/templates/blink/intent_to_implement.html
+++ b/templates/blink/intent_to_implement.html
@ -55,7 +55,11 @@
  {% endif %}

  {% for stage in stage_info.ot_stages %}
-    {% if stage.ot_chromium_trial_name %}
+    {% if stage.ot_display_name %}
+      <br><br><h4>Origin Trial Name</h4>
+      {{stage.ot_display_name}}
+
+    {% endif %}{% if stage.ot_chromium_trial_name %}
      <br><br><h4>Chromium Trial Name</h4>
      {{stage.ot_chromium_trial_name}}