Merge "Add option to not publish drafts in reply dialog"

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

@@ -263,12 +263,18 @@ limitations under the License.
         </template>
       </section>
       <section class="draftsContainer" hidden$="[[_computeHideDraftList(diffDrafts)]]">
-        <h3>[[_computeDraftsTitle(diffDrafts)]]</h3>
+        <div class="includeComments">
+          <input type="checkbox" id="includeComments"
+              checked="{{_includeComments::change}}">
+          <label for="includeComments">Publish [[_computeDraftsTitle(diffDrafts)]]</label>
+        </div>
         <gr-comment-list
+            id="commentList"
             comments="[[diffDrafts]]"
             change-num="[[change._number]]"
             project-config="[[projectConfig]]"
-            patch-num="[[patchNum]]"></gr-comment-list>
+            patch-num="[[patchNum]]"
+            hidden$="[[!_includeComments]]"></gr-comment-list>
       </section>
       <section class="actionsContainer">
         <gr-button primary class="action send" on-tap="_sendTapHandler">Send</gr-button>

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

@@ -90,6 +90,10 @@
       },
       _owner: Object,
       _pendingConfirmationDetails: Object,
+      _includeComments: {
+        type: Boolean,
+        value: true,
+      },
       _reviewers: Array,
       _reviewerPendingConfirmation: {
         type: Object,
@@ -246,9 +250,9 @@
       return {reviewer: reviewerId, confirmed: confirmed};
     },
 
-    send: function() {
+    send: function(includeComments) {
       var obj = {
-        drafts: 'PUBLISH_ALL_REVISIONS',
+        drafts: includeComments ? 'PUBLISH_ALL_REVISIONS' : 'KEEP',
         labels: {},
       };
 
@@ -304,6 +308,7 @@
         }
         this.disabled = false;
         this.draft = '';
+        this._includeComments = true;
         this.fire('send', null, {bubbles: false});
         return accountAdditions;
       }.bind(this)).catch(function(err) {
@@ -525,8 +530,8 @@
 
     _sendTapHandler: function(e) {
       e.preventDefault();
-      this.send().then(function(keep) {
+      this.send(this._includeComments).then(function(keepReviewers) {
-        this._purgeReviewersPendingRemove(false, keep);
+        this._purgeReviewersPendingRemove(false, keepReviewers);
       }.bind(this));
     },
 

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

@@ -126,6 +126,69 @@ limitations under the License.
       MockInteractions.tap(element.$$('.cancel'));
     });
 
+    test('default to publishing drafts with reply', function(done) {
+      // Async tick is needed because iron-selector content is distributed and
+      // distributed content requires an observer to be set up.
+      // Note: Double flush seems to be needed in Safari. {@see Issue 4963}.
+      flush(function() {
+        flush(function() {
+          element.draft = 'I wholeheartedly disapprove';
+
+          var saveReviewStub = sandbox.stub(element, '_saveReview',
+              function(review) {
+            assert.deepEqual(review, {
+              drafts: 'PUBLISH_ALL_REVISIONS',
+              labels: {},
+              message: 'I wholeheartedly disapprove',
+              reviewers: [],
+            });
+            assert.isFalse(element.$.commentList.hidden);
+            done();
+            return Promise.resolve({ok: true});
+          });
+
+          // This is needed on non-Blink engines most likely due to the ways in
+          // which the dom-repeat elements are stamped.
+          flush(function() {
+            MockInteractions.tap(element.$$('.send'));
+          });
+        });
+      });
+    });
+
+    test('keep drafts with reply', function(done) {
+      MockInteractions.tap(element.$$('#includeComments'));
+      assert.equal(element._includeComments, false);
+
+      // Async tick is needed because iron-selector content is distributed and
+      // distributed content requires an observer to be set up.
+      // Note: Double flush seems to be needed in Safari. {@see Issue 4963}.
+      flush(function() {
+        flush(function() {
+          element.draft = 'I wholeheartedly disapprove';
+
+          var saveReviewStub = sandbox.stub(element, '_saveReview',
+              function(review) {
+            assert.deepEqual(review, {
+              drafts: 'KEEP',
+              labels: {},
+              message: 'I wholeheartedly disapprove',
+              reviewers: [],
+            });
+            assert.isTrue(element.$.commentList.hidden);
+            done();
+            return Promise.resolve({ok: true});
+          });
+
+          // This is needed on non-Blink engines most likely due to the ways in
+          // which the dom-repeat elements are stamped.
+          flush(function() {
+            MockInteractions.tap(element.$$('.send'));
+          });
+        });
+      });
+    });
+
     test('label picker', function(done) {
       element.revisions = {};
       element.patchNum = '';
@@ -147,7 +210,7 @@ limitations under the License.
               'iron-selector[data-label="Verified"] > ' +
               'gr-button[data-value="-1"]'));
 
-          var saveReviewStub = sinon.stub(element, '_saveReview',
+          var saveReviewStub = sandbox.stub(element, '_saveReview',
               function(review) {
             assert.deepEqual(review, {
               drafts: 'PUBLISH_ALL_REVISIONS',
@@ -165,7 +228,6 @@ limitations under the License.
             assert.isFalse(element.disabled,
                 'Element should be enabled when done sending reply.');
             assert.equal(element.draft.length, 0);
-            saveReviewStub.restore();
             done();
           });
 
@@ -440,14 +502,13 @@ limitations under the License.
 
     test('only send labels that have changed', function(done) {
       flush(function() {
-        var saveReviewStub = sinon.stub(element, '_saveReview',
+        var saveReviewStub = sandbox.stub(element, '_saveReview',
             function(review) {
           assert.deepEqual(review.labels, {Verified: -1});
           return Promise.resolve({ok: true});
         });
 
         element.addEventListener('send', function() {
-          saveReviewStub.restore();
           done();
         });
         // Without wrapping this test in flush(), the below two calls to