Add support for patch range in change view

This is prep for adding support for diffing against a base patch set in the file list. Bug: Issue 3936 Change-Id: Ia8880846e8da3307772c222b762d4bfa6ec275f2
2016-05-10 15:59:55 -04:00
parent 38b4325867
commit bf25362a25
7 changed files with 116 additions and 50 deletions
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -220,7 +220,7 @@ limitations under the License.
            <gr-change-star change="{{_change}}" hidden$="[[!_loggedIn]]"></gr-change-star>
            <a href$="[[_computeChangePermalink(_change._number)]]">[[_change._number]]</a><span>:</span>
            <span>[[_change.subject]]</span>
-            <span class="changeStatus">[[_computeChangeStatus(_change, _patchNum)]]</span>
+            <span class="changeStatus">[[_computeChangeStatus(_change, _patchRange.patchNum)]]</span>
          </span>
          <span class="header-actions">
            <gr-button hidden
@@ -233,7 +233,7 @@ limitations under the License.
              <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)]]">
+                  <option value$="[[patchNumber]]" selected$="[[_computePatchIndexIsSelected(index, _patchRange.patchNum)]]">
                    <span>[[patchNumber]]</span>
                    /
                    <span>[[_computeLatestPatchNum(_change)]]</span>
@@ -253,7 +253,7 @@ limitations under the License.
          <gr-change-actions id="actions"
              actions="[[_change.actions]]"
              change-num="[[_changeNum]]"
-              patch-num="[[_patchNum]]"
+              patch-num="[[_patchRange.patchNum]]"
              commit-info="[[_commitInfo]]"
              on-reload-change="_handleReloadChange"></gr-change-actions>
        </div>
@@ -267,15 +267,16 @@ limitations under the License.
          <div class="relatedChanges">
            <gr-related-changes-list id="relatedChanges"
                change="[[_change]]"
-                patch-num="[[_patchNum]]"></gr-related-changes-list>
+                patch-num="[[_patchRange.patchNum]]"></gr-related-changes-list>
          </div>
        </div>
      </section>
      <gr-file-list id="fileList"
          change-num="[[_changeNum]]"
-          patch-num="[[_patchNum]]"
+          patch-num="{{_patchRange.patchNum}}"
          comments="[[_comments]]"
          drafts="[[_diffDrafts]]"
+          revisions="[[_change.revisions]]"
          selected-index="{{viewState.selectedFileIndex}}"></gr-file-list>
      <gr-messages-list id="messageList"
          change-num="[[_changeNum]]"
@@ -288,7 +289,7 @@ limitations under the License.
    <gr-overlay id="downloadOverlay" with-backdrop>
      <gr-download-dialog
          change="[[_change]]"
-          patch-num="[[_patchNum]]"
+          patch-num="[[_patchRange.patchNum]]"
          config="[[serverConfig.download]]"
          on-close="_handleDownloadDialogClose"></gr-download-dialog>
    </gr-overlay>
@@ -297,7 +298,7 @@ limitations under the License.
        with-backdrop>
      <gr-reply-dialog id="replyDialog"
          change-num="[[_changeNum]]"
-          patch-num="[[_patchNum]]"
+          patch-num="[[_patchRange.patchNum]]"
          labels="[[_change.labels]]"
          permitted-labels="[[_change.permitted_labels]]"
          diff-drafts="[[_diffDrafts]]"
--- 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
@@ -56,7 +56,7 @@
      _commitInfo: Object,
      _changeNum: String,
      _diffDrafts: Object,
-      _patchNum: String,
+      _patchRange: Object,
      _allPatchSets: {
        type: Array,
        computed: '_computeAllPatchSets(_change)',
@@ -174,13 +174,13 @@
      if (value.view != this.tagName.toLowerCase()) { return; }

      this._changeNum = value.changeNum;
-      this._patchNum = value.patchNum;
-      if (this.viewState.changeNum != this._changeNum ||
-          this.viewState.patchNum != this._patchNum) {
-        this.set('viewState.selectedFileIndex', 0);
-        this.set('viewState.changeNum', this._changeNum);
-        this.set('viewState.patchNum', this._patchNum);
-      }
+      this._patchRange = {
+        patchNum: value.patchNum,
+        basePatchNum: value.basePatchNum || 'PARENT',
+      };
+
+      this._resetStateIfNecessary();
+
      if (!this._changeNum) {
        return;
      }
@@ -207,15 +207,31 @@

        this.$.jsAPI.handleEvent(this.$.jsAPI.EventType.SHOW_CHANGE, {
          change: this._change,
-          patchNum: this._patchNum,
+          patchNum: this._patchRange.patchNum,
        });
      }.bind(this));
    },

+    _resetStateIfNecessary: function() {
+      // If the change number or patch range is different, then reset the
+      // selected file index.
+      if (this.viewState.changeNum !== this._changeNum ||
+          this.viewState.patchRange.basePatchNum !==
+              this._patchRange.basePatchNum ||
+          this.viewState.patchRange.patchNum !== this._patchRange.patchNum) {
+        this.set('viewState.selectedFileIndex', 0);
+        this.set('viewState.changeNum', this._changeNum);
+        this.set('viewState.patchRange', this._patchRange);
+      }
+    },
+
    _changeChanged: function(change) {
      if (!change) { return; }
-      this._patchNum = this._patchNum ||
-          change.revisions[change.current_revision]._number;
+      this.set('_patchRange.basePatchNum',
+          this._patchRange.basePatchNum || 'PARENT');
+      this.set('_patchRange.patchNum',
+          this._patchRange.patchNum ||
+              change.revisions[change.current_revision]._number);

      var title = change.subject + ' (' + change.change_id.substr(0, 9) + ')';
      this.fire('title-change', {title: title});
@@ -368,7 +384,7 @@

    _getCommitInfo: function() {
      return this.$.restAPI.getChangeCommitInfo(
-          this._changeNum, this._patchNum).then(
+          this._changeNum, this._patchRange.patchNum).then(
              function(commitInfo) {
                this._commitInfo = commitInfo;
              }.bind(this));
@@ -415,7 +431,7 @@

      this._resetHeaderEl();

-      if (this._patchNum) {
+      if (this._patchRange.patchNum) {
        return reloadPatchNumDependentResources().then(function() {
          return detailCompletes;
        }).then(reloadDetailDependentResources);
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -83,7 +83,10 @@ limitations under the License.

    test('patch num change', function(done) {
      element._changeNum = '42';
-      element._patchNum = 2;
+      element._patchRange = {
+        basePatchNum: 'PARENT',
+        patchNum: 2,
+      };
      element._change = {
        change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
        revisions: {
@@ -133,7 +136,10 @@ limitations under the License.

    test('change status new', function() {
      element._changeNum = '1';
-      element._patchNum = 1;
+      element._patchRange = {
+        basePatchNum: 'PARENT',
+        patchNum: 1,
+      };
      element._change = {
        change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
        revisions: {
@@ -149,7 +155,10 @@ limitations under the License.

    test('change status draft', function() {
      element._changeNum = '1';
-      element._patchNum = 1;
+      element._patchRange = {
+        basePatchNum: 'PARENT',
+        patchNum: 1,
+      };
      element._change = {
        change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
        revisions: {
@@ -165,7 +174,10 @@ limitations under the License.

    test('revision status draft', function() {
      element._changeNum = '1';
-      element._patchNum = 2;
+      element._patchRange = {
+        basePatchNum: 'PARENT',
+        patchNum: 1,
+      };
      element._change = {
        change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
        revisions: {
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -28,8 +28,14 @@ limitations under the License.
        display: flex;
        padding: .1em .25em;
      }
-      .header {
+      header {
+        display: flex;
        font-weight: bold;
+        justify-content: space-between;
+        margin-bottom: .5em;
+      }
+      header label {
+        font-weight: normal;
      }
      .positionIndicator,
      .reviewed,
@@ -59,20 +65,21 @@ limitations under the License.
      }
      .path {
        flex: 1;
-        overflow: hidden;
        padding-left: .35em;
        text-decoration: none;
-        text-overflow: ellipsis;
        white-space: nowrap;
      }
      .path:hover :first-child {
        text-decoration: underline;
      }
-      .oldPath {
-        color: #999;
+      .path,
+      .path div {
        overflow: hidden;
        text-overflow: ellipsis;
      }
+      .oldPath {
+        color: #999;
+      }
      .comments,
      .stats {
        text-align: right;
@@ -116,14 +123,13 @@ limitations under the License.
        }
      }
    </style>
-    <div class="row header">
-      <div class="positionIndicator"></div>
-      <div class="reviewed" hidden$="[[!_loggedIn]]" hidden></div>
-      <div class="status"></div>
-      <div class="path">Path</div>
-      <div class="comments">Comments</div>
-      <div class="stats">Stats</div>
-    </div>
+    <header>
+      <div>Files</div>
+      <label hidden>
+        Diff against
+        <select></select>
+      </label>
+    </header>
    <template is="dom-repeat" items="{{_files}}" as="file">
      <div class="row" selected$="[[_computeFileSelected(index, selectedIndex)]]">
        <div class="positionIndicator">&#x25b6;</div>
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -24,6 +24,7 @@
      changeNum: String,
      comments: Object,
      drafts: Object,
+      revisions: Object,
      selectedIndex: {
        type: Number,
        notify: true,
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
@@ -68,15 +68,37 @@
    page('/q/:query', queryHandler);

    page(/^\/(\d+)\/?/, function(ctx) {
-      page.redirect('/c/' + ctx.params[0]);
+      page.redirect('/c/' + encodeURIComponent(ctx.params[0]));
    });

-    page('/c/:changeNum/:patchNum?', function(data) {
-      data.params.view = 'gr-change-view';
-      app.params = data.params;
+    // Matches /c/<changeNum>/[<basePatchNum>..][<patchNum>].
+    page(/^\/c\/(\d+)\/?(((\d+)(\.\.(\d+))?))?$/, function(ctx) {
+      // Parameter order is based on the regex group number matched.
+      console.info(ctx)
+      var params = {
+        changeNum: ctx.params[0],
+        basePatchNum: ctx.params[3],
+        patchNum: ctx.params[5],
+        view: 'gr-change-view',
+      };
+
+      // Don't allow diffing the same patch number against itself.
+      if (params.basePatchNum != null &&
+          params.basePatchNum === params.patchNum) {
+        page.redirect('/c/' +
+            encodeURIComponent(params.changeNum) +
+            '/' +
+            encodeURIComponent(params.patchNum) +
+            '/');
+        return;
+      }
+      normalizePatchRangeParams(params);
+      app.params = params;
    });

+    // Matches /c/<changeNum>/[<basePatchNum>..]<patchNum>/<path>.
    page(/^\/c\/(\d+)\/((\d+)(\.\.(\d+))?)\/(.+)/, function(ctx) {
+      // Parameter order is based on the regex group number matched.
      var params = {
        changeNum: ctx.params[0],
        basePatchNum: ctx.params[2],
@@ -84,19 +106,27 @@
        path: ctx.params[5],
        view: 'gr-diff-view',
      };
-      // Don't allow diffing the same patch number against itself because WHY?
-      if (params.basePatchNum == params.patchNum) {
-        page.redirect('/c/' + params.changeNum + '/' + params.patchNum + '/' +
-            params.path);
+      // Don't allow diffing the same patch number against itself.
+      if (params.basePatchNum === params.patchNum) {
+        page.redirect('/c/' +
+            encodeURIComponent(params.changeNum) +
+            '/' +
+            encodeURIComponent(params.patchNum) +
+            '/' +
+            encodeURIComponent(params.path));
        return;
      }
-      if (!params.patchNum) {
-        params.patchNum = params.basePatchNum;
-        delete(params.basePatchNum);
-      }
+      normalizePatchRangeParams(params);
      app.params = params;
    });

+    function normalizePatchRangeParams(params) {
+      if (params.basePatchNum && !params.patchNum) {
+        params.patchNum = params.basePatchNum;
+        params.basePatchNum = null;
+      }
+    }
+
    page.start();
  });
 })();
--- a/polygerrit-ui/app/elements/gr-app.js
+++ b/polygerrit-ui/app/elements/gr-app.js
@@ -68,7 +68,7 @@
      this._viewState = {
        changeView: {
          changeNum: null,
-          patchNum: null,
+          patchRange: null,
          selectedFileIndex: 0,
          showReplyDialog: false,
          diffMode: null,