From 4e9540998b578fad9f20dde0dd8c4b333abe2b03 Mon Sep 17 00:00:00 2001
From: Thomas Shafer <tshafer@google.com>
Date: Tue, 12 Mar 2019 13:44:59 -0700
Subject: [PATCH] Rename params variable to optionsHex

Params is a confusing variable name as it implies key value pairs.
optionsHex is the variable name used in the first call to _getChangeDetail.

Change-Id: I20a646faa49fb657de5beee318724a3db244bee6
---
 .../gr-rest-api-interface.js                     | 16 +++++++++-------
 .../gr-rest-api-interface_test.html              | 12 ++++++------
 2 files changed, 15 insertions(+), 13 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 ec14af5067..849972f1f7 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
@@ -353,7 +353,7 @@
 
     /**
      * @param {string} url
-     * @param {?Object=} opt_params URL params, key-value hash.
+     * @param {?Object|string=} opt_params URL params, key-value hash.
      * @return {string}
      */
     _urlWithParams(url, opt_params) {
@@ -1352,30 +1352,32 @@
      * @param {function()=} opt_cancelCondition
      */
     getDiffChangeDetail(changeNum, opt_errFn, opt_cancelCondition) {
-      const params = this.listChangesOptionsToHex(
+      const optionsHex = this.listChangesOptionsToHex(
           this.ListChangesOption.ALL_COMMITS,
           this.ListChangesOption.ALL_REVISIONS,
           this.ListChangesOption.SKIP_MERGEABLE
       );
-      return this._getChangeDetail(changeNum, params, opt_errFn,
+      return this._getChangeDetail(changeNum, optionsHex, opt_errFn,
           opt_cancelCondition);
     },
 
     /**
      * @param {number|string} changeNum
+     * @param {string|undefined} optionsHex list changes options in hex
      * @param {function(?Response, string=)=} opt_errFn
      * @param {function()=} opt_cancelCondition
      */
-    _getChangeDetail(changeNum, params, opt_errFn, opt_cancelCondition) {
+    _getChangeDetail(changeNum, optionsHex, opt_errFn, opt_cancelCondition) {
       return this.getChangeActionURL(changeNum, null, '/detail').then(url => {
-        const urlWithParams = this._urlWithParams(url, params);
+        const urlWithParams = this._urlWithParams(url, optionsHex);
+        const params = {O: optionsHex};
         const req = {
           url,
           errFn: opt_errFn,
           cancelCondition: opt_cancelCondition,
-          params: {O: params},
+          params,
           fetchOptions: this._etags.getOptions(urlWithParams),
-          anonymizedUrl: '/changes/*~*/detail?O=' + params,
+          anonymizedUrl: '/changes/*~*/detail?O=' + optionsHex,
         };
         return this._fetchRawJSON(req).then(response => {
           if (response && response.status === 304) {
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 4ab1e044ce..ef4e401e64 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
@@ -1151,12 +1151,12 @@ limitations under the License.
       test('_getChangeDetail passes params to ETags decorator', () => {
         const changeNum = 4321;
         element._projectLookup[changeNum] = 'test';
-        const params = {foo: 'bar'};
         const expectedUrl =
-            window.CANONICAL_PATH + '/changes/test~4321/detail?foo=bar';
+            window.CANONICAL_PATH + '/changes/test~4321/detail?'+
+            '0=5&1=1&2=6&3=7&4=1&5=4';
         sandbox.stub(element._etags, 'getOptions');
         sandbox.stub(element._etags, 'collect');
-        return element._getChangeDetail(changeNum, params).then(() => {
+        return element._getChangeDetail(changeNum, '516714').then(() => {
           assert.isTrue(element._etags.getOptions.calledWithExactly(
               expectedUrl));
           assert.equal(element._etags.collect.lastCall.args[0], expectedUrl);
@@ -1169,7 +1169,7 @@ limitations under the License.
             .returns(Promise.resolve(''));
         sandbox.stub(element, '_fetchRawJSON')
             .returns(Promise.resolve({ok: false, status: 500}));
-        return element._getChangeDetail(123, {}, errFn).then(() => {
+        return element._getChangeDetail(123, '516714', errFn).then(() => {
           assert.isTrue(errFn.called);
         });
       });
@@ -1185,7 +1185,7 @@ limitations under the License.
           parsed: mockResponse,
           raw: JSON.stringify(mockResponse),
         }));
-        return element._getChangeDetail(1).then(() => {
+        return element._getChangeDetail(1, '516714').then(() => {
           assert.equal(Object.keys(element._projectLookup).length, 1);
           assert.equal(element._projectLookup[1], 'test');
         });
@@ -1216,7 +1216,7 @@ limitations under the License.
             ok: true,
           }));
 
-          return element._getChangeDetail(123, {}).then(detail => {
+          return element._getChangeDetail(123, '516714').then(detail => {
             assert.isFalse(getPayloadSpy.called);
             assert.isTrue(collectSpy.calledOnce);
             const cachedResponse = element._etags.getCachedPayload(requestUrl);