Remove custom URL parameter parsing in favor of URLSearchParams
Release-Notes: skip
Google-Bug-Id: b/244279450
Change-Id: Idd36303d079f17a9b82aab3e3e27ccfdc480e194
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 79b7d44..3912de7 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -79,6 +79,7 @@
} from '../../../models/views/documentation';
import {PluginViewModel, PluginViewState} from '../../../models/views/plugin';
import {SearchViewModel, SearchViewState} from '../../../models/views/search';
+import {DashboardSection} from '../../../utils/dashboard-util';
const RoutePattern = {
ROOT: '/',
@@ -245,16 +246,6 @@
const LINE_ADDRESS_PATTERN = /^([ab]?)(\d+)$/;
/**
- * Pattern to recognize '+' in url-encoded strings for replacement with ' '.
- */
-const PLUS_PATTERN = /\+/g;
-
-/**
- * Pattern to recognize leading '?' in window.location.search, for stripping.
- */
-const QUESTION_PATTERN = /^\?*/;
-
-/**
* GWT UI would use @\d+ at the end of a path to indicate linenum.
*/
const LEGACY_LINENUM_PATTERN = /@([ab]?\d+)$/;
@@ -277,8 +268,6 @@
});
})();
-type QueryStringItem = [string, string]; // [key, value]
-
export const routerToken = define<GrRouter>('router');
export class GrRouter implements Finalizable {
@@ -918,48 +907,6 @@
}
/**
- * Decode an application/x-www-form-urlencoded string.
- *
- * @param qs The application/x-www-form-urlencoded string.
- * @return The decoded string.
- */
- private decodeQueryString(qs: string) {
- return decodeURIComponent(qs.replace(PLUS_PATTERN, ' '));
- }
-
- /**
- * Parse a query string (e.g. window.location.search) into an array of
- * name/value pairs.
- *
- * @param qs The application/x-www-form-urlencoded query string.
- * @return An array of name/value pairs, where each
- * element is a 2-element array.
- */
- parseQueryString(qs: string): Array<QueryStringItem> {
- qs = qs.replace(QUESTION_PATTERN, '');
- if (!qs) {
- return [];
- }
- const params: Array<[string, string]> = [];
- qs.split('&').forEach(param => {
- const idx = param.indexOf('=');
- let name;
- let value;
- if (idx < 0) {
- name = this.decodeQueryString(param);
- value = '';
- } else {
- name = this.decodeQueryString(param.substring(0, idx));
- value = this.decodeQueryString(param.substring(idx + 1));
- }
- if (name) {
- params.push([name, value]);
- }
- });
- return params;
- }
-
- /**
* Handle dashboard routes. These may be user, or project dashboards.
*/
handleDashboardRoute(data: PageContext) {
@@ -984,62 +931,38 @@
});
}
- /**
- * Handle custom dashboard routes.
- *
- * @param qs Optional query string associated with the route.
- * If not given, window.location.search is used. (Used by tests).
- */
- handleCustomDashboardRoute(
- _: PageContext,
- qs: string = window.location.search
- ) {
- const queryParams = this.parseQueryString(qs);
- let title = 'Custom Dashboard';
- const titleParam = queryParams.find(
- elem => elem[0].toLowerCase() === 'title'
- );
- if (titleParam) {
- title = titleParam[1];
- }
- // Dashboards support a foreach param which adds a base query to any
- // additional query.
- const forEachParam = queryParams.find(
- elem => elem[0].toLowerCase() === 'foreach'
- );
- let forEachQuery: string | null = null;
- if (forEachParam) {
- forEachQuery = forEachParam[1];
- }
- const sectionParams = queryParams.filter(
- elem =>
- elem[0] &&
- elem[1] &&
- elem[0].toLowerCase() !== 'title' &&
- elem[0].toLowerCase() !== 'foreach'
- );
- const sections = sectionParams.map(elem => {
- const query = forEachQuery ? `${forEachQuery} ${elem[1]}` : elem[1];
- return {
- name: elem[0],
- query,
- };
- });
+ handleCustomDashboardRoute(ctx: PageContext) {
+ const queryParams = new URLSearchParams(ctx.querystring);
- if (sections.length > 0) {
- const state: DashboardViewState = {
- view: GerritView.DASHBOARD,
- user: 'self',
- sections,
- title,
- };
- this.setState(state);
- this.dashboardViewModel.updateState(state);
+ let title = 'Custom Dashboard';
+ const titleParam = queryParams.get('title');
+ if (titleParam) title = titleParam;
+ queryParams.delete('title');
+
+ let forEachQuery = '';
+ const forEachParam = queryParams.get('foreach');
+ if (forEachParam) forEachQuery = forEachParam + ' ';
+ queryParams.delete('foreach');
+
+ const sections: DashboardSection[] = [];
+ for (const [name, query] of queryParams) {
+ if (!name || !query) continue;
+ sections.push({name, query: `${forEachQuery}${query}`});
+ }
+
+ if (sections.length === 0) {
+ this.redirect('/dashboard/self');
return Promise.resolve();
}
- // Redirect /dashboard/ -> /dashboard/self.
- this.redirect('/dashboard/self');
+ const state: DashboardViewState = {
+ view: GerritView.DASHBOARD,
+ user: 'self',
+ sections,
+ title,
+ };
+ this.setState(state);
+ this.dashboardViewModel.updateState(state);
return Promise.resolve();
}
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
index 96078cc..a766176 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
@@ -630,12 +630,13 @@
});
test('no user specified', () => {
- const data = {
+ const data: PageContext = {
...createPageContext(),
canonicalPath: '/dashboard/',
params: {0: ''},
+ querystring: '',
};
- return router.handleCustomDashboardRoute(data, '').then(() => {
+ return router.handleCustomDashboardRoute(data).then(() => {
assert.isFalse(setStateStub.called);
assert.isTrue(redirectStub.called);
assert.equal(redirectStub.lastCall.args[0], '/dashboard/self');
@@ -643,68 +644,65 @@
});
test('custom dashboard without title', () => {
- const data = {
+ const data: PageContext = {
...createPageContext(),
canonicalPath: '/dashboard/',
params: {0: ''},
+ querystring: '?a=b&c&d=e',
};
- return router
- .handleCustomDashboardRoute(data, '?a=b&c&d=e')
- .then(() => {
- assert.isFalse(redirectStub.called);
- assert.isTrue(setStateStub.calledOnce);
- assert.deepEqual(setStateStub.lastCall.args[0], {
- view: GerritView.DASHBOARD,
- user: 'self',
- sections: [
- {name: 'a', query: 'b'},
- {name: 'd', query: 'e'},
- ],
- title: 'Custom Dashboard',
- });
+ return router.handleCustomDashboardRoute(data).then(() => {
+ assert.isFalse(redirectStub.called);
+ assert.isTrue(setStateStub.calledOnce);
+ assert.deepEqual(setStateStub.lastCall.args[0], {
+ view: GerritView.DASHBOARD,
+ user: 'self',
+ sections: [
+ {name: 'a', query: 'b'},
+ {name: 'd', query: 'e'},
+ ],
+ title: 'Custom Dashboard',
});
+ });
});
test('custom dashboard with title', () => {
- const data = {
+ const data: PageContext = {
...createPageContext(),
canonicalPath: '/dashboard/',
params: {0: ''},
+ querystring: '?a=b&c&d=&=e&title=t',
};
- return router
- .handleCustomDashboardRoute(data, '?a=b&c&d=&=e&title=t')
- .then(() => {
- assert.isFalse(redirectToLoginStub.called);
- assert.isFalse(redirectStub.called);
- assert.isTrue(setStateStub.calledOnce);
- assert.deepEqual(setStateStub.lastCall.args[0], {
- view: GerritView.DASHBOARD,
- user: 'self',
- sections: [{name: 'a', query: 'b'}],
- title: 't',
- });
+ return router.handleCustomDashboardRoute(data).then(() => {
+ assert.isFalse(redirectToLoginStub.called);
+ assert.isFalse(redirectStub.called);
+ assert.isTrue(setStateStub.calledOnce);
+ assert.deepEqual(setStateStub.lastCall.args[0], {
+ view: GerritView.DASHBOARD,
+ user: 'self',
+ sections: [{name: 'a', query: 'b'}],
+ title: 't',
});
+ });
});
test('custom dashboard with foreach', () => {
- const data = {
+ const data: PageContext = {
...createPageContext(),
canonicalPath: '/dashboard/',
params: {0: ''},
+ querystring: '?a=b&c&d=&=e&foreach=is:open',
};
- return router
- .handleCustomDashboardRoute(data, '?a=b&c&d=&=e&foreach=is:open')
- .then(() => {
- assert.isFalse(redirectToLoginStub.called);
- assert.isFalse(redirectStub.called);
- assert.isTrue(setStateStub.calledOnce);
- assert.deepEqual(setStateStub.lastCall.args[0], {
- view: GerritView.DASHBOARD,
- user: 'self',
- sections: [{name: 'a', query: 'is:open b'}],
- title: 'Custom Dashboard',
- });
+ return router.handleCustomDashboardRoute(data).then(() => {
+ assert.isFalse(redirectToLoginStub.called);
+ assert.isFalse(redirectStub.called);
+ assert.isTrue(setStateStub.calledOnce);
+ assert.deepEqual(setStateStub.lastCall.args[0], {
+ view: GerritView.DASHBOARD,
+ user: 'self',
+ sections: [{name: 'a', query: 'is:open b'}],
+ title: 'Custom Dashboard',
});
+ });
});
});
@@ -1336,35 +1334,4 @@
assert.isFalse(redirectStub.called);
});
});
-
- suite('parseQueryString', () => {
- test('empty queries', () => {
- assert.deepEqual(router.parseQueryString(''), []);
- assert.deepEqual(router.parseQueryString('?'), []);
- assert.deepEqual(router.parseQueryString('??'), []);
- assert.deepEqual(router.parseQueryString('&&&'), []);
- });
-
- test('url decoding', () => {
- assert.deepEqual(router.parseQueryString('+'), [[' ', '']]);
- assert.deepEqual(router.parseQueryString('???+%3d+'), [[' = ', '']]);
- assert.deepEqual(
- router.parseQueryString('%6e%61%6d%65=%76%61%6c%75%65'),
- [['name', 'value']]
- );
- });
-
- test('multiple parameters', () => {
- assert.deepEqual(router.parseQueryString('a=b&c=d&e=f'), [
- ['a', 'b'],
- ['c', 'd'],
- ['e', 'f'],
- ]);
- assert.deepEqual(router.parseQueryString('&a=b&&&e=f&c'), [
- ['a', 'b'],
- ['e', 'f'],
- ['c', ''],
- ]);
- });
- });
});
