From 22a86edc7f2376e3632588ab08ee4b6553ca3529 Mon Sep 17 00:00:00 2001
From: Dhruv Srivastava <dhruvsri@google.com>
Date: Tue, 17 Dec 2019 16:46:18 +0100
Subject: [PATCH] Show revert change and revert submission options instead of 2
 buttons

Change-Id: I5b7d6b6b5335637ffeff599ec62a16792fe46604
---
 .../gr-change-actions/gr-change-actions.html  |   3 +
 .../gr-change-actions/gr-change-actions.js    |  49 ++---
 .../gr-change-actions_test.html               | 172 ++++++++++++++----
 .../gr-confirm-revert-dialog.html             |  49 ++++-
 .../gr-confirm-revert-dialog.js               | 117 +++++++++++-
 .../gr-confirm-revert-dialog_test.html        |  11 +-
 .../gr-confirm-revert-submission-dialog.js    |   6 +-
 ...confirm-revert-submission-dialog_test.html |  10 +-
 8 files changed, 341 insertions(+), 76 deletions(-)

diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
index f4bd6a6ee5..283bd743c5 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
@@ -205,6 +205,9 @@ limitations under the License.
       <gr-confirm-revert-dialog id="confirmRevertDialog"
           class="confirmDialog"
           on-confirm="_handleRevertDialogConfirm"
+          commit-message="[[commitMessage]]"
+          change="[[change]]"
+          changes="[[_revertChanges]]"
           on-cancel="_handleConfirmDialogCancel"
           hidden></gr-confirm-revert-dialog>
       <gr-confirm-revert-submission-dialog id="confirmRevertSubmissionDialog"
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index 2c4ca82caa..a4ed899375 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -192,6 +192,11 @@
   const AWAIT_CHANGE_ATTEMPTS = 5;
   const AWAIT_CHANGE_TIMEOUT_MS = 1000;
 
+  const REVERT_TYPES = {
+    REVERT_SINGLE_CHANGE: 1,
+    REVERT_SUBMISSION: 2,
+  };
+
   /**
    * @appliesMixin Gerrit.FireMixin
    * @appliesMixin Gerrit.PatchSetMixin
@@ -421,6 +426,7 @@
           type: Boolean,
           value: true,
         },
+        _revertChanges: Array,
       };
     }
 
@@ -915,16 +921,13 @@
       return null;
     }
 
-    _modifyRevertMsg() {
-      return this.$.jsAPI.modifyRevertMsg(this.change,
-          this.$.confirmRevertDialog.message, this.commitMessage);
-    }
-
     showRevertDialog() {
-      this.$.confirmRevertDialog.populateRevertMessage(
-          this.commitMessage, this.change.current_revision);
-      this.$.confirmRevertDialog.message = this._modifyRevertMsg();
-      this._showActionDialog(this.$.confirmRevertDialog);
+      const query = 'submissionid:' + this.change.submission_id;
+      this.$.restAPI.getChanges('', query)
+          .then(changes => {
+            this._revertChanges = changes;
+            this._showActionDialog(this.$.confirmRevertDialog);
+          });
     }
 
     showRevertSubmissionDialog() {
@@ -932,7 +935,7 @@
       this.$.restAPI.getChanges('', query)
           .then(changes => {
             this.$.confirmRevertSubmissionDialog.
-                populateRevertSubmissionMessage(
+                _populateRevertSubmissionMessage(
                     this.commitMessage, this.change, changes);
             this._showActionDialog(this.$.confirmRevertSubmissionDialog);
           });
@@ -1143,20 +1146,24 @@
       );
     }
 
-    _handleRevertDialogConfirm() {
+    _handleRevertDialogConfirm(e) {
+      const revertType = e.detail.revertType;
+      const message = e.detail.message;
       const el = this.$.confirmRevertDialog;
       this.$.overlay.close();
       el.hidden = true;
-      this._fireAction('/revert', this.actions.revert, false,
-          {message: el.message});
-    }
-
-    _handleRevertSubmissionDialogConfirm() {
-      const el = this.$.confirmRevertSubmissionDialog;
-      this.$.overlay.close();
-      el.hidden = true;
-      this._fireAction('/revert_submission', this.actions.revert_submission,
-          false, {message: el.message});
+      switch (revertType) {
+        case REVERT_TYPES.REVERT_SINGLE_CHANGE:
+          this._fireAction('/revert', this.actions.revert, false,
+              {message});
+          break;
+        case REVERT_TYPES.REVERT_SUBMISSION:
+          this._fireAction('/revert_submission', this.actions.revert_submission,
+              false, {message});
+          break;
+        default:
+          console.error('invalid revert type');
+      }
     }
 
     _handleAbandonDialogConfirm() {
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
index ec5c5a5485..e4ea218390 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -37,6 +37,7 @@ limitations under the License.
 </test-fixture>
 
 <script>
+  // TODO(dhruvsri): remove use of _populateRevertMessage as it's private
   suite('gr-change-actions tests', () => {
     let element;
     let sandbox;
@@ -795,12 +796,12 @@ limitations under the License.
     });
 
     suite('revert change', () => {
-      let alertStub;
       let fireActionStub;
 
       setup(() => {
         fireActionStub = sandbox.stub(element, '_fireAction');
-        alertStub = sandbox.stub(window, 'alert');
+        element.commitMessage = 'random commit message';
+        element.change.current_revision = 'abcdef';
         element.actions = {
           revert: {
             method: 'POST',
@@ -813,50 +814,149 @@ limitations under the License.
       });
 
       test('revert change with plugin hook', done => {
+        const newRevertMsg = 'Modified revert msg';
+        sandbox.stub(element.$.confirmRevertDialog, '_modifyRevertMsg',
+            () => newRevertMsg);
         element.change = {
           current_revision: 'abc1234',
         };
-        const newRevertMsg = 'Modified revert msg';
-        sandbox.stub(element, '_modifyRevertMsg',
-            () => newRevertMsg);
-        sandbox.stub(element.$.confirmRevertDialog, 'populateRevertMessage',
-            () => 'original msg');
+        sandbox.stub(element.$.confirmRevertDialog,
+            '_populateRevertSubmissionMessage', () => 'original msg');
         flush(() => {
-          const revertButton =
-              element.$$('gr-button[data-action-key="revert"]');
+          const revertButton = element.shadowRoot
+              .querySelector('gr-button[data-action-key="revert"]');
           MockInteractions.tap(revertButton);
-
-          assert.equal(element.$.confirmRevertDialog.message, newRevertMsg);
-          done();
+          flush(() => {
+            assert.equal(element.$.confirmRevertDialog.message, newRevertMsg);
+            done();
+          });
         });
       });
 
-      test('works', () => {
-        element.change = {
-          current_revision: 'abc1234',
-        };
-        sandbox.stub(element.$.confirmRevertDialog, 'populateRevertMessage',
-            () => 'original msg');
-        const revertButton = element.$$('gr-button[data-action-key="revert"]');
-        MockInteractions.tap(revertButton);
+      suite('revert change submitted together', () => {
+        setup(() => {
+          element.change = {
+            submission_id: '199',
+            current_revision: '2000',
+          };
+          sandbox.stub(element.$.restAPI, 'getChanges')
+              .returns(Promise.resolve([
+                {change_id: '12345678901234', topic: 'T', subject: 'random'},
+                {change_id: '23456', topic: 'T', subject: 'a'.repeat(100)},
+              ]));
+        });
 
-        element.$.confirmRevertDialog.message = 'foo message';
-        element._handleRevertDialogConfirm();
-        assert.notOk(alertStub.called);
+        test('confirm revert dialog shows both options', done => {
+          const revertButton = element.shadowRoot
+              .querySelector('gr-button[data-action-key="revert"]');
+          MockInteractions.tap(revertButton);
+          flush(() => {
+            const confirmRevertDialog = element.$.confirmRevertDialog;
+            const revertSingleChangeLabel = confirmRevertDialog
+                .shadowRoot.querySelector('.revertSingleChange');
+            const revertSubmissionLabel = confirmRevertDialog.
+                shadowRoot.querySelector('.revertSubmission');
+            assert(revertSingleChangeLabel.innerText.trim() ===
+                'Revert single change');
+            assert(revertSubmissionLabel.innerText.trim() ===
+                'Revert entire submission (2 Changes)');
+            let expectedMsg = 'Revert submission 199' + '\n\n' +
+              'Reason for revert: <INSERT REASONING HERE>' + '\n' +
+              'Reverted Changes:' + '\n' +
+              '1234567890:random' + '\n' +
+              '23456:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...' +
+              '\n';
+            assert.equal(confirmRevertDialog.message, expectedMsg);
+            const radioInputs = confirmRevertDialog.shadowRoot
+                .querySelectorAll('input[name="revertOptions"]');
+            MockInteractions.tap(radioInputs[0]);
+            flush(() => {
+              expectedMsg = 'Revert "random commit message"\n\nThis reverts '
+               + 'commit 2000.\n\nReason'
+               + ' for revert: <INSERT REASONING HERE>\n';
+              assert.equal(confirmRevertDialog.message, expectedMsg);
+              done();
+            });
+          });
+        });
 
-        const action = {
-          __key: 'revert',
-          __type: 'change',
-          __primary: false,
-          enabled: true,
-          label: 'Revert',
-          method: 'POST',
-          title: 'Revert the change',
-        };
-        assert.deepEqual(fireActionStub.lastCall.args, [
-          '/revert', action, false, {
-            message: 'foo message',
-          }]);
+        test('message modification is retained on switching', done => {
+          const revertButton = element.shadowRoot
+              .querySelector('gr-button[data-action-key="revert"]');
+          const confirmRevertDialog = element.$.confirmRevertDialog;
+          MockInteractions.tap(revertButton);
+          flush(() => {
+            const radioInputs = confirmRevertDialog.shadowRoot
+                .querySelectorAll('input[name="revertOptions"]');
+            const revertSubmissionMsg = 'Revert submission 199' + '\n\n' +
+            'Reason for revert: <INSERT REASONING HERE>' + '\n' +
+            'Reverted Changes:' + '\n' +
+            '1234567890:random' + '\n' +
+            '23456:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...' +
+            '\n';
+            const singleChangeMsg =
+            'Revert "random commit message"\n\nThis reverts '
+              + 'commit 2000.\n\nReason'
+              + ' for revert: <INSERT REASONING HERE>\n';
+            assert.equal(confirmRevertDialog.message, revertSubmissionMsg);
+            const newRevertMsg = revertSubmissionMsg + 'random';
+            const newSingleChangeMsg = singleChangeMsg + 'random';
+            confirmRevertDialog.message = newRevertMsg;
+            MockInteractions.tap(radioInputs[0]);
+            flush(() => {
+              assert.equal(confirmRevertDialog.message, singleChangeMsg);
+              confirmRevertDialog.message = newSingleChangeMsg;
+              MockInteractions.tap(radioInputs[1]);
+              flush(() => {
+                assert.equal(confirmRevertDialog.message, newRevertMsg);
+                MockInteractions.tap(radioInputs[0]);
+                flush(() => {
+                  assert.equal(confirmRevertDialog.message, newSingleChangeMsg);
+                  done();
+                });
+              });
+            });
+          });
+        });
+      });
+
+      suite('revert single change', () => {
+        setup(() => {
+          element.change = {
+            submission_id: '199',
+            current_revision: '2000',
+          };
+          sandbox.stub(element.$.restAPI, 'getChanges')
+              .returns(Promise.resolve([
+                {change_id: '12345678901234', topic: 'T', subject: 'random'},
+              ]));
+        });
+
+        test('confirm revert dialog shows one radio button', done => {
+          const revertButton = element.shadowRoot
+              .querySelector('gr-button[data-action-key="revert"]');
+          MockInteractions.tap(revertButton);
+          flush(() => {
+            const confirmRevertDialog = element.$.confirmRevertDialog;
+            const radioInputs = confirmRevertDialog.shadowRoot
+                .querySelectorAll('input[name="revertOptions"]');
+            assert.equal(radioInputs.length, 1);
+            const msg = 'Revert "random commit message"\n\n'
+              + 'This reverts commit 2000.\n\nReason '
+              + 'for revert: <INSERT REASONING HERE>\n';
+            assert.equal(confirmRevertDialog.message, msg);
+            const confirmButton = element.$.confirmRevertDialog.shadowRoot
+                .querySelector('gr-dialog')
+                .shadowRoot.querySelector('#confirm');
+            MockInteractions.tap(confirmButton);
+            flush(() => {
+              assert.equal(fireActionStub.getCall(0).args[0], '/revert');
+              assert.equal(fireActionStub.getCall(0).args[1].__key, 'revert');
+              assert.equal(fireActionStub.getCall(0).args[3].message, msg);
+              done();
+            });
+          });
+        });
       });
     });
 
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.html b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.html
index fc3a8c160b..7bffe8a525 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.html
@@ -21,6 +21,7 @@ limitations under the License.
 <link rel="import" href="../../shared/gr-dialog/gr-dialog.html">
 <link rel="import" href="../../../styles/shared-styles.html">
 <link rel="import" href="../../plugins/gr-endpoint-decorator/gr-endpoint-decorator.html">
+<link rel="import" href="../../shared/gr-js-api-interface/gr-js-api-interface.html">
 
 <dom-module id="gr-confirm-revert-dialog">
   <template>
@@ -37,6 +38,13 @@ limitations under the License.
         display: block;
         width: 100%;
       }
+      .revertSubmissionLayout {
+        display: flex;
+      }
+      .label {
+        margin-left: var(--spacing-m);
+        margin-bottom: var(--spacing-m);
+      }
       iron-autogrow-textarea {
         font-family: var(--monospace-font-family);
         font-size: var(--font-size-mono);
@@ -50,7 +58,45 @@ limitations under the License.
         on-cancel="_handleCancelTap">
       <div class="header" slot="header">Revert Merged Change</div>
       <div class="main" slot="main">
+        <div class="revertSubmissionLayout">
+          <input
+            name="revertOptions"
+            type="radio"
+            id="revertSingleChange"
+            on-change="_handleRevertSingleChangeClicked"
+            checked="[[_computeIfSingleRevert(_revertType)]]">
+          <label for="revertSingleChange" class="label revertSingleChange">
+            Revert single change
+          </label>
+        </div>
+        <template is="dom-if" if="[[_showRevertSubmission]]">
+          <div on-click="_handleRevertSubmissionClicked" class="revertSubmissionLayout">
+            <input
+              name="revertOptions"
+              type="radio"
+              id="revertSubmission"
+              checked="[[_computeIfRevertSubmission(_revertType)]]">
+            <label for="revertSubmission" class="label revertSubmission">
+              Revert entire submission ([[changes.length]] Changes)
+            </label>
+        </template>
         <gr-endpoint-decorator name="confirm-revert-change">
+          <!-- Duplicating the text-area as a plugin in the case of a single
+          revert will override the entire textarea which should not happen
+          for multiple revert -->
+          <template is="dom-if" if="[[_computeIfSingleRevert(_revertType)]]">
+            <label for="messageInput">
+                Revert Commit Message
+              </label>
+            <iron-autogrow-textarea
+              id="messageInput"
+              class="message"
+              autocomplete="on"
+              max-rows="15"
+              bind-value="{{message}}"></iron-autogrow-textarea>
+          </template>
+        </gr-endpoint-decorator>
+        <template is="dom-if" if="[[_computeIfRevertSubmission(_revertType)]]">
           <label for="messageInput">
             Revert Commit Message
           </label>
@@ -60,9 +106,10 @@ limitations under the License.
               autocomplete="on"
               max-rows="15"
               bind-value="{{message}}"></iron-autogrow-textarea>
-        </gr-endpoint-decorator>
+        </template>
       </div>
     </gr-dialog>
+    <gr-js-api-interface id="jsAPI"></gr-js-api-interface>
   </template>
   <script src="gr-confirm-revert-dialog.js"></script>
 </dom-module>
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js
index bf727ec861..438440e05f 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js
@@ -19,6 +19,13 @@
 
   const ERR_COMMIT_NOT_FOUND =
       'Unable to find the commit hash of this change.';
+  const CHANGE_SUBJECT_LIMIT = 50;
+
+  // TODO(dhruvsri): clean up repeated definitions after moving to js modules
+  const REVERT_TYPES = {
+    REVERT_SINGLE_CHANGE: 1,
+    REVERT_SUBMISSION: 2,
+  };
 
   /**
    * @appliesMixin Gerrit.FireMixin
@@ -45,12 +52,55 @@
     static get properties() {
       return {
         message: String,
+        _revertType: {
+          type: Number,
+          value: REVERT_TYPES.REVERT_SINGLE_CHANGE,
+        },
+        _showRevertSubmission: {
+          type: Boolean,
+          value: false,
+        },
+        changes: {
+          type: Array,
+          value() { return []; },
+        },
+        change: Object,
+        commitMessage: String,
       };
     }
 
-    populateRevertMessage(message, commitHash) {
+    static get observers() {
+      return [
+        'onInputUpdate(change, commitMessage, changes)',
+      ];
+    }
+
+    _computeIfSingleRevert(revertType) {
+      return revertType === REVERT_TYPES.REVERT_SINGLE_CHANGE;
+    }
+
+    _computeIfRevertSubmission(revertType) {
+      return revertType === REVERT_TYPES.REVERT_SUBMISSION;
+    }
+
+    _modifyRevertMsg(change, commitMessage, message) {
+      return this.$.jsAPI.modifyRevertMsg(change,
+          message, commitMessage);
+    }
+
+    onInputUpdate(change, commitMessage, changes) {
+      if (!change || !changes) return;
+      this._populateRevertSingleChangeMessage(
+          change, commitMessage, change.current_revision);
+      if (changes.length > 1) {
+        this._populateRevertSubmissionMessage(
+            change, changes);
+      }
+    }
+
+    _populateRevertSingleChangeMessage(change, commitMessage, commitHash) {
       // Figure out what the revert title should be.
-      const originalTitle = message.split('\n')[0];
+      const originalTitle = (commitMessage || '').split('\n')[0];
       const revertTitle = `Revert "${originalTitle}"`;
       if (!commitHash) {
         this.fire('show-alert', {message: ERR_COMMIT_NOT_FOUND});
@@ -58,20 +108,77 @@
       }
       const revertCommitText = `This reverts commit ${commitHash}.`;
 
-      this.message = `${revertTitle}\n\n${revertCommitText}\n\n` +
+      this.revertSingleChangeMessage =
+          `${revertTitle}\n\n${revertCommitText}\n\n` +
           `Reason for revert: <INSERT REASONING HERE>\n`;
+      // This is to give plugins a chance to update message
+      this.revertSingleChangeMessage =
+          this._modifyRevertMsg(change, commitMessage,
+              this.revertSingleChangeMessage);
+      this.message = this.revertSingleChangeMessage;
+    }
+
+    _getTrimmedChangeSubject(subject) {
+      if (!subject) return '';
+      if (subject.length < CHANGE_SUBJECT_LIMIT) return subject;
+      return subject.substring(0, CHANGE_SUBJECT_LIMIT) + '...';
+    }
+
+    _modifyRevertSubmissionMsg(change) {
+      return this.$.jsAPI.modifyRevertSubmissionMsg(change,
+          this.revertSubmissionMessage, this.commitMessage);
+    }
+
+    _populateRevertSubmissionMessage(change, changes) {
+      // Follow the same convention of the revert
+      const commitHash = change.current_revision;
+      if (!commitHash) {
+        this.fire('show-alert', {message: ERR_COMMIT_NOT_FOUND});
+        return;
+      }
+      if (!changes || changes.length <= 1) return;
+      const submissionId = change.submission_id;
+      const revertTitle = 'Revert submission ' + submissionId;
+      this.changes = changes;
+      this.revertSubmissionMessage = revertTitle + '\n\n' +
+          'Reason for revert: <INSERT REASONING HERE>\n';
+      this.revertSubmissionMessage += 'Reverted Changes:\n';
+      changes.forEach(change => {
+        this.revertSubmissionMessage += change.change_id.substring(0, 10) + ':'
+          + this._getTrimmedChangeSubject(change.subject) + '\n';
+      });
+      this.revertSubmissionMessage = this._modifyRevertSubmissionMsg(change);
+      this.message = this.revertSubmissionMessage;
+      this._revertType = REVERT_TYPES.REVERT_SUBMISSION;
+      this._showRevertSubmission = true;
+    }
+
+    _handleRevertSingleChangeClicked() {
+      if (this._revertType === REVERT_TYPES.REVERT_SINGLE_CHANGE) return;
+      this.revertSubmissionMessage = this.message;
+      this.message = this.revertSingleChangeMessage;
+      this._revertType = REVERT_TYPES.REVERT_SINGLE_CHANGE;
+    }
+
+    _handleRevertSubmissionClicked() {
+      if (this._revertType === REVERT_TYPES.REVERT_SUBMISSION) return;
+      this._revertType = REVERT_TYPES.REVERT_SUBMISSION;
+      this.revertSingleChangeMessage = this.message;
+      this.message = this.revertSubmissionMessage;
     }
 
     _handleConfirmTap(e) {
       e.preventDefault();
       e.stopPropagation();
-      this.fire('confirm', null, {bubbles: false});
+      this.fire('confirm', {revertType: this._revertType,
+        message: this.message}, {bubbles: false});
     }
 
     _handleCancelTap(e) {
       e.preventDefault();
       e.stopPropagation();
-      this.fire('cancel', null, {bubbles: false});
+      this.fire('cancel', {revertType: this._revertType},
+          {bubbles: false});
     }
   }
 
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html
index dbdfba229a..1d28d3226f 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.html
@@ -50,13 +50,14 @@ limitations under the License.
       assert.isNotOk(element.message);
       const alertStub = sandbox.stub();
       element.addEventListener('show-alert', alertStub);
-      element.populateRevertMessage('not a commitHash in sight', undefined);
+      element._populateRevertSingleChangeMessage({},
+          'not a commitHash in sight', undefined);
       assert.isTrue(alertStub.calledOnce);
     });
 
     test('single line', () => {
       assert.isNotOk(element.message);
-      element.populateRevertMessage(
+      element._populateRevertSingleChangeMessage({},
           'one line commit\n\nChange-Id: abcdefg\n',
           'abcd123');
       const expected = 'Revert "one line commit"\n\n' +
@@ -67,7 +68,7 @@ limitations under the License.
 
     test('multi line', () => {
       assert.isNotOk(element.message);
-      element.populateRevertMessage(
+      element._populateRevertSingleChangeMessage({},
           'many lines\ncommit\n\nmessage\n\nChange-Id: abcdefg\n',
           'abcd123');
       const expected = 'Revert "many lines"\n\n' +
@@ -78,7 +79,7 @@ limitations under the License.
 
     test('issue above change id', () => {
       assert.isNotOk(element.message);
-      element.populateRevertMessage(
+      element._populateRevertSingleChangeMessage({},
           'much lines\nvery\n\ncommit\n\nBug: Issue 42\nChange-Id: abcdefg\n',
           'abcd123');
       const expected = 'Revert "much lines"\n\n' +
@@ -89,7 +90,7 @@ limitations under the License.
 
     test('revert a revert', () => {
       assert.isNotOk(element.message);
-      element.populateRevertMessage(
+      element._populateRevertSingleChangeMessage({},
           'Revert "one line commit"\n\nChange-Id: abcdefg\n',
           'abcd123');
       const expected = 'Revert "Revert "one line commit""\n\n' +
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog.js
index d59f5cd1b4..ae8dfa53c1 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog.js
@@ -50,7 +50,7 @@
       };
     }
 
-    getTrimmedChangeSubject(subject) {
+    _getTrimmedChangeSubject(subject) {
       if (!subject) return '';
       if (subject.length < CHANGE_SUBJECT_LIMIT) return subject;
       return subject.substring(0, CHANGE_SUBJECT_LIMIT) + '...';
@@ -61,7 +61,7 @@
           this.message, this.commitMessage);
     }
 
-    populateRevertSubmissionMessage(message, change, changes) {
+    _populateRevertSubmissionMessage(message, change, changes) {
       // Follow the same convention of the revert
       const commitHash = change.current_revision;
       if (!commitHash) {
@@ -77,7 +77,7 @@
       changes = changes || [];
       changes.forEach(change => {
         this.message += change.change_id.substring(0, 10) + ': ' +
-          this.getTrimmedChangeSubject(change.subject) + '\n';
+          this._getTrimmedChangeSubject(change.subject) + '\n';
       });
       this.message = this._modifyRevertSubmissionMsg(change);
     }
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog_test.html b/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog_test.html
index cc4bd548fa..af99c7e6ee 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-submission-dialog/gr-confirm-revert-submission-dialog_test.html
@@ -51,7 +51,7 @@ limitations under the License.
       assert.isNotOk(element.message);
       const alertStub = sandbox.stub();
       element.addEventListener('show-alert', alertStub);
-      element.populateRevertSubmissionMessage(
+      element._populateRevertSubmissionMessage(
           'not a commitHash in sight'
       );
       assert.isTrue(alertStub.calledOnce);
@@ -59,7 +59,7 @@ limitations under the License.
 
     test('single line', () => {
       assert.isNotOk(element.message);
-      element.populateRevertSubmissionMessage(
+      element._populateRevertSubmissionMessage(
           'one line commit\n\nChange-Id: abcdefg\n',
           'abcd123');
       const expected = 'Revert submission\n\n' +
@@ -69,7 +69,7 @@ limitations under the License.
 
     test('multi line', () => {
       assert.isNotOk(element.message);
-      element.populateRevertSubmissionMessage(
+      element._populateRevertSubmissionMessage(
           'many lines\ncommit\n\nmessage\n\nChange-Id: abcdefg\n',
           'abcd123');
       const expected = 'Revert submission\n\n' +
@@ -79,7 +79,7 @@ limitations under the License.
 
     test('issue above change id', () => {
       assert.isNotOk(element.message);
-      element.populateRevertSubmissionMessage(
+      element._populateRevertSubmissionMessage(
           'test \nvery\n\ncommit\n\nBug: Issue 42\nChange-Id: abcdefg\n',
           'abcd123');
       const expected = 'Revert submission\n\n' +
@@ -89,7 +89,7 @@ limitations under the License.
 
     test('revert a revert', () => {
       assert.isNotOk(element.message);
-      element.populateRevertSubmissionMessage(
+      element._populateRevertSubmissionMessage(
           'Revert "one line commit"\n\nChange-Id: abcdefg\n',
           'abcd123');
       const expected = 'Revert submission\n\n' +