Add derived status chips 'active' and 'ready to submit'

If there are no other existing statuses, add a derived status chip, either 'active' or 'ready to submit' depending on if there are any unmet approvals or not. Bug: Issue 7951, Issue 7947 Change-Id: I509ee53374c72db8cad65b19495e59a4c594ec6e
2017-12-22 12:57:11 -08:00
parent c4d42c43c8
commit 020e1be456
10 changed files with 115 additions and 43 deletions
--- a/polygerrit-ui/app/behaviors/rest-client-behavior/rest-client-behavior.html
+++ b/polygerrit-ui/app/behaviors/rest-client-behavior/rest-client-behavior.html
@@ -119,7 +119,13 @@ limitations under the License.
      return status === this.ChangeStatus.NEW;
    },

-    changeStatuses(change) {
+    /**
+     * @param {!Object} change
+     * @param {!Object=} opt_options
+     *
+     * @return {!Array}
+     */
+    changeStatuses(change, opt_options) {
      const states = [];
      if (change.status === this.ChangeStatus.MERGED) {
        states.push('Merged');
@@ -131,9 +137,25 @@ limitations under the License.
      }
      if (change.work_in_progress) { states.push('WIP'); }
      if (change.is_private) { states.push('Private'); }
+
+      // If there are any pre-defined statuses, only return those. Otherwise,
+      // will determine the derived status.
+      if (states.length || !opt_options) { return states; }
+
+      // If no missing requirements, either active or ready to submit.
+      if (opt_options.readyToSubmit) {
+        states.push('Ready to submit');
+      } else {
+        // Otherwise it is active.
+        states.push('Active');
+      }
      return states;
    },

+    /**
+     * @param {!Object} change
+     * @return {String}
+     */
    changeStatusString(change) {
      return this.changeStatuses(change).join(', ');
    },
--- a/polygerrit-ui/app/behaviors/rest-client-behavior/rest-client-behavior_test.html
+++ b/polygerrit-ui/app/behaviors/rest-client-behavior/rest-client-behavior_test.html
@@ -87,10 +87,20 @@ limitations under the License.
        labels: {},
        mergeable: true,
      };
-      const statuses = element.changeStatuses(change);
-      const statusString = element.changeStatusString(change);
+      let statuses = element.changeStatuses(change);
+      let statusString = element.changeStatusString(change);
      assert.deepEqual(statuses, []);
      assert.equal(statusString, '');
+
+      statuses = element.changeStatuses(change,
+          {readyToSubmit: false, includeDerived: true});
+      assert.deepEqual(statuses, ['Active']);
+
+      // With no missing labels
+      statuses = element.changeStatuses(change,
+          {readyToSubmit: true, includeDerived: true});
+      statusString = element.changeStatusString(change);
+      assert.deepEqual(statuses, ['Ready to submit']);
    });

    test('Merge conflict', () => {
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
@@ -313,17 +313,17 @@ limitations under the License.
            <div hidden$="[[!_isWip]]">
              Work in progress
            </div>
-            <div hidden$="[[!_showMissingLabels(change.labels)]]">
-              [[_computeMissingLabelsHeader(change.labels)]]
+            <div hidden$="[[!_showMissingLabels(missingLabels)]]">
+              [[_computeMissingLabelsHeader(missingLabels)]]
              <ul id="missingLabels">
                <template
                    is="dom-repeat"
-                    items="[[_computeMissingLabels(change.labels)]]">
+                    items="[[missingLabels]]">
                  <li>[[item]]</li>
                </template>
              </ul>
            </div>
-            <div hidden$="[[_showMissingRequirements(change.labels, _isWip)]]">
+            <div hidden$="[[_showMissingRequirements(missingLabels, _isWip)]]">
              Ready to submit
            </div>
          </span>
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
@@ -40,6 +40,7 @@
      /** @type {?} */
      revision: Object,
      commitInfo: Object,
+      missingLabels: Array,
      mutable: Boolean,
      /**
       * @type {{ note_db_enabled: string }}
@@ -311,29 +312,17 @@
      return isNewChange && hasLabels;
    },

-    _computeMissingLabels(labels) {
-      const missingLabels = [];
-      for (const label in labels) {
-        if (!labels.hasOwnProperty(label)) { continue; }
-        const obj = labels[label];
-        if (!obj.optional && !obj.approved) {
-          missingLabels.push(label);
-        }
-      }
-      return missingLabels;
-    },
-
-    _computeMissingLabelsHeader(labels) {
+    _computeMissingLabelsHeader(missingLabels) {
      return 'Needs label' +
-          (this._computeMissingLabels(labels).length > 1 ? 's' : '') + ':';
+          (missingLabels.length > 1 ? 's' : '') + ':';
    },

-    _showMissingLabels(labels) {
-      return !!this._computeMissingLabels(labels).length;
+    _showMissingLabels(missingLabels) {
+      return !!missingLabels.length;
    },

-    _showMissingRequirements(labels, workInProgress) {
-      return workInProgress || this._showMissingLabels(labels);
+    _showMissingRequirements(missingLabels, workInProgress) {
+      return workInProgress || this._showMissingLabels(missingLabels);
    },

    _computeProjectURL(project) {
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
@@ -103,21 +103,12 @@ limitations under the License.
    });

    test('show missing labels', () => {
-      let labels = {};
-      assert.isFalse(element._showMissingLabels(labels));
-      labels = {test: {}};
-      assert.isTrue(element._showMissingLabels(labels));
-      assert.deepEqual(element._computeMissingLabels(labels), ['test']);
-      labels.test.approved = true;
-      assert.isFalse(element._showMissingLabels(labels));
-      labels.test.approved = false;
-      labels.test.optional = true;
-      assert.isFalse(element._showMissingLabels(labels));
-      labels.test.optional = false;
-      labels.test2 = {};
-      assert.isTrue(element._showMissingLabels(labels));
-      assert.deepEqual(element._computeMissingLabels(labels),
-          ['test', 'test2']);
+      let missingLabels = [];
+      assert.isFalse(element._showMissingLabels(missingLabels));
+      missingLabels = ['test'];
+      assert.isTrue(element._showMissingLabels(missingLabels));
+      missingLabels.push('test2');
+      assert.isTrue(element._showMissingLabels(missingLabels));
    });

    test('weblinks use Gerrit.Nav interface', () => {
@@ -178,6 +169,7 @@ limitations under the License.

    test('determines whether to show "Ready to Submit" label', () => {
      const showMissingSpy = sandbox.spy(element, '_showMissingRequirements');
+      element.missingLabels = ['bojack'];
      element.change = {status: 'NEW', submit_type: 'CHERRY_PICK', labels: {
        test: {
          all: [{_account_id: 1, name: 'bojack', value: 1}],
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -364,6 +364,7 @@ limitations under the License.
              revision="[[_currentRevision]]"
              commit-info="[[_commitInfo]]"
              server-config="[[_serverConfig]]"
+              missing-labels="[[_missingLabels]]"
              mutable="[[_loggedIn]]"
              on-show-reply-dialog="_handleShowReplyDialog">
          </gr-change-metadata>
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -174,6 +174,11 @@
      },
      _loading: Boolean,
      /** @type {?} */
+      _missingLabels: {
+        type: Array,
+        computed: '_computeMissingLabels(_change.labels)',
+      },
+      /** @type {?} */
      _projectConfig: Object,
      _rebaseOnCurrent: Boolean,
      _replyButtonLabel: {
@@ -198,7 +203,7 @@
      },
      _changeStatuses: {
        type: String,
-        computed: '_computeChangeStatusChips(_change)',
+        computed: '_computeChangeStatusChips(_change, _missingLabels)',
      },
      _commitCollapsed: {
        type: Boolean,
@@ -340,8 +345,28 @@
      this._editingCommitMessage = false;
    },

-    _computeChangeStatusChips(change) {
-      return this.changeStatuses(change);
+    _computeMissingLabels(labels) {
+      const missingLabels = [];
+      for (const label in labels) {
+        if (!labels.hasOwnProperty(label)) { continue; }
+        const obj = labels[label];
+        if (!obj.optional && !obj.approved) {
+          missingLabels.push(label);
+        }
+      }
+      return missingLabels;
+    },
+
+    _readyToSubmit(missingLabels) {
+      return missingLabels.length === 0;
+    },
+
+    _computeChangeStatusChips(change, missingLabels) {
+      const options = {
+        readyToSubmit: this._readyToSubmit(missingLabels),
+        includeDerived: true,
+      };
+      return this.changeStatuses(change, options);
    },

    _computeHideEditCommitMessage(loggedIn, editing, change) {
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -347,6 +347,22 @@ limitations under the License.
      assert.equal(statusChips.length, 2);
    });

+    test('_computeMissingLabels', () => {
+      let labels = {};
+      assert.equal(element._computeMissingLabels(labels).length, 0);
+      labels = {test: {}};
+      assert.deepEqual(element._computeMissingLabels(labels), ['test']);
+      labels.test.approved = true;
+      assert.equal(element._computeMissingLabels(labels).length, 0);
+      labels.test.approved = false;
+      labels.test.optional = true;
+      assert.equal(element._computeMissingLabels(labels).length, 0);
+      labels.test.optional = false;
+      labels.test2 = {};
+      assert.deepEqual(element._computeMissingLabels(labels),
+          ['test', 'test2']);
+    });
+
    test('diff preferences open when open-diff-prefs is fired', () => {
      const overlayOpenStub = sandbox.stub(element.$.fileList,
          'openDiffPrefs');
--- a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.html
+++ b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.html
@@ -50,6 +50,9 @@ limitations under the License.
      :host(.active) .chip {
        background-color: #29b6f6;
      }
+      :host(.ready-to-submit) .chip {
+        background-color: #e10ca3;
+      }
      :host(.custom) .chip {
        background-color: #825cc2;
      }
--- a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status_test.html
@@ -80,6 +80,20 @@ limitations under the License.
      assert.isTrue(element.classList.contains('private'));
    });

+    test('active', () => {
+      element.status = 'Active';
+      assert.equal(element.$$('.chip').innerText, element.status);
+      assert.isUndefined(element.tooltipText);
+      assert.isTrue(element.classList.contains('active'));
+    });
+
+    test('ready to submit', () => {
+      element.status = 'Ready to submit';
+      assert.equal(element.$$('.chip').innerText, element.status);
+      assert.isUndefined(element.tooltipText);
+      assert.isTrue(element.classList.contains('ready-to-submit'));
+    });
+
    test('updating status removes the previous class', () => {
      element.status = 'Private';
      assert.isTrue(element.classList.contains('private'));