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>(