Convert all PLUGIN_LIST routes to one unified regex pattern
Our goal is that all routes become regex patterns. At the moment some of
them are strings, which means that they will get special handling by
page.js, most importantly it will construct its own regex pattern for
tokens such as `:filter`. Apart from not wanting to deal with as few as
possible page.js specialities and trying to be consistent with all the
route patterns, these special page.js tokens also come along with a
fundamental problem: They use `[^/]+` and thus don't match `/`, which at
least for `repo` matching is a problem and creates extra complexity.
Release-Notes: skip
Change-Id: I109507a4dc84aacb1f98bf2f4db83780e335d644
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 8e386e5..a181348 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -192,10 +192,8 @@
PLUGINS: /^\/plugins\/(.+)$/,
- // Matches /admin/plugins[,<offset>][/].
- PLUGIN_LIST_OFFSET: /^\/admin\/plugins(,(\d+))?(\/)?$/,
- PLUGIN_LIST_FILTER: '/admin/plugins/q/filter::filter',
- PLUGIN_LIST_FILTER_OFFSET: '/admin/plugins/q/filter::filter,:offset',
+ // Matches /admin/plugins with optional filter and offset.
+ PLUGIN_LIST: /^\/admin\/plugins\/?(?:\/q\/filter:(.*?))?(?:,(\d+))?$/,
QUERY: /^\/q\/([^,]+)(,(\d+))?$/,
@@ -837,21 +835,7 @@
);
this.mapRoute(
- RoutePattern.PLUGIN_LIST_OFFSET,
- 'handlePluginListOffsetRoute',
- ctx => this.handlePluginListOffsetRoute(ctx),
- true
- );
-
- this.mapRoute(
- RoutePattern.PLUGIN_LIST_FILTER_OFFSET,
- 'handlePluginListFilterOffsetRoute',
- ctx => this.handlePluginListFilterOffsetRoute(ctx),
- true
- );
-
- this.mapRoute(
- RoutePattern.PLUGIN_LIST_FILTER,
+ RoutePattern.PLUGIN_LIST,
'handlePluginListFilterRoute',
ctx => this.handlePluginListFilterRoute(ctx),
true
@@ -1415,36 +1399,12 @@
this.redirect(ctx.path + ',general');
}
- handlePluginListOffsetRoute(ctx: PageContext) {
- const state: AdminViewState = {
- view: GerritView.ADMIN,
- adminView: AdminChildView.PLUGINS,
- offset: ctx.params[1] ?? '0',
- filter: null,
- };
- // Note that router model view must be updated before view models.
- this.setState(state);
- this.adminViewModel.setState(state);
- }
-
- handlePluginListFilterOffsetRoute(ctx: PageContext) {
- const state: AdminViewState = {
- view: GerritView.ADMIN,
- adminView: AdminChildView.PLUGINS,
- offset: ctx.params['offset'] ?? '0',
- filter: ctx.params['filter'],
- };
- // Note that router model view must be updated before view models.
- this.setState(state);
- this.adminViewModel.setState(state);
- }
-
handlePluginListFilterRoute(ctx: PageContext) {
const state: AdminViewState = {
view: GerritView.ADMIN,
adminView: AdminChildView.PLUGINS,
- offset: '0',
- filter: ctx.params['filter'] ?? null,
+ offset: ctx.params[1] ?? '0',
+ filter: ctx.params[0] ?? null,
};
// Note that router model view must be updated before view models.
this.setState(state);
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 d486f66..8019ac9 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
@@ -157,9 +157,7 @@
'handleGroupRoute',
'handleGroupSelfRedirectRoute',
'handleNewAgreementsRoute',
- 'handlePluginListFilterOffsetRoute',
'handlePluginListFilterRoute',
- 'handlePluginListOffsetRoute',
'handlePluginListRoute',
'handleRepoCommandsRoute',
'handleRepoEditFileRoute',
@@ -799,36 +797,33 @@
});
});
- suite('PLUGIN_LIST_*', () => {
- test('PLUGIN_LIST_OFFSET', async () => {
- // PLUGIN_LIST_OFFSET: /^\/admin\/plugins(,(\d+))?(\/)?$/,
- await checkUrlToState('/admin/plugins', {
- ...createAdminPluginsViewState(),
- });
- await checkUrlToState('/admin/plugins/', {
- ...createAdminPluginsViewState(),
- });
- await checkUrlToState('/admin/plugins,42', {
- ...createAdminPluginsViewState(),
- offset: '42',
- });
+ test('PLUGIN_LIST', async () => {
+ await checkUrlToState('/admin/plugins', {
+ ...createAdminPluginsViewState(),
});
-
- test('PLUGIN_LIST_FILTER_OFFSET', async () => {
- // PLUGIN_LIST_FILTER_OFFSET: '/admin/plugins/q/filter::filter,:offset',
- await checkUrlToState('/admin/plugins/q/filter:foo,42', {
- ...createAdminPluginsViewState(),
- offset: '42',
- filter: 'foo',
- });
+ await checkUrlToState('/admin/plugins/', {
+ ...createAdminPluginsViewState(),
});
-
- test('PLUGIN_LIST_FILTER', async () => {
- // PLUGIN_LIST_FILTER: '/admin/plugins/q/filter::filter',
- await checkUrlToState('/admin/plugins/q/filter:foo', {
- ...createAdminPluginsViewState(),
- filter: 'foo',
- });
+ await checkUrlToState('/admin/plugins,42', {
+ ...createAdminPluginsViewState(),
+ offset: '42',
+ });
+ await checkUrlToState('/admin/plugins/q/filter:foo', {
+ ...createAdminPluginsViewState(),
+ filter: 'foo',
+ });
+ await checkUrlToState('/admin/plugins/q/filter:foo%2F%20%2525%252F', {
+ ...createAdminPluginsViewState(),
+ filter: 'foo/ %/',
+ });
+ await checkUrlToState('/admin/plugins/q/filter:foo,42', {
+ ...createAdminPluginsViewState(),
+ offset: '42',
+ filter: 'foo',
+ });
+ await checkUrlToState('/admin/plugins/q/filter:foo,asdf', {
+ ...createAdminPluginsViewState(),
+ filter: 'foo,asdf',
});
});
