From 61e68854733f5b96adb0ec4cce74a019df393a9c Mon Sep 17 00:00:00 2001
From: Yann Dago <mr.yanndago@gmail.com>
Date: Thu, 9 Mar 2023 16:18:20 -0500
Subject: [PATCH] Add Enterprise feature flow UI/UX changes (#2802)

* Hide edit button on feature page for enterprises
* After creating an enterprise feature go to the edit all page at the first rollout section
* Open all section on the feature page for enterprise by default
* Adding stage for enterprise features only adds rollout feature
* Remove the action of setting a stage in enterprise features
* Update scrolling behavior to scroll to the element from the hash if no form field is found
* Remove the edit icon from the feature page that navigates to the edit page
* Add an id to the "edit all" section headers to serve as an anchor for navigations
---
 .../elements/chromedash-add-stage-dialog.js   | 10 +++++++--
 .../elements/chromedash-feature-detail.js     |  9 +++++---
 .../elements/chromedash-feature-page.js       |  6 ++---
 .../elements/chromedash-guide-editall-page.js | 21 ++++++++++++++----
 .../elements/chromedash-guide-metadata.js     | 12 +++++-----
 .../chromedash-guide-metadata_test.js         |  2 +-
 .../elements/chromedash-guide-stage-page.js   | 22 ++++++++++---------
 .../elements/chromedash-process-overview.js   |  4 +++-
 client-src/elements/form-definition.js        |  1 -
 client-src/elements/utils.js                  | 10 +++++++++
 pages/guide.py                                |  2 +-
 11 files changed, 68 insertions(+), 31 deletions(-)

diff --git a/client-src/elements/chromedash-add-stage-dialog.js b/client-src/elements/chromedash-add-stage-dialog.js
index 1a746d61..876b141f 100644
--- a/client-src/elements/chromedash-add-stage-dialog.js
+++ b/client-src/elements/chromedash-add-stage-dialog.js
@@ -1,6 +1,7 @@
 import {LitElement, css, html} from 'lit';
 import {SHARED_STYLES} from '../sass/shared-css.js';
 import {CREATEABLE_STAGES, FORMS_BY_STAGE_TYPE} from './form-definition.js';
+import {renderHTMLIf} from './utils.js';
 
 
 let addStageDialogEl;
@@ -90,17 +91,22 @@ class ChromedashAddStageDialog extends LitElement {
   }
 
   renderStageSelect() {
+    const offerChoice = CREATEABLE_STAGES[this.featureType].length > 1;
+    const initialValue = offerChoice ? 0 : CREATEABLE_STAGES[this.featureType][0];
+    this.canSubmit = !offerChoice;
     return html`
     <div id="controls">
       <sl-select
         placement="top" hoist
-        value=0
+        value=${initialValue}
         id="stage_create_select"
         size="small"
         @sl-change=${this.checkCanSubmit}
         style="width:16rem"
       >
-        <sl-option value="0" disabled>Select a stage to create</sl-option>
+        ${renderHTMLIf(
+          offerChoice,
+          html`<sl-option value="0" disabled>Select a stage to create</sl-option>`)}
         ${this.renderSelectMenuItems()}
       </sl-select>
       <sl-button variant="primary"
diff --git a/client-src/elements/chromedash-feature-detail.js b/client-src/elements/chromedash-feature-detail.js
index 5d248acd..ff7960ee 100644
--- a/client-src/elements/chromedash-feature-detail.js
+++ b/client-src/elements/chromedash-feature-detail.js
@@ -492,7 +492,11 @@ class ChromedashFeatureDetail extends LitElement {
         ${this.renderSectionFields(fields, {})}
       </section>
     `;
-    return this.renderSection('Metadata', content);
+    return this.renderSection(
+      'Metadata',
+      content,
+      /* isActive=*/false,
+      /* defaultOpen=*/this.feature.is_enterprise_feature);
   }
 
   renderGateChip(feStage, gate) {
@@ -593,8 +597,7 @@ class ChromedashFeatureDetail extends LitElement {
         ${this.renderSectionFields(fields, feStage)}
       </section>
     `;
-
-    const defaultOpen = (feStage.id == this.openStage);
+    const defaultOpen = this.feature.is_enterprise_feature || (feStage.id == this.openStage);
     return this.renderSection(name, content, isActive, defaultOpen);
   }
 
diff --git a/client-src/elements/chromedash-feature-page.js b/client-src/elements/chromedash-feature-page.js
index 94a406ea..abce46a6 100644
--- a/client-src/elements/chromedash-feature-page.js
+++ b/client-src/elements/chromedash-feature-page.js
@@ -2,7 +2,7 @@ import {LitElement, css, html, nothing} from 'lit';
 import './chromedash-feature-detail';
 import './chromedash-gantt';
 import {openApprovalsDialog} from './chromedash-approvals-dialog';
-import {autolink, showToastMessage} from './utils.js';
+import {autolink, renderHTMLIf, showToastMessage} from './utils.js';
 import {SHARED_STYLES} from '../sass/shared-css.js';
 
 const INACTIVE_STATES = [
@@ -288,13 +288,13 @@ export class ChromedashFeaturePage extends LitElement {
               <iron-icon icon="chromestatus:link"></iron-icon>
             </a>
           </span>
-          ${canEdit ? html`
+          ${renderHTMLIf(canEdit && !this.feature.is_enterprise_feature, html`
             <span class="tooltip" title="Edit this feature">
               <a href="/guide/edit/${this.featureId}" class="editfeature" data-tooltip>
                 <iron-icon icon="chromestatus:create"></iron-icon>
               </a>
             </span>
-          `: nothing}
+          `)}
         </div>
         <h2 id="breadcrumbs">
           <a href="${this.contextLink}">
diff --git a/client-src/elements/chromedash-guide-editall-page.js b/client-src/elements/chromedash-guide-editall-page.js
index 3f34c34f..1324ed3b 100644
--- a/client-src/elements/chromedash-guide-editall-page.js
+++ b/client-src/elements/chromedash-guide-editall-page.js
@@ -8,10 +8,12 @@ import {
   FLAT_METADATA_FIELDS,
   FLAT_ENTERPRISE_METADATA_FIELDS,
   FORMS_BY_STAGE_TYPE,
-  FLAT_TRIAL_EXTENSION_FIELDS} from './form-definition';
+  FLAT_TRIAL_EXTENSION_FIELDS,
+  STAGE_SHORT_NAMES} from './form-definition';
 import {SHARED_STYLES} from '../sass/shared-css.js';
 import {FORM_STYLES} from '../sass/forms-css.js';
 import {STAGE_SPECIFIC_FIELDS} from './form-field-enums.js';
+import {openAddStageDialog} from './chromedash-add-stage-dialog';
 
 
 export class ChromedashGuideEditallPage extends LitElement {
@@ -122,7 +124,8 @@ export class ChromedashGuideEditallPage extends LitElement {
   }
 
   getNextPage() {
-    return this.nextPage || `/guide/edit/${this.featureId}`;
+    return this.nextPage || this.feature.is_enterprise_feature ?
+    `/feature/${this.featureId}` : `/guide/edit/${this.featureId}`;
   }
 
   renderSubheader() {
@@ -174,9 +177,10 @@ export class ChromedashGuideEditallPage extends LitElement {
         </chromedash-form-field>
       `;
     });
-
+    const id = `${STAGE_SHORT_NAMES[feStage.stage_type] || 'metadata'}${this.sameTypeRendered}`
+      .toLowerCase();
     return html`
-    <h3>${sectionName}</h3>
+    <h3 id="${id}">${sectionName}</h3>
     <section class="flat_form">
       ${formFieldEls}
     </section>
@@ -243,6 +247,14 @@ export class ChromedashGuideEditallPage extends LitElement {
     return stageIds.join(',');
   }
 
+  renderAddStageButton() {
+    return html`
+    <sl-button size="small" @click="${
+        () => openAddStageDialog(this.feature.id, this.feature.feature_type_int)}">
+      Add stage
+    </sl-button>`;
+  }
+
   renderForm() {
     const formattedFeature = formatFeatureForEdit(this.feature);
     const stageIds = this.getAllStageIds();
@@ -256,6 +268,7 @@ export class ChromedashGuideEditallPage extends LitElement {
         <chromedash-form-table ${ref(this.registerHandlers)}>
           ${formsToRender}
         </chromedash-form-table>
+        ${this.renderAddStageButton()}
 
         <section class="final_buttons">
           <input class="button" type="submit" value="Submit">
diff --git a/client-src/elements/chromedash-guide-metadata.js b/client-src/elements/chromedash-guide-metadata.js
index 67f375f4..602b6d33 100644
--- a/client-src/elements/chromedash-guide-metadata.js
+++ b/client-src/elements/chromedash-guide-metadata.js
@@ -1,6 +1,6 @@
 import {LitElement, css, html, nothing} from 'lit';
 import {ref} from 'lit/directives/ref.js';
-import {autolink, flattenSections} from './utils.js';
+import {autolink, flattenSections, renderHTMLIf} from './utils.js';
 import './chromedash-form-table';
 import './chromedash-form-field';
 import {ENTERPRISE_FEATURE_CATEGORIES_DISPLAYNAME} from './form-field-enums';
@@ -195,10 +195,12 @@ export class ChromedashGuideMetadata extends LitElement {
               <td>${this.feature.feature_type}</td>
             </tr>
 
-            <tr>
-              <th>Process stage</th>
-              <td>${this.feature.intent_stage}</td>
-            </tr>
+            ${renderHTMLIf(!this.feature.is_enterprise_feature, html`
+              <tr>
+                <th>Process stage</th>
+                <td>${this.feature.intent_stage}</td>
+              </tr>`,
+            )}
 
             ${this.feature.tags && !this.feature.is_enterprise_feature ? html`
               <tr>
diff --git a/client-src/elements/chromedash-guide-metadata_test.js b/client-src/elements/chromedash-guide-metadata_test.js
index 085a1a94..d6a505fd 100644
--- a/client-src/elements/chromedash-guide-metadata_test.js
+++ b/client-src/elements/chromedash-guide-metadata_test.js
@@ -111,7 +111,7 @@ describe('chromedash-guide-metadata', () => {
     // feature feature type is listed
     assert.include(metadataDiv.innerHTML, 'fake feature type');
     // feature intent stage is listed
-    assert.include(metadataDiv.innerHTML, 'fake intent stage');
+    assert.notInclude(metadataDiv.innerHTML, 'fake intent stage');
     // feature tag is listed
     assert.notInclude(metadataDiv.innerHTML, 'tag_one');
     // feature status is listed
diff --git a/client-src/elements/chromedash-guide-stage-page.js b/client-src/elements/chromedash-guide-stage-page.js
index b624b058..ac6c0703 100644
--- a/client-src/elements/chromedash-guide-stage-page.js
+++ b/client-src/elements/chromedash-guide-stage-page.js
@@ -226,16 +226,18 @@ export class ChromedashGuideStagePage extends LitElement {
   }
 
   renderSections(formattedFeature, stageSections) {
-    const formSections = [
-      html`
-      <section class="stage_form">
-        <chromedash-form-field
-          name="set_stage"
-          value=${this.isActiveStage}
-          ?disabled=${this.isActiveStage}>
-        </chromedash-form-field>
-      </section>`,
-    ];
+    const formSections = [];
+    if (!formattedFeature.is_enterprise_feature) {
+      formSections.push(
+        html`
+        <section class="stage_form">
+          <chromedash-form-field
+            name="set_stage"
+            value=${this.isActiveStage}
+            ?disabled=${this.isActiveStage}>
+          </chromedash-form-field>
+        </section>`);
+    }
 
     stageSections.forEach(section => {
       if (section.isImplementationSection) {
diff --git a/client-src/elements/chromedash-process-overview.js b/client-src/elements/chromedash-process-overview.js
index dd3328d0..e7acd93c 100644
--- a/client-src/elements/chromedash-process-overview.js
+++ b/client-src/elements/chromedash-process-overview.js
@@ -245,7 +245,9 @@ export class ChromedashProcessOverview extends LitElement {
     // Choose button based on active stage.
     const buttonHref = `/guide/stage/${featureId}/${processStage.outgoing_stage}/${feStage.id}`;
     let button = html`<a href="${buttonHref}">Edit</a>`;
-    if (isActive) {
+    if (this.feature.is_enterprise_feature) {
+      button = html`<a href="${buttonHref}" class="button primary">Edit</a>`;
+    } else if (isActive) {
       button = html`<a href="${buttonHref}" class="button primary">Update</a>`;
     } else if (this.isPriorStage(processStage)) {
       button = html`<a href="${buttonHref}">Revisit</a>`;
diff --git a/client-src/elements/form-definition.js b/client-src/elements/form-definition.js
index 2e4243f2..b8068894 100644
--- a/client-src/elements/form-definition.js
+++ b/client-src/elements/form-definition.js
@@ -689,7 +689,6 @@ export const CREATEABLE_STAGES = {
   ],
   [FEATURE_TYPES.FEATURE_TYPE_ENTERPRISE_ID[0]]: [
     STAGE_ENT_ROLLOUT,
-    STAGE_ENT_SHIPPED,
   ],
 };
 
diff --git a/client-src/elements/utils.js b/client-src/elements/utils.js
index 9675f66f..528f4f42 100644
--- a/client-src/elements/utils.js
+++ b/client-src/elements/utils.js
@@ -1,6 +1,7 @@
 // This file contains helper functions for our elements.
 
 import {markupAutolinks} from './autolink.js';
+import {nothing} from 'lit';
 
 let toastEl;
 
@@ -85,6 +86,10 @@ export function setupScrollToHash(pageElement) {
           fieldRow.scrollIntoView({
             block: 'center', behavior: 'smooth',
           });
+        } else {
+          el.scrollIntoView({
+            behavior: 'smooth',
+          });
         }
       }
     }
@@ -101,3 +106,8 @@ export function setupScrollToHash(pageElement) {
     scrollToElement(hash);
   }
 }
+
+/* Returns a html template if the condition is true, otherwise returns an empty html */
+export function renderHTMLIf(condition, originalHTML) {
+  return condition ? originalHTML : nothing;
+}
diff --git a/pages/guide.py b/pages/guide.py
index 9737f85a..ca51ed69 100644
--- a/pages/guide.py
+++ b/pages/guide.py
@@ -190,7 +190,7 @@ class EnterpriseFeatureCreateHandler(FeatureCreateHandler):
 
     # TODO(jrobbins): Make this be /feature/ID after ability to edit
     # from the feature detail page is complete.
-    redirect_url = '/guide/edit/' + str(key.integer_id())
+    redirect_url = '/guide/editall/' + str(key.integer_id()) + '#rollout1'
     return self.redirect(redirect_url)