Merge "RevisionActions: Do not alter server response" into stable-3.1

6 files changed, 23 insertions, 102 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 000756f897..e12f10def9 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
@@ -179,7 +179,7 @@ limitations under the License.
           on-cancel="_handleConfirmDialogCancel"
           branch="[[change.branch]]"
           has-parent="[[hasParent]]"
-          rebase-on-current="[[_revisionRebaseAction.rebaseOnCurrent]]"
+          rebase-on-current="[[_computeRebaseOnCurrent(_revisionRebaseAction)]]"
           hidden></gr-confirm-rebase-dialog>
       <gr-confirm-cherrypick-dialog id="confirmCherrypick"
           class="confirmDialog"
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 89642e53df..9c0c38fc2d 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
@@ -433,9 +433,7 @@
     },
 
     _getRebaseAction(revisionActions) {
-      return this._getRevisionAction(revisionActions, 'rebase',
-          {rebaseOnCurrent: null}
-      );
+      return this._getRevisionAction(revisionActions, 'rebase', null);
     },
 
     _getRevisionAction(revisionActions, actionName, emptyActionValue) {
@@ -459,7 +457,7 @@
       return this._getRevisionActions().then(revisionActions => {
         if (!revisionActions) { return; }
 
-        this.revisionActions = this._updateRebaseAction(revisionActions);
+        this.revisionActions = revisionActions;
         this._sendShowRevisionActions({
           change: this.change,
           revisionActions,
@@ -483,18 +481,6 @@
       );
     },
 
-    _updateRebaseAction(revisionActions) {
-      if (revisionActions && revisionActions.rebase) {
-        revisionActions.rebase.rebaseOnCurrent =
-            !!revisionActions.rebase.enabled;
-        this._parentIsCurrent = !revisionActions.rebase.enabled;
-        revisionActions.rebase.enabled = true;
-      } else {
-        this._parentIsCurrent = true;
-      }
-      return revisionActions;
-    },
-
     _changeChanged() {
       this.reload();
     },
@@ -1045,8 +1031,9 @@
     },
 
     _calculateDisabled(action, hasKnownChainState) {
-      if (action.__key === 'rebase' && hasKnownChainState === false) {
-        return true;
+      if (action.__key === 'rebase') {
+        // Rebase button is only disabled when change has no parent(s).
+        return hasKnownChainState === false;
       }
       return !action.enabled;
     },
@@ -1483,6 +1470,13 @@
       });
     },
 
+    _computeRebaseOnCurrent(revisionRebaseAction) {
+      if (revisionRebaseAction) {
+        return !!revisionRebaseAction.enabled;
+      }
+      return null;
+    },
+
     /**
      * Occasionally, a change created by a change action is not yet knwon to the
      * API for a brief time. Wait for the given change number to be recognized.
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 520e03f1e1..74d262ab6c 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
@@ -363,7 +363,7 @@ limitations under the License.
 
       action.enabled = false;
       assert.equal(
-          element._calculateDisabled(action, hasKnownChainState), true);
+          element._calculateDisabled(action, hasKnownChainState), false);
     });
 
     test('rebase change', done => {
@@ -385,7 +385,6 @@ limitations under the License.
         };
         assert.isTrue(fetchChangesStub.called);
         element._handleRebaseConfirm({detail: {base: '1234'}});
-        rebaseAction.rebaseOnCurrent = true;
         assert.deepEqual(fireActionStub.lastCall.args,
             ['/rebase', rebaseAction, true, {base: '1234'}]);
         done();
@@ -1569,57 +1568,23 @@ limitations under the License.
       assert.strictEqual(element.$.confirmRebase.rebaseOnCurrent, null);
     });
 
-    test('_updateRebaseAction sets _parentIsCurrent on no rebase', () => {
-      const currentRevisionActions = {
-        cherrypick: {
-          enabled: true,
-          label: 'Cherry Pick',
-          method: 'POST',
-          title: 'cherrypick',
-        },
-      };
-      element._parentIsCurrent = undefined;
-      element._updateRebaseAction(currentRevisionActions);
-      assert.isTrue(element._parentIsCurrent);
-    });
-
-    test('_updateRebaseAction', () => {
-      const currentRevisionActions = {
-        cherrypick: {
-          enabled: true,
-          label: 'Cherry Pick',
-          method: 'POST',
-          title: 'cherrypick',
-        },
-        rebase: {
-          enabled: true,
-          label: 'Rebase',
-          method: 'POST',
-          title: 'Rebase onto tip of branch or parent change',
-        },
+    test('_computeRebaseOnCurrent', () => {
+      const rebaseAction = {
+        enabled: true,
+        label: 'Rebase',
+        method: 'POST',
+        title: 'Rebase onto tip of branch or parent change',
       };
-      element._parentIsCurrent = undefined;
 
-      // Rebase enabled should always end up true.
       // When rebase is enabled initially, rebaseOnCurrent should be set to
       // true.
-      assert.equal(element._updateRebaseAction(currentRevisionActions),
-          currentRevisionActions);
+      assert.isTrue(element._computeRebaseOnCurrent(rebaseAction));
 
-      assert.isTrue(currentRevisionActions.rebase.enabled);
-      assert.isTrue(currentRevisionActions.rebase.rebaseOnCurrent);
-      assert.isFalse(element._parentIsCurrent);
-
-      delete currentRevisionActions.rebase.enabled;
+      delete rebaseAction.enabled;
 
       // When rebase is not enabled initially, rebaseOnCurrent should be set to
       // false.
-      assert.equal(element._updateRebaseAction(currentRevisionActions),
-          currentRevisionActions);
-
-      assert.isTrue(currentRevisionActions.rebase.enabled);
-      assert.isFalse(currentRevisionActions.rebase.rebaseOnCurrent);
-      assert.isTrue(element._parentIsCurrent);
+      assert.isFalse(element._computeRebaseOnCurrent(rebaseAction));
     });
   });
 </script>
diff --git 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
index 20c3d15287..0c929b46dd 100644
--- 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
@@ -203,7 +203,6 @@
       _loading: Boolean,
       /** @type {?} */
       _projectConfig: Object,
-      _rebaseOnCurrent: Boolean,
       _replyButtonLabel: {
         type: String,
         value: 'Reply',
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index 03735c90c6..8e4af15233 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -1200,12 +1200,6 @@
         reportEndpointAsIs: true,
       };
       return this._getChangeURLAndFetch(req).then(revisionActions => {
-        // The rebase button on change screen is always enabled.
-        if (revisionActions.rebase) {
-          revisionActions.rebase.rebaseOnCurrent =
-              !!revisionActions.rebase.enabled;
-          revisionActions.rebase.enabled = true;
-        }
         return revisionActions;
       });
     },
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
index 1781ce70df..635e0f5065 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
@@ -321,37 +321,6 @@ limitations under the License.
           ]);
     });
 
-    suite('rebase action', () => {
-      let resolve_fetchJSON;
-      setup(() => {
-        sandbox.stub(element._restApiHelper, 'fetchJSON').returns(
-            new Promise(resolve => {
-              resolve_fetchJSON = resolve;
-            }));
-      });
-
-      test('no rebase on current', done => {
-        element.getChangeRevisionActions('42', '1337').then(
-            response => {
-              assert.isTrue(response.rebase.enabled);
-              assert.isFalse(response.rebase.rebaseOnCurrent);
-              done();
-            });
-        resolve_fetchJSON({rebase: {}});
-      });
-
-      test('rebase on current', done => {
-        element.getChangeRevisionActions('42', '1337').then(
-            response => {
-              assert.isTrue(response.rebase.enabled);
-              assert.isTrue(response.rebase.rebaseOnCurrent);
-              done();
-            });
-        resolve_fetchJSON({rebase: {enabled: true}});
-      });
-    });
-
-
     test('server error', done => {
       const getResponseObjectStub = sandbox.stub(element, 'getResponseObject');
       window.fetch.returns(Promise.resolve({ok: false}));