From 9eb69a0593875ba8425d5a74a2e01fe05218be85 Mon Sep 17 00:00:00 2001
From: Andrew Bonventre <andybons@chromium.org>
Date: Tue, 15 Dec 2015 12:33:58 -0500
Subject: [PATCH] Add dropdown to jump to patch set in change view

Feature: Issue 3645
Change-Id: I5b38ce872295c5d39790d3768ca581f07c050785
---
 polygerrit-ui/app/elements/gr-ajax.html       |   7 +
 .../app/elements/gr-change-view.html          | 156 +++++++++++++-----
 polygerrit-ui/app/elements/gr-file-list.html  |  50 +++---
 polygerrit-ui/app/scripts/app.js              |   2 +-
 .../app/test/gr-change-view-test.html         |  60 +++++++
 polygerrit-ui/app/test/gr-file-list-test.html | 126 ++++++++++++++
 polygerrit-ui/app/test/index.html             |   1 +
 7 files changed, 331 insertions(+), 71 deletions(-)
 create mode 100644 polygerrit-ui/app/test/gr-file-list-test.html

diff --git a/polygerrit-ui/app/elements/gr-ajax.html b/polygerrit-ui/app/elements/gr-ajax.html
index 9671b37e6d..97f35281a2 100644
--- a/polygerrit-ui/app/elements/gr-ajax.html
+++ b/polygerrit-ui/app/elements/gr-ajax.html
@@ -79,6 +79,13 @@ limitations under the License.
         },
       },
 
+      ready: function() {
+        // Used for debugging which element a request came from.
+        var headers = this.$.xhr.headers;
+        headers['x-requesting-element-id'] = this.id || 'gr-ajax (no id)';
+        this.$.xhr.headers = headers;
+      },
+
       generateRequest: function() {
         return this.$.xhr.generateRequest();
       },
diff --git a/polygerrit-ui/app/elements/gr-change-view.html b/polygerrit-ui/app/elements/gr-change-view.html
index 8bef101571..35153d42c6 100644
--- a/polygerrit-ui/app/elements/gr-change-view.html
+++ b/polygerrit-ui/app/elements/gr-change-view.html
@@ -38,6 +38,7 @@ limitations under the License.
         height: 4.1em;
       }
       .header {
+        align-items: center;
         background-color: var(--view-background-color);
         display: flex;
         max-width: var(--max-constrained-width);
@@ -57,6 +58,15 @@ limitations under the License.
         overflow: hidden;
         text-overflow: ellipsis;
       }
+      .patchSelectLabel {
+        margin-left: var(--default-horizontal-margin);
+      }
+      .header select {
+        margin-left: .5em;
+      }
+      .header gr-reply-dropdown {
+        margin-left: var(--default-horizontal-margin);
+      }
       section {
         margin: 10px 0;
         padding: 10px var(--default-horizontal-margin);
@@ -87,26 +97,39 @@ limitations under the License.
       }
     </style>
     <gr-ajax id="detailXHR"
-        url="[[_computeDetailPath(changeNum)]]"
+        url="[[_computeDetailPath(_changeNum)]]"
         params="[[_computeDetailQueryParams()]]"
-        last-response="{{change}}"
+        last-response="{{_change}}"
         loading="{{_loading}}"></gr-ajax>
     <gr-ajax id="commentsXHR"
-        url="[[_computeCommentsPath(changeNum)]]"
-        last-response="{{comments}}"></gr-ajax>
+        url="[[_computeCommentsPath(_changeNum)]]"
+        last-response="{{_comments}}"></gr-ajax>
+    <gr-ajax id="commitInfoXHR"
+        url="[[_computeCommitInfoPath(_changeNum, _patchNum)]]"
+        last-response="{{_commitInfo}}"></gr-ajax>
     <div class="container loading" hidden$="{{!_loading}}">Loading...</div>
     <div class="container" hidden$="{{_loading}}">
       <div class="headerContainer">
         <div class="header">
           <h2>
