Fix entering specific revision in rebase dialog

The previous implementation for base selection relied on gr-autocomplete
firing a commit event to set the public base property. This did not work
for entering a value that does not exist as an autocompleted value --
`commit` events are fired only upon autocompletion.

This implementation instead gets the base on confirm and adds it to the
event detail.

Bug: Issue 8412
Change-Id: I479d11fca62b14880243b2f2d71bff72085a92fc

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

@@ -949,9 +949,9 @@
       this.$.overlay.close();
     },
 
-    _handleRebaseConfirm() {
+    _handleRebaseConfirm(e) {
       const el = this.$.confirmRebase;
-      const payload = {base: el.base};
+      const payload = {base: e.detail.base};
       this.$.overlay.close();
       el.hidden = true;
       this._fireAction('/rebase', this.revisionActions.rebase, true, payload);

polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html

@@ -80,6 +80,7 @@ limitations under the License.
 
           return Promise.reject('bad url');
         },
+        getProjectConfig() { return Promise.resolve({}); },
       });
 
       element = fixture('basic');
@@ -350,24 +351,9 @@ limitations under the License.
           title: 'Rebase onto tip of branch or parent change',
         };
         assert.isTrue(fetchChangesStub.called);
-        // rebase on other
-        element.$.confirmRebase.base = '1234';
-        element._handleRebaseConfirm();
+        element._handleRebaseConfirm({detail: {base: '1234'}});
         assert.deepEqual(fireActionStub.lastCall.args,
           ['/rebase', rebaseAction, true, {base: '1234'}]);
-
-        // rebase on parent
-        element.$.confirmRebase.base = null;
-        element._handleRebaseConfirm();
-        assert.deepEqual(fireActionStub.lastCall.args,
-          ['/rebase', rebaseAction, true, {base: null}]);
-
-        // rebase on tip
-        element.$.confirmRebase.base = '';
-        element._handleRebaseConfirm();
-        assert.deepEqual(fireActionStub.lastCall.args,
-          ['/rebase', rebaseAction, true, {base: ''}]);
-
         done();
       });
     });

polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.html

@@ -106,9 +106,8 @@ limitations under the License.
               id="parentInput"
               query="[[_query]]"
               no-debounce
-              text="{{_inputText}}"
+              text="{{_text}}"
               on-tap="_handleEnterChangeNumberTap"
-              on-commit="_handleBaseSelected"
               allow-non-suggested-values
               placeholder="Change number, ref, or commit hash">
           </gr-autocomplete>

polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.js

@@ -33,16 +33,11 @@
      */
 
     properties: {
-      /**
-       * Weird API usage requires this to be String or Null. Add this so
-       * the closure compiler doesn't complain.
-       * @type {?string} */
-      base: String,
       branch: String,
       changeNumber: Number,
       hasParent: Boolean,
       rebaseOnCurrent: Boolean,
-      _inputText: String,
+      _text: String,
       _query: {
         type: Function,
         value() {
@@ -107,22 +102,6 @@
       return !(!rebaseOnCurrent && !hasParent);
     },
 
-    _handleConfirmTap(e) {
-      e.preventDefault();
-      this._inputText = '';
-      this.fire('confirm', null, {bubbles: false});
-    },
-
-    _handleCancelTap(e) {
-      e.preventDefault();
-      this._inputText = '';
-      this.fire('cancel', null, {bubbles: false});
-    },
-
-    _handleRebaseOnOther() {
-      this.$.parentInput.focus();
-    },
-
     /**
      * There is a subtle but important difference between setting the base to an
      * empty string and omitting it entirely from the payload. An empty string
@@ -130,16 +109,29 @@
      * rebased on top of the target branch. Leaving out the base implies that it
      * should be rebased on top of its current parent.
      */
-    _handleRebaseOnTip() {
-      this.base = '';
+    _getSelectedBase() {
+      if (this.$.rebaseOnParentInput.checked) { return null; }
+      if (this.$.rebaseOnTipInput.checked) { return ''; }
+      // Change numbers will have their description appended by the
+      // autocomplete.
+      return this._text.split(':')[0];
     },
 
-    _handleRebaseOnParent() {
-      this.base = null;
+    _handleConfirmTap(e) {
+      e.preventDefault();
+      this.dispatchEvent(new CustomEvent('confirm',
+          {detail: {base: this._getSelectedBase()}}));
+      this._text = '';
     },
 
-    _handleBaseSelected(e) {
-      this.base = e.detail.value;
+    _handleCancelTap(e) {
+      e.preventDefault();
+      this.dispatchEvent(new CustomEvent('cancel'));
+      this._text = '';
+    },
+
+    _handleRebaseOnOther() {
+      this.$.parentInput.focus();
     },
 
     _handleEnterChangeNumberTap() {
@@ -153,13 +145,10 @@
     _updateSelectedOption(rebaseOnCurrent, hasParent) {
       if (this._displayParentOption(rebaseOnCurrent, hasParent)) {
         this.$.rebaseOnParentInput.checked = true;
-        this._handleRebaseOnParent();
       } else if (this._displayTipOption(rebaseOnCurrent, hasParent)) {
         this.$.rebaseOnTipInput.checked = true;
-        this._handleRebaseOnTip();
       } else {
         this.$.rebaseOnOtherInput.checked = true;
-        this._handleRebaseOnOther();
       }
     },
   });

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

@@ -91,13 +91,26 @@ limitations under the License.
     });
 
     test('input cleared on cancel or submit', () => {
-      element._inputText = '123';
+      element._text = '123';
       element.$.confirmDialog.fire('confirm');
-      assert.equal(element._inputText, '');
+      assert.equal(element._text, '');
 
-      element._inputText = '123';
+      element._text = '123';
       element.$.confirmDialog.fire('cancel');
-      assert.equal(element._inputText, '');
+      assert.equal(element._text, '');
+    });
+
+    test('_getSelectedBase', () => {
+      element._text = '5fab321c';
+      element.$.rebaseOnParentInput.checked = true;
+      assert.equal(element._getSelectedBase(), null);
+      element.$.rebaseOnParentInput.checked = false;
+      element.$.rebaseOnTipInput.checked = true;
+      assert.equal(element._getSelectedBase(), '');
+      element.$.rebaseOnTipInput.checked = false;
+      assert.equal(element._getSelectedBase(), element._text);
+      element._text = '101: Test';
+      assert.equal(element._getSelectedBase(), '101');
     });
 
     suite('parent suggestions', () => {
@@ -167,9 +180,9 @@ limitations under the License.
       test('input text change triggers function', () => {
         sandbox.spy(element, '_getRecentChanges');
         element.$.parentInput.noDebounce = true;
-        element.$.parentInput.text = '1';
+        element._text = '1';
         assert.isTrue(element._getRecentChanges.calledOnce);
-        element._inputText = '12';
+        element._text = '12';
         assert.isTrue(element._getRecentChanges.calledTwice);
       });
     });