Migrate more form definitions and render guide/verify_accuracy form without unsafeHTML (#2204)

* Refactor input form fields by specifying attributes * Remove unsafeHTML and slots in chromedash-form-field * Update the unit test and remove unused properties * Fix hard-coded input field name for radios * Render guide/edit form without unsafeHTML * Update guide page unit tests * no longer need to pass in overviewForm * Refactor verify_accuracy page and migrate its form to js * Remove unused code * Resolve web test complaints * Make changes based on changes from previous PR
2022-09-06 20:35:35 -04:00 · 2022-09-06 20:35:35 -04:00 · d0b954b880
--- a/internals/core_models.py
+++ b/internals/core_models.py
@ -237,31 +237,32 @@ class Feature(DictModel):
          'val': standard_maturity_val,
        },
      }
+      del d['standard_maturity']
      d['tag_review_status'] = REVIEW_STATUS_CHOICES[self.tag_review_status]
      d['security_review_status'] = REVIEW_STATUS_CHOICES[
          self.security_review_status]
      d['privacy_review_status'] = REVIEW_STATUS_CHOICES[
          self.privacy_review_status]
      d['resources'] = {
-        'samples': self.sample_links or [],
-        'docs': self.doc_links or [],
+        'samples': d.pop('sample_links', []),
+        'docs': d.pop('doc_links', []),
      }