-            <a href$="[[_computeChangePath(change._number)]]">[[change._number]]</a><span>:</span>
-            <span>[[change.subject]]</span>
+            <a href$="[[_computeChangePath(_change._number)]]">[[_change._number]]</a><span>:</span>
+            <span>[[_change.subject]]</span>
           </h2>
+          <label class="patchSelectLabel" for="patchSetSelect">Patch set</label>
+          <select id="patchSetSelect" on-change="_handlePatchChange">
+            <template is="dom-repeat" items="{{_allPatchSets}}" as="patchNumber">
+              <option value$="[[patchNumber]]" selected$="[[_computePatchIndexIsSelected(index, _patchNum)]]">
+                <span>[[patchNumber]]</span>
+                /
+                <span>[[_computeLatestPatchNum(_change)]]</span>
+              </option>
+            </template>
+          </select>
           <gr-reply-dropdown id="replyDropdown"
-              change-num="[[changeNum]]"
-              patch-num="[[_computePatchNum(change.current_revision)]]"
-              labels="[[change.labels]]"
-              permitted-labels="[[change.permitted_labels]]"
+              change-num="[[_changeNum]]"
+              patch-num="[[_patchNum]]"
+              labels="[[_change.labels]]"
+              permitted-labels="[[_change.permitted_labels]]"
               on-send="_handleReplySent"
               hidden$="[[!_loggedIn]]">Reply</gr-reply-dropdown>
         </div>
@@ -115,13 +138,13 @@ limitations under the License.
         <table>
           <tr>
             <td class="changeInfo-label">Owner</td>
-            <td>[[change.owner.name]]</td>
+            <td>[[_change.owner.name]]</td>
           </tr>
           <tr>
             <td class="changeInfo-label">Reviewers</td>
             <td>
               <template is="dom-repeat"
-                        items="[[_computeReviewers(change.labels, change.owner)]]"
+                        items="[[_computeReviewers(_change.labels, _change.owner)]]"
                         as="reviewer">
                 <div>[[reviewer.name]]</div>
               </template>
@@ -129,15 +152,15 @@ limitations under the License.
           </tr>
           <tr>
             <td class="changeInfo-label">Project</td>
-            <td>[[change.project]]</td>
+            <td>[[_change.project]]</td>
           </tr>
           <tr>
             <td class="changeInfo-label">Branch</td>
-            <td>[[change.branch]]</td>
+            <td>[[_change.branch]]</td>
           </tr>
           <tr>
             <td class="changeInfo-label">Topic</td>
-            <td>[[change.topic]]</td>
+            <td>[[_change.topic]]</td>
           </tr>
           <tr>
             <td class="changeInfo-label">Strategy</td>
@@ -147,21 +170,20 @@ limitations under the License.
             <td class="changeInfo-label">Updated</td>
             <td>
               <gr-date-formatter
-                  date-str="[[change.updated]]"></gr-date-formatter>
+                  date-str="[[_change.updated]]"></gr-date-formatter>
             </td>
           </tr>
         </table>
       </section>
-      <section class="summary">[[_computeCurrentRevisionMessage(change)]]</section>
+      <section class="summary">[[_commitInfo.message]]</section>
       <gr-file-list id="fileList"
-          change-num="[[changeNum]]"
-          patch-num="[[_computePatchNum(change.current_revision)]]"
-          revision="[[change.current_revision]]"
-          comments="[[comments]]"></gr-file-list>
+          change-num="[[_changeNum]]"
+          patch-num="[[_patchNum]]"
+          comments="[[_comments]]"></gr-file-list>
       <gr-messages-list
-          change-num="[[changeNum]]"
-          messages="[[change.messages]]"
-          comments="[[comments]]"></gr-messages-list>
+          change-num="[[_changeNum]]"
+          messages="[[_change.messages]]"
+          comments="[[_comments]]"></gr-messages-list>
     </div>
   </template>
   <script>
