Merge "Centralize URL upgrading to within router"
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
index 7e305bb..1658da8 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -228,7 +228,7 @@
});
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); });
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index 35c1885..bd0ef38 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -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;
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index 1ed5918..2a8cf63 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -835,7 +835,6 @@
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 @@
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 @@
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 @@
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>
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.js b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
index e30c1fd..6a0cf8d 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
@@ -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 => {
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 5ec81e5..fbd5b1b 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
@@ -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);
},
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 49e82d4..e9c5de8 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
@@ -463,7 +463,6 @@
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 @@
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 @@
});
});
- 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({}));
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index 956d0b3..fe3646b 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -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 => {
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
index eac758f..54ec90e 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
@@ -269,7 +269,7 @@
});
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 @@
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 @@
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 @@
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'});
});
