opendev/gerrit
Code Issues Proposed changes

Merge changes I7b542ddb,Ie9bea7f9

Browse Source 
* changes:
  Call Gerrit.Nav.upgradeUrl from valid views
  Add function upgradeUrl to gr-navigation
This commit is contained in:
Kasper Nilsson
2017-07-28 17:00:05 +00:00
committed by Gerrit Code Review
parent 81b9c31e21 f89608d903
commit 49e3d1f47f
6 changed files with 99 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
View File

@@ -908,11 +908,11 @@
_getChangeDetail() {
return this.$.restAPI.getChangeDetail(this._changeNum,
this._handleGetChangeDetailError.bind(this)).then(
change => {
this._handleGetChangeDetailError.bind(this)).then(change => {
if (!change) {
return '';
}
this._upgradeUrl(change, this.params);
// Issue 4190: Coalesce missing topics to null.
if (!change.topic) { change.topic = null; }
if (!change.reviewer_updates) {
@@ -945,6 +945,13 @@
});
},
_upgradeUrl(change, params) {
const project = change.project;
if (!params.project || project !== params.project) {
Gerrit.Nav.upgradeUrl(Object.assign({}, params, {project}));
}
},
_getComments() {
return this.$.restAPI.getDiffComments(this._changeNum).then(
comments => {

28
polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
View File

@@ -818,6 +818,7 @@ limitations under the License.
test('topic is coalesced to null', done => {
sandbox.stub(element, '_changeChanged');
sandbox.stub(element, '_upgradeUrl');
sandbox.stub(element.$.restAPI, 'getChangeDetail', () => {
return Promise.resolve({
id: '123456789',
@@ -835,6 +836,7 @@ limitations under the License.
test('commit sha is populated from getChangeDetail', done => {
sandbox.stub(element, '_changeChanged');
sandbox.stub(element, '_upgradeUrl');
sandbox.stub(element.$.restAPI, 'getChangeDetail', () => {
return Promise.resolve({
id: '123456789',
@@ -1236,5 +1238,31 @@ limitations under the License.
element.dispatchEvent(new CustomEvent('topic-changed'));
assert.isTrue(element.$.relatedChanges.reload.calledOnce);
});
suite('_upgradeUrl calls', () => {
let upgradeStub;
const mockChange = {project: 'test'};
setup(() => {
upgradeStub = sandbox.stub(window.Gerrit.Nav, 'upgradeUrl');
});
test('app.params.project undefined', () => {
element._upgradeUrl(mockChange, {});
assert.isTrue(upgradeStub.called);
assert.deepEqual(upgradeStub.lastCall.args[0], mockChange);
});
test('app.params.project differs from change.project', () => {
element._upgradeUrl(mockChange, {project: 'demo'});
assert.isTrue(upgradeStub.called);
assert.deepEqual(upgradeStub.lastCall.args[0], mockChange);
});
test('app.params.project === change.project', () => {
element._upgradeUrl(mockChange, {project: 'test'});
assert.isFalse(upgradeStub.called);
});
});
});
</script>

14
polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html
View File

@@ -72,6 +72,9 @@ limitations under the License.
/** @type {Function} */
_generateUrl: uninitialized,
/** @type {Function} */
_upgradeUrl: uninitialized,
_checkPatchRange(patchNum, basePatchNum) {
if (basePatchNum && !patchNum) {
throw new Error('Cannot use base patch number without patch number.');
@@ -80,17 +83,20 @@ limitations under the License.
/**
* Setup router implementation.
* @param {Function} handleNavigate
* @param {Function} navigate
* @param {Function} generateUrl
* @param {Function} upgradeUrl
*/
setup(navigate, generateUrl) {
setup(navigate, generateUrl, upgradeUrl) {
this._navigate = navigate;
this._generateUrl = generateUrl;
this._upgradeUrl = upgradeUrl;
},
destroy() {
this._navigate = uninitialized;
this._generateUrl = uninitialized;
this._upgradeUrl = uninitialized;
},
/**
@@ -229,6 +235,10 @@ limitations under the License.
}
this._navigate(relativeUrl);
},
upgradeUrl(params) {
this._upgradeUrl(params);
},
};
})(window);
</script>

15
polygerrit-ui/app/elements/core/gr-router/gr-router.js
View File

@@ -46,10 +46,18 @@
page.base(base);
}
const upgradeUrl = params => {
const url = generateUrl(params);
if (url !== window.location.pathname) {
history.replaceState(null, null, url);
app.params = params;
}
};
const restAPI = document.createElement('gr-rest-api-interface');
const reporting = getReporting();
Gerrit.Nav.setup(url => { page.show(url); }, generateUrl);
Gerrit.Nav.setup(url => { page.show(url); }, generateUrl, upgradeUrl);
// Middleware
page((ctx, next) => {
@@ -311,7 +319,7 @@
if (params.basePatchNum &&
patchNumEquals(params.basePatchNum, params.patchNum)) {
params.basePatchNum = null;
history.replaceState(null, null, generateUrl(params));
upgradeUrl(params);
} else if (params.basePatchNum && !params.patchNum) {
params.patchNum = params.basePatchNum;
params.basePatchNum = null;
@@ -333,10 +341,9 @@
path: ctx.params[8],
view: ctx.params[8] ? Gerrit.Nav.View.DIFF : Gerrit.Nav.View.CHANGE,
};
normalizePatchRangeParams(params);
app.params = params;
history.replaceState(null, null, generateUrl(params));
upgradeUrl(params);
});
// Matches /c/<changeNum>/[<basePatchNum>..][<patchNum>][/].

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

@@ -161,10 +161,17 @@
},
_getChangeDetail(changeNum) {
return this.$.restAPI.getDiffChangeDetail(changeNum).then(
change => {
this._change = change;
});
return this.$.restAPI.getDiffChangeDetail(changeNum).then(change => {
this._change = change;
this._upgradeUrl(change, this.params);
});
},
_upgradeUrl(change, params) {
const project = change.project;
if (!params.project || project !== params.project) {
Gerrit.Nav.upgradeUrl(Object.assign({}, params, {project}));
}
},
_getFiles(changeNum, patchRangeRecord) {

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

@@ -463,6 +463,7 @@ limitations under the License.
stub('gr-rest-api-interface', {
getDiffComments() { return Promise.resolve({}); },
});
sandbox.stub(element, '_upgradeUrl');
const saveReviewedStub = sandbox.stub(element, '_saveReviewedState',
() => Promise.resolve());
sandbox.stub(element.$.diff, 'reload');
@@ -497,6 +498,7 @@ limitations under the License.
stub('gr-rest-api-interface', {
getDiffComments() { return Promise.resolve({}); },
});
sandbox.stub(element, '_upgradeUrl');
sandbox.stub(element.$.diff, 'reload');
sandbox.stub(element, '_loadHash');
@@ -734,5 +736,31 @@ limitations under the License.
assert.isNull(result.next);
});
});
suite('_upgradeUrl calls', () => {
let upgradeStub;
const mockChange = {project: 'test'};
setup(() => {
upgradeStub = sandbox.stub(window.Gerrit.Nav, 'upgradeUrl');
});
test('app.params.project undefined', () => {
element._upgradeUrl(mockChange, {});
assert.isTrue(upgradeStub.called);
assert.deepEqual(upgradeStub.lastCall.args[0], mockChange);
});
test('app.params.project differs from change.project', () => {
element._upgradeUrl(mockChange, {project: 'demo'});
assert.isTrue(upgradeStub.called);
assert.deepEqual(upgradeStub.lastCall.args[0], mockChange);
});
test('app.params.project === change.project', () => {
element._upgradeUrl(mockChange, {project: 'test'});
assert.isFalse(upgradeStub.called);
});
});
});
</script>