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/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();
}
});
