From 7dc7f91bc49b69c1533891a9d81e3a7f27d98ccd Mon Sep 17 00:00:00 2001
From: Logan Hanks <logan@google.com>
Date: Mon, 12 Nov 2018 15:54:23 -0800
Subject: [PATCH] Conditionally add PUSH_CERTIFICATES change option

If the server config reports that signed pushes are enabled, then we
need to add the PUSH_CERTIFICATES bit to our change detail request
options. This change will fetch the server config (if it's not already
pending or resolved) and wait for it to resolve before sending the
change detail request.

Change-Id: I8a919a50a278588aa342fa31529911274fec9ca5
---
 .../gr-rest-api-interface.js                  | 36 +++++++------
 .../gr-rest-api-interface_test.html           | 53 +++++++++++++++----
 2 files changed, 64 insertions(+), 25 deletions(-)

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 2a1ad9eec1..fd25908f13 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
@@ -1297,21 +1297,27 @@
      * @param {function()=} opt_cancelCondition
      */
     getChangeDetail(changeNum, opt_errFn, opt_cancelCondition) {
-      const options = this.listChangesOptionsToHex(
-          this.ListChangesOption.ALL_COMMITS,
-          this.ListChangesOption.ALL_REVISIONS,
-          this.ListChangesOption.CHANGE_ACTIONS,
-          this.ListChangesOption.CURRENT_ACTIONS,
-          this.ListChangesOption.DETAILED_LABELS,
-          this.ListChangesOption.DOWNLOAD_COMMANDS,
-          this.ListChangesOption.MESSAGES,
-          this.ListChangesOption.SUBMITTABLE,
-          this.ListChangesOption.WEB_LINKS,
-          this.ListChangesOption.SKIP_MERGEABLE
-      );
-      return this._getChangeDetail(
-          changeNum, options, opt_errFn, opt_cancelCondition)
-          .then(GrReviewerUpdatesParser.parse);
+      const options = [
+        this.ListChangesOption.ALL_COMMITS,
+        this.ListChangesOption.ALL_REVISIONS,
+        this.ListChangesOption.CHANGE_ACTIONS,
+        this.ListChangesOption.CURRENT_ACTIONS,
+        this.ListChangesOption.DETAILED_LABELS,
+        this.ListChangesOption.DOWNLOAD_COMMANDS,
+        this.ListChangesOption.MESSAGES,
+        this.ListChangesOption.SUBMITTABLE,
+        this.ListChangesOption.WEB_LINKS,
+        this.ListChangesOption.SKIP_MERGEABLE,
+      ];
+      return this.getConfig(false).then(config => {
+        if (config.receive && config.receive.enable_signed_push) {
+          options.push(this.ListChangesOption.PUSH_CERTIFICATES);
+        }
+        const optionsHex = this.listChangesOptionsToHex(...options);
+        return this._getChangeDetail(
+            changeNum, optionsHex, opt_errFn, opt_cancelCondition)
+            .then(GrReviewerUpdatesParser.parse);
+      });
     },
 
     /**
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 eaac5efd69..b3496ce1f4 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
@@ -722,15 +722,6 @@ limitations under the License.
       assert.deepEqual(element._send.lastCall.args[0].body, {token: 'foo'});
     });
 
-    test('GrReviewerUpdatesParser.parse is used', () => {
-      sandbox.stub(GrReviewerUpdatesParser, 'parse').returns(
-          Promise.resolve('foo'));
-      return element.getChangeDetail(42).then(result => {
-        assert.isTrue(GrReviewerUpdatesParser.parse.calledOnce);
-        assert.equal(result, 'foo');
-      });
-    });
-
     test('setAccountStatus', () => {
       sandbox.stub(element, '_send').returns(Promise.resolve('OOO'));
       element._cache.set('/accounts/self/detail', {});
@@ -1031,7 +1022,49 @@ limitations under the License.
       });
     });
 
-    suite('_getChangeDetail', () => {
+    suite('getChangeDetail', () => {
+      suite('change detail options', () => {
+        let toHexStub;
+
+        setup(() => {
+          toHexStub = sandbox.stub(element, 'listChangesOptionsToHex',
+              options => 'deadbeef');
+          sandbox.stub(element, '_getChangeDetail',
+              async (changeNum, options) => ({changeNum, options}));
+        });
+
+        test('signed pushes disabled', async () => {
+          const {PUSH_CERTIFICATES} = element.ListChangesOption;
+          sandbox.stub(element, 'getConfig', async () => ({}));
+          const {changeNum, options} = await element.getChangeDetail(123);
+          assert.strictEqual(123, changeNum);
+          assert.strictEqual('deadbeef', options);
+          assert.isTrue(toHexStub.calledOnce);
+          assert.isFalse(toHexStub.lastCall.args.includes(PUSH_CERTIFICATES));
+        });
+
+        test('signed pushes enabled', async () => {
+          const {PUSH_CERTIFICATES} = element.ListChangesOption;
+          sandbox.stub(element, 'getConfig', async () => {
+            return {receive: {enable_signed_push: true}};
+          });
+          const {changeNum, options} = await element.getChangeDetail(123);
+          assert.strictEqual(123, changeNum);
+          assert.strictEqual('deadbeef', options);
+          assert.isTrue(toHexStub.calledOnce);
+          assert.isTrue(toHexStub.lastCall.args.includes(PUSH_CERTIFICATES));
+        });
+      });
+
+      test('GrReviewerUpdatesParser.parse is used', () => {
+        sandbox.stub(GrReviewerUpdatesParser, 'parse').returns(
+            Promise.resolve('foo'));
+        return element.getChangeDetail(42).then(result => {
+          assert.isTrue(GrReviewerUpdatesParser.parse.calledOnce);
+          assert.equal(result, 'foo');
+        });
+      });
+
       test('_getChangeDetail passes params to ETags decorator', () => {
         const changeNum = 4321;
         element._projectLookup[changeNum] = 'test';