opendev/gerrit
Code Issues Proposed changes

Handle cursor positions that don't yield addresses

Browse Source 
The gr-diff-cursor#getAddress method returns null when it has no
position or no address corresponds to it's position. However, the
gr-diff-view#_onLineSelected method did not account for this case and
attempted to use null addresses to construct URLs when moving the cursor
to a "File" line of a diff.

With this change, the diff view neither uses line number nor the diff
side when constructing URLs if the cursor does not yield an address.

Change-Id: I628658295bca1f49e0c2d3484e2e0d01e71bcd91
This commit is contained in:
Wyatt Allen
2017-09-15 11:55:35 -07:00
parent 695ae74ed1
commit 1bd466a24b
2 changed files with 17 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
View File

@@ -734,10 +734,11 @@
this.$.cursor.moveToLineNumber(detail.number, detail.side);
if (!this._change) { return; }
const cursorAddress = this.$.cursor.getAddress();
const number = cursorAddress ? cursorAddress.number : undefined;
const leftSide = cursorAddress ? cursorAddress.leftSide : undefined;
const url = Gerrit.Nav.getUrlForDiffById(this._changeNum,
this._change.project, this._path, this._patchRange.patchNum,
this._patchRange.basePatchNum, cursorAddress.number,
cursorAddress.leftSide);
this._patchRange.basePatchNum, number, leftSide);
history.replaceState(null, '', url);
},

14
polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
View File

@@ -659,6 +659,20 @@ limitations under the License.
assert.isTrue(getUrlStub.called);
});
test('_onLineSelected w/o line address', () => {
const getUrlStub = sandbox.stub(Gerrit.Nav, 'getUrlForDiffById');
sandbox.stub(history, 'replaceState');
sandbox.stub(element.$.cursor, 'moveToLineNumber');
sandbox.stub(element.$.cursor, 'getAddress').returns(null);
element._changeNum = 321;
element._change = {_number: 321, project: 'foo/bar'};
element._patchRange = {basePatchNum: '3', patchNum: '5'};
element._onLineSelected({}, {number: 123, side: 'right'});
assert.isTrue(getUrlStub.calledOnce);
assert.isUndefined(getUrlStub.lastCall.args[5]);
assert.isUndefined(getUrlStub.lastCall.args[6]);
});
test('_getDiffViewMode', () => {
// No user prefs or change view state set.
assert.equal(element._getDiffViewMode(), 'SIDE_BY_SIDE');