Render change edits on change and diff screens
ORIGINALLY: Iafd4b80af53624027c347f0f443b4cdfde292e29
This change makes the change view and the diff view change edit aware.
Mutation operations on edit itself, like editing files in editor, are
beyond the scope of this change.
Change edits must be fetched with a separate change edit endpoint and
merged into the change.revisions object. The _number of an editInfo is
'edit'. It has a special property called 'basePatchNum' that marks
which patch set the edit is based on. In patch set selectors, the edit
is sorted right after its basePatchNum.
Alternative implementation considerations:
It could be easier to handle edits on the Polygerrit UI, if
GET /changes/<id>/detail endpoint
would optionally include the change edit in the resulting
change.revsions map.
TODOs:
* Overwrite change.current_revision with change edit commit in some
use cases
* Mark the change edit as Change Edit in the header of the change view
* Allow for modification of the edit in the change/diff view
* Disable commenting on files in edit patchsets
* Modify file list rows to have appropriate actions when edit is
selected (e.g. allow revert, disable marking as reviewed)
* Identify and whitelist valid change/revision actions for an edit
Bug: Issue 4437
Change-Id: Ia4690d20954de730cd625ac76920e849beb12f7c
diff --git a/polygerrit-ui/app/behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html b/polygerrit-ui/app/behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html
index ec7541b..139edf5 100644
--- a/polygerrit-ui/app/behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html
+++ b/polygerrit-ui/app/behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html
@@ -30,6 +30,8 @@
/** @polymerBehavior Gerrit.PatchSetBehavior */
const PatchSetBehavior = {
+ // Corresponds to the patchNum for an edit patchset.
+ EDIT_NAME: 'edit',
/**
* As patchNum can be either a string (e.g. 'edit', 'PARENT') OR a number,
@@ -58,6 +60,55 @@
},
/**
+ * Find change edit base revision if change edit exists.
+ *
+ * @param {!Array<!Object>} revisions The revisions array.
+ * @return {Object} change edit parent revision or null if change edit
+ * doesn't exist.
+ */
+ findEditParentRevision(revisions) {
+ const editInfo =
+ revisions.find(info => info._number === PatchSetBehavior.EDIT_NAME);
+
+ if (!editInfo) { return null; }
+
+ return revisions.find(info => info._number === editInfo.basePatchNum) ||
+ null;
+ },
+
+ /**
+ * Find change edit base patch set number if change edit exists.
+ *
+ * @param {!Array<!Object>} revisions The revisions array.
+ * @return {number} Change edit patch set number or -1.
+ */
+ findEditParentPatchNum(revisions) {
+ const revisionInfo = PatchSetBehavior.findEditParentRevision(revisions);
+ return revisionInfo ? revisionInfo._number : -1;
+ },
+
+ /**
+ * Sort given revisions array according to the patch set number. The sort
+ * algorithm is change edit aware. Change edit has patch set number equals
+ * 0, but must appear after the patch set it was based on. Example: change
+ * edit is based on patch set 2, and another patch set was uploaded after
+ * change edit creation, the sorted order should be: 1, 2, (0|edit), 3.
+ *
+ * @param {!Array<!Object>} revisions The revisions array
+ * @return {!Array<!Object>} The sorted {revisions} array
+ */
+ sortRevisions(revisions) {
+ const editParent = PatchSetBehavior.findEditParentPatchNum(revisions);
+ // Map a normal patchNum to 2 * (patchNum - 1) + 1... I.e. 1 -> 1,
+ // 2 -> 3, 3 -> 5, etc.
+ // Map an edit to the patchNum of parent*2... I.e. edit on 2 -> 4.
+ const num = r => r._number === PatchSetBehavior.EDIT_NAME ?
+ 2 * editParent :
+ 2 * (r._number - 1) + 1;
+ return revisions.sort((a, b) => num(a) - num(b));
+ },
+
+ /**
* Construct a chronological list of patch sets derived from change details.
* Each element of this list is an object with the following properties:
*
@@ -69,21 +120,26 @@
* property and its log of change messages.
*
* @param {Object} change The change details
- * @return {Array<Object>} Sorted list of patch set objects, as described
+ * @return {!Array<!Object>} Sorted list of patch set objects, as described
* above
*/
computeAllPatchSets(change) {
if (!change) { return []; }
- const patchNums = [];
- for (const commit in change.revisions) {
- if (change.revisions.hasOwnProperty(commit)) {
- patchNums.push({
- num: change.revisions[commit]._number,
- desc: change.revisions[commit].description,
- });
- }
+ let patchNums = [];
+ if (change.revisions &&
+ Object.keys(change.revisions).length) {
+ patchNums =
+ PatchSetBehavior.sortRevisions(Object.values(change.revisions))
+ .map(e => {
+ // TODO(kaspern): Mark which patchset an edit was made on, if an
+ // edit exists -- perhaps with a temporary description.
+ return {
+ num: e._number,
+ desc: e.description,
+ };
+ });
}
- patchNums.sort((a, b) => { return a.num - b.num; });
+ patchNums.sort((a, b) => a.num - b.num);
return PatchSetBehavior._computeWipForPatchSets(change, patchNums);
},
@@ -91,9 +147,9 @@
* Populate the wip properties of the given list of patch sets.
*
* @param {Object} change The change details
- * @param {Array<Object>} patchNums Sorted list of patch set objects, as
+ * @param {!Array<!Object>} patchNums Sorted list of patch set objects, as
* generated by computeAllPatchSets
- * @return {Array<Object>} The given list of patch set objects, with the
+ * @return {!Array<!Object>} The given list of patch set objects, with the
* wip property set on each of them
*/
_computeWipForPatchSets(change, patchNums) {
@@ -122,15 +178,20 @@
computeLatestPatchNum(allPatchSets) {
if (!allPatchSets || !allPatchSets.length) { return undefined; }
+ if (allPatchSets[allPatchSets.length - 1].num ===
+ PatchSetBehavior.EDIT_NAME) {
+ return allPatchSets[allPatchSets.length - 2].num;
+ }
return allPatchSets[allPatchSets.length - 1].num;
},
/**
* Check whether there is no newer patch than the latest patch that was
* available when this change was loaded.
- * @return {Promise} A promise that yields true if the latest patch has been
- * loaded, and false if a newer patch has been uploaded in the meantime.
- * The promise is rejected on network error.
+ *
+ * @return {Promise<boolean>} A promise that yields true if the latest patch
+ * has been loaded, and false if a newer patch has been uploaded in the
+ * meantime. The promise is rejected on network error.
*/
fetchIsLatestKnown(change, restAPI) {
const knownLatest = PatchSetBehavior.computeLatestPatchNum(
@@ -145,6 +206,18 @@
return actualLatest <= knownLatest;
});
},
+
+ /**
+ * @param {number|string} patchNum
+ * @param {!Array<!Object>} revisions A sorted array of revisions.
+ *
+ * @return {number} The index of the revision with the given patchNum.
+ */
+ findSortedIndex(patchNum, revisions) {
+ revisions = revisions || [];
+ const findNum = rev => rev._number + '' === patchNum + '';
+ return revisions.findIndex(findNum);
+ },
};
window.Gerrit = window.Gerrit || {};
diff --git a/polygerrit-ui/app/behaviors/gr-patch-set-behavior/gr-patch-set-behavior_test.html b/polygerrit-ui/app/behaviors/gr-patch-set-behavior/gr-patch-set-behavior_test.html
index 2a5caed..c2ccccd 100644
--- a/polygerrit-ui/app/behaviors/gr-patch-set-behavior/gr-patch-set-behavior_test.html
+++ b/polygerrit-ui/app/behaviors/gr-patch-set-behavior/gr-patch-set-behavior_test.html
@@ -173,5 +173,63 @@
assert.isTrue(equals('edit', 'edit'));
assert.isTrue(equals('PARENT', 'PARENT'));
});
+
+ test('findEditParentRevision', () => {
+ const findParent = Gerrit.PatchSetBehavior.findEditParentRevision;
+ let revisions = [
+ {_number: 0},
+ {_number: 1},
+ {_number: 2},
+ ];
+ assert.strictEqual(findParent(revisions), null);
+
+ revisions = [...revisions, {_number: 'edit', basePatchNum: 3}];
+ assert.strictEqual(findParent(revisions), null);
+
+ revisions = [...revisions, {_number: 3}];
+ assert.deepEqual(findParent(revisions), {_number: 3});
+ });
+
+ test('findEditParentPatchNum', () => {
+ const findNum = Gerrit.PatchSetBehavior.findEditParentPatchNum;
+ let revisions = [
+ {_number: 0},
+ {_number: 1},
+ {_number: 2},
+ ];
+ assert.equal(findNum(revisions), -1);
+
+ revisions =
+ [...revisions, {_number: 'edit', basePatchNum: 3}, {_number: 3}];
+ assert.deepEqual(findNum(revisions), 3);
+ });
+
+ test('sortRevisions', () => {
+ const sort = Gerrit.PatchSetBehavior.sortRevisions;
+ const revisions = [
+ {_number: 0},
+ {_number: 2},
+ {_number: 1},
+ ];
+ const sorted = [
+ {_number: 0},
+ {_number: 1},
+ {_number: 2},
+ ];
+
+ assert.deepEqual(sort(revisions), sorted);
+
+ // Edit patchset should follow directly after its basePatchNum.
+ revisions.push({_number: 'edit', basePatchNum: 2});
+ sorted.push({_number: 'edit', basePatchNum: 2});
+ assert.deepEqual(sort(revisions), sorted);
+
+ revisions[3].basePatchNum = 0;
+ const edit = sorted.pop();
+ edit.basePatchNum = 0;
+ // Edit patchset should be at index 1.
+ sorted.splice(1, 0, edit);
+ assert.deepEqual(sort(revisions), sorted);
+ });
});
</script>
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 cb304d8..4e0e07c 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
@@ -459,8 +459,9 @@
<select>
<template is="dom-repeat" items="[[_allPatchSets]]"
as="patchNum">
- <option value$="[[patchNum.num]]"
- disabled$="[[_computePatchSetDisabled(patchNum.num, _patchRange.basePatchNum)]]">
+ <option
+ value$="[[patchNum.num]]"
+ disabled$="[[_computePatchSetDisabled(patchNum.num, _patchRange.basePatchNum, _sortedRevisions)]]">
[[patchNum.num]]
/
[[computeLatestPatchNum(_allPatchSets)]]
@@ -511,7 +512,7 @@
patch-range="{{_patchRange}}"
comments="[[_comments]]"
drafts="[[_diffDrafts]]"
- revisions="[[_change.revisions]]"
+ revisions="[[_sortedRevisions]]"
project-config="[[_projectConfig]]"
selected-index="{{viewState.selectedFileIndex}}"
diff-view-mode="{{viewState.diffMode}}"
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 1a20e7c..8866ce5 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
@@ -181,6 +181,7 @@
value: true,
},
_updateCheckTimerHandle: Number,
+ _sortedRevisions: Array,
},
behaviors: [
@@ -195,6 +196,7 @@
observers: [
'_labelsChanged(_change.labels.*)',
'_paramsAndChangeChanged(params, _change)',
+ '_updateSortedRevisions(_change.revisions.*)',
],
keyBindings: {
@@ -243,6 +245,11 @@
}
},
+ _updateSortedRevisions(revisionsRecord) {
+ const revisions = revisionsRecord.base;
+ this._sortedRevisions = this.sortRevisions(Object.values(revisions));
+ },
+
_computePrefsButtonHidden(prefs, loggedIn) {
return !loggedIn || !prefs;
},
@@ -684,13 +691,15 @@
/**
* Determines if a patch number should be disabled based on value of the
* basePatchNum from gr-file-list.
- * @param {Number} patchNum Patch number available in dropdown
- * @param {Number|String} basePatchNum Base patch number from file list
- * @return {Boolean}
+ * @param {number} patchNum Patch number available in dropdown
+ * @param {number|string} basePatchNum Base patch number from file list
+ * @return {boolean}
*/
_computePatchSetDisabled(patchNum, basePatchNum) {
- basePatchNum = basePatchNum === 'PARENT' ? 0 : basePatchNum;
- return parseInt(patchNum, 10) <= parseInt(basePatchNum, 10);
+ if (basePatchNum === 'PARENT') { return false; }
+
+ return this.findSortedIndex(patchNum, this._sortedRevisions) <=
+ this.findSortedIndex(basePatchNum, this._sortedRevisions);
},
_computeLabelNames(labels) {
@@ -901,12 +910,24 @@
},
_getChangeDetail() {
- return this.$.restAPI.getChangeDetail(this._changeNum,
- this._handleGetChangeDetailError.bind(this)).then(change => {
+ const detailCompletes = this.$.restAPI.getChangeDetail(
+ this._changeNum, this._handleGetChangeDetailError.bind(this));
+ const editCompletes = this._getEdit();
+
+ return Promise.all([detailCompletes, editCompletes])
+ .then(([change, edit]) => {
if (!change) {
return '';
}
this._upgradeUrl(change, this.params);
+ if (edit) {
+ change.revisions[edit.commit.commit] = {
+ _number: this.EDIT_NAME,
+ basePatchNum: edit.base_patch_set_number,
+ commit: edit.commit,
+ fetch: edit.fetch,
+ };
+ }
// Issue 4190: Coalesce missing topics to null.
if (!change.topic) { change.topic = null; }
if (!change.reviewer_updates) {
@@ -952,6 +973,10 @@
});
},
+ _getEdit() {
+ return this.$.restAPI.getChangeEdit(this._changeNum, true);
+ },
+
_getLatestCommitMessage() {
return this.$.restAPI.getChangeCommitInfo(this._changeNum,
this.computeLatestPatchNum(this._allPatchSets)).then(commitInfo => {
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 31d1591..25f1192 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
@@ -58,6 +58,10 @@
});
suite('keyboard shortcuts', () => {
+ setup(() => {
+ sandbox.stub(element, '_updateSortedRevisions');
+ });
+
test('S should toggle the CL star', () => {
const starStub = sandbox.stub(element.$.changeStar, 'toggleStar');
MockInteractions.pressAndReleaseKeyOn(element, 83, null, 's');
@@ -233,6 +237,12 @@
});
test('_computePatchSetDisabled', () => {
+ element._sortedRevisions = [
+ {_number: 1},
+ {_number: 2},
+ {_number: element.EDIT_NAME, basePatchNum: 2},
+ {_number: 3},
+ ];
let basePatchNum = 'PARENT';
let patchNum = 1;
assert.equal(element._computePatchSetDisabled(patchNum, basePatchNum),
@@ -243,6 +253,12 @@
patchNum = 2;
assert.equal(element._computePatchSetDisabled(patchNum, basePatchNum),
false);
+ basePatchNum = element.EDIT_NAME;
+ assert.equal(element._computePatchSetDisabled(patchNum, basePatchNum),
+ true);
+ patchNum = '3';
+ assert.equal(element._computePatchSetDisabled(patchNum, basePatchNum),
+ false);
});
test('_prepareCommitMsgForLinkify', () => {
@@ -470,6 +486,7 @@
});
test('change num change', () => {
+ sandbox.stub(element, '_updateSortedRevisions');
element._changeNum = null;
element._patchRange = {
basePatchNum: 'PARENT',
@@ -852,6 +869,37 @@
});
});
+ test('edit is added to change', () => {
+ sandbox.stub(element, '_changeChanged');
+ sandbox.stub(element, '_upgradeUrl');
+ sandbox.stub(element.$.restAPI, 'getChangeDetail', () => {
+ return Promise.resolve({
+ id: '123456789',
+ labels: {},
+ current_revision: 'foo',
+ revisions: {foo: {commit: {}}},
+ });
+ });
+ sandbox.stub(element, '_getEdit', () => {
+ return Promise.resolve({
+ base_patch_set_number: 1,
+ commit: {commit: 'bar'},
+ });
+ });
+
+ return element._getChangeDetail().then(() => {
+ const revs = element._change.revisions;
+ assert.equal(Object.keys(revs).length, 2);
+ assert.deepEqual(revs['foo'], {commit: {commit: 'foo'}});
+ assert.deepEqual(revs['bar'], {
+ _number: element.EDIT_NAME,
+ basePatchNum: 1,
+ commit: {commit: 'bar'},
+ fetch: undefined,
+ });
+ });
+ });
+
test('reply dialog focus can be controlled', () => {
const FocusTarget = element.$.replyDialog.FocusTarget;
const openStub = sandbox.stub(element, '_openReplyDialog');
@@ -966,6 +1014,7 @@
suite('reply dialog tests', () => {
setup(() => {
sandbox.stub(element.$.replyDialog, '_draftChanged');
+ sandbox.stub(element, '_updateSortedRevisions');
sandbox.stub(element.$.replyDialog, 'fetchIsLatestKnown',
() => { return Promise.resolve(true); });
element._change = {labels: {}};
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 dc64042..8c58559 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
@@ -234,7 +234,7 @@
items="[[computeAllPatchSets(change)]]"
as="patchNum">
<option
- disabled$="[[_computePatchSetDisabled(patchNum.num, patchRange.patchNum)]]"
+ disabled$="[[_computePatchSetDisabled(patchNum.num, patchRange.patchNum, revisions)]]"
value$="[[patchNum.num]]">
[[patchNum.num]]
[[patchNum.desc]]
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 8560717..a7025f5 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
@@ -41,7 +41,8 @@
changeNum: String,
comments: Object,
drafts: Object,
- revisions: Object,
+ // Already sorted by the change-view.
+ revisions: Array,
projectConfig: Object,
selectedIndex: {
type: Number,
@@ -116,6 +117,7 @@
type: Boolean,
observer: '_loadingChanged',
},
+ _sortedRevisions: Array,
},
behaviors: [
@@ -231,7 +233,8 @@
},
_computePatchSetDisabled(patchNum, currentPatchNum) {
- return parseInt(patchNum, 10) >= parseInt(currentPatchNum, 10);
+ return this.findSortedIndex(patchNum, this.revisions) >=
+ this.findSortedIndex(currentPatchNum, this.revisions);
},
_togglePathExpanded(path) {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index 9be19a7..c9ad238 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -664,21 +664,29 @@
revisions: {
rev1: {_number: 1},
rev2: {_number: 2},
- rev3: {_number: 3},
+ rev3: {_number: 'edit', basePatchNum: 2},
+ rev4: {_number: 3},
},
};
+ element.revisions = [
+ {_number: 1},
+ {_number: 2},
+ {_number: 'edit', basePatchNum: 2},
+ {_number: 3},
+ ];
+
flush(() => {
const selectEl = element.$.patchChange;
assert.equal(selectEl.nativeSelect.value, 'PARENT');
assert.isTrue(element.$$('option[value="3"]').hasAttribute('disabled'));
selectEl.addEventListener('change', () => {
- assert.equal(selectEl.nativeSelect.value, '2');
- assert(navStub.lastCall.calledWithExactly(element.change, '3', '2'),
- 'Should navigate to /c/42/2..3');
+ assert.equal(selectEl.nativeSelect.value, 'edit');
+ assert(navStub.lastCall.calledWithExactly(element.change, '3', 'edit'),
+ 'Should navigate to /c/42/edit..3');
navStub.restore();
done();
});
- selectEl.nativeSelect.value = '2';
+ selectEl.nativeSelect.value = 'edit';
element.fire('change', {}, {node: selectEl.nativeSelect});
});
});
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 f290dd2..ef2d3aa 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
@@ -39,6 +39,7 @@
const encode = window.Gerrit.URLEncodingBehavior.encodeURL;
const patchNumEquals = window.Gerrit.PatchSetBehavior.patchNumEquals;
+ const EDIT_NAME = window.Gerrit.PatchSetBehavior.EDIT_NAME;
function startRouter(generateUrl) {
const base = window.Gerrit.BaseUrlBehavior.getBaseUrl();
@@ -419,18 +420,27 @@
if (params.basePatchNum &&
patchNumEquals(params.basePatchNum, params.patchNum)) {
params.basePatchNum = null;
- upgradeUrl(params);
} else if (params.basePatchNum && !params.patchNum) {
params.patchNum = params.basePatchNum;
params.basePatchNum = null;
}
+ // In GWTUI, edits are represented in URLs with either 0 or 'edit'.
+ // TODO(kaspern): Remove this normalization when GWT UI is gone.
+ if (patchNumEquals(params.basePatchNum, 0)) {
+ params.basePatchNum = EDIT_NAME;
+ }
+ if (patchNumEquals(params.patchNum, 0)) {
+ params.patchNum = EDIT_NAME;
+ }
+ upgradeUrl(params);
};
// Matches
- // /c/<project>/+/<changeNum>/[<basePatchNum>..][<patchNum>]/[path].
+ // /c/<project>/+/<changeNum>/[<basePatchNum|edit>..][<patchNum|edit>]/[path].
// TODO(kaspern): Migrate completely to project based URLs, with backwards
// compatibility for change-only.
- page(/^\/c\/(.+)\/\+\/(\d+)(\/?((\d+)(\.\.(\d+))?(\/(.+))?))?\/?$/,
+ // eslint-disable-next-line max-len
+ page(/^\/c\/(.+)\/\+\/(\d+)(\/?((\d+|edit)(\.\.(\d+|edit))?(\/(.+))?))?\/?$/,
ctx => {
// Parameter order is based on the regex group number matched.
const params = {
@@ -448,7 +458,7 @@
});
// Matches /c/<changeNum>/[<basePatchNum>..][<patchNum>][/].
- page(/^\/c\/(\d+)\/?(((\d+)(\.\.(\d+))?))?\/?$/, ctx => {
+ page(/^\/c\/(\d+)\/?(((\d+|edit)(\.\.(\d+|edit))?))?\/?$/, ctx => {
// Parameter order is based on the regex group number matched.
const params = {
changeNum: ctx.params[0],
@@ -462,7 +472,7 @@
});
// Matches /c/<changeNum>/[<basePatchNum>..]<patchNum>/<path>.
- page(/^\/c\/(\d+)\/((\d+)(\.\.(\d+))?)\/(.+)/, ctx => {
+ page(/^\/c\/(\d+)\/((\d+|edit)(\.\.(\d+|edit))?)\/(.+)/, ctx => {
// Check if path has an '@' which indicates it was using GWT style line
// numbers. Even if the filename had an '@' in it, it would have already
// been URI encoded. Redirect to hash version of path.
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index 70d19e5..2908f85 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -271,7 +271,7 @@
change-num="[[_changeNum]]"
patch-range="[[_patchRange]]"
files-weblinks="[[_filesWeblinks]]"
- available-patches="[[_computeAvailablePatches(_change.revisions)]]"
+ available-patches="[[_computeAvailablePatches(_change.revisions, _change.revisions.*)]]"
revisions="[[_change.revisions]]">
</gr-patch-range-select>
<span class="download desktop">
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index 5574fd6..0f6a07d 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -176,6 +176,10 @@
}
},
+ _getChangeEdit(changeNum) {
+ return this.$.restAPI.getChangeEdit(this._changeNum);
+ },
+
_getFiles(changeNum, patchRangeRecord) {
const patchRange = patchRangeRecord.base;
return this.$.restAPI.getChangeFilePathsAsSpeciallySortedArray(
@@ -495,7 +499,17 @@
promises.push(this._loadComments());
- Promise.all(promises).then(() => {
+ promises.push(this._getChangeEdit(this._changeNum));
+
+ Promise.all(promises).then(r => {
+ const edit = r[4];
+ if (edit) {
+ this.set('_change.revisions.' + edit.commit.commit, {
+ _number: this.EDIT_NAME,
+ basePatchNum: edit.base_patch_set_number,
+ commit: edit.commit,
+ });
+ }
this._loading = false;
this.$.diff.comments = this._commentsForDiff;
this.$.diff.reload();
@@ -562,13 +576,8 @@
return patchStr;
},
- _computeAvailablePatches(revisions) {
- const patchNums = [];
- if (!revisions) { return patchNums; }
- for (const rev of Object.values(revisions)) {
- patchNums.push(rev._number);
- }
- return patchNums.sort((a, b) => { return a - b; });
+ _computeAvailablePatches(revs) {
+ return this.sortRevisions(Object.values(revs)).map(e => e._number);
},
/**
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js
index e9437bd..9a4000f 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js
@@ -30,10 +30,13 @@
observer: '_updateSelected',
},
revisions: Object,
+ _sortedRevisions: Array,
_rightSelected: String,
_leftSelected: String,
},
+ observers: ['_updateSortedRevisions(revisions.*)'],
+
behaviors: [Gerrit.PatchSetBehavior],
_updateSelected() {
@@ -41,6 +44,11 @@
this._leftSelected = this.patchRange.basePatchNum;
},
+ _updateSortedRevisions(revisionsRecord) {
+ const revisions = revisionsRecord.base;
+ this._sortedRevisions = this.sortRevisions(Object.values(revisions));
+ },
+
_handlePatchChange(e) {
const leftPatch = this._leftSelected;
const rightPatch = this._rightSelected;
@@ -53,12 +61,15 @@
},
_computeLeftDisabled(patchNum, patchRange) {
- return parseInt(patchNum, 10) >= parseInt(patchRange.patchNum, 10);
+ return this.findSortedIndex(patchNum, this._sortedRevisions) >=
+ this.findSortedIndex(patchRange.patchNum, this._sortedRevisions);
},
_computeRightDisabled(patchNum, patchRange) {
if (patchRange.basePatchNum == 'PARENT') { return false; }
- return parseInt(patchNum, 10) <= parseInt(patchRange.basePatchNum, 10);
+
+ return this.findSortedIndex(patchNum, this._sortedRevisions) <=
+ this.findSortedIndex(patchRange.basePatchNum, this._sortedRevisions);
},
// On page load, the dom-if for options getting added occurs after
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.html b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.html
index abe3e5d..2fe5986 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.html
@@ -36,16 +36,26 @@
<script>
suite('gr-patch-range-select tests', () => {
let element;
+ let sandbox;
setup(() => {
element = fixture('basic');
+ sandbox = sinon.sandbox.create();
});
+ teardown(() => sandbox.restore());
+
test('enabled/disabled options', () => {
const patchRange = {
basePatchNum: 'PARENT',
patchNum: '3',
};
+ element._sortedRevisions = [
+ {_number: 1},
+ {_number: 2},
+ {_number: element.EDIT_NAME, basePatchNum: 2},
+ {_number: 3},
+ ];
for (const patchNum of ['1', '2', '3']) {
assert.isFalse(element._computeRightDisabled(patchNum, patchRange));
}
@@ -54,18 +64,21 @@
}
assert.isTrue(element._computeLeftDisabled('3', patchRange));
- patchRange.basePatchNum = '2';
+ patchRange.basePatchNum = element.EDIT_NAME;
assert.isTrue(element._computeLeftDisabled('3', patchRange));
assert.isTrue(element._computeRightDisabled('1', patchRange));
assert.isTrue(element._computeRightDisabled('2', patchRange));
assert.isFalse(element._computeRightDisabled('3', patchRange));
+ assert.isTrue(element._computeRightDisabled(element.EDIT_NAME, patchRange));
});
test('navigation', done => {
- const showStub = sinon.stub(page, 'show');
+ sandbox.stub(element, '_computeLeftDisabled').returns(false);
+ sandbox.stub(element, '_computeRightDisabled').returns(false);
+ const showStub = sandbox.stub(page, 'show');
const leftSelectEl = element.$.leftPatchSelect;
const rightSelectEl = element.$.rightPatchSelect;
- const blurSpy = sinon.spy(leftSelectEl, 'blur');
+ const blurSpy = sandbox.spy(leftSelectEl, 'blur');
element.changeNum = '42';
element.path = 'path/to/file.txt';
element.availablePatches = ['1', '2', '3'];
@@ -89,7 +102,6 @@
assert(showStub.lastCall.calledWithExactly(
'/c/42/1..3/path/to/file.txt'),
'Should navigate to /c/42/1..3/path/to/file.txt');
- showStub.restore();
done();
}
});
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 f20255b..d37c07b 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
@@ -694,10 +694,10 @@
return this.fetchJSON(
this.getChangeActionURL(changeNum, patchNum, '/actions')).then(
revisionActions => {
- // The rebase button on change screen is always enabled.
+ // The rebase button on change screen is always enabled.
if (revisionActions.rebase) {
revisionActions.rebase.rebaseOnCurrent =
- !!revisionActions.rebase.enabled;
+ !!revisionActions.rebase.enabled;
revisionActions.rebase.enabled = true;
}
return revisionActions;
@@ -899,6 +899,16 @@
.then(() => this.send('POST', url, review, opt_errFn, opt_ctx));
},
+ getChangeEdit(changeNum, opt_download_commands) {
+ return this.getLoggedIn().then(loggedIn => {
+ return loggedIn ?
+ this.fetchJSON(
+ this.getChangeActionURL(changeNum, null, '/edit/'), null, null,
+ opt_download_commands ? {'download-commands': true} : null) :
+ false;
+ });
+ },
+
getFileInChangeEdit(changeNum, path) {
return this.send('GET',
this.getChangeActionURL(changeNum, null,
