Merge changes I7ed36dd1,Idf9b07b7
* changes:
Rewrite router tests for GROUP_LIST in a more robust way
Let gr-router use a local instance of page.js instead of a global one
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 fac7a83..00cb938 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -307,6 +307,8 @@
private view?: GerritView;
+ readonly page = page.create();
+
constructor(
private readonly reporting: ReportingService,
private readonly routerModel: RouterModel,
@@ -343,7 +345,7 @@
}
if (browserUrl.toString() !== stateUrl.toString()) {
- page.replace(
+ this.page.replace(
stateUrl.toString(),
null,
/* init: */ false,
@@ -360,6 +362,7 @@
subscription.unsubscribe();
}
this.subscriptions = [];
+ this.page.stop();
}
start() {
@@ -402,7 +405,7 @@
redirect(url: string) {
this._isRedirecting = true;
- page.redirect(url);
+ this.page.redirect(url);
}
/**
@@ -431,7 +434,9 @@
*/
redirectToLogin(returnUrl: string) {
const basePath = getBaseUrl() || '';
- page('/login/' + encodeURIComponent(returnUrl.substring(basePath.length)));
+ this.page(
+ '/login/' + encodeURIComponent(returnUrl.substring(basePath.length))
+ );
}
/**
@@ -504,7 +509,7 @@
handler: (ctx: PageContext) => void,
authRedirect?: boolean
) {
- page(
+ this.page(
pattern,
(ctx, next) => this.loadUserMiddleware(ctx, next),
ctx => {
@@ -555,14 +560,14 @@
* page.show() eventually just calls `window.history.pushState()`.
*/
setUrl(url: string) {
- page.show(url);
+ this.page.show(url);
}
/**
* Navigate to this URL, but replace the current URL in the history instead of
* adding a new one (which is what `setUrl()` would do).
*
- * page.redirect() eventually just calls `window.history.replaceState()`.
+ * this.page.redirect() eventually just calls `window.history.replaceState()`.
*/
replaceUrl(url: string) {
this.redirect(url);
@@ -585,10 +590,10 @@
startRouter() {
const base = getBaseUrl();
if (base) {
- page.base(base);
+ this.page.base(base);
}
- page.exit('*', (_, next) => {
+ this.page.exit('*', (_, next) => {
if (!this._isRedirecting) {
this.reporting.beforeLocationChanged();
}
@@ -599,7 +604,7 @@
// Remove the tracking param 'usp' (User Source Parameter) from the URL,
// just to have users look at cleaner URLs.
- page((ctx, next) => {
+ this.page((ctx, next) => {
if (window.URLSearchParams) {
const pathname = toPathname(ctx.canonicalPath);
const searchParams = toSearchParams(ctx.canonicalPath);
@@ -615,7 +620,7 @@
});
// Middleware
- page((ctx, next) => {
+ this.page((ctx, next) => {
document.body.scrollTop = 0;
if (ctx.hash.match(RoutePattern.PLUGIN_SCREEN)) {
@@ -986,7 +991,7 @@
this.handleDefaultRoute()
);
- page.start();
+ this.page.start();
}
/**
@@ -1153,7 +1158,7 @@
const state: AdminViewState = {
view: GerritView.ADMIN,
adminView: AdminChildView.GROUPS,
- offset: ctx.params[1] || 0,
+ offset: ctx.params[1] ?? '0',
filter: null,
openCreateModal: ctx.hash === 'create',
};
@@ -1168,6 +1173,7 @@
adminView: AdminChildView.GROUPS,
offset: ctx.params['offset'],
filter: ctx.params['filter'],
+ openCreateModal: false,
};
// Note that router model view must be updated before view models.
this.setState(state);
@@ -1178,7 +1184,9 @@
const state: AdminViewState = {
view: GerritView.ADMIN,
adminView: AdminChildView.GROUPS,
+ offset: ctx.params[1] ?? '0',
filter: ctx.params['filter'] || null,
+ openCreateModal: false,
};
// 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 112c776..af44739 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
@@ -5,12 +5,13 @@
*/
import '../../../test/common-test-setup';
import './gr-router';
-import {page, PageContext} from '../../../utils/page-wrapper-utils';
+import {Page, PageContext} from '../../../utils/page-wrapper-utils';
import {
stubBaseUrl,
stubRestApi,
addListenerForTest,
waitEventLoop,
+ waitUntilCalled,
} from '../../../test/test-utils';
import {GrRouter, routerToken, _testOnly_RoutePattern} from './gr-router';
import {GerritView} from '../../../services/router/router-model';
@@ -25,7 +26,7 @@
} from '../../../types/common';
import {AppElementParams} from '../../gr-app-types';
import {assert} from '@open-wc/testing';
-import {AdminChildView} from '../../../models/views/admin';
+import {AdminChildView, AdminViewState} from '../../../models/views/admin';
import {RepoDetailView} from '../../../models/views/repo';
import {GroupDetailView} from '../../../models/views/group';
import {ChangeChildView, ChangeViewState} from '../../../models/views/change';
@@ -38,12 +39,15 @@
createRevision,
} from '../../../test/test-data-generators';
import {ParsedChangeInfo} from '../../../types/types';
+import {ViewState} from '../../../models/views/base';
suite('gr-router tests', () => {
let router: GrRouter;
+ let page: Page;
setup(() => {
router = testResolver(routerToken);
+ page = router.page;
});
test('getHashFromCanonicalPath', () => {
@@ -289,6 +293,19 @@
assert.deepEqual(setStateStub.lastCall.args[0], params);
}
+ async function checkUrlToState<T extends ViewState>(url: string, state: T) {
+ setStateStub.reset();
+ router.page.show(url);
+ await waitUntilCalled(setStateStub, 'setState');
+ assert.deepEqual(setStateStub.lastCall.firstArg, state);
+ }
+
+ async function checkUrlNotMatched(url: string) {
+ handlePassThroughRoute.reset();
+ router.page.show(url);
+ await waitUntilCalled(handlePassThroughRoute, 'handlePassThroughRoute');
+ }
+
function createPageContext(): PageContext {
return {
canonicalPath: '',
@@ -304,6 +321,7 @@
redirectStub = sinon.stub(router, 'redirect');
setStateStub = sinon.stub(router, 'setState');
handlePassThroughRoute = sinon.stub(router, 'handlePassThroughRoute');
+ router.startRouter();
});
test('handleLegacyProjectDashboardRoute', () => {
@@ -735,55 +753,55 @@
});
});
- test('handleGroupListOffsetRoute', () => {
- const ctx = createPageContext();
- assertctxToParams(ctx, 'handleGroupListOffsetRoute', {
+ test('list of groups', async () => {
+ const defaultState: AdminViewState = {
view: GerritView.ADMIN,
adminView: AdminChildView.GROUPS,
- offset: 0,
- filter: null,
+ offset: '0',
openCreateModal: false,
- });
-
- ctx.params[1] = '42';
- assertctxToParams(ctx, 'handleGroupListOffsetRoute', {
- view: GerritView.ADMIN,
- adminView: AdminChildView.GROUPS,
- offset: '42',
filter: null,
- openCreateModal: false,
- });
+ };
- ctx.hash = 'create';
- assertctxToParams(ctx, 'handleGroupListOffsetRoute', {
- view: GerritView.ADMIN,
- adminView: AdminChildView.GROUPS,
- offset: '42',
- filter: null,
+ await checkUrlToState('/admin/groups', defaultState);
+ await checkUrlToState('/admin/groups/', defaultState);
+ await checkUrlToState('/admin/groups#create', {
+ ...defaultState,
openCreateModal: true,
});
- });
-
- test('handleGroupListFilterOffsetRoute', () => {
- const ctx = {
- ...createPageContext(),
- params: {filter: 'foo', offset: '42'},
- };
- assertctxToParams(ctx, 'handleGroupListFilterOffsetRoute', {
- view: GerritView.ADMIN,
- adminView: AdminChildView.GROUPS,
- offset: '42',
- filter: 'foo',
+ await checkUrlToState('/admin/groups,123', {
+ ...defaultState,
+ offset: '123',
});
- });
-
- test('handleGroupListFilterRoute', () => {
- const ctx = {...createPageContext(), params: {filter: 'foo'}};
- assertctxToParams(ctx, 'handleGroupListFilterRoute', {
- view: GerritView.ADMIN,
- adminView: AdminChildView.GROUPS,
- filter: 'foo',
+ await checkUrlToState('/admin/groups,123#create', {
+ ...defaultState,
+ offset: '123',
+ openCreateModal: true,
});
+
+ await checkUrlToState('/admin/groups/q/filter:asdf', {
+ ...defaultState,
+ filter: 'asdf',
+ });
+ await checkUrlToState('/admin/groups/q/filter:asdf,123', {
+ ...defaultState,
+ filter: 'asdf',
+ offset: '123',
+ });
+ // #create is ignored when filtering
+ await checkUrlToState('/admin/groups/q/filter:asdf,123#create', {
+ ...defaultState,
+ filter: 'asdf',
+ offset: '123',
+ });
+ // filter is decoded (twice)
+ await checkUrlToState(
+ '/admin/groups/q/filter:XX%20XX%2520XX%252FXX%3FXX',
+ {...defaultState, filter: 'XX XX XX/XX?XX'}
+ );
+
+ // Slash must be double encoded in `filter` param.
+ await checkUrlNotMatched('/admin/groups/q/filter:asdf/qwer,11');
+ await checkUrlNotMatched('/admin/groups/q/filter:asdf%2Fqwer,11');
});
test('handleGroupRoute', () => {
diff --git a/polygerrit-ui/app/utils/page-wrapper-utils.ts b/polygerrit-ui/app/utils/page-wrapper-utils.ts
index 78e78ed..2e5c7b42 100644
--- a/polygerrit-ui/app/utils/page-wrapper-utils.ts
+++ b/polygerrit-ui/app/utils/page-wrapper-utils.ts
@@ -17,6 +17,7 @@
replace(path: string, state: null, init: boolean, dispatch: boolean): void;
base(url: string): void;
start(): void;
+ stop(): void;
exit(pattern: string | RegExp, ...pageCallback: PageCallback[]): void;
}
@@ -37,6 +38,7 @@
next: PageNextCallback
) => void;
-// TODO: Convert page usages to the real types and remove this file of wrapper
-// types. Also remove workarounds in rollup config.
-export const page = pagejs as unknown as Page;
+// Must only be used by gr-router!
+// TODO: Move this into gr-router. Note that there is a Google import rule
+// that would need to be modified.
+export const page = pagejs as unknown as {create(): Page};