@@ -189,9 +211,19 @@ limitations under the License.
           type: Object,
           observer: '_paramsChanged',
         },
-        changeNum: Number,
-        comments: Object,
 
+        _comments: Object,
+        _change: {
+          type: Object,
+          observer: '_changeChanged',
+        },
+        _commitInfo: Object,
+        _changeNum: String,
+        _patchNum: String,
+        _allPatchSets: {
+          type: Array,
+          computed: '_computeAllPatchSets(_change)',
+        },
         _loggedIn: {
           type: Boolean,
           value: false,
@@ -240,20 +272,38 @@ limitations under the License.
         el.classList.toggle('pinned', window.scrollY >= top);
       },
 
+      _handlePatchChange: function(e) {
+        var patchNum = e.target.value;
+        var currentPatchNum =
+            this._change.revisions[this._change.current_revision]._number
+        if (patchNum == currentPatchNum) {
+          page.show(this._computeChangePath(this._changeNum));
+          return;
+        }
+        page.show(this._computeChangePath(this._changeNum) + '/' + patchNum);
+      },
+
       _handleReplySent: function(e) {
         this._reload();
       },
 
       _paramsChanged: function(value) {
-        this.changeNum = value.changeNum;
-        if (!this.changeNum) {
-          this.change = null;
-          this.comments = null;
+        this._changeNum = value.changeNum;
+        this._patchNum = value.patchNum;
+        if (!this._changeNum) {
+          this._change = null;
+          this._comments = null;
           return;
         }
         this._reload();
       },
 
+      _changeChanged: function(change) {
+        if (!change) { return; }
+        this._patchNum = this._patchNum ||
+            change.revisions[change.current_revision]._number;
+      },
+
       _computeChangePath: function(changeNum) {
         return '/c/' + changeNum;
       },
@@ -262,32 +312,22 @@ limitations under the License.
         return '/changes/' + changeNum + '/detail';
       },
 
-      _computeCommitInfoPath: function(changeNum, commitHash) {
-        return '/changes/' + changeNum + '/revisions/' + commitHash + '/commit';
+      _computeCommitInfoPath: function(changeNum, patchNum) {
+        return Changes.baseURL(changeNum, patchNum) + '/commit?links';
       },
 
       _computeCommentsPath: function(changeNum) {
         return '/changes/' + changeNum + '/comments';
       },
 
-      _computePatchNum: function(revision) {
-        return this.change && this.change.revisions[revision]._number;
-      },
-
       _computeDetailQueryParams: function() {
         var options = Changes.listChangesOptionsToHex(
-            Changes.ListChangesOption.CURRENT_REVISION,
-            Changes.ListChangesOption.CURRENT_COMMIT,
+            Changes.ListChangesOption.ALL_REVISIONS,
             Changes.ListChangesOption.CHANGE_ACTIONS
         );
         return { O: options };
       },
 
-      _computeCurrentRevisionMessage: function(change) {
-        return change &&
-            change.revisions[change.current_revision].commit.message;
-      },
-
       _computeReviewers: function(labels, owner) {
         var reviewers =
             (labels['Code-Review'] && labels['Code-Review'].all) || [];
@@ -297,6 +337,22 @@ limitations under the License.
         });
       },
 
