opendev/gerrit
Code Issues Proposed changes

Make file list URLs explicitly depend on patchNum

Browse Source 
The Polymer binding to generate diff URLs had depended on the patch
range object, but this would be created before the patch number was
known, leaving it undefined inside the range object. With this change,
the `_computeDiffURL` method explicitly depends on the range components,
rather than the object that houses them.

Bug: Issue 6978
Change-Id: I871a9f340057cc7db8eda143d7bf110ea955019b
This commit is contained in:
Wyatt Allen
2017-08-09 11:00:11 -07:00
parent 389d241c30
commit acc46838db
3 changed files with 19 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
View File

@@ -267,9 +267,9 @@ limitations under the License.
[[_computeFileStatus(file.status)]]
</div>
<span
data-url="[[_computeDiffURL(change, patchRange, file.__path)]]"
data-url="[[_computeDiffURL(change, patchRange.patchNum, patchRange.basePatchNum, file.__path)]]"
class$="[[_computePathClass(file.__path, _expandedFilePaths.*)]]">
<a href$="[[_computeDiffURL(change, patchRange, file.__path)]]">
<a href$="[[_computeDiffURL(change, patchRange.patchNum, patchRange.basePatchNum, file.__path)]]">
<span title$="[[_computeFileDisplayName(file.__path)]]"
class="fullFileName">
[[_computeFileDisplayName(file.__path)]]

5
polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
View File

@@ -662,9 +662,8 @@
return status || 'M';
},
_computeDiffURL(change, patchRange, path) {
return Gerrit.Nav.getUrlForDiff(change, path, patchRange.patchNum,
patchRange.basePatchNum);
_computeDiffURL(change, patchNum, basePatchNum, path) {
return Gerrit.Nav.getUrlForDiff(change, path, patchNum, basePatchNum);
},
_computeFileDisplayName(path) {

15
polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
View File

@@ -988,6 +988,21 @@ limitations under the License.
element.flushDebouncer('loading-change');
assert.isFalse(element.classList.contains('loading'));
});
test('no execute _computeDiffURL before patchNum is knwon', done => {
const urlStub = sandbox.stub(element, '_computeDiffURL');
element.change = {_number: 123};
element.patchRange = {patchNum: undefined, basePatchNum: 'PARENT'};
element._files = [{__path: 'foo/bar.cpp'}];
flush(() => {
assert.isFalse(urlStub.called);
element.set('patchRange.patchNum', 4);
flush(() => {
assert.isTrue(urlStub.called);
done();
});
});
});
});
suite('gr-file-list inline diff tests', () => {