-      d['tags'] = self.search_tags or []
+      d['tags'] = d.pop('search_tags', [])
      d['editors'] = d.pop('editors', [])
      d['creator'] = d.pop('creator', None)
      d['browsers'] = {
        'chrome': {
-          'bug': self.bug_url or None,
-          'blink_components': self.blink_components or [],
-          'devrel': self.devrel or [],
-          'owners': self.owner or [],
+          'bug': d.pop('bug_url', None),
+          'blink_components': d.pop('blink_components', []),
+          'devrel': d.pop('devrel', []),
+          'owners': d.pop('owner', []),
          'origintrial': self.impl_status_chrome == ORIGIN_TRIAL,
          'intervention': self.impl_status_chrome == INTERVENTION,
          'prefixed': d.pop('prefixed', False),
          'flag': self.impl_status_chrome == BEHIND_A_FLAG,
          'status': {
            'text': IMPLEMENTATION_STATUS[self.impl_status_chrome],
-            'val': self.impl_status_chrome or None
+            'val': d.pop('impl_status_chrome', None)
          },
          'desktop': d.pop('shipped_milestone', None),
          'android': d.pop('shipped_android_milestone', None),
--- a/pages/guide.py
+++ b/pages/guide.py
@ -599,18 +599,5 @@ class FeatureVerifyAccuracy(FeatureEditStage):
    if redirect_resp:
      return redirect_resp

-    f, _ = self.get_feature_and_process(feature_id)
-    feature_edit_dict = f.format_for_edit()
-
-    forms_title = "Accuracy last verified at time of creation."
-    if f.accurate_as_of is not None:
-      date = f.accurate_as_of.strftime("%Y-%m-%d")
-      forms_title = f"Accuracy last verified {date}."
-    form = guideforms.Verify_Accuracy(feature_edit_dict)
-    forms = [(forms_title, str(form), list(form.fields))]
-    template_data = {
-        'feature': f,
-        'feature_id': f.key.integer_id(),
-        'forms': json.dumps(forms),
-    }
+    template_data = {'feature_id': feature_id}
    return template_data
--- a/pages/guideforms.py
+++ b/pages/guideforms.py
@ -861,18 +861,6 @@ Flat_Ship = define_form_class_using_shared_fields(
     'shipped_milestone', 'shipped_android_milestone',
     'shipped_ios_milestone', 'shipped_webview_milestone'))

-Verify_Accuracy = define_form_class_using_shared_fields(
-    'Verify_Accuracy',
-    ('summary', 'owner', 'editors', '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',
-    'ot_milestone_desktop_end', 'ot_milestone_webview_start',
-    'ot_milestone_webview_end', 'shipped_android_milestone',
-    'shipped_ios_milestone', 'shipped_milestone', 'shipped_webview_milestone',
-    'accurate_as_of')
-)
-
 FIELD_NAME_TO_DISPLAY_TYPE = {
    'doc_links': 'urllist',
    'explainer_links': 'urllist',
--- a/static/elements/chromedash-guide-verify-accuracy-page.js
+++ b/static/elements/chromedash-guide-verify-accuracy-page.js
@ -1,7 +1,9 @@
 import {LitElement, css, html} from 'lit';
-import {unsafeHTML} from 'lit/directives/unsafe-html.js';
+import {ref} from 'lit/directives/ref.js';
 import {showToastMessage} from './utils.js';
+import './chromedash-form-field';
 import './chromedash-form-table';
+import {formatFeatureForEdit, VERIFY_ACCURACY_FORM_FIELDS} from './form-definition';
 import {SHARED_STYLES} from '../sass/shared-css.js';
 import {FORM_STYLES} from '../sass/forms-css.js';

@ -19,7 +21,6 @@ export class ChromedashGuideVerifyAccuracyPage extends LitElement {
    return {
      featureId: {type: Number},
      feature: {type: Object},
-      forms: {type: String},
      loading: {type: Boolean},
    };
  }
@ -28,7 +29,6 @@ export class ChromedashGuideVerifyAccuracyPage extends LitElement {
    super();
    this.featureId = 0;
    this.feature = {};
-    this.forms = '';
    this.loading = true;
  }

@ -53,19 +53,17 @@ export class ChromedashGuideVerifyAccuracyPage extends LitElement {

  /* Add the form's event listener after Shoelace event listeners are attached
   * see more at https://github.com/GoogleChrome/chromium-dashboard/issues/2014 */
-  firstUpdated() {
-    /* TODO(kevinshen56714): remove the timeout once the form fields are all
-     * migrated to frontend, we need it now because the unsafeHTML(this.overviewForm)
-     * delays the Shoelace event listener attachment */
-    setTimeout(() => {
-      const hiddenTokenField = this.shadowRoot.querySelector('input[name=token]');
-      hiddenTokenField.form.addEventListener('submit', (event) => {
-        this.handleFormSubmission(event, hiddenTokenField);
-      });
-    }, 1000);
+  async registerFormSubmitHandler(el) {
+    if (!el) return;
+
+    await el.updateComplete;
+    const hiddenTokenField = this.shadowRoot.querySelector('input[name=token]');
+    hiddenTokenField.form.addEventListener('submit', (event) => {
+      this.handleFormSubmit(event, hiddenTokenField);
+    });
  }

-  handleFormSubmission(event, hiddenTokenField) {
+  handleFormSubmit(event, hiddenTokenField) {
    event.preventDefault();

    // get the XSRF token and update it if it's expired before submission
@ -79,15 +77,6 @@ export class ChromedashGuideVerifyAccuracyPage extends LitElement {
    window.location.href = `/guide/edit/${this.featureId}`;
  }

-  // get a comma-spearated list of field names
-  getFormFields() {
-    let fields = [];
-    JSON.parse(this.forms).map((form) => {
-      fields = [...fields, ...form[2]];
-    });
-    return fields.join();
-  }
-
  renderSkeletons() {
    return html`
      <h3><sl-skeleton effect="sheen"></sl-skeleton></h3>
@ -121,19 +110,27 @@ export class ChromedashGuideVerifyAccuracyPage extends LitElement {
  }

  renderForm() {
+    const title = this.feature.accurate_as_of ?
+      `Accuracy last verified ${this.feature.accurate_as_of.split(' ')[0]}.` :
+      'Accuracy last verified at time of creation.';
+    const formattedFeature = formatFeatureForEdit(this.feature);
+
    return html`
      <form name="feature_form" method="POST" action="/guide/verify_accuracy/${this.featureId}">
        <input type="hidden" name="token">
-        <input type="hidden" name="form_fields" value=${this.getFormFields()}>
+        <input type="hidden" name="form_fields" value=${VERIFY_ACCURACY_FORM_FIELDS.join()}>

-        ${JSON.parse(this.forms).map(([sectionName, form]) => html`
-          <h3>${sectionName}</h3>
-          <section class="flat_form">
-            <chromedash-form-table>
-              ${unsafeHTML(form)}
-            </chromedash-form-table>
-          </section>
-        `)}
+        <h3>${title}</h3>
+        <section class="flat_form">
+          <chromedash-form-table ${ref(this.registerFormSubmitHandler)}>
+          ${VERIFY_ACCURACY_FORM_FIELDS.map((field) => html`
+            <chromedash-form-field
+              name=${field}
+              value=${formattedFeature[field]}>
+            </chromedash-form-field>
+          `)}
+          </chromedash-form-table>
+        </section>

        <section class="final_buttons">
          <input class="button" type="submit" value="Submit">
--- a/static/elements/chromedash-guide-verify-accuracy-page_test.js
+++ b/static/elements/chromedash-guide-verify-accuracy-page_test.js
@ -1,6 +1,7 @@
 import {html} from 'lit';
 import {assert, fixture} from '@open-wc/testing';
 import {ChromedashGuideVerifyAccuracyPage} from './chromedash-guide-verify-accuracy-page';
+import {VERIFY_ACCURACY_FORM_FIELDS} from './form-definition';
 import './chromedash-toast';
 import '../js-src/cs-client';
 import sinon from 'sinon';
@ -18,24 +19,30 @@ describe('chromedash-guide-verify-accuracy-page', () => {
      chrome: {
        blink_components: ['Blink'],
        owners: ['fake chrome owner one', 'fake chrome owner two'],
-        status: {text: 'fake chrome status text'},
+        status: {
+          milestone_str: 'No active development',
+          text: 'No active development',
+          val: 1},
      },
-      ff: {view: {text: 'fake ff view text'}},
-      safari: {view: {text: 'fake safari view text'}},
-      webdev: {view: {text: 'fake webdev view text'}},
+      ff: {view: {text: 'No signal', val: 5}},
+      safari: {view: {text: 'No signal', val: 5}},
+      webdev: {view: {text: 'No signal', val: 4}},
+      other: {view: {}},
    },
    resources: {
      samples: ['fake sample link one', 'fake sample link two'],
      docs: ['fake doc link one', 'fake doc link two'],
    },
    standards: {
-      spec: 'fake spec link',
-      maturity: {text: 'Unknown standards status - check spec link for status'},
+      maturity: {
+        short_text: 'Incubation',
+        text: 'Specification being incubated in a Community Group',
+        val: 3,
+      },
+      status: {text: 'Editor\'s Draft', val: 4},
    },
    tags: ['tag_one'],
  });
-  /* TODO: create a proper fake data once the form generation is migrated to frontend */
-  const forms = '[["fake section name", "", ["fake field 1", "fake field 2"]]]';

  /* window.csClient and <chromedash-toast> are initialized at _base.html
   * which are not available here, so we initialize them before each test.
@ -72,8 +79,7 @@ describe('chromedash-guide-verify-accuracy-page', () => {

    const component = await fixture(
      html`<chromedash-guide-verify-accuracy-page
-             .featureId=${featureId}
-             .forms=${forms}>
+             .featureId=${featureId}>
           </chromedash-guide-verify-accuracy-page>`);
    assert.exists(component);
    assert.instanceOf(component, ChromedashGuideVerifyAccuracyPage);
@ -89,7 +95,7 @@ describe('chromedash-guide-verify-accuracy-page', () => {
    assert.exists(featureForm);
    assert.include(featureForm.innerHTML, '<input type="hidden" name="token">');
    assert.include(featureForm.innerHTML,
-      '<input type="hidden" name="form_fields" value="fake field 1,fake field 2">');
+      `<input type="hidden" name="form_fields" value="${VERIFY_ACCURACY_FORM_FIELDS.join()}">`);
    assert.include(featureForm.innerHTML, '<section class="final_buttons">');
  });
 });
--- a/static/elements/form-definition.js
+++ b/static/elements/form-definition.js
@ -6,14 +6,62 @@ const COMMA_SEPARATED_FIELDS = ['owner', 'editors', 'spec_mentors',

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

+/* Convert the format of feature object fetched from API into those for edit.
+ * The feature object from API is formatted by the format_for_template method of
+ * the Feature class in internals/core_models.py
+ */
 export function formatFeatureForEdit(feature) {
  const formattedFeature = {
    ...feature,
    category: feature.category_int,
    feature_type: feature.feature_type_int,
    intent_stage: feature.intent_stage_int,
-    standard_maturity: feature.standard_maturity || STANDARD_MATURITY_CHOICES.UNKNOWN_STD,
-    blink_components: feature.blink_components[0],
+
+    // accurate_as_of field should always be checked, regardless of
+    // the current value. This is only necessary if the feature
+    // has been created before this field was added.
+    accurate_as_of: true,
+
+    // from feature.standards
+    spec_link: feature.standards.spec,
+    standardization: feature.standards.status.val,
+    standard_maturity: feature.standards.maturity.val || STANDARD_MATURITY_CHOICES.UNKNOWN_STD,
+
+    // from feature.resources
+    sample_links: feature.resources.samples,
+    doc_links: feature.resources.docs,
+
+    search_tags: feature.tag,
+
+    // from feature.browsers.chrome
+    blink_components: feature.browsers.chrome.blink_components[0],
+    bug_url: feature.browsers.chrome.bug,
+    devrel: feature.browsers.chrome.devrel,
+    owner: feature.browsers.chrome.owners,
+    prefixed: feature.browsers.chrome.prefixed,
+    impl_status_chrome: feature.browsers.chrome.status.val,
+    shipped_milestone: feature.browsers.chrome.desktop,
+    shipped_android_milestone: feature.browsers.chrome.android,
+    shipped_webview_milestone: feature.browsers.chrome.webview,
+    shipped_ios_milestone: feature.browsers.chrome.ios,
+
+    // from feature.browsers.ff
+    ff_views: feature.browsers.ff.view.val,
+    ff_views_link: feature.browsers.ff.view.url,
+    ff_views_notes: feature.browsers.ff.view.notes,
+
+    // from feature.browsers.safari
+    safari_views: feature.browsers.safari.view.val,
+    safari_views_link: feature.browsers.safari.view.url,
+    safari_views_notes: feature.browsers.safari.view.notes,
+
+    // from feature.browsers.webdev
+    web_dev_views: feature.browsers.webdev.view.val,
+    web_dev_views_link: feature.browsers.webdev.view.url,
+    web_dev_views_notes: feature.browsers.webdev.view.notes,
+
+    // from feature.browsers.other
+    other_views_notes: feature.browsers.other.view.notes,
  };

  COMMA_SEPARATED_FIELDS.map((field) => {
@ -39,3 +87,14 @@ export const METADATA_FORM_FIELDS = [
  'blink_components',
  'bug_url', 'launch_bug_url',
 ];
+
+export const VERIFY_ACCURACY_FORM_FIELDS = [
+  'summary', 'owner', 'editors', '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',
+  'ot_milestone_desktop_end', 'ot_milestone_webview_start',
+  'ot_milestone_webview_end', 'shipped_android_milestone',
+  'shipped_ios_milestone', 'shipped_milestone', 'shipped_webview_milestone',
+  'accurate_as_of',
+];
--- a/templates/guide/verify_accuracy.html
+++ b/templates/guide/verify_accuracy.html
@ -3,7 +3,6 @@

 {% block content %}
  <chromedash-guide-verify-accuracy-page
-    featureId="{{ feature_id }}"
-    forms="{{ forms }}">
+    featureId="{{ feature_id }}">
  </chromedash-guide-verify-accuracy-page>
 {% endblock %}