+      _computeLatestPatchNum: function(change) {
+        return change.revisions[change.current_revision]._number;
+      },
+
+      _computeAllPatchSets: function(change) {
+        var patchNums = [];
+        for (var rev in change.revisions) {
+          patchNums.push(change.revisions[rev]._number);
+        }
+        return patchNums.sort();
+      },
+
+      _computePatchIndexIsSelected: function(index, patchNum) {
+         return this._allPatchSets[index] == patchNum;
+      },
+
       _handleKey: function(e) {
         if (util.shouldSupressKeyboardShortcut(e)) { return; }
         e.preventDefault();
@@ -311,9 +367,19 @@ limitations under the License.
       },
 
       _reload: function() {
-        this.$.detailXHR.generateRequest();
+        var detailCompletes = this.$.detailXHR.generateRequest().completes;
         this.$.commentsXHR.generateRequest();
-        this.$.fileList.reload();
+        var reloadCommitInfoAndFileList = function() {
+          this.$.commitInfoXHR.generateRequest();
+          this.$.fileList.reload();
+        }.bind(this);
+
+        if (this._patchNum) {
+          reloadCommitInfoAndFileList();
+        } else {
+          // The patch number is reliant on the change detail request.
+          detailCompletes.then(reloadCommitInfoAndFileList);
+        }
       },
 
     });
diff --git a/polygerrit-ui/app/elements/gr-file-list.html b/polygerrit-ui/app/elements/gr-file-list.html
index dcc96ac8ca..de04f559ba 100644
--- a/polygerrit-ui/app/elements/gr-file-list.html
+++ b/polygerrit-ui/app/elements/gr-file-list.html
@@ -49,10 +49,10 @@ limitations under the License.
       }
     </style>
     <gr-ajax id="filesXHR"
-        url="[[_computeFilesURL(changeNum, revision)]]"
+        url="[[_computeFilesURL(changeNum, patchNum)]]"
         on-response="_handleResponse"></gr-ajax>
     <gr-ajax id="draftsXHR"
-        url="[[_computeDraftsURL(changeNum, revision)]]"
+        url="[[_computeDraftsURL(changeNum, patchNum)]]"
         last-response="{{_drafts}}"></gr-ajax>
     </gr-ajax>
     <div class="tableContainer">
@@ -73,8 +73,8 @@ limitations under the License.
                  href$="[[_computeDiffURL(changeNum, patchNum, file.__path)]]">[[file.__path]]</a>
             </td>
             <td>
-              <span class="drafts">[[_computeDraftsString(_drafts, file.__path)]]</span>
-              <span class="comments">[[_computeCommentsString(comments, file.__path)]]</span>
+              <span class="drafts">[[_computeDraftsString(_drafts, patchNum, file.__path)]]</span>
+              <span class="comments">[[_computeCommentsString(comments, patchNum, file.__path)]]</span>
             </td>
             <td>
               +<span>[[file.lines_inserted]]</span> lines,
@@ -93,46 +93,46 @@ limitations under the License.
       is: 'gr-file-list',
 
       properties: {
-        patchNum: Number,
-        changeNum: Number,
-        revision: String,
+        patchNum: String,
+        changeNum: String,
         comments: Object,
 
         _drafts: Object,
       },
 
