Merge "Navigate change legacy routes to current change route"
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index 4a6f009..8e70eb9 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -177,8 +177,8 @@
CHANGE_ID_QUERY: /^\/id\/(I[0-9a-f]{40})$/,
- // Matches /c/<changeNum>/[<basePatchNum>..][<patchNum>][/].
- CHANGE_LEGACY: /^\/c\/(\d+)\/?(((-?\d+|edit)(\.\.(\d+|edit))?))?\/?$/,
+ // Matches /c/<changeNum>/[*][/].
+ CHANGE_LEGACY: /^\/c\/(\d+)\/(.*)$/,
CHANGE_NUMBER_LEGACY: /^\/(\d+)\/?/,
// Matches
@@ -210,10 +210,6 @@
// Matches /c/<project>/+/<changeNum>/[<patchNum|edit>]/<path>,edit[#lineNum]
DIFF_EDIT: /^\/c\/(.+)\/\+\/(\d+)\/(\d+|edit)\/(.+),edit(#\d+)?$/,
- // Matches non-project-relative
- // /c/<changeNum>/[<basePatchNum>..]<patchNum>/<path>.
- DIFF_LEGACY: /^\/c\/(\d+)\/((-?\d+|edit)(\.\.(\d+|edit))?)\/(.+)/,
-
// Matches diff routes using @\d+ to specify a file name (whether or not
// the project name is included).
// eslint-disable-next-line max-len
@@ -287,15 +283,6 @@
type QueryStringItem = [string, string]; // [key, value]
-type GenerateUrlLegacyChangeViewParameters = Omit<
- GenerateUrlChangeViewParameters,
- 'project'
->;
-type GenerateUrlLegacyDiffViewParameters = Omit<
- GenerateUrlDiffViewParameters,
- 'project'
->;
-
interface PatchRangeParams {
patchNum?: PatchSetNum;
basePatchNum?: BasePatchSetNum;
@@ -679,37 +666,6 @@
}
/**
- * Given a set of params without a project, gets the project from the rest
- * API project lookup and then sets the app params.
- */
- _normalizeLegacyRouteParams(
- params: Readonly<
- | GenerateUrlLegacyChangeViewParameters
- | GenerateUrlLegacyDiffViewParameters
- >
- ) {
- if (!params.changeNum) {
- return Promise.resolve();
- }
-
- return this.restApiService
- .getFromProjectLookup(params.changeNum)
- .then(project => {
- // Show a 404 and terminate if the lookup request failed. Attempting
- // to redirect after failing to get the project loops infinitely.
- if (!project) {
- this._show404();
- return;
- }
- const updatedParams:
- | GenerateUrlChangeViewParameters
- | GenerateUrlDiffViewParameters = {...params, project};
- this._normalizePatchRangeParams(updatedParams);
- this._redirect(this._generateUrl(updatedParams));
- });
- }
-
- /**
* Normalizes the params object, and determines if the URL needs to be
* modified to fit the proper schema.
*
@@ -1100,8 +1056,6 @@
this._mapRoute(RoutePattern.CHANGE_LEGACY, '_handleChangeLegacyRoute');
- this._mapRoute(RoutePattern.DIFF_LEGACY, '_handleDiffLegacyRoute');
-
this._mapRoute(RoutePattern.AGREEMENTS, '_handleAgreementsRoute', true);
this._mapRoute(
@@ -1666,41 +1620,26 @@
}
_handleChangeLegacyRoute(ctx: PageContextWithQueryMap) {
- // Parameter order is based on the regex group number matched.
- const params: GenerateUrlLegacyChangeViewParameters = {
- changeNum: Number(ctx.params[0]) as NumericChangeId,
- basePatchNum: convertToPatchSetNum(ctx.params[3]) as BasePatchSetNum,
- patchNum: convertToPatchSetNum(ctx.params[5]),
- view: GerritView.CHANGE,
- querystring: ctx.querystring,
- };
-
- this._normalizeLegacyRouteParams(params);
+ const changeNum = Number(ctx.params[0]) as NumericChangeId;
+ if (!changeNum) {
+ this._show404();
+ return;
+ }
+ this.restApiService.getFromProjectLookup(changeNum).then(project => {
+ // Show a 404 and terminate if the lookup request failed. Attempting
+ // to redirect after failing to get the project loops infinitely.
+ if (!project) {
+ this._show404();
+ return;
+ }
+ this._redirect(`/c/${project}/+/${changeNum}/${ctx.params[1]}`);
+ });
}
_handleLegacyLinenum(ctx: PageContextWithQueryMap) {
this._redirect(ctx.path.replace(LEGACY_LINENUM_PATTERN, '#$1'));
}
- _handleDiffLegacyRoute(ctx: PageContextWithQueryMap) {
- // Parameter order is based on the regex group number matched.
- const params: GenerateUrlLegacyDiffViewParameters = {
- changeNum: Number(ctx.params[0]) as NumericChangeId,
- basePatchNum: convertToPatchSetNum(ctx.params[2]) as BasePatchSetNum,
- patchNum: convertToPatchSetNum(ctx.params[4]),
- path: ctx.params[5],
- view: GerritView.DIFF,
- };
-
- const address = this._parseLineAddress(ctx.hash);
- if (address) {
- params.leftSide = address.leftSide;
- params.lineNum = address.lineNum;
- }
-
- this._normalizeLegacyRouteParams(params);
- }
-
_handleDiffEditRoute(ctx: PageContextWithQueryMap) {
// Parameter order is based on the regex group number matched.
const project = ctx.params[0] as RepoName;
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
index 4c7855b..b91bf0c 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
@@ -192,7 +192,6 @@
'_handleDiffRoute',
'_handleDefaultRoute',
'_handleChangeLegacyRoute',
- '_handleDiffLegacyRoute',
'_handleDocumentationRedirectRoute',
'_handleDocumentationSearchRoute',
'_handleDocumentationSearchRedirectRoute',
@@ -536,66 +535,6 @@
});
suite('param normalization', () => {
- let projectLookupStub;
- let generateUrlStub;
-
- setup(() => {
- projectLookupStub = stubRestApi('getFromProjectLookup');
- generateUrlStub = sinon.stub(element, '_generateUrl');
- });
-
- suite('_normalizeLegacyRouteParams', () => {
- let rangeStub;
- let redirectStub;
- let show404Stub;
-
- setup(() => {
- rangeStub = sinon.stub(element, '_normalizePatchRangeParams')
- .returns(Promise.resolve());
- redirectStub = sinon.stub(element, '_redirect');
- show404Stub = sinon.stub(element, '_show404');
- });
-
- test('w/o changeNum', () => {
- projectLookupStub.returns(Promise.resolve('foo/bar'));
- const params = {};
- return element._normalizeLegacyRouteParams(params).then(() => {
- assert.isFalse(generateUrlStub.calledOnce);
- assert.isFalse(projectLookupStub.called);
- assert.isFalse(rangeStub.called);
- assert.isFalse(redirectStub.called);
- assert.isFalse(show404Stub.called);
- });
- });
-
- test('w/ changeNum', () => {
- projectLookupStub.returns(Promise.resolve('foo/bar'));
- const params = {changeNum: 1234};
-
- return element._normalizeLegacyRouteParams(params).then(() => {
- assert.isTrue(generateUrlStub.calledOnce);
- const updatedParams = generateUrlStub.lastCall.args[0];
- assert.isTrue(projectLookupStub.called);
- assert.isTrue(rangeStub.called);
- assert.equal(updatedParams.project, 'foo/bar');
- assert.isTrue(redirectStub.calledOnce);
- assert.isFalse(show404Stub.called);
- });
- });
-
- test('halts on project lookup failure', () => {
- projectLookupStub.returns(Promise.resolve(undefined));
- const params = {changeNum: 1234};
- return element._normalizeLegacyRouteParams(params).then(() => {
- assert.isFalse(generateUrlStub.calledOnce);
- assert.isTrue(projectLookupStub.called);
- assert.isFalse(rangeStub.called);
- assert.isFalse(redirectStub.called);
- assert.isTrue(show404Stub.calledOnce);
- });
- });
- });
-
suite('_normalizePatchRangeParams', () => {
test('range n..n normalizes to n', () => {
const params = {basePatchNum: 4, patchNum: 4};
@@ -1367,58 +1306,19 @@
assert.isTrue(redirectStub.calledWithExactly('/c/12345'));
});
- test('_handleChangeLegacyRoute', () => {
- const normalizeRouteStub = sinon.stub(element,
- '_normalizeLegacyRouteParams');
+ test('_handleChangeLegacyRoute', async () => {
+ stubRestApi('getFromProjectLookup').returns(Promise.resolve('project'));
const ctx = {
params: [
1234, // 0 Change number
- null, // 1 Unused
- null, // 2 Unused
- 6, // 3 Base patch number
- null, // 4 Unused
- 9, // 5 Patch number
+ 'comment/6789',
],
querystring: '',
};
element._handleChangeLegacyRoute(ctx);
- assert.isTrue(normalizeRouteStub.calledOnce);
- assert.deepEqual(normalizeRouteStub.lastCall.args[0], {
- changeNum: 1234,
- basePatchNum: 6,
- patchNum: 9,
- view: GerritView.CHANGE,
- querystring: '',
- });
- });
-
- test('_handleDiffLegacyRoute', () => {
- const normalizeRouteStub = sinon.stub(element,
- '_normalizeLegacyRouteParams');
- const ctx = {
- params: [
- 1234, // 0 Change number
- null, // 1 Unused
- 3, // 2 Base patch number
- null, // 3 Unused
- 8, // 4 Patch number
- 'foo/bar', // 5 Diff path
- ],
- path: '/c/1234/3..8/foo/bar',
- hash: 'b123',
- };
- element._handleDiffLegacyRoute(ctx);
- assert.isFalse(redirectStub.called);
- assert.isTrue(normalizeRouteStub.calledOnce);
- assert.deepEqual(normalizeRouteStub.lastCall.args[0], {
- changeNum: 1234,
- basePatchNum: 3,
- patchNum: 8,
- view: GerritView.DIFF,
- path: 'foo/bar',
- lineNum: 123,
- leftSide: true,
- });
+ await flush();
+ assert.isTrue(redirectStub.calledWithExactly('/c/project/+/1234' +
+ '/comment/6789'));
});
test('_handleLegacyLinenum w/ @321', () => {
