Add tests for DOCUMENTATION_SEARCH routes and stop using page.js 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: I014e8a1d7fadbc80ad68d044da2010ac6caea713
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 e75bc32..8e386e5 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -104,6 +104,8 @@
import {Route, ViewState} from '../../../models/views/base';
import {Model} from '../../../models/model';
+// TODO: Move all patterns to view model files and use the `Route` interface,
+// which will enforce using `RegExp` in its `urlPattern` property.
const RoutePattern = {
ROOT: '/',
@@ -255,7 +257,7 @@
PLUGIN_SCREEN: /^\/x\/([\w-]+)\/([\w-]+)\/?/,
- DOCUMENTATION_SEARCH_FILTER: '/Documentation/q/filter::filter',
+ DOCUMENTATION_SEARCH_FILTER: /^\/Documentation\/q\/filter:(.*)$/,
DOCUMENTATION_SEARCH: /^\/Documentation\/q\/(.*)$/,
DOCUMENTATION: /^\/Documentation(\/)?(.+)?/,
};
@@ -1778,7 +1780,7 @@
handleDocumentationSearchRoute(ctx: PageContext) {
const state: DocumentationViewState = {
view: GerritView.DOCUMENTATION_SEARCH,
- filter: ctx.params['filter'] || null,
+ filter: ctx.params[0] ?? '',
};
// 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 35ce54c..d486f66 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
@@ -1030,5 +1030,40 @@
screen: 'bar',
});
});
+
+ test('DOCUMENTATION_SEARCH*', async () => {
+ // DOCUMENTATION_SEARCH_FILTER: '/Documentation/q/filter::filter',
+ // DOCUMENTATION_SEARCH: /^\/Documentation\/q\/(.*)$/,
+ await checkRedirect(
+ '/Documentation/q/asdf',
+ '/Documentation/q/filter:asdf'
+ );
+ await checkRedirect(
+ '/Documentation/q/as%20df',
+ '/Documentation/q/filter:as%20df'
+ );
+
+ await checkUrlToState('/Documentation/q/filter:', {
+ view: GerritView.DOCUMENTATION_SEARCH,
+ filter: '',
+ });
+ await checkUrlToState('/Documentation/q/filter:asdf', {
+ view: GerritView.DOCUMENTATION_SEARCH,
+ filter: 'asdf',
+ });
+ // Percent decoding works fine. page.js decodes twice, so the only problem
+ // is having `%25` in the URL, because the first decoding pass will yield
+ // `%`, and then the second decoding pass will throw `URI malformed`.
+ await checkUrlToState('/Documentation/q/filter:as%20%2fdf', {
+ view: GerritView.DOCUMENTATION_SEARCH,
+ filter: 'as /df',
+ });
+ // We accept and process double-encoded values, but only *require* it for
+ // the percent symbol `%`.
+ await checkUrlToState('/Documentation/q/filter:as%252f%2525df', {
+ view: GerritView.DOCUMENTATION_SEARCH,
+ filter: 'as/%df',
+ });
+ });
});
});
diff --git a/polygerrit-ui/app/models/views/documentation.ts b/polygerrit-ui/app/models/views/documentation.ts
index b564d64..6789695 100644
--- a/polygerrit-ui/app/models/views/documentation.ts
+++ b/polygerrit-ui/app/models/views/documentation.ts
@@ -10,7 +10,7 @@
export interface DocumentationViewState extends ViewState {
view: GerritView.DOCUMENTATION_SEARCH;
- filter?: string | null;
+ filter: string;
}
export const documentationViewModelToken = define<DocumentationViewModel>(
