Revert "Revert "Add Start Review button to Change View""

This reverts commit 8a3995e7b4.

Reason for revert: Rolling forward as planned.

Change-Id: I3169377c0369a74b0cf67218044858d83765c239

polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js

@@ -310,7 +310,7 @@ class GrChangeView extends mixinBehaviors( [
       _replyButtonLabel: {
         type: String,
         value: 'Reply',
-        computed: '_computeReplyButtonLabel(_diffDrafts.*, _canStartReview)',
+        computed: '_computeReplyButtonLabel(_diffDrafts.*)',
       },
       _selectedPatchSet: String,
       _shownFileCount: Number,
@@ -922,6 +922,23 @@ class GrChangeView extends mixinBehaviors( [
     this._openReplyDialog(this.$.replyDialog.FocusTarget.ANY);
   }
 
+  _handleReadyTap(e) {
+    e.preventDefault();
+    const button = e && e.target;
+    if (button) {
+      button.loading = true;
+    }
+    return this.$.restAPI.startReview(this._changeNum)
+        .then(result => {
+          this._reload(result);
+        })
+        .finally(() => {
+          if (button) {
+            button.loading = false;
+          }
+        });
+  }
+
   _handleOpenDiffPrefs() {
     this.$.fileList.openDiffPrefs();
   }
@@ -1361,16 +1378,12 @@ class GrChangeView extends mixinBehaviors( [
     return result;
   }
 
-  _computeReplyButtonLabel(changeRecord, canStartReview) {
+  _computeReplyButtonLabel(changeRecord) {
     // Polymer 2: check for undefined
-    if ([changeRecord, canStartReview].some(arg => arg === undefined)) {
+    if ([changeRecord].some(arg => arg === undefined)) {
       return 'Reply';
     }
 
-    if (canStartReview) {
-      return 'Start review';
-    }
-
     const drafts = (changeRecord && changeRecord.base) || {};
     const draftCount = Object.keys(drafts)
         .reduce((count, file) => count + drafts[file].length, 0);
@@ -1879,7 +1892,7 @@ class GrChangeView extends mixinBehaviors( [
 
   _computeCanStartReview(change) {
     return !!(change.actions && change.actions.ready &&
-        change.actions.ready.enabled);
+      change.actions.ready.enabled);
   }
 
   _computeReplyDisabled() { return false; }

polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js

@@ -446,11 +446,20 @@ export const htmlTemplate = html`
                   title="[[createTitle(Shortcut.OPEN_REPLY_DIALOG,
                         ShortcutSection.ACTIONS)]]"
                   hidden$="[[!_loggedIn]]"
-                  primary=""
+                  primary$="[[!_canStartReview]]"
                   disabled="[[_replyDisabled]]"
                   on-click="_handleReplyTap"
                   >[[_replyButtonLabel]]</gr-button
                 >
+                <gr-button
+                  id="startReviewBtn"
+                  class="startReview"
+                  title="Switches change state from 'Work in Progress' to 'Active'."
+                  hidden="[[!_canStartReview]]"
+                  primary$="[[_canStartReview]]"
+                  on-click="_handleReadyTap"
+                  >Start review</gr-button
+                >
               </div>
               <div id="commitMessage" class="commitMessage">
                 <gr-editable-content

polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html

@@ -1062,7 +1062,7 @@ suite('gr-change-view tests', () => {
     const getLabel = element._computeReplyButtonLabel;
 
     assert.equal(getLabel(null, false), 'Reply');
-    assert.equal(getLabel(null, true), 'Start review');
+    assert.equal(getLabel(null, true), 'Reply');
 
     const changeRecord = {base: null};
     assert.equal(getLabel(changeRecord, false), 'Reply');
@@ -1078,9 +1078,7 @@ suite('gr-change-view tests', () => {
   });
 
   test('start review button when owner of WIP change', () => {
-    assert.equal(
-        element._computeReplyButtonLabel(null, true),
-        'Start review');
+    assert.equal(element.$.startReviewBtn.innerHTML, 'Start review');
   });
 
   test('comment events properly update diff drafts', () => {

polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js

@@ -250,7 +250,7 @@ class GrReplyDialog extends mixinBehaviors( [
       },
       _sendDisabled: {
         type: Boolean,
-        computed: '_computeSendButtonDisabled(_sendButtonLabel, ' +
+        computed: '_computeSendButtonDisabled(canBeStarted, ' +
           'draftCommentThreads, draft, _reviewersMutated, _labelsChanged, ' +
           '_includeComments, disabled, _commentEditing)',
         observer: '_sendDisabledChanged',
@@ -871,7 +871,8 @@ class GrReplyDialog extends mixinBehaviors( [
   }
 
   _computeSendButtonLabel(canBeStarted) {
-    return canBeStarted ? ButtonLabels.START_REVIEW : ButtonLabels.SEND;
+    return canBeStarted ? ButtonLabels.SEND + ' and ' +
+        ButtonLabels.START_REVIEW : ButtonLabels.SEND;
   }
 
   _computeSendButtonTooltip(canBeStarted) {
@@ -883,11 +884,11 @@ class GrReplyDialog extends mixinBehaviors( [
   }
 
   _computeSendButtonDisabled(
-      buttonLabel, draftCommentThreads, text, reviewersMutated,
+      canBeStarted, draftCommentThreads, text, reviewersMutated,
       labelsChanged, includeComments, disabled, commentEditing) {
     // Polymer 2: check for undefined
     if ([
-      buttonLabel,
+      canBeStarted,
       draftCommentThreads,
       text,
       reviewersMutated,
@@ -900,7 +901,7 @@ class GrReplyDialog extends mixinBehaviors( [
     }
 
     if (commentEditing || disabled) { return true; }
-    if (buttonLabel === ButtonLabels.START_REVIEW) { return false; }
+    if (canBeStarted === true) { return false; }
     const hasDrafts = includeComments && draftCommentThreads.length;
     return !hasDrafts && !text.length && !reviewersMutated && !labelsChanged;
   }

polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html

@@ -955,7 +955,7 @@ suite('gr-reply-dialog tests', () => {
         'Send');
     assert.equal(
         element._computeSendButtonLabel(true),
-        'Start review');
+        'Send and Start review');
   });
 
   test('_handle400Error reviewrs and CCs', done => {
@@ -1138,10 +1138,11 @@ suite('gr-reply-dialog tests', () => {
     });
   });
 
-  test('_computeSendButtonDisabled', () => {
+  test('_computeSendButtonDisabled_canBeStarted', () => {
     const fn = element._computeSendButtonDisabled.bind(element);
+    // Mock canBeStarted
     assert.isFalse(fn(
-        /* buttonLabel= */ 'Start review',
+        /* canBeStarted= */ true,
         /* draftCommentThreads= */ [],
         /* text= */ '',
         /* reviewersMutated= */ false,
@@ -1150,8 +1151,13 @@ suite('gr-reply-dialog tests', () => {
         /* disabled= */ false,
         /* commentEditing= */ false
     ));
+  });
+
+  test('_computeSendButtonDisabled_allFalse', () => {
+    const fn = element._computeSendButtonDisabled.bind(element);
+    // Mock everything false
     assert.isTrue(fn(
-        /* buttonLabel= */ 'Send',
+        /* canBeStarted= */ false,
         /* draftCommentThreads= */ [],
         /* text= */ '',
         /* reviewersMutated= */ false,
@@ -1160,9 +1166,13 @@ suite('gr-reply-dialog tests', () => {
         /* disabled= */ false,
         /* commentEditing= */ false
     ));
-    // Mock nonempty comment draft array, with seding comments.
+  });
+
+  test('_computeSendButtonDisabled_draftCommentsSend', () => {
+    const fn = element._computeSendButtonDisabled.bind(element);
+    // Mock nonempty comment draft array, with sending comments.
     assert.isFalse(fn(
-        /* buttonLabel= */ 'Send',
+        /* canBeStarted= */ false,
         /* draftCommentThreads= */ [{comments: [{__draft: true}]}],
         /* text= */ '',
         /* reviewersMutated= */ false,
@@ -1171,9 +1181,13 @@ suite('gr-reply-dialog tests', () => {
         /* disabled= */ false,
         /* commentEditing= */ false
     ));
-    // Mock nonempty comment draft array, without seding comments.
+  });
+
+  test('_computeSendButtonDisabled_draftCommentsDoNotSend', () => {
+    const fn = element._computeSendButtonDisabled.bind(element);
+    // Mock nonempty comment draft array, without sending comments.
     assert.isTrue(fn(
-        /* buttonLabel= */ 'Send',
+        /* canBeStarted= */ false,
         /* draftCommentThreads= */ [{comments: [{__draft: true}]}],
         /* text= */ '',
         /* reviewersMutated= */ false,
@@ -1182,9 +1196,13 @@ suite('gr-reply-dialog tests', () => {
         /* disabled= */ false,
         /* commentEditing= */ false
     ));
+  });
+
+  test('_computeSendButtonDisabled_changeMessage', () => {
+    const fn = element._computeSendButtonDisabled.bind(element);
     // Mock nonempty change message.
     assert.isFalse(fn(
-        /* buttonLabel= */ 'Send',
+        /* canBeStarted= */ false,
         /* draftCommentThreads= */ {},
         /* text= */ 'test',
         /* reviewersMutated= */ false,
@@ -1193,9 +1211,13 @@ suite('gr-reply-dialog tests', () => {
         /* disabled= */ false,
         /* commentEditing= */ false
     ));
+  });
+
+  test('_computeSendButtonDisabled_reviewersChanged', () => {
+    const fn = element._computeSendButtonDisabled.bind(element);
     // Mock reviewers mutated.
     assert.isFalse(fn(
-        /* buttonLabel= */ 'Send',
+        /* canBeStarted= */ false,
         /* draftCommentThreads= */ {},
         /* text= */ '',
         /* reviewersMutated= */ true,
@@ -1204,9 +1226,13 @@ suite('gr-reply-dialog tests', () => {
         /* disabled= */ false,
         /* commentEditing= */ false
     ));
+  });
+
+  test('_computeSendButtonDisabled_labelsChanged', () => {
+    const fn = element._computeSendButtonDisabled.bind(element);
     // Mock labels changed.
     assert.isFalse(fn(
-        /* buttonLabel= */ 'Send',
+        /* canBeStarted= */ false,
         /* draftCommentThreads= */ {},
         /* text= */ '',
         /* reviewersMutated= */ false,
@@ -1215,9 +1241,13 @@ suite('gr-reply-dialog tests', () => {
         /* disabled= */ false,
         /* commentEditing= */ false
     ));
+  });
+
+  test('_computeSendButtonDisabled_dialogDisabled', () => {
+    const fn = element._computeSendButtonDisabled.bind(element);
     // Whole dialog is disabled.
     assert.isTrue(fn(
-        /* buttonLabel= */ 'Send',
+        /* canBeStarted= */ false,
         /* draftCommentThreads= */ {},
         /* text= */ '',
         /* reviewersMutated= */ false,