Update URL generation in gr-diff-view Bug: Issue 6446 Change-Id: I21be5ac4e3e89390745a6f04582cbfb9dc6535a3
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 627806c..c707af7 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
@@ -15,8 +15,6 @@ --> <link rel="import" href="../../../bower_components/polymer/polymer.html"> -<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html"> -<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html"> <link rel="import" href="../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html"> <link rel="import" href="../../../behaviors/rest-client-behavior/rest-client-behavior.html"> <link rel="import" href="../../../bower_components/iron-dropdown/iron-dropdown.html"> @@ -197,7 +195,7 @@ ready-for-measure="[[!_loading]]"> <header> <h3> - <a href$="[[_computeChangePath(_changeNum, _patchRange.*, _change.revisions)]]"> + <a href$="[[_computeChangePath(_change, _patchRange.*, _change.revisions)]]"> [[_changeNum]]</a><span>:</span> <span>[[_change.subject]]</span> <span class="dash">—</span> @@ -222,7 +220,7 @@ items="[[_fileList]]" as="path" initial-count="75"> - <a href$="[[_computeDiffURL(_changeNum, _patchRange.*, path)]]" + <a href$="[[_computeDiffURL(_change, _patchRange.*, path)]]" selected$="[[_computeFileSelected(path, _path)]]" data-key-nav$="[[_computeKeyNav(path, _path, _fileList)]]" on-tap="_handleFileTap">[[_computeFileDisplayName(path)]]</a> @@ -244,13 +242,16 @@ </h3> <div class="navLinks desktop"> <a class="navLink" - href$="[[_computeNavLinkURL(_path, _fileList, -1, 1)]]">Prev</a> + href$="[[_computeNavLinkURL(_change, _path, _fileList, -1, 1)]]"> + Prev</a> / <a class="navLink" - href$="[[_computeUpURL(_changeNum, _patchRange, _change, _change.revisions)]]">Up</a> + href$="[[_computeChangePath(_change, _patchRange.*, _change.revisions)]]"> + Up</a> / <a class="navLink" - href$="[[_computeNavLinkURL(_path, _fileList, 1, 1)]]">Next</a> + href$="[[_computeNavLinkURL(_change, _path, _fileList, 1, 1)]]"> + Next</a> </div> </header> <div class="subHeader"> @@ -295,11 +296,13 @@ </div> <div class="fileNav mobile"> <a class="mobileNavLink" - href$="[[_computeNavLinkURL(_path, _fileList, -1, 1)]]"><</a> + href$="[[_computeNavLinkURL(_change, _path, _fileList, -1, 1)]]"> + <</a> <div class="fullFileName mobile">[[_computeFileDisplayName(_path)]] </div> <a class="mobileNavLink" - href$="[[_computeNavLinkURL(_path, _fileList, 1, 1)]]">></a> + href$="[[_computeNavLinkURL(_change, _path, _fileList, 1, 1)]]"> + ></a> </div> </gr-fixed-panel> <div class="loading" hidden$="[[!_loading]]">Loading...</div>
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 c34f00f..9815983 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
@@ -112,10 +112,8 @@ }, behaviors: [ - Gerrit.BaseUrlBehavior, Gerrit.KeyboardShortcutBehavior, Gerrit.RESTClientBehavior, - Gerrit.URLEncodingBehavior, ], observers: [ @@ -256,15 +254,15 @@ _moveToPreviousFileWithComment() { if (this._commentSkips && this._commentSkips.previous) { - page.show(this._getDiffURL(this._changeNum, this._patchRange, - this._commentSkips.previous)); + Gerrit.Nav.navigateToDiff(this._change, this._commentSkips.previous, + this._patchRange.patchNum, this._patchRange.basePatchNum); } }, _moveToNextFileWithComment() { if (this._commentSkips && this._commentSkips.next) { - page.show(this._getDiffURL(this._changeNum, this._patchRange, - this._commentSkips.next)); + Gerrit.Nav.navigateToDiff(this._change, this._commentSkips.next, + this._patchRange.patchNum, this._patchRange.basePatchNum); } }, @@ -363,25 +361,26 @@ _navToChangeView() { if (!this._changeNum || !this._patchRange.patchNum) { return; } - - page.show(this._getChangePath( - this._changeNum, + this._navigateToChange( + this._change, this._patchRange, - this._change && this._change.revisions)); - }, - - _computeUpURL(changeNum, patchRange, change, changeRevisions) { - return this._getChangePath( - changeNum, - patchRange, - change && changeRevisions); + this._change && this._change.revisions); }, _navToFile(path, fileList, direction) { - const url = this._computeNavLinkURL(path, fileList, direction); - if (!url) { return; } + const newPath = this._getNavLinkPath(path, fileList, direction); + if (!newPath) { return; } - page.show(this._computeNavLinkURL(path, fileList, direction)); + if (newPath.up) { + this._navigateToChange( + this._change, + this._patchRange, + this._change && this._change.revisions); + return; + } + + Gerrit.Nav.navigateToDiff(this._change, newPath.path, + this._patchRange.patchNum, this._patchRange.basePatchNum); }, /** @@ -395,7 +394,38 @@ * @return {?string} The next URL when proceeding in the specified * direction. */ - _computeNavLinkURL(path, fileList, direction, opt_noUp) { + _computeNavLinkURL(change, path, fileList, direction, opt_noUp) { + const newPath = this._getNavLinkPath(path, fileList, direction, opt_noUp); + if (!newPath) { return null; } + + if (newPath.up) { + return this._getChangePath( + this._change, + this._patchRange, + this._change && this._change.revisions); + } + return this._getDiffUrl(this._change, this._patchRange, newPath.path); + }, + + /** + * Gives an object representing the target of navigating either left or + * right through the change. The resulting object will have one of the + * following forms: + * * {path: "<target file path>"} - When another file path should be the + * result of the navigation. + * * {up: true} - When the result of navigating should go back to the + * change view. + * * null - When no navigation is possible for the given direction. + * + * @param {?string} path The path of the current file being shown. + * @param {Array.<string>} fileList The list of files in this change and + * patch range. + * @param {number} direction Either 1 (next file) or -1 (prev file). + * @param {(number|boolean)} opt_noUp Whether to return to the change view + * when advancing the file goes outside the bounds of fileList. + * @return {Object} + */ + _getNavLinkPath(path, fileList, direction, opt_noUp) { if (!path || fileList.length === 0) { return null; } let idx = fileList.indexOf(path); @@ -403,7 +433,7 @@ const file = direction > 0 ? fileList[0] : fileList[fileList.length - 1]; - return this._getDiffURL(this._changeNum, this._patchRange, file); + return {path: file}; } idx += direction; @@ -411,12 +441,10 @@ // outside the bounds of [0, fileList.length). if (idx < 0 || idx > fileList.length - 1) { if (opt_noUp) { return null; } - return this._getChangePath( - this._changeNum, - this._patchRange, - this._change && this._change.revisions); + return {up: true}; } - return this._getDiffURL(this._changeNum, this._patchRange, fileList[idx]); + + return {path: fileList[idx]}; }, _paramsChanged(value) { @@ -506,13 +534,13 @@ this._fileList.indexOf(path)); }, - _getDiffURL(changeNum, patchRange, path) { - return this.getBaseUrl() + '/c/' + changeNum + '/' + - this._patchRangeStr(patchRange) + '/' + this.encodeURL(path, true); + _getDiffUrl(change, patchRange, path) { + return Gerrit.Nav.getUrlForDiff(change, path, patchRange.patchNum, + patchRange.basePatchNum); }, - _computeDiffURL(changeNum, patchRangeRecord, path) { - return this._getDiffURL(changeNum, patchRangeRecord.base, path); + _computeDiffURL(change, patchRangeRecord, path) { + return this._getDiffUrl(change, patchRangeRecord.base, path); }, _patchRangeStr(patchRange) { @@ -533,28 +561,39 @@ return patchNums.sort((a, b) => { return a - b; }); }, - _getChangePath(changeNum, patchRange, revisions) { - const base = this.getBaseUrl() + '/c/' + changeNum + '/'; - - // The change may not have loaded yet, making revisions unavailable. - if (!revisions) { - return base + this._patchRangeStr(patchRange); - } - + /** + * When the latest patch of the change is selected (and there is no base + * patch) then the patch range need not appear in the URL. Return a patch + * range object with undefined values when a range is not needed. + */ + _getChangeUrlRange(patchRange, revisions) { + let patchNum = undefined; + let basePatchNum = undefined; let latestPatchNum = -1; - for (const rev of Object.values(revisions)) { + for (const rev of Object.values(revisions || {})) { latestPatchNum = Math.max(latestPatchNum, rev._number); } if (patchRange.basePatchNum !== 'PARENT' || parseInt(patchRange.patchNum, 10) !== latestPatchNum) { - return base + this._patchRangeStr(patchRange); + patchNum = patchRange.patchNum; + basePatchNum = patchRange.basePatchNum; } - - return base; + return {patchNum, basePatchNum}; }, - _computeChangePath(changeNum, patchRangeRecord, revisions) { - return this._getChangePath(changeNum, patchRangeRecord.base, revisions); + _getChangePath(change, patchRange, revisions) { + const range = this._getChangeUrlRange(patchRange, revisions); + return Gerrit.Nav.getUrlForChange(change, range.patchNum, + range.basePatchNum); + }, + + _navigateToChange(change, patchRange, revisions) { + const range = this._getChangeUrlRange(patchRange, revisions); + Gerrit.Nav.navigateToChange(change, range.patchNum, range.basePatchNum); + }, + + _computeChangePath(change, patchRangeRecord, revisions) { + return this._getChangePath(change, patchRangeRecord.base, revisions); }, _computeFileDisplayName(path) { @@ -599,7 +638,8 @@ _handleMobileSelectChange(e) { const path = Polymer.dom(e).rootTarget.value; - page.show(this._getDiffURL(this._changeNum, this._patchRange, path)); + Gerrit.Nav.navigateToDiff(this._change, path, this._patchRange.patchNum, + this._patchRange.basePatchNum); }, _showDropdownTapHandler(e) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html index c9bd1f8..10be37a 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
@@ -45,6 +45,8 @@ let element; let sandbox; + const PARENT = 'PARENT'; + setup(() => { sandbox = sinon.sandbox.create(); @@ -73,10 +75,11 @@ test('keyboard shortcuts', () => { element._changeNum = '42'; element._patchRange = { - basePatchNum: 'PARENT', + basePatchNum: PARENT, patchNum: '10', }; element._change = { + _number: 42, revisions: { a: {_number: 10}, }, @@ -85,31 +88,33 @@ element._path = 'glados.txt'; element.changeViewState.selectedFileIndex = 1; - const showStub = sandbox.stub(page, 'show'); + const diffNavStub = sandbox.stub(Gerrit.Nav, 'navigateToDiff'); + const changeNavStub = sandbox.stub(Gerrit.Nav, 'navigateToChange'); + MockInteractions.pressAndReleaseKeyOn(element, 85, null, 'u'); - assert(showStub.lastCall.calledWithExactly('/c/42/'), + assert(changeNavStub.lastCall.calledWith(element._change), 'Should navigate to /c/42/'); MockInteractions.pressAndReleaseKeyOn(element, 221, null, ']'); - assert(showStub.lastCall.calledWithExactly('/c/42/10/wheatley.md'), - 'Should navigate to /c/42/10/wheatley.md'); + assert(diffNavStub.lastCall.calledWith(element._change, 'wheatley.md', + '10', PARENT), 'Should navigate to /c/42/10/wheatley.md'); element._path = 'wheatley.md'; assert.equal(element.changeViewState.selectedFileIndex, 2); MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); - assert(showStub.lastCall.calledWithExactly('/c/42/10/glados.txt'), - 'Should navigate to /c/42/10/glados.txt'); + assert(diffNavStub.lastCall.calledWith(element._change, 'glados.txt', + '10', PARENT), 'Should navigate to /c/42/10/glados.txt'); element._path = 'glados.txt'; assert.equal(element.changeViewState.selectedFileIndex, 1); MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); - assert(showStub.lastCall.calledWithExactly('/c/42/10/chell.go'), - 'Should navigate to /c/42/10/chell.go'); + assert(diffNavStub.lastCall.calledWith(element._change, 'chell.go', '10', + PARENT), 'Should navigate to /c/42/10/chell.go'); element._path = 'chell.go'; assert.equal(element.changeViewState.selectedFileIndex, 0); MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); - assert(showStub.lastCall.calledWithExactly('/c/42/'), + assert(changeNavStub.lastCall.calledWith(element._change), 'Should navigate to /c/42/'); assert.equal(element.changeViewState.selectedFileIndex, 0); @@ -155,6 +160,7 @@ patchNum: '10', }; element._change = { + _number: 42, revisions: { a: {_number: 10}, }, @@ -162,11 +168,12 @@ element._fileList = ['chell.go', 'glados.txt', 'wheatley.md']; element._path = 'glados.txt'; - const showStub = sandbox.stub(page, 'show'); + const diffNavStub = sandbox.stub(Gerrit.Nav, 'navigateToDiff'); + const changeNavStub = sandbox.stub(Gerrit.Nav, 'navigateToChange'); MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a'); - assert.isTrue(showStub.notCalled, 'The `a` keyboard shortcut should ' + - 'only work when the user is logged in.'); + assert.isTrue(changeNavStub.notCalled, 'The `a` keyboard shortcut ' + + 'should only work when the user is logged in.'); assert.isNull(window.sessionStorage.getItem( 'changeView.showReplyDialog')); @@ -174,40 +181,45 @@ MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a'); assert.isTrue(element.changeViewState.showReplyDialog); - assert(showStub.lastCall.calledWithExactly('/c/42/5..10'), - 'Should navigate to /c/42/5..10'); + assert(changeNavStub.lastCall.calledWithExactly(element._change, '10', + '5'), 'Should navigate to /c/42/5..10'); MockInteractions.pressAndReleaseKeyOn(element, 85, null, 'u'); - assert(showStub.lastCall.calledWithExactly('/c/42/5..10'), - 'Should navigate to /c/42/5..10'); + assert(changeNavStub.lastCall.calledWithExactly(element._change, '10', + '5'), 'Should navigate to /c/42/5..10'); MockInteractions.pressAndReleaseKeyOn(element, 221, null, ']'); - assert(showStub.lastCall.calledWithExactly('/c/42/5..10/wheatley.md'), + assert(diffNavStub.lastCall.calledWithExactly(element._change, + 'wheatley.md', '10', '5'), 'Should navigate to /c/42/5..10/wheatley.md'); element._path = 'wheatley.md'; MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); - assert(showStub.lastCall.calledWithExactly('/c/42/5..10/glados.txt'), + assert(diffNavStub.lastCall.calledWithExactly(element._change, + 'glados.txt', '10', '5'), 'Should navigate to /c/42/5..10/glados.txt'); element._path = 'glados.txt'; MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); - assert(showStub.lastCall.calledWithExactly('/c/42/5..10/chell.go'), + assert(diffNavStub.lastCall.calledWithExactly(element._change, 'chell.go', + '10', '5'), 'Should navigate to /c/42/5..10/chell.go'); element._path = 'chell.go'; MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); - assert(showStub.lastCall.calledWithExactly('/c/42/5..10'), + assert(changeNavStub.lastCall.calledWithExactly(element._change, '10', + '5'), 'Should navigate to /c/42/5..10'); }); test('keyboard shortcuts with old patch number', () => { element._changeNum = '42'; element._patchRange = { - basePatchNum: 'PARENT', + basePatchNum: PARENT, patchNum: '1', }; element._change = { + _number: 42, revisions: { a: {_number: 1}, b: {_number: 2}, @@ -216,11 +228,12 @@ element._fileList = ['chell.go', 'glados.txt', 'wheatley.md']; element._path = 'glados.txt'; - const showStub = sandbox.stub(page, 'show'); + const diffNavStub = sandbox.stub(Gerrit.Nav, 'navigateToDiff'); + const changeNavStub = sandbox.stub(Gerrit.Nav, 'navigateToChange'); MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a'); - assert.isTrue(showStub.notCalled, 'The `a` keyboard shortcut should ' + - 'only work when the user is logged in.'); + assert.isTrue(changeNavStub.notCalled, 'The `a` keyboard shortcut ' + + 'should only work when the user is logged in.'); assert.isNull(window.sessionStorage.getItem( 'changeView.showReplyDialog')); @@ -228,31 +241,33 @@ MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a'); assert.isTrue(element.changeViewState.showReplyDialog); - assert(showStub.lastCall.calledWithExactly('/c/42/1'), - 'Should navigate to /c/42/1'); + assert(changeNavStub.lastCall.calledWithExactly(element._change, '1', + PARENT), 'Should navigate to /c/42/1'); MockInteractions.pressAndReleaseKeyOn(element, 85, null, 'u'); - assert(showStub.lastCall.calledWithExactly('/c/42/1'), - 'Should navigate to /c/42/1'); + assert(changeNavStub.lastCall.calledWithExactly(element._change, '1', + PARENT), 'Should navigate to /c/42/1'); MockInteractions.pressAndReleaseKeyOn(element, 221, null, ']'); - assert(showStub.lastCall.calledWithExactly('/c/42/1/wheatley.md'), + assert(diffNavStub.lastCall.calledWithExactly(element._change, + 'wheatley.md', '1', PARENT), 'Should navigate to /c/42/1/wheatley.md'); element._path = 'wheatley.md'; MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); - assert(showStub.lastCall.calledWithExactly('/c/42/1/glados.txt'), + assert(diffNavStub.lastCall.calledWithExactly(element._change, + 'glados.txt', '1', PARENT), 'Should navigate to /c/42/1/glados.txt'); element._path = 'glados.txt'; MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); - assert(showStub.lastCall.calledWithExactly('/c/42/1/chell.go'), - 'Should navigate to /c/42/1/chell.go'); + assert(diffNavStub.lastCall.calledWithExactly(element._change, 'chell.go', + '1', PARENT), 'Should navigate to /c/42/1/chell.go'); element._path = 'chell.go'; MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); - assert(showStub.lastCall.calledWithExactly('/c/42/1'), - 'Should navigate to /c/42/1'); + assert(changeNavStub.lastCall.calledWithExactly(element._change, '1', + PARENT), 'Should navigate to /c/42/1'); }); test('Diff preferences hidden when no prefs or logged out', () => { @@ -287,148 +302,152 @@ assert.isTrue(overlayOpenStub.called); }); - test('go up to change via kb without change loaded', () => { - element._changeNum = '42'; - element._patchRange = { - basePatchNum: 'PARENT', - patchNum: '1', - }; + suite('url params', () => { + setup(() => { + sandbox.stub(Gerrit.Nav, 'getUrlForDiff', (c, p, pn, bpn) => { + return `${c._number}-${p}-${pn}-${bpn}`; + }); + sandbox.stub(Gerrit.Nav, 'getUrlForChange', (c, pn, bpn) => { + return `${c._number}-${pn}-${bpn}`; + }); + }); - element._fileList = ['chell.go', 'glados.txt', 'wheatley.md']; - element._path = 'glados.txt'; + test('jump to file dropdown', () => { + element._changeNum = '42'; + element._patchRange = { + basePatchNum: PARENT, + patchNum: '10', + }; + element._change = {_number: 42}; + element._fileList = ['chell.go', 'glados.txt', 'wheatley.md']; + element._path = 'glados.txt'; + flushAsynchronousOperations(); + const linkEls = + Polymer.dom(element.root).querySelectorAll('.dropdown-content > a'); + assert.equal(linkEls.length, 3); + assert.isFalse(linkEls[0].hasAttribute('selected')); + assert.isTrue(linkEls[1].hasAttribute('selected')); + assert.isFalse(linkEls[2].hasAttribute('selected')); + assert.equal(linkEls[0].getAttribute('data-key-nav'), '['); + assert.equal(linkEls[1].getAttribute('data-key-nav'), ''); + assert.equal(linkEls[2].getAttribute('data-key-nav'), ']'); + assert.equal(linkEls[0].getAttribute('href'), '42-chell.go-10-PARENT'); + assert.equal(linkEls[1].getAttribute('href'), + '42-glados.txt-10-PARENT'); + assert.equal(linkEls[2].getAttribute('href'), + '42-wheatley.md-10-PARENT'); - const showStub = sandbox.stub(page, 'show'); + assert.equal(element._computeFileDisplayName('/foo/bar/baz'), + '/foo/bar/baz'); + assert.equal(element._computeFileDisplayName('/COMMIT_MSG'), + 'Commit message'); + assert.equal(element._computeFileDisplayName('/MERGE_LIST'), + 'Merge list'); + }); - MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a'); - assert.isTrue(showStub.notCalled, 'The `a` keyboard shortcut should ' + - 'only work when the user is logged in.'); - assert.isNull(window.sessionStorage.getItem( - 'changeView.showReplyDialog')); + test('jump to file dropdown with patch range', () => { + element._changeNum = '42'; + element._patchRange = { + basePatchNum: '5', + patchNum: '10', + }; + element._change = {_number: 42}; + element._fileList = ['chell.go', 'glados.txt', 'wheatley.md']; + element._path = 'glados.txt'; + flushAsynchronousOperations(); + const linkEls = + Polymer.dom(element.root).querySelectorAll('.dropdown-content > a'); + assert.equal(linkEls.length, 3); + assert.isFalse(linkEls[0].hasAttribute('selected')); + assert.isTrue(linkEls[1].hasAttribute('selected')); + assert.isFalse(linkEls[2].hasAttribute('selected')); + assert.equal(linkEls[0].getAttribute('data-key-nav'), '['); + assert.equal(linkEls[1].getAttribute('data-key-nav'), ''); + assert.equal(linkEls[2].getAttribute('data-key-nav'), ']'); + assert.equal(linkEls[0].getAttribute('href'), '42-chell.go-10-5'); + assert.equal(linkEls[1].getAttribute('href'), '42-glados.txt-10-5'); + assert.equal(linkEls[2].getAttribute('href'), '42-wheatley.md-10-5'); + }); - element._loggedIn = true; - MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a'); - assert.isTrue(element.changeViewState.showReplyDialog); + test('prev/up/next links', () => { + element._changeNum = '42'; + element._patchRange = { + basePatchNum: PARENT, + patchNum: '10', + }; + element._change = { + _number: 42, + revisions: { + a: {_number: 10}, + }, + }; + element._fileList = ['chell.go', 'glados.txt', 'wheatley.md']; + element._path = 'glados.txt'; + flushAsynchronousOperations(); + const linkEls = Polymer.dom(element.root).querySelectorAll('.navLink'); + assert.equal(linkEls.length, 3); + assert.equal(linkEls[0].getAttribute('href'), '42-chell.go-10-PARENT'); + assert.equal(linkEls[1].getAttribute('href'), '42-undefined-undefined'); + assert.equal(linkEls[2].getAttribute('href'), + '42-wheatley.md-10-PARENT'); + element._path = 'wheatley.md'; + flushAsynchronousOperations(); + assert.equal(linkEls[0].getAttribute('href'), + '42-glados.txt-10-PARENT'); + assert.equal(linkEls[1].getAttribute('href'), '42-undefined-undefined'); + assert.isFalse(linkEls[2].hasAttribute('href')); + element._path = 'chell.go'; + flushAsynchronousOperations(); + assert.isFalse(linkEls[0].hasAttribute('href')); + assert.equal(linkEls[1].getAttribute('href'), '42-undefined-undefined'); + assert.equal(linkEls[2].getAttribute('href'), + '42-glados.txt-10-PARENT'); + element._path = 'not_a_real_file'; + flushAsynchronousOperations(); + assert.equal(linkEls[0].getAttribute('href'), + '42-wheatley.md-10-PARENT'); + assert.equal(linkEls[1].getAttribute('href'), '42-undefined-undefined'); + assert.equal(linkEls[2].getAttribute('href'), '42-chell.go-10-PARENT'); + }); - assert(showStub.lastCall.calledWithExactly('/c/42/1'), - 'Should navigate to /c/42/1'); - - MockInteractions.pressAndReleaseKeyOn(element, 85, null, 'u'); - assert(showStub.lastCall.calledWithExactly('/c/42/1'), - 'Should navigate to /c/42/1'); - - MockInteractions.pressAndReleaseKeyOn(element, 221, null, ']'); - assert(showStub.lastCall.calledWithExactly('/c/42/1/wheatley.md'), - 'Should navigate to /c/42/1/wheatley.md'); - element._path = 'wheatley.md'; - - MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); - assert(showStub.lastCall.calledWithExactly('/c/42/1/glados.txt'), - 'Should navigate to /c/42/1/glados.txt'); - element._path = 'glados.txt'; - - MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); - assert(showStub.lastCall.calledWithExactly('/c/42/1/chell.go'), - 'Should navigate to /c/42/1/chell.go'); - element._path = 'chell.go'; - - MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); - assert(showStub.lastCall.calledWithExactly('/c/42/1'), - 'Should navigate to /c/42/1'); - }); - - test('jump to file dropdown', () => { - element._changeNum = '42'; - element._patchRange = { - basePatchNum: 'PARENT', - patchNum: '10', - }; - element._fileList = ['chell.go', 'glados.txt', 'wheatley.md']; - element._path = 'glados.txt'; - flushAsynchronousOperations(); - const linkEls = - Polymer.dom(element.root).querySelectorAll('.dropdown-content > a'); - assert.equal(linkEls.length, 3); - assert.isFalse(linkEls[0].hasAttribute('selected')); - assert.isTrue(linkEls[1].hasAttribute('selected')); - assert.isFalse(linkEls[2].hasAttribute('selected')); - assert.equal(linkEls[0].getAttribute('data-key-nav'), '['); - assert.equal(linkEls[1].getAttribute('data-key-nav'), ''); - assert.equal(linkEls[2].getAttribute('data-key-nav'), ']'); - assert.equal(linkEls[0].getAttribute('href'), '/c/42/10/chell.go'); - assert.equal(linkEls[1].getAttribute('href'), '/c/42/10/glados.txt'); - assert.equal(linkEls[2].getAttribute('href'), '/c/42/10/wheatley.md'); - - assert.equal(element._computeFileDisplayName('/foo/bar/baz'), - '/foo/bar/baz'); - assert.equal(element._computeFileDisplayName('/COMMIT_MSG'), - 'Commit message'); - assert.equal(element._computeFileDisplayName('/MERGE_LIST'), - 'Merge list'); - }); - - test('jump to file dropdown with patch range', () => { - element._changeNum = '42'; - element._patchRange = { - basePatchNum: '5', - patchNum: '10', - }; - element._fileList = ['chell.go', 'glados.txt', 'wheatley.md']; - element._path = 'glados.txt'; - flushAsynchronousOperations(); - const linkEls = - Polymer.dom(element.root).querySelectorAll('.dropdown-content > a'); - assert.equal(linkEls.length, 3); - assert.isFalse(linkEls[0].hasAttribute('selected')); - assert.isTrue(linkEls[1].hasAttribute('selected')); - assert.isFalse(linkEls[2].hasAttribute('selected')); - assert.equal(linkEls[0].getAttribute('data-key-nav'), '['); - assert.equal(linkEls[1].getAttribute('data-key-nav'), ''); - assert.equal(linkEls[2].getAttribute('data-key-nav'), ']'); - assert.equal(linkEls[0].getAttribute('href'), '/c/42/5..10/chell.go'); - assert.equal(linkEls[1].getAttribute('href'), '/c/42/5..10/glados.txt'); - assert.equal(linkEls[2].getAttribute('href'), '/c/42/5..10/wheatley.md'); - }); - - test('prev/up/next links', () => { - element._changeNum = '42'; - element._patchRange = { - basePatchNum: 'PARENT', - patchNum: '10', - }; - element._change = { - revisions: { - a: {_number: 10}, - }, - }; - element._fileList = ['chell.go', 'glados.txt', 'wheatley.md']; - element._path = 'glados.txt'; - flushAsynchronousOperations(); - const linkEls = Polymer.dom(element.root).querySelectorAll('.navLink'); - assert.equal(linkEls.length, 3); - assert.equal(linkEls[0].getAttribute('href'), '/c/42/10/chell.go'); - assert.equal(linkEls[1].getAttribute('href'), '/c/42/'); - assert.equal(linkEls[2].getAttribute('href'), '/c/42/10/wheatley.md'); - element._path = 'wheatley.md'; - flushAsynchronousOperations(); - assert.equal(linkEls[0].getAttribute('href'), '/c/42/10/glados.txt'); - assert.equal(linkEls[1].getAttribute('href'), '/c/42/'); - assert.isFalse(linkEls[2].hasAttribute('href')); - element._path = 'chell.go'; - flushAsynchronousOperations(); - assert.isFalse(linkEls[0].hasAttribute('href')); - assert.equal(linkEls[1].getAttribute('href'), '/c/42/'); - assert.equal(linkEls[2].getAttribute('href'), '/c/42/10/glados.txt'); - element._path = 'not_a_real_file'; - flushAsynchronousOperations(); - assert.equal(linkEls[0].getAttribute('href'), '/c/42/10/wheatley.md'); - assert.equal(linkEls[1].getAttribute('href'), '/c/42/'); - assert.equal(linkEls[2].getAttribute('href'), '/c/42/10/chell.go'); + test('prev/up/next links with patch range', () => { + element._changeNum = '42'; + element._patchRange = { + basePatchNum: '5', + patchNum: '10', + }; + element._change = { + _number: 42, + revisions: { + a: {_number: 5}, + b: {_number: 10}, + }, + }; + element._fileList = ['chell.go', 'glados.txt', 'wheatley.md']; + element._path = 'glados.txt'; + flushAsynchronousOperations(); + const linkEls = Polymer.dom(element.root).querySelectorAll('.navLink'); + assert.equal(linkEls.length, 3); + assert.equal(linkEls[0].getAttribute('href'), '42-chell.go-10-5'); + assert.equal(linkEls[1].getAttribute('href'), '42-10-5'); + assert.equal(linkEls[2].getAttribute('href'), '42-wheatley.md-10-5'); + element._path = 'wheatley.md'; + flushAsynchronousOperations(); + assert.equal(linkEls[0].getAttribute('href'), '42-glados.txt-10-5'); + assert.equal(linkEls[1].getAttribute('href'), '42-10-5'); + assert.isFalse(linkEls[2].hasAttribute('href')); + element._path = 'chell.go'; + flushAsynchronousOperations(); + assert.isFalse(linkEls[0].hasAttribute('href')); + assert.equal(linkEls[1].getAttribute('href'), '42-10-5'); + assert.equal(linkEls[2].getAttribute('href'), '42-glados.txt-10-5'); + }); }); test('download link', () => { element._changeNum = '42'; element._patchRange = { - basePatchNum: 'PARENT', + basePatchNum: PARENT, patchNum: '10', }; element._fileList = ['chell.go', 'glados.txt', 'wheatley.md']; @@ -438,38 +457,6 @@ '/changes/42/revisions/10/patch?zip&path=glados.txt'); }); - test('prev/up/next links with patch range', () => { - element._changeNum = '42'; - element._patchRange = { - basePatchNum: '5', - patchNum: '10', - }; - element._change = { - revisions: { - a: {_number: 5}, - b: {_number: 10}, - }, - }; - element._fileList = ['chell.go', 'glados.txt', 'wheatley.md']; - element._path = 'glados.txt'; - flushAsynchronousOperations(); - const linkEls = Polymer.dom(element.root).querySelectorAll('.navLink'); - assert.equal(linkEls.length, 3); - assert.equal(linkEls[0].getAttribute('href'), '/c/42/5..10/chell.go'); - assert.equal(linkEls[1].getAttribute('href'), '/c/42/5..10'); - assert.equal(linkEls[2].getAttribute('href'), '/c/42/5..10/wheatley.md'); - element._path = 'wheatley.md'; - flushAsynchronousOperations(); - assert.equal(linkEls[0].getAttribute('href'), '/c/42/5..10/glados.txt'); - assert.equal(linkEls[1].getAttribute('href'), '/c/42/5..10'); - assert.isFalse(linkEls[2].hasAttribute('href')); - element._path = 'chell.go'; - flushAsynchronousOperations(); - assert.isFalse(linkEls[0].hasAttribute('href')); - assert.equal(linkEls[1].getAttribute('href'), '/c/42/5..10'); - assert.equal(linkEls[2].getAttribute('href'), '/c/42/5..10/glados.txt'); - }); - test('file review status', done => { stub('gr-rest-api-interface', { getDiffComments() { return Promise.resolve({}); }, @@ -608,14 +595,6 @@ assert.isTrue(replaceStateStub.called); }); - test('_getDiffURL encodes special characters', () => { - const changeNum = 123; - const patchRange = {basePatchNum: 123, patchNum: 456}; - const path = 'c++/cpp.cpp'; - assert.equal(element._getDiffURL(changeNum, patchRange, path), - '/c/123/123..456/c%252B%252B/cpp.cpp'); - }); - test('_getDiffViewMode', () => { // No user prefs or change view state set. assert.equal(element._getDiffViewMode(), 'SIDE_BY_SIDE'); @@ -665,7 +644,7 @@ stub('gr-rest-api-interface', { getDiffComments() { return Promise.resolve({ - 'path/to/file/one.cpp': [{patch_set: 'PARENT', message: 'lorem'}], + 'path/to/file/one.cpp': [{patch_set: PARENT, message: 'lorem'}], 'path-to/file/two.py': [{patch_set: 2, message: 'ipsum'}], }); },