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
diff --git 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
index 7ab23a1..8c6c216 100644
--- 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 @@
<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 @@
<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 @@
<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 @@
<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 @@
<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 @@
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]]"
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 c17c51c..a8012a0 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
@@ -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);
diff --git 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
index ee08ba1..7fcbd32 100644
--- 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 @@
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 @@
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 @@
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 @@
test('revision status draft', function() {
element._changeNum = '1';
- element._patchNum = 2;
+ element._patchRange = {
+ basePatchNum: 'PARENT',
+ patchNum: 1,
+ };
element._change = {
change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
revisions: {
diff --git 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
index f42cdb6..104b27f2 100644
--- 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 @@
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 @@
}
.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 @@
}
}
</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">▶</div>
diff --git 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
index de28184..47b6abd 100644
--- 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,
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.js b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
index abd2a2c..41bf824 100644
--- 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();
});
})();
diff --git a/polygerrit-ui/app/elements/gr-app.js b/polygerrit-ui/app/elements/gr-app.js
index 8897967..684120e 100644
--- 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,