-      observers: [
-        '_changeNumOrRevisionChanged(changeNum, revision)',
-      ],
-
       reload: function() {
-        if (!!this.changeNum && !!this.revision) {
+        if (!!this.changeNum && !!this.patchNum) {
           this.$.filesXHR.generateRequest();
-          this.$.draftsXHR.generateRequest();
+          app.accountReady.then(function() {
+            if (!app.loggedIn) { return; }
+            this.$.draftsXHR.generateRequest();
+          }.bind(this));
         }
       },
 
-      _changeNumOrRevisionChanged: function(changeNum, revision) {
-        this.reload();
+      _computeFilesURL: function(changeNum, patchNum) {
+        return Changes.baseURL(changeNum, patchNum) + '/files';
       },
 
-      _computeFilesURL: function(changeNum, revision) {
-        return Changes.baseURL(changeNum, revision) + '/files';
-      },
-
-      _computeCommentsString: function(comments, path) {
-        var num = (comments[path] || []).length;
+      _computeCommentsString: function(comments, patchNum, path) {
+        var patchComments = (comments[path] || []).filter(function(c) {
+          return c.patch_set == patchNum;
+        });
+        var num = patchComments.length;
         if (num == 0) { return ''; }
         if (num == 1) { return '1 comment'; }
         if (num > 1) { return num + ' comments'; };
       },
 
-      _computeDraftsURL: function(changeNum, revision) {
-        return Changes.baseURL(changeNum, revision) + '/drafts';
+      _computeDraftsURL: function(changeNum, patchNum) {
+        return Changes.baseURL(changeNum, patchNum) + '/drafts';
       },
 
-      _computeDraftsString: function(drafts, path) {
-        var num = (drafts[path] || []).length;
+      _computeDraftsString: function(drafts, patchNum, path) {
+        var patchDrafts = (drafts[path] || []).filter(function(d) {
+          return d.patch_set == patchNum;
+        });
+        var num = patchDrafts.length;
         if (num == 0) { return ''; }
         if (num == 1) { return '1 draft'; }
         if (num > 1) { return num + ' drafts'; };
diff --git a/polygerrit-ui/app/scripts/app.js b/polygerrit-ui/app/scripts/app.js
index 3ceb90dc1d..2ab2c5c9ca 100644
--- a/polygerrit-ui/app/scripts/app.js
+++ b/polygerrit-ui/app/scripts/app.js
@@ -61,7 +61,7 @@ window.addEventListener('WebComponentsReady', function() {
     page.redirect('/c/' + ctx.params[0]);
   });
 
-  page('/c/:changeNum', function(data) {
+  page('/c/:changeNum/:patchNum?', function(data) {
     app.route = 'gr-change-view';
     app.params = data.params;
   });
diff --git a/polygerrit-ui/app/test/gr-change-view-test.html b/polygerrit-ui/app/test/gr-change-view-test.html
index 4cf1d4c18d..867156084f 100644
--- a/polygerrit-ui/app/test/gr-change-view-test.html
+++ b/polygerrit-ui/app/test/gr-change-view-test.html
@@ -37,9 +37,25 @@ limitations under the License.
 <script>
   suite('gr-change-view tests', function() {
     var element;
+    var server;
 
     setup(function() {
       element = fixture('basic');
+      server = sinon.fakeServer.create();
+      // Eat any requests made by elements in this suite.
+      server.respondWith(
+        'GET',
+        /\/changes\/(.*)/,
+        [
+          200,
+          { 'Content-Type': 'application/json' },
+          ')]}\'\n{}',
+        ]
+      );
+    });
+
+    teardown(function() {
+      server.restore();
     });
 
     test('keyboard shortcuts', function() {
@@ -57,5 +73,49 @@ limitations under the License.
       assert.isFalse(dropdownEl.opened);
     });
 
+    test('patch num change', function(done) {
+      element._changeNum = '42';
+      element._patchNum = 2;
+      element._change = {
+        revisions: {
+          rev2: { _number: 2 },
+          rev1: { _number: 1 },
+          rev3: { _number: 3 },
+        },
+        current_revision: 'rev3',
+      };
+      flushAsynchronousOperations();
+      var selectEl = element.$$('.header select');
+      assert.ok(selectEl);
+      var optionEls =
+          Polymer.dom(element.root).querySelectorAll('.header option');
+      assert.equal(optionEls.length, 3);
+      assert.isFalse(
+          element.$$('.header option[value="1"]').hasAttribute('selected'));
+      assert.isTrue(
+          element.$$('.header option[value="2"]').hasAttribute('selected'));
+      assert.isFalse(
+          element.$$('.header option[value="3"]').hasAttribute('selected'));
+
+      var showStub = sinon.stub(page, 'show');
+
+      var numEvents = 0;
+      selectEl.addEventListener('change', function(e) {
+        numEvents++;
+        if (numEvents == 1) {
+          assert(showStub.lastCall.calledWithExactly('/c/42/1'),
+              'Should navigate to /c/42/1');
+          selectEl.value = '3';
+          element.fire('change', {}, {node: selectEl});
+        } else if (numEvents == 2) {
+          assert(showStub.lastCall.calledWithExactly('/c/42'),
+              'Should navigate to /c/42');
+          showStub.restore();
+          done();
+        }
+      });
+      selectEl.value = '1';
+      element.fire('change', {}, {node: selectEl});
+    });
   });
 </script>
diff --git a/polygerrit-ui/app/test/gr-file-list-test.html b/polygerrit-ui/app/test/gr-file-list-test.html
new file mode 100644
index 0000000000..f474f15c35
--- /dev/null
+++ b/polygerrit-ui/app/test/gr-file-list-test.html
@@ -0,0 +1,126 @@
+<!DOCTYPE html>
+<!--
+Copyright (C) 2015 The Android Open Source Project
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+
+<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
+<title>gr-file-list</title>
+
+<script src="../../bower_components/webcomponentsjs/webcomponents.min.js"></script>
+<script src="../../bower_components/web-component-tester/browser.js"></script>
+<script src="../../bower_components/page/page.js"></script>
+<script src="../scripts/changes.js"></script>
+<script src="../scripts/fake-app.js"></script>
+<script src="../scripts/util.js"></script>
+
+<link rel="import" href="../../bower_components/iron-test-helpers/iron-test-helpers.html">
+<link rel="import" href="../elements/gr-file-list.html">
+
+<test-fixture id="basic">
+  <template>
+    <gr-file-list></gr-file-list>
+  </template>
+</test-fixture>
+
+<script>
+  suite('gr-file-list tests', function() {
+    var element;
+    var server;
+
+    setup(function() {
+      element = fixture('basic');
+      server = sinon.fakeServer.create();
+      server.respondWith(
+        'GET',
+        '/changes/42/revisions/1/files',
+        [
+          200,
+          { 'Content-Type': 'application/json' },
+          ')]}\'\n' +
+          JSON.stringify({
+            '/COMMIT_MSG': {
+              status: 'A',
+              lines_inserted: 9,
+              size_delta: 317,
+              size: 317
+            },
+            'myfile.txt': {
+              lines_inserted: 35,
+              size_delta: 1146,
+              size: 1167
+            }
+          }),
+        ]
+      );
+      server.respondWith(
+        'GET',
+        '/changes/42/revisions/2/files',
+        [
+          200,
+          { 'Content-Type': 'application/json' },
+          ')]}\'\n' +
+          JSON.stringify({
+            '/COMMIT_MSG': {
+              status: 'A',
+              lines_inserted: 9,
+              size_delta: 317,
+              size: 317
+            },
+            'myfile.txt': {
+              lines_inserted: 35,
+              size_delta: 1146,
+              size: 1167
+            },
+            'file_added_in_rev2.txt': {
+              lines_inserted: 98,
+              size_delta: 234,
+              size: 136
+            }
+          }),
+        ]
+      );
+    });
+
+    teardown(function() {
+      server.restore();
+    });
+
+    test('requests', function(done) {
+      element.changeNum = '42';
+      element.patchNum = '1';
+      element.reload();
+      server.respond();
+
+      element.async(function() {
+        var filenames = element.files.map(function(f) {
+          return f.__path;
+        });
+        assert.deepEqual(filenames, ['/COMMIT_MSG', 'myfile.txt']);
+
+        element.patchNum = '2';
+        element.reload();
+        server.respond();
+        element.async(function() {
+          filenames = element.files.map(function(f) {
+            return f.__path;
+          });
+          assert.deepEqual(filenames,
+              ['/COMMIT_MSG', 'file_added_in_rev2.txt', 'myfile.txt']);
+          done();
+        }, 1);
+      }, 1)
+    });
+  });
+</script>
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html
index 42cd24f7c0..231f960006 100644
--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -33,6 +33,7 @@ limitations under the License.
     'gr-diff-side-test.html',
     'gr-diff-test.html',
     'gr-diff-view-test.html',
+    'gr-file-list-test.html',
     'gr-reply-dropdown-test.html',
     'gr-search-bar-test.html',
   ].forEach(function(file) {