opendev/gerrit
Code Issues Proposed changes

Merge "Centralize URL upgrading to within router"

Browse Source
This commit is contained in:
Kasper Nilsson
2017-08-15 21:24:03 +00:00
committed by Gerrit Code Review
parent be79d1ec62 eb7e7b80cf
commit 833e15263e
8 changed files with 25 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
View File

@@ -228,7 +228,7 @@ limitations under the License.
});
test('submit change', done => {
sandbox.stub(element.$.restAPI, '_getFromProjectLookup')
sandbox.stub(element.$.restAPI, 'getFromProjectLookup')
.returns(Promise.resolve('test'));
sandbox.stub(element, 'fetchIsLatestKnown',
() => { return Promise.resolve(true); });

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

@@ -958,7 +958,6 @@
if (!change) {
return '';
}
this._upgradeUrl(change, this.params);
this._processEdit(change, edit);
// Issue 4190: Coalesce missing topics to null.
if (!change.topic) { change.topic = null; }
@@ -995,13 +994,6 @@
});
},
_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 => {
this._comments = comments;

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

@@ -835,7 +835,6 @@ 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',
@@ -853,7 +852,6 @@ 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',
@@ -871,7 +869,6 @@ limitations under the License.
test('edit is added to change', () => {
sandbox.stub(element, '_changeChanged');
sandbox.stub(element, '_upgradeUrl');
sandbox.stub(element.$.restAPI, 'getChangeDetail', () => {
return Promise.resolve({
id: '123456789',
@@ -1333,31 +1330,5 @@ limitations under the License.
element._processEdit(mockChange = _.cloneDeep(change), edit);
assert.equal(element._patchRange.patchNum, 'baz');
});
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>

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

@@ -67,6 +67,21 @@
Gerrit.Nav.setup(url => { page.show(url); }, generateUrl, upgradeUrl);
/**
* Given a set of params without a project, gets the project from the rest
* API project lookup and then sets the app params.
*
* @param {?Object} params
*/
const normalizeLegacyRouteParams = params => {
if (!params.changeNum) { return; }
restAPI.getFromProjectLookup(params.changeNum).then(project => {
params.project = project;
normalizePatchRangeParams(params);
});
};
// Middleware
page((ctx, next) => {
document.body.scrollTop = 0;
@@ -464,7 +479,6 @@
};
normalizePatchRangeParams(params);
app.params = params;
upgradeUrl(params);
restAPI.setInProjectLookup(params.changeNum, params.project);
});
@@ -478,8 +492,7 @@
view: Gerrit.Nav.View.CHANGE,
};
normalizePatchRangeParams(params);
app.params = params;
normalizeLegacyRouteParams(params);
});
// Matches /c/<changeNum>/[<basePatchNum>..]<patchNum>/<path>.
@@ -502,8 +515,7 @@
view: Gerrit.Nav.View.DIFF,
};
normalizePatchRangeParams(params);
app.params = params;
normalizeLegacyRouteParams(params);
});
page(/^\/settings\/(agreements|new-agreement)/, loadUser, data => {

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

@@ -179,17 +179,9 @@
_getChangeDetail(changeNum) {
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}));
}
},
_getChangeEdit(changeNum) {
return this.$.restAPI.getChangeEdit(this._changeNum);
},

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

@@ -463,7 +463,6 @@ 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');
@@ -509,7 +508,6 @@ 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');
@@ -732,32 +730,6 @@ limitations under the License.
});
});
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);
});
});
test('_computeEditLoaded', () => {
const callCompute = range => element._computeEditLoaded({base: range});
assert.isFalse(callCompute({}));

4
polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
View File

@@ -1556,7 +1556,7 @@
// stack every time _changeBaseURL is called without a project.
const projectPromise = opt_project ?
Promise.resolve(opt_project) :
this._getFromProjectLookup(changeNum);
this.getFromProjectLookup(changeNum);
return projectPromise.then(project => {
let url = `/changes/${encodeURIComponent(project)}~${changeNum}`;
if (opt_patchNum) {
@@ -1732,7 +1732,7 @@
* @param {string|number} changeNum
* @return {!Promise<string|undefined>}
*/
_getFromProjectLookup(changeNum) {
getFromProjectLookup(changeNum) {
const project = this._projectLookup[changeNum];
if (project) { return Promise.resolve(project); }
return this.getChange(changeNum).then(change => {

10
polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
View File

@@ -269,7 +269,7 @@ limitations under the License.
});
test('differing patch diff comments are properly grouped', done => {
sandbox.stub(element, '_getFromProjectLookup')
sandbox.stub(element, 'getFromProjectLookup')
.returns(Promise.resolve('test'));
sandbox.stub(element, 'fetchJSON', url => {
if (url === '/changes/test~42/revisions/1') {
@@ -780,11 +780,11 @@ limitations under the License.
assert.deepEqual(element._projectLookup, {test: 'project'});
});
suite('_getFromProjectLookup', () => {
suite('getFromProjectLookup', () => {
test('getChange fails', () => {
sandbox.stub(element, 'getChange')
.returns(Promise.resolve());
return element._getFromProjectLookup().then(val => {
return element.getFromProjectLookup().then(val => {
assert.strictEqual(val, undefined);
assert.deepEqual(element._projectLookup, {});
});
@@ -793,7 +793,7 @@ limitations under the License.
test('getChange succeeds, no project', () => {
sandbox.stub(element, 'getChange')
.returns(Promise.resolve({}));
return element._getFromProjectLookup().then(val => {
return element.getFromProjectLookup().then(val => {
assert.strictEqual(val, undefined);
assert.deepEqual(element._projectLookup, {});
});
@@ -802,7 +802,7 @@ limitations under the License.
test('getChange succeeds with project', () => {
sandbox.stub(element, 'getChange')
.returns(Promise.resolve({project: 'project'}));
return element._getFromProjectLookup('test').then(val => {
return element.getFromProjectLookup('test').then(val => {
assert.equal(val, 'project');
assert.deepEqual(element._projectLookup, {test: 'project'});
});