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 7e305bba6c..1658da8bce 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 @@ 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); }); 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 35c1885b37..bd0ef38c13 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 1ed59181bd..2a8cf63a0f 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 @@ 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); - }); - }); }); 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 e30c1fd36f..6a0cf8d840 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//[..]/. @@ -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 5ec81e5c22..fbd5b1b793 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 49e82d4d73..e9c5de850e 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 @@ 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({})); 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 956d0b3f4f..fe3646b59b 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} */ - _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 eac758fa57..54ec90e5ab 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 @@ 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'}); });