Stop using page.js directly from random components
Instead use the navigation service abstraction.
`navService.setUrl()` is a 1:1 replacement for `page.show()`.
With change the only direct dependency on page.js will be in gr-router.
That is beneficial because of general encapsulation, but more concretely
it will allow us to stop using the *global* page.js instance. The router
will then be able to instantiate its own instance, which is also
beneficial for testing.
Release-Notes: skip
Change-Id: Icc1470e50eca1624482be171498abc017a408aa1
diff --git a/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.ts b/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.ts
index 8d5689c..96688e9 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.ts
@@ -6,7 +6,6 @@
import '@polymer/iron-input/iron-input';
import '../../../styles/gr-form-styles';
import '../../../styles/shared-styles';
-import {page} from '../../../utils/page-wrapper-utils';
import {GroupId, GroupName} from '../../../types/common';
import {getAppContext} from '../../../services/app-context';
import {formStyles} from '../../../styles/gr-form-styles';
@@ -16,6 +15,8 @@
import {BindValueChangeEvent} from '../../../types/events';
import {fireEvent} from '../../../utils/event-util';
import {createGroupUrl} from '../../../models/views/group';
+import {resolve} from '../../../models/dependency';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
declare global {
interface HTMLElementTagNameMap {
@@ -32,6 +33,8 @@
private readonly restApiService = getAppContext().restApiService;
+ private readonly getNavigation = resolve(this, navigationToken);
+
static override get styles() {
return [
formStyles,
@@ -86,8 +89,7 @@
return this.restApiService.getGroupConfig(name).then(group => {
if (!group) return;
const groupId = String(group.group_id!) as GroupId;
- // TODO: Use navigation service instead of `page.show()` directly.
- page.show(createGroupUrl({groupId}));
+ this.getNavigation().setUrl(createGroupUrl({groupId}));
});
});
}
diff --git a/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog_test.ts b/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog_test.ts
index 2a0b539..6e36d8c 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog_test.ts
@@ -6,7 +6,6 @@
import '../../../test/common-test-setup';
import './gr-create-group-dialog';
import {GrCreateGroupDialog} from './gr-create-group-dialog';
-import {page} from '../../../utils/page-wrapper-utils';
import {
mockPromise,
queryAndAssert,
@@ -15,6 +14,8 @@
import {IronInputElement} from '@polymer/iron-input';
import {GroupId} from '../../../types/common';
import {fixture, html, assert} from '@open-wc/testing';
+import {testResolver} from '../../../test/common-test-setup';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
suite('gr-create-group-dialog tests', () => {
let element: GrCreateGroupDialog;
@@ -68,9 +69,9 @@
Promise.resolve({id: 'testId551' as GroupId, group_id: 551})
);
- const showStub = sinon.stub(page, 'show');
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
await element.handleCreateGroup();
- assert.isTrue(showStub.calledWith('/admin/groups/551'));
+ assert.isTrue(setUrlStub.calledWith('/admin/groups/551'));
});
test('test for unsuccessful group creation', async () => {
@@ -81,8 +82,8 @@
Promise.resolve({id: 'testId551' as GroupId, group_id: 551})
);
- const showStub = sinon.stub(page, 'show');
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
await element.handleCreateGroup();
- assert.isFalse(showStub.called);
+ assert.isFalse(setUrlStub.called);
});
});
diff --git a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
index bb59ccc..ed57830 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
@@ -7,7 +7,6 @@
import '../../shared/gr-autocomplete/gr-autocomplete';
import '../../shared/gr-button/gr-button';
import '../../shared/gr-select/gr-select';
-import {page} from '../../../utils/page-wrapper-utils';
import {
BranchName,
GroupId,
@@ -24,6 +23,8 @@
import {fireEvent} from '../../../utils/event-util';
import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
import {createRepoUrl} from '../../../models/views/repo';
+import {resolve} from '../../../models/dependency';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
declare global {
interface HTMLElementTagNameMap {
@@ -71,6 +72,8 @@
private readonly restApiService = getAppContext().restApiService;
+ private readonly getNavigation = resolve(this, navigationToken);
+
constructor() {
super();
this.query = (input: string) => this.getRepoSuggestions(input);
@@ -195,8 +198,7 @@
);
if (repoRegistered.status === 201) {
this.repoCreated = true;
- // TODO: Use navigation service instead of `page.show()` directly.
- page.show(createRepoUrl({repo: this.repoConfig.name}));
+ this.getNavigation().setUrl(createRepoUrl({repo: this.repoConfig.name}));
}
return repoRegistered;
}
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list_test.ts b/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list_test.ts
index 9b9cf29..9a67fda 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list_test.ts
@@ -6,7 +6,6 @@
import '../../../test/common-test-setup';
import './gr-repo-detail-list';
import {GrRepoDetailList} from './gr-repo-detail-list';
-import {page} from '../../../utils/page-wrapper-utils';
import {
addListenerForTest,
mockPromise,
@@ -34,6 +33,8 @@
import {GrListView} from '../../shared/gr-list-view/gr-list-view';
import {fixture, html, assert} from '@open-wc/testing';
import {RepoDetailView} from '../../../models/views/repo';
+import {testResolver} from '../../../test/common-test-setup';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
function branchGenerator(counter: number) {
return {
@@ -95,7 +96,7 @@
html`<gr-repo-detail-list></gr-repo-detail-list>`
);
element.detailType = RepoDetailView.BRANCHES;
- sinon.stub(page, 'show');
+ sinon.stub(testResolver(navigationToken), 'setUrl');
});
suite('list of repo branches', () => {
@@ -2338,7 +2339,7 @@
html`<gr-repo-detail-list></gr-repo-detail-list>`
);
element.detailType = RepoDetailView.TAGS;
- sinon.stub(page, 'show');
+ sinon.stub(testResolver(navigationToken), 'setUrl');
});
suite('list of repo tags', () => {
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.ts b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.ts
index 65f5a8c..906b733 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.ts
@@ -6,7 +6,6 @@
import '../../../test/common-test-setup';
import './gr-repo-list';
import {GrRepoList} from './gr-repo-list';
-import {page} from '../../../utils/page-wrapper-utils';
import {
mockPromise,
queryAndAssert,
@@ -23,6 +22,8 @@
import {GrListView} from '../../shared/gr-list-view/gr-list-view';
import {fixture, html, assert} from '@open-wc/testing';
import {AdminChildView, AdminViewState} from '../../../models/views/admin';
+import {testResolver} from '../../../test/common-test-setup';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
function createRepo(name: string, counter: number) {
return {
@@ -51,7 +52,7 @@
let repos: ProjectInfoWithName[];
setup(async () => {
- sinon.stub(page, 'show');
+ sinon.stub(testResolver(navigationToken), 'setUrl');
element = await fixture(html`<gr-repo-list></gr-repo-list>`);
});
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
index faaee0b..d2ba2c9 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
@@ -6,7 +6,6 @@
import '../gr-change-list/gr-change-list';
import '../gr-repo-header/gr-repo-header';
import '../gr-user-header/gr-user-header';
-import {page} from '../../../utils/page-wrapper-utils';
import {
AccountDetailInfo,
AccountId,
@@ -28,6 +27,7 @@
import {resolve} from '../../../models/dependency';
import {subscribe} from '../../lit/subscription-controller';
import {userModelToken} from '../../../models/user/user-model';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
const LIMIT_OPERATOR_PATTERN = /\blimit:(\d+)/i;
@@ -81,6 +81,8 @@
private readonly getViewModel = resolve(this, searchViewModelToken);
+ private readonly getNavigation = resolve(this, navigationToken);
+
constructor() {
super();
this.addEventListener('next-page', () => this.handleNextPage());
@@ -282,15 +284,13 @@
// private but used in test
handleNextPage() {
if (!this.nextArrow || !this.changesPerPage) return;
- // TODO: Use navigation service instead of `page.show()` directly.
- page.show(this.computeNavLink(1));
+ this.getNavigation().setUrl(this.computeNavLink(1));
}
// private but used in test
handlePreviousPage() {
if (!this.prevArrow || !this.changesPerPage) return;
- // TODO: Use navigation service instead of `page.show()` directly.
- page.show(this.computeNavLink(-1));
+ this.getNavigation().setUrl(this.computeNavLink(-1));
}
// private but used in test
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_test.ts
index f4bd8bd..decc253 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_test.ts
@@ -6,7 +6,6 @@
import '../../../test/common-test-setup';
import './gr-change-list-view';
import {GrChangeListView} from './gr-change-list-view';
-import {page} from '../../../utils/page-wrapper-utils';
import {query, queryAndAssert} from '../../../test/test-utils';
import {createChange} from '../../../test/test-data-generators';
import {ChangeInfo} from '../../../api/rest-api';
@@ -14,6 +13,8 @@
import {GrChangeList} from '../gr-change-list/gr-change-list';
import {GrChangeListSection} from '../gr-change-list-section/gr-change-list-section';
import {GrChangeListItem} from '../gr-change-list-item/gr-change-list-item';
+import {testResolver} from '../../../test/common-test-setup';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
suite('gr-change-list-view tests', () => {
let element: GrChangeListView;
@@ -158,7 +159,7 @@
});
test('handleNextPage', async () => {
- const showStub = sinon.stub(page, 'show');
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
element.changes = Array(25)
.fill(0)
.map(_ => createChange());
@@ -166,7 +167,7 @@
element.loading = false;
await element.updateComplete;
element.handleNextPage();
- assert.isFalse(showStub.called);
+ assert.isFalse(setUrlStub.called);
element.changes = Array(25)
.fill(0)
@@ -174,11 +175,11 @@
element.loading = false;
await element.updateComplete;
element.handleNextPage();
- assert.isTrue(showStub.called);
+ assert.isTrue(setUrlStub.called);
});
test('handlePreviousPage', async () => {
- const showStub = sinon.stub(page, 'show');
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
element.offset = 0;
element.changes = Array(25)
.fill(0)
@@ -187,11 +188,11 @@
element.loading = false;
await element.updateComplete;
element.handlePreviousPage();
- assert.isFalse(showStub.called);
+ assert.isFalse(setUrlStub.called);
element.offset = 25;
await element.updateComplete;
element.handlePreviousPage();
- assert.isTrue(showStub.called);
+ assert.isTrue(setUrlStub.called);
});
});
diff --git a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts
index 0092193..ea5e9f3 100644
--- a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts
+++ b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts
@@ -6,10 +6,11 @@
import '../../../test/common-test-setup';
import './gr-documentation-search';
import {GrDocumentationSearch} from './gr-documentation-search';
-import {page} from '../../../utils/page-wrapper-utils';
import {queryAndAssert, stubRestApi} from '../../../test/test-utils';
import {DocResult} from '../../../types/common';
import {fixture, html, assert} from '@open-wc/testing';
+import {testResolver} from '../../../test/common-test-setup';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
function documentationGenerator(counter: number) {
return {
@@ -31,7 +32,7 @@
let documentationSearches: DocResult[];
setup(async () => {
- sinon.stub(page, 'show');
+ sinon.stub(testResolver(navigationToken), 'setUrl');
element = await fixture(
html`<gr-documentation-search></gr-documentation-search>`
);
diff --git a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.ts b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.ts
index dad802a..f5837e3 100644
--- a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.ts
+++ b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.ts
@@ -7,13 +7,14 @@
import '../gr-button/gr-button';
import '../gr-icon/gr-icon';
import {encodeURL, getBaseUrl} from '../../../utils/url-util';
-import {page} from '../../../utils/page-wrapper-utils';
import {fireEvent} from '../../../utils/event-util';
import {debounce, DelayedTask} from '../../../utils/async-util';
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, PropertyValues, css, html} from 'lit';
import {customElement, property} from 'lit/decorators.js';
import {BindValueChangeEvent} from '../../../types/events';
+import {resolve} from '../../../models/dependency';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
const REQUEST_DEBOUNCE_INTERVAL_MS = 200;
@@ -48,6 +49,8 @@
private reloadTask?: DelayedTask;
+ private readonly getNavigation = resolve(this, navigationToken);
+
override disconnectedCallback() {
this.reloadTask?.cancel();
super.disconnectedCallback();
@@ -165,12 +168,12 @@
() => {
if (!this.isConnected || !this.path) return;
if (filter) {
- // TODO: Use navigation service instead of `page.show()` directly.
- page.show(`${this.path}/q/filter:${encodeURL(filter, false)}`);
+ this.getNavigation().setUrl(
+ `${this.path}/q/filter:${encodeURL(filter, false)}`
+ );
return;
}
- // TODO: Use navigation service instead of `page.show()` directly.
- page.show(this.path);
+ this.getNavigation().setUrl(this.path);
},
REQUEST_DEBOUNCE_INTERVAL_MS
);
diff --git a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.ts b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.ts
index bf94e8f..ecc1c25 100644
--- a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.ts
@@ -6,10 +6,11 @@
import '../../../test/common-test-setup';
import './gr-list-view';
import {GrListView} from './gr-list-view';
-import {page} from '../../../utils/page-wrapper-utils';
import {queryAndAssert, stubBaseUrl} from '../../../test/test-utils';
import {GrButton} from '../gr-button/gr-button';
import {fixture, html, assert} from '@open-wc/testing';
+import {testResolver} from '../../../test/common-test-setup';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
suite('gr-list-view tests', () => {
let element: GrListView;
@@ -84,7 +85,9 @@
let resolve: (url: string) => void;
const promise = new Promise(r => (resolve = r));
element.path = '/admin/projects';
- sinon.stub(page, 'show').callsFake(r => resolve(r));
+ sinon
+ .stub(testResolver(navigationToken), 'setUrl')
+ .callsFake(r => resolve(r));
element.filter = 'test';
await element.updateComplete;
