Merge "Add new user preference "allowBrowserNotifications""
diff --git a/Documentation/user-upload.txt b/Documentation/user-upload.txt
index 2bfc62d..8c51207 100644
--- a/Documentation/user-upload.txt
+++ b/Documentation/user-upload.txt
@@ -340,7 +340,7 @@
To avoid confusion in parsing the git ref, at least the following characters
must be percent-encoded: " %^@.~-+_:/!". Note that some of the reserved
characters (like tilde) are not escaped in the standard URL encoding rules,
-so a language-provided function (e.g. encodeURIComponent(), in javascript)
+so a language-provided function (e.g. encodeURIComponent(), in JavaScript)
might not suffice. To be safest, you might consider percent-encoding all
non-alphanumeric characters (and all multibyte UTF-8 code points).
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
index f31dc13..088002c 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.ts
@@ -18,7 +18,7 @@
import '../gr-repo-detail-list/gr-repo-detail-list';
import '../gr-repo-list/gr-repo-list';
import {getBaseUrl} from '../../../utils/url-util';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {
AdminNavLinksOption,
@@ -122,6 +122,8 @@
private readonly routerModel = getAppContext().routerModel;
+ private readonly getNavigation = resolve(this, navigationToken);
+
constructor() {
super();
subscribe(
@@ -549,7 +551,7 @@
if (this.selectedIsCurrentPage(selected)) return;
if (selected.url === undefined) return;
if (this.reloading) return;
- GerritNav.navigateToRelativeUrl(selected.url);
+ this.getNavigation().setUrl(selected.url);
}
isAdminView(): boolean {
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts
index 532c17f..d65d171 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts
@@ -6,7 +6,6 @@
import '../../../test/common-test-setup';
import './gr-admin-view';
import {AdminSubsectionLink, GrAdminView} from './gr-admin-view';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {stubBaseUrl, stubElement, stubRestApi} from '../../../test/test-utils';
import {GerritView} from '../../../services/router/router-model';
@@ -19,6 +18,8 @@
import {AdminChildView} from '../../../models/views/admin';
import {GroupDetailView} from '../../../models/views/group';
import {RepoDetailView} from '../../../models/views/repo';
+import {testResolver} from '../../../test/common-test-setup';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
function createAdminCapabilities() {
return {
@@ -456,10 +457,7 @@
parent: 'my-repo' as RepoName,
},
];
- const navigateToRelativeUrlStub = sinon.stub(
- GerritNav,
- 'navigateToRelativeUrl'
- );
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
const selectedIsCurrentPageSpy = sinon.spy(
element,
'selectedIsCurrentPage'
@@ -475,14 +473,14 @@
);
assert.equal(selectedIsCurrentPageSpy.callCount, 1);
// Doesn't trigger navigation from the page select menu.
- assert.isFalse(navigateToRelativeUrlStub.called);
+ assert.isFalse(setUrlStub.called);
// When explicitly changed, navigation is called
queryAndAssert<GrDropdownList>(element, '#pageSelect').value =
'repogeneral';
await queryAndAssert<GrDropdownList>(element, '#pageSelect').updateComplete;
assert.equal(selectedIsCurrentPageSpy.callCount, 2);
- assert.isTrue(navigateToRelativeUrlStub.calledOnce);
+ assert.isTrue(setUrlStub.calledOnce);
});
test('selectedIsCurrentPage', () => {
diff --git a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.ts b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.ts
index 6305292..cee0fa4 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.ts
@@ -10,7 +10,7 @@
import '../../shared/gr-autocomplete/gr-autocomplete';
import '../../shared/gr-button/gr-button';
import '../../shared/gr-select/gr-select';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {
RepoName,
BranchName,
@@ -28,6 +28,7 @@
import {subscribe} from '../../lit/subscription-controller';
import {configModelToken} from '../../../models/config/config-model';
import {resolve} from '../../../models/dependency';
+import {createChangeUrl} from '../../../models/views/change';
const SUGGESTIONS_LIMIT = 15;
const REF_PREFIX = 'refs/heads/';
@@ -71,6 +72,8 @@
private readonly configModel = resolve(this, configModelToken);
+ private readonly getNavigation = resolve(this, navigationToken);
+
constructor() {
super();
this.query = (input: string) => this.getRepoBranchesSuggestions(input);
@@ -223,9 +226,9 @@
this.baseChange,
this.baseCommit || undefined
)
- .then(changeCreated => {
- if (!changeCreated) return;
- GerritNav.navigateToChange(changeCreated);
+ .then(change => {
+ if (!change) return;
+ this.getNavigation().setUrl(createChangeUrl({change}));
});
}
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
index 202ef88..c716d65 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
@@ -17,6 +17,7 @@
AccountInfo,
GroupInfo,
GroupName,
+ ServerInfo,
} from '../../../types/common';
import {
AutocompleteQuery,
@@ -40,6 +41,9 @@
import {customElement, property, query, state} from 'lit/decorators.js';
import {ifDefined} from 'lit/directives/if-defined.js';
import {getAccountSuggestions} from '../../../utils/account-util';
+import {subscribe} from '../../lit/subscription-controller';
+import {configModelToken} from '../../../models/config/config-model';
+import {resolve} from '../../../models/dependency';
const SAVING_ERROR_TEXT =
'Group may not exist, or you may not have ' + 'permission to add it';
@@ -101,10 +105,21 @@
private readonly restApiService = getAppContext().restApiService;
+ private readonly getConfigModel = resolve(this, configModelToken);
+
+ private serverConfig?: ServerInfo;
+
constructor() {
super();
+ subscribe(
+ this,
+ () => this.getConfigModel().serverConfig$,
+ config => {
+ this.serverConfig = config;
+ }
+ );
this.queryMembers = input =>
- getAccountSuggestions(input, this.restApiService);
+ getAccountSuggestions(input, this.restApiService, this.serverConfig);
this.queryIncludedGroup = input => this.getGroupSuggestions(input);
}
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members_test.ts b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members_test.ts
index 2879c51..6c65dd6 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members_test.ts
@@ -29,6 +29,7 @@
import {getAccountSuggestions} from '../../../utils/account-util';
import {getAppContext} from '../../../services/app-context';
import {fixture, html, assert} from '@open-wc/testing';
+import {createServerInfo} from '../../../test/test-data-generators';
suite('gr-group-members tests', () => {
let element: GrGroupMembers;
@@ -102,6 +103,7 @@
name: 'test-account',
email: 'test.account@example.com' as EmailAddress,
username: 'test123',
+ display_name: 'display-test-account',
},
{
_account_id: 1001439 as AccountId,
@@ -498,7 +500,8 @@
test('getAccountSuggestions empty', async () => {
const accounts = await getAccountSuggestions(
'nonexistent',
- getAppContext().restApiService
+ getAppContext().restApiService,
+ createServerInfo()
);
assert.equal(accounts.length, 0);
});
@@ -506,10 +509,14 @@
test('getAccountSuggestions non-empty', async () => {
const accounts = await getAccountSuggestions(
'test-',
- getAppContext().restApiService
+ getAppContext().restApiService,
+ createServerInfo()
);
assert.equal(accounts.length, 3);
- assert.equal(accounts[0].name, 'test-account <test.account@example.com>');
+ assert.equal(
+ accounts[0].name,
+ 'display-test-account <test.account@example.com>'
+ );
assert.equal(accounts[1].name, 'test-admin <test.admin@example.com>');
assert.equal(accounts[2].name, 'test-git');
});
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
index d47926b..21ab184 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
@@ -5,7 +5,7 @@
*/
import '../gr-access-section/gr-access-section';
import {encodeURL, getBaseUrl, singleDecodeURL} from '../../../utils/url-util';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {toSortedPermissionsArray} from '../../../utils/access-util';
import {
RepoName,
@@ -41,6 +41,8 @@
import {assertIsDefined} from '../../../utils/common-util';
import {ValueChangedEvent} from '../../../types/events';
import {ifDefined} from 'lit/directives/if-defined.js';
+import {resolve} from '../../../models/dependency';
+import {createChangeUrl} from '../../../models/views/change';
const NOTHING_TO_SAVE = 'No changes to save.';
@@ -109,6 +111,8 @@
private readonly restApiService = getAppContext().restApiService;
+ private readonly getNavigation = resolve(this, navigationToken);
+
constructor() {
super();
this.query = () => this.getInheritFromSuggestions();
@@ -692,7 +696,7 @@
return this.restApiService
.setRepoAccessRightsForReview(this.repo, obj)
.then(change => {
- GerritNav.navigateToChange(change);
+ this.getNavigation().setUrl(createChangeUrl({change}));
})
.finally(() => {
this.modified = false;
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.ts b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.ts
index 9fd2a54..85d5c21 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.ts
@@ -6,7 +6,7 @@
import '../../../test/common-test-setup';
import './gr-repo-access';
import {GrRepoAccess} from './gr-repo-access';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {toSortedPermissionsArray} from '../../../utils/access-util';
import {
addListenerForTest,
@@ -32,6 +32,7 @@
import {GrPermission} from '../gr-permission/gr-permission';
import {createChange} from '../../../test/test-data-generators';
import {fixture, html, assert} from '@open-wc/testing';
+import {testResolver} from '../../../test/common-test-setup';
suite('gr-repo-access tests', () => {
let element: GrRepoAccess;
@@ -1440,7 +1441,7 @@
stubRestApi('getRepoAccessRights').returns(
Promise.resolve(JSON.parse(JSON.stringify(accessRes)))
);
- const navigateToChangeStub = sinon.stub(GerritNav, 'navigateToChange');
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
let resolver: (value: Response | PromiseLike<Response>) => void;
const saveStub = stubRestApi('setRepoAccessRights').returns(
new Promise(r => (resolver = r))
@@ -1459,7 +1460,7 @@
resolver!({status: 200} as Response);
await element.updateComplete;
assert.isTrue(saveStub.called);
- assert.isTrue(navigateToChangeStub.notCalled);
+ assert.isTrue(setUrlStub.notCalled);
});
test('handleSaveForReview', async () => {
@@ -1490,7 +1491,7 @@
stubRestApi('getRepoAccessRights').returns(
Promise.resolve(JSON.parse(JSON.stringify(accessRes)))
);
- const navigateToChangeStub = sinon.stub(GerritNav, 'navigateToChange');
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
let resolver: (value: ChangeInfo | PromiseLike<ChangeInfo>) => void;
const saveForReviewStub = stubRestApi(
'setRepoAccessRightsForReview'
@@ -1511,8 +1512,9 @@
resolver!(createChange());
await element.updateComplete;
assert.isTrue(saveForReviewStub.called);
+ assert.isTrue(setUrlStub.called);
assert.isTrue(
- navigateToChangeStub.lastCall.calledWithExactly(createChange())
+ setUrlStub.lastCall.args?.[0]?.includes(`${createChange()._number}`)
);
});
});
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts b/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts
index e761d1b..93f9159 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts
@@ -10,7 +10,7 @@
import '../../shared/gr-dialog/gr-dialog';
import '../../shared/gr-overlay/gr-overlay';
import '../gr-create-change-dialog/gr-create-change-dialog';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {
BranchName,
ConfigInfo,
@@ -34,6 +34,7 @@
import {customElement, query, property, state} from 'lit/decorators.js';
import {assertIsDefined} from '../../../utils/common-util';
import {createEditUrl} from '../../../models/views/edit';
+import {resolve} from '../../../models/dependency';
const GC_MESSAGE = 'Garbage collection completed successfully.';
const CONFIG_BRANCH = 'refs/meta/config' as BranchName;
@@ -74,6 +75,8 @@
private readonly restApiService = getAppContext().restApiService;
+ private readonly getNavigation = resolve(this, navigationToken);
+
override connectedCallback() {
super.connectedCallback();
fireTitleChange(this, 'Repo Commands');
@@ -284,7 +287,7 @@
return;
}
- GerritNav.navigateToRelativeUrl(
+ this.getNavigation().setUrl(
createEditUrl({
changeNum: change._number,
project: change.project,
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands_test.ts b/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands_test.ts
index eb2c81b..77caf5e 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands_test.ts
@@ -6,7 +6,6 @@
import '../../../test/common-test-setup';
import './gr-repo-commands';
import {GrRepoCommands} from './gr-repo-commands';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {
addListenerForTest,
mockPromise,
@@ -150,7 +149,6 @@
setup(() => {
createChangeStub = stubRestApi('createChange');
- sinon.stub(GerritNav, 'navigateToRelativeUrl');
handleSpy = sinon.spy(element, 'handleEditRepoConfig');
alertStub = sinon.stub();
element.repo = 'test' as RepoName;
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts
index 84f32b4..5728529 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts
@@ -219,12 +219,7 @@
private handleOpenChanges() {
for (const change of this.selectedChanges) {
- window.open(
- createChangeUrl({
- changeNum: change._number,
- project: change.project,
- })
- );
+ window.open(createChangeUrl({change, usp: 'bulk-vote'}));
}
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
index 396a21b..69b5a1f 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
@@ -15,7 +15,7 @@
import '../gr-change-list-column-requirements-summary/gr-change-list-column-requirements-summary';
import '../gr-change-list-column-requirement/gr-change-list-column-requirement';
import '../../shared/gr-tooltip-content/gr-tooltip-content';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getDisplayName} from '../../../utils/display-name-util';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
@@ -114,6 +114,8 @@
private readonly getBulkActionsModel = resolve(this, bulkActionsModelToken);
+ private readonly getNavigation = resolve(this, navigationToken);
+
constructor() {
super();
subscribe(
@@ -645,7 +647,7 @@
// gr-change-list-item such as account links, which will bubble through
// without triggering this extra navigation.
if (this.change && e.composedPath()[0] === this) {
- GerritNav.navigateToChange(this.change);
+ this.getNavigation().setUrl(createChangeUrl({change: this.change}));
}
};
@@ -656,11 +658,7 @@
private computeChangeURL() {
if (!this.change) return '';
- return createChangeUrl({
- changeNum: this.change._number,
- project: this.change.project,
- usp: this.usp,
- });
+ return createChangeUrl({change: this.change, usp: this.usp});
}
private computeRepoUrl() {
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
index 5e9d040..cec1dee 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
@@ -32,7 +32,7 @@
TopicName,
} from '../../../types/common';
import {StandardLabels} from '../../../utils/label-util';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import './gr-change-list-item';
import {GrChangeListItem} from './gr-change-list-item';
import {
@@ -45,6 +45,7 @@
} from '../../../models/bulk-actions/bulk-actions-model';
import {createTestAppContext} from '../../../test/test-app-context-init';
import {ColumnNames} from '../../../constants/constants';
+import {testResolver} from '../../../test/common-test-setup';
suite('gr-change-list-item tests', () => {
const account = createAccountWithId();
@@ -309,7 +310,7 @@
});
test('clicking item navigates to change', async () => {
- const navStub = sinon.stub(GerritNav);
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
element.change = change;
await element.updateComplete;
@@ -317,7 +318,8 @@
element.click();
await element.updateComplete;
- assert.isTrue(navStub.navigateToChange.calledWithExactly(change));
+ assert.isTrue(setUrlStub.called);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/a/test/repo/+/42');
});
test('renders', async () => {
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 937276d..0ac9903 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
@@ -7,7 +7,7 @@
import '../gr-repo-header/gr-repo-header';
import '../gr-user-header/gr-user-header';
import {page} from '../../../utils/page-wrapper-utils';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {
AccountDetailInfo,
AccountId,
@@ -30,6 +30,7 @@
} from '../../../models/views/search';
import {resolve} from '../../../models/dependency';
import {subscribe} from '../../lit/subscription-controller';
+import {createChangeUrl} from '../../../models/views/change';
const LOOKUP_QUERY_PATTERNS: RegExp[] = [
/^\s*i?[0-9a-f]{7,40}\s*$/i, // CHANGE_ID
@@ -111,6 +112,8 @@
private readonly getViewModel = resolve(this, searchViewModelToken);
+ private readonly getNavigation = resolve(this, navigationToken);
+
constructor() {
super();
this.addEventListener('next-page', () => this.handleNextPage());
@@ -302,11 +305,10 @@
if (this.query && changes.length === 1) {
for (const queryPattern of LOOKUP_QUERY_PATTERNS) {
if (this.query.match(queryPattern)) {
- // "Back"/"Forward" buttons work correctly only with
- // opt_redirect options
- GerritNav.navigateToChange(changes[0], {
- redirect: true,
- });
+ // "Back"/"Forward" buttons work correctly only with replaceUrl()
+ this.getNavigation().replaceUrl(
+ createChangeUrl({change: changes[0]})
+ );
return;
}
}
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 f88f668..860f8a3 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
@@ -7,9 +7,8 @@
import './gr-change-list-view';
import {GrChangeListView} from './gr-change-list-view';
import {page} from '../../../utils/page-wrapper-utils';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {
- mockPromise,
query,
stubRestApi,
queryAndAssert,
@@ -24,6 +23,8 @@
} from '../../../api/rest-api';
import {fixture, html, waitUntil, assert} from '@open-wc/testing';
import {GerritView} from '../../../services/router/router-model';
+import {testResolver} from '../../../test/common-test-setup';
+import {SinonStub} from 'sinon';
const CHANGE_ID = 'IcA3dAB3edAB9f60B8dcdA6ef71A75980e4B7127';
const COMMIT_HASH = '12345678';
@@ -275,7 +276,10 @@
});
suite('query based navigation', () => {
- setup(() => {});
+ let replaceUrlStub: SinonStub;
+ setup(() => {
+ replaceUrlStub = sinon.stub(testResolver(navigationToken), 'replaceUrl');
+ });
teardown(async () => {
await element.updateComplete;
@@ -285,82 +289,52 @@
test('Searching for a change ID redirects to change', async () => {
const change = {...createChange(), _number: 1 as NumericChangeId};
sinon.stub(element, 'getChanges').returns(Promise.resolve([change]));
- const promise = mockPromise();
- sinon.stub(GerritNav, 'navigateToChange').callsFake((url, opt) => {
- assert.equal(url, change);
- // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
- assert.isTrue(opt!.redirect);
- promise.resolve();
- });
- element.viewState = {
- view: GerritView.SEARCH,
- query: CHANGE_ID,
- offset: '',
- };
- await promise;
+ element.viewState = {view: GerritView.SEARCH, query: CHANGE_ID};
+ await element.updateComplete;
+
+ assert.isTrue(replaceUrlStub.called);
+ assert.equal(replaceUrlStub.lastCall.firstArg, '/c/test-project/+/1');
});
test('Searching for a change num redirects to change', async () => {
const change = {...createChange(), _number: 1 as NumericChangeId};
sinon.stub(element, 'getChanges').returns(Promise.resolve([change]));
- const promise = mockPromise();
- sinon.stub(GerritNav, 'navigateToChange').callsFake((url, opt) => {
- assert.equal(url, change);
- // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
- assert.isTrue(opt!.redirect);
- promise.resolve();
- });
- element.viewState = {view: GerritView.SEARCH, query: '1', offset: ''};
- await promise;
+ element.viewState = {view: GerritView.SEARCH, query: '1'};
+ await element.updateComplete;
+
+ assert.isTrue(replaceUrlStub.called);
+ assert.equal(replaceUrlStub.lastCall.firstArg, '/c/test-project/+/1');
});
test('Commit hash redirects to change', async () => {
const change = {...createChange(), _number: 1 as NumericChangeId};
sinon.stub(element, 'getChanges').returns(Promise.resolve([change]));
- const promise = mockPromise();
- sinon.stub(GerritNav, 'navigateToChange').callsFake((url, opt) => {
- assert.equal(url, change);
- // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
- assert.isTrue(opt!.redirect);
- promise.resolve();
- });
- element.viewState = {
- view: GerritView.SEARCH,
- query: COMMIT_HASH,
- offset: '',
- };
- await promise;
+ element.viewState = {view: GerritView.SEARCH, query: COMMIT_HASH};
+ await element.updateComplete;
+
+ assert.isTrue(replaceUrlStub.called);
+ assert.equal(replaceUrlStub.lastCall.firstArg, '/c/test-project/+/1');
});
test('Searching for an invalid change ID searches', async () => {
sinon.stub(element, 'getChanges').returns(Promise.resolve([]));
- const stub = sinon.stub(GerritNav, 'navigateToChange');
- element.viewState = {
- view: GerritView.SEARCH,
- query: CHANGE_ID,
- offset: '',
- };
+ element.viewState = {view: GerritView.SEARCH, query: CHANGE_ID};
await element.updateComplete;
- assert.isFalse(stub.called);
+ assert.isFalse(replaceUrlStub.called);
});
test('Change ID with multiple search results searches', async () => {
sinon.stub(element, 'getChanges').returns(Promise.resolve(undefined));
- const stub = sinon.stub(GerritNav, 'navigateToChange');
- element.viewState = {
- view: GerritView.SEARCH,
- query: CHANGE_ID,
- offset: '',
- };
+ element.viewState = {view: GerritView.SEARCH, query: CHANGE_ID};
await element.updateComplete;
- assert.isFalse(stub.called);
+ assert.isFalse(replaceUrlStub.called);
});
});
});
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
index 343dee2..fb7b354 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
@@ -9,7 +9,7 @@
import {GrChangeListItem} from '../gr-change-list-item/gr-change-list-item';
import '../../plugins/gr-endpoint-decorator/gr-endpoint-decorator';
import {getAppContext} from '../../../services/app-context';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {GrCursorManager} from '../../shared/gr-cursor-manager/gr-cursor-manager';
@@ -34,6 +34,8 @@
import {GrChangeListSection} from '../gr-change-list-section/gr-change-list-section';
import {Execution} from '../../../constants/reporting';
import {ValueChangedEvent} from '../../../types/events';
+import {resolve} from '../../../models/dependency';
+import {createChangeUrl} from '../../../models/views/change';
export interface ChangeListSection {
countLabel?: string;
@@ -143,6 +145,8 @@
private readonly shortcuts = new ShortcutController(this);
+ private readonly getNavigation = resolve(this, navigationToken);
+
private cursor = new GrCursorManager();
constructor() {
@@ -388,7 +392,7 @@
private async openChange() {
const change = await this.changeForIndex(this.selectedIndex);
- if (change) GerritNav.navigateToChange(change);
+ if (change) this.getNavigation().setUrl(createChangeUrl({change}));
}
private nextPage() {
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
index dbb436f..a15da85 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
@@ -6,7 +6,7 @@
import '../../../test/common-test-setup';
import './gr-change-list';
import {GrChangeList, computeRelativeIndex} from './gr-change-list';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {
pressKey,
query,
@@ -31,6 +31,7 @@
import {GrChangeListSection} from '../gr-change-list-section/gr-change-list-section';
import {fixture, assert} from '@open-wc/testing';
import {html} from 'lit';
+import {testResolver} from '../../../test/common-test-setup';
suite('gr-change-list basic tests', () => {
let element: GrChangeList;
@@ -224,16 +225,11 @@
assert.equal(element.selectedIndex, 2);
assert.isTrue(elementItems[2].selected);
- const navStub = sinon.stub(GerritNav, 'navigateToChange');
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
assert.equal(element.selectedIndex, 2);
pressKey(element, Key.ENTER);
- await waitUntil(() => navStub.callCount >= 1);
- await element.updateComplete;
- assert.deepEqual(
- navStub.lastCall.args[0],
- {...createChange(), _number: 2 as NumericChangeId},
- 'Should navigate to /c/2/'
- );
+ await waitUntil(() => setUrlStub.callCount >= 1);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/2');
pressKey(element, 'k');
await element.updateComplete;
@@ -241,15 +237,11 @@
assert.equal(element.selectedIndex, 1);
- const prevCount = navStub.callCount;
+ const prevCount = setUrlStub.callCount;
pressKey(element, Key.ENTER);
- await waitUntil(() => navStub.callCount > prevCount);
- assert.deepEqual(
- navStub.lastCall.args[0],
- {...createChange(), _number: 1 as NumericChangeId},
- 'Should navigate to /c/1/'
- );
+ await waitUntil(() => setUrlStub.callCount > prevCount);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/1');
pressKey(element, 'k');
pressKey(element, 'k');
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
index 9d2c458..77a0eb2 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
@@ -18,10 +18,7 @@
import '../gr-confirm-revert-dialog/gr-confirm-revert-dialog';
import '../gr-confirm-submit-dialog/gr-confirm-submit-dialog';
import '../../../styles/shared-styles';
-import {
- GerritNav,
- navigationToken,
-} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {getAppContext} from '../../../services/app-context';
import {CURRENT} from '../../../utils/patch-set-util';
@@ -110,6 +107,7 @@
import {Interaction} from '../../../constants/reporting';
import {rootUrl} from '../../../utils/url-util';
import {createSearchUrl} from '../../../models/views/search';
+import {createChangeUrl} from '../../../models/views/change';
const ERR_BRANCH_EMPTY = 'The destination branch can’t be empty.';
const ERR_COMMIT_EMPTY = 'The commit message can’t be empty.';
@@ -1845,20 +1843,24 @@
this.waitForChangeReachable(revertChangeInfo._number)
.then(() => this.setReviewOnRevert(revertChangeInfo._number))
.then(() => {
- GerritNav.navigateToChange(revertChangeInfo);
+ this.getNavigation().setUrl(
+ createChangeUrl({change: revertChangeInfo})
+ );
});
break;
}
case RevisionActions.CHERRYPICK: {
const cherrypickChangeInfo: ChangeInfo = obj as unknown as ChangeInfo;
this.waitForChangeReachable(cherrypickChangeInfo._number).then(() => {
- GerritNav.navigateToChange(cherrypickChangeInfo);
+ this.getNavigation().setUrl(
+ createChangeUrl({change: cherrypickChangeInfo})
+ );
});
break;
}
case ChangeActions.DELETE:
if (action.__type === ActionType.CHANGE) {
- GerritNav.navigateToRelativeUrl(rootUrl());
+ this.getNavigation().setUrl(rootUrl());
}
break;
case ChangeActions.WIP:
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
index 5ae2733..1b98c5d 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
@@ -5,10 +5,7 @@
*/
import '../../../test/common-test-setup';
import './gr-change-actions';
-import {
- GerritNav,
- navigationToken,
-} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {
createAccountWithId,
@@ -2460,7 +2457,6 @@
sendStub = stubRestApi('executeChangeAction').returns(
Promise.resolve(new Response())
);
- sinon.stub(GerritNav, 'navigateToChange');
});
test('change action', async () => {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 7d7d8b8..ed752e6 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -15,7 +15,7 @@
import '../../shared/gr-change-star/gr-change-star';
import '../../shared/gr-change-status/gr-change-status';
import '../../shared/gr-editable-content/gr-editable-content';
-import '../../shared/gr-linked-text/gr-linked-text';
+import '../../shared/gr-markdown/gr-markdown';
import '../../shared/gr-overlay/gr-overlay';
import '../../shared/gr-tooltip-content/gr-tooltip-content';
import '../gr-change-actions/gr-change-actions';
@@ -37,7 +37,7 @@
import {GrEditConstants} from '../../edit/gr-edit-constants';
import {pluralize} from '../../../utils/string-util';
import {querySelectorAll, windowLocationReload} from '../../../utils/dom-util';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {RevisionInfo as RevisionInfoClass} from '../../shared/revision-info/revision-info';
@@ -191,7 +191,6 @@
const MIN_LINES_FOR_COMMIT_COLLAPSE = 18;
-const REVIEWERS_REGEX = /^(R|CC)=/gm;
const MIN_CHECK_INTERVAL_SECS = 0;
const REPLY_REFIT_DEBOUNCE_INTERVAL_MS = 500;
@@ -586,6 +585,8 @@
private readonly shortcutsController = new ShortcutController(this);
+ private readonly getNavigation = resolve(this, navigationToken);
+
constructor() {
super();
this.setupListeners();
@@ -957,7 +958,7 @@
/* Account for border and padding and rounding errors. */
max-width: calc(72ch + 2px + 2 * var(--spacing-m) + 0.4px);
}
- .commitMessage gr-linked-text {
+ .commitMessage gr-markdown {
word-break: break-word;
}
#commitMessageEditor {
@@ -1458,12 +1459,9 @@
.commitCollapsible=${this.computeCommitCollapsible()}
remove-zero-width-space=""
>
- <gr-linked-text
- pre=""
- .content=${this.latestCommitMessage}
- .config=${this.projectConfig?.commentlinks}
- remove-zero-width-space=""
- ></gr-linked-text>
+ <gr-markdown
+ .content=${this.latestCommitMessage ?? ''}
+ ></gr-markdown>
</gr-editable-content>
</div>
<h3 class="assistive-tech-only">Comments and Checks Summary</h3>
@@ -1822,7 +1820,7 @@
return;
}
- this.latestCommitMessage = this.prepareCommitMsgForLinkify(message);
+ this.latestCommitMessage = message;
this.editingCommitMessage = false;
this.reloadWindow();
})
@@ -2096,8 +2094,8 @@
* anymore. The app element makes sure that an obsolete change view is not
* shown anymore, so if the change view is still and doing some update to
* itself, then that is not dangerous. But for example it should not call
- * navigateToChange() anymore. That would very likely cause erroneous
- * behavior.
+ * the navigation service's set/replaceUrl() methods anymore. That would very
+ * likely cause erroneous behavior.
*/
private isChangeObsolete() {
// While this.changeNum is undefined the change view is fresh and has just
@@ -2289,8 +2287,7 @@
assertIsDefined(this.patchRange, 'patchRange');
const hash = PREFIX + e.detail.id;
const url = createChangeUrl({
- changeNum: this.change._number,
- project: this.change.project,
+ change: this.change,
patchNum: this.patchRange.patchNum,
basePatchNum: this.patchRange.basePatchNum,
edit: this.getEditMode(),
@@ -2428,8 +2425,7 @@
private computeChangeUrl(forceReload?: boolean) {
if (!this.change) return undefined;
return createChangeUrl({
- changeNum: this.change._number,
- project: this.change.project,
+ change: this.change,
forceReload: !!forceReload,
});
}
@@ -2515,9 +2511,9 @@
fireAlert(this, 'Base is already selected.');
return;
}
- GerritNav.navigateToChange(this.change, {
- patchNum: this.patchRange.patchNum,
- });
+ this.getNavigation().setUrl(
+ createChangeUrl({change: this.change, patchNum: this.patchRange.patchNum})
+ );
}
// Private but used in tests.
@@ -2529,9 +2525,12 @@
fireAlert(this, 'Left is already base.');
return;
}
- GerritNav.navigateToChange(this.change, {
- patchNum: this.patchRange.basePatchNum as RevisionPatchSetNum,
- });
+ this.getNavigation().setUrl(
+ createChangeUrl({
+ change: this.change,
+ patchNum: this.patchRange.basePatchNum as RevisionPatchSetNum,
+ })
+ );
}
// Private but used in tests.
@@ -2543,10 +2542,13 @@
fireAlert(this, 'Latest is already selected.');
return;
}
- GerritNav.navigateToChange(this.change, {
- patchNum: latestPatchNum,
- basePatchNum: this.patchRange.basePatchNum,
- });
+ this.getNavigation().setUrl(
+ createChangeUrl({
+ change: this.change,
+ patchNum: latestPatchNum,
+ basePatchNum: this.patchRange.basePatchNum,
+ })
+ );
}
// Private but used in tests.
@@ -2558,10 +2560,13 @@
fireAlert(this, 'Right is already latest.');
return;
}
- GerritNav.navigateToChange(this.change, {
- patchNum: latestPatchNum,
- basePatchNum: this.patchRange.patchNum as BasePatchSetNum,
- });
+ this.getNavigation().setUrl(
+ createChangeUrl({
+ change: this.change,
+ patchNum: latestPatchNum,
+ basePatchNum: this.patchRange.patchNum as BasePatchSetNum,
+ })
+ );
}
// Private but used in tests.
@@ -2576,7 +2581,9 @@
fireAlert(this, 'Already diffing base against latest.');
return;
}
- GerritNav.navigateToChange(this.change, {patchNum: latestPatchNum});
+ this.getNavigation().setUrl(
+ createChangeUrl({change: this.change, patchNum: latestPatchNum})
+ );
}
private handleToggleChangeStar() {
@@ -2605,7 +2612,7 @@
private determinePageBack() {
// Default backPage to root if user came to change view page
// via an email link, etc.
- GerritNav.navigateToRelativeUrl(this.backPage || rootUrl());
+ this.getNavigation().setUrl(this.backPage || rootUrl());
}
private handleLabelRemoved(
@@ -2663,14 +2670,6 @@
this.changeViewAriaHidden = true;
}
- // Private but used in tests.
- prepareCommitMsgForLinkify(msg: string) {
- // TODO(wyatta) switch linkify sequence, see issue 5526.
- // This is a zero-with space. It is added to prevent the linkify library
- // from including R= or CC= as part of the email address.
- return msg.replace(REVIEWERS_REGEX, '$1=\u200B');
- }
-
/**
* Utility function to make the necessary modifications to a change in the
* case an edit exists.
@@ -2800,9 +2799,7 @@
throw new Error('Could not find latest Revision Sha');
const currentRevision = this.change.revisions[latestRevisionSha];
if (currentRevision.commit && currentRevision.commit.message) {
- this.latestCommitMessage = this.prepareCommitMsgForLinkify(
- currentRevision.commit.message
- );
+ this.latestCommitMessage = currentRevision.commit.message;
} else {
this.latestCommitMessage = null;
}
@@ -2855,9 +2852,7 @@
.getChangeCommitInfo(this.changeNum, lastpatchNum)
.then(commitInfo => {
if (!commitInfo) return;
- this.latestCommitMessage = this.prepareCommitMsgForLinkify(
- commitInfo.message
- );
+ this.latestCommitMessage = commitInfo.message;
});
}
@@ -2902,9 +2897,9 @@
loadData(isLocationChange?: boolean, clearPatchset?: boolean) {
if (this.isChangeObsolete()) return Promise.resolve();
if (clearPatchset && this.change) {
- GerritNav.navigateToChange(this.change, {
- forceReload: true,
- });
+ this.getNavigation().setUrl(
+ createChangeUrl({change: this.change, forceReload: true})
+ );
return Promise.resolve();
}
this.loading = true;
@@ -3218,7 +3213,7 @@
break;
case GrEditConstants.Actions.OPEN.id:
assertIsDefined(this.patchRange.patchNum, 'patchset number');
- GerritNav.navigateToRelativeUrl(
+ this.getNavigation().setUrl(
createEditUrl({
changeNum: this.change._number,
project: this.change.project,
@@ -3263,7 +3258,8 @@
);
if (editInfo) {
- GerritNav.navigateToChange(this.change, {patchNum: EDIT});
+ const url = createChangeUrl({change: this.change, patchNum: EDIT});
+ this.getNavigation().setUrl(url);
return;
}
@@ -3276,20 +3272,26 @@
) {
patchNum = this.patchRange.patchNum;
}
- GerritNav.navigateToChange(this.change, {
- patchNum,
- isEdit: true,
- forceReload: true,
- });
+ this.getNavigation().setUrl(
+ createChangeUrl({
+ change: this.change,
+ patchNum,
+ edit: true,
+ forceReload: true,
+ })
+ );
}
private handleStopEditTap() {
assertIsDefined(this.change, 'change');
assertIsDefined(this.patchRange, 'patchRange');
- GerritNav.navigateToChange(this.change, {
- patchNum: this.patchRange.patchNum,
- forceReload: true,
- });
+ this.getNavigation().setUrl(
+ createChangeUrl({
+ change: this.change,
+ patchNum: this.patchRange.patchNum,
+ forceReload: true,
+ })
+ );
}
private resetReplyOverlayFocusStops() {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 0e6fb71..49248ea 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -19,7 +19,7 @@
} from '../../../constants/constants';
import {GrEditConstants} from '../../edit/gr-edit-constants';
import {_testOnly_resetEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {EventType, PluginApi} from '../../../api/plugin';
import {
@@ -100,13 +100,11 @@
import {GrCopyLinks} from '../gr-copy-links/gr-copy-links';
import {ChangeViewState} from '../../../models/views/change';
import {rootUrl} from '../../../utils/url-util';
+import {testResolver} from '../../../test/common-test-setup';
suite('gr-change-view tests', () => {
let element: GrChangeView;
-
- let navigateToChangeStub: SinonStubbedMember<
- typeof GerritNav.navigateToChange
- >;
+ let setUrlStub: sinon.SinonStub;
const ROBOT_COMMENTS_LIMIT = 10;
@@ -331,7 +329,7 @@
setup(async () => {
// Since pluginEndpoints are global, must reset state.
_testOnly_resetEndpoints();
- navigateToChangeStub = sinon.stub(GerritNav, 'navigateToChange');
+ setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
stubRestApi('getConfig').returns(
Promise.resolve({
@@ -435,9 +433,7 @@
id="commitMessageEditor"
remove-zero-width-space=""
>
- <gr-linked-text pre="" remove-zero-width-space="">
- <span id="output" slot="insert"> </span>
- </gr-linked-text>
+ <gr-markdown></gr-markdown>
</gr-editable-content>
</div>
<h3 class="assistive-tech-only">
@@ -585,10 +581,8 @@
basePatchNum: 1 as BasePatchSetNum,
};
element.handleDiffAgainstBase();
- assert(navigateToChangeStub.called);
- const args = navigateToChangeStub.getCall(0).args;
- assert.equal(args[0], element.change);
- assert.equal(args[1]!.patchNum, 3 as RevisionPatchSetNum);
+ assert.isTrue(setUrlStub.called);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/3');
});
test('handleDiffAgainstLatest', () => {
@@ -601,11 +595,8 @@
patchNum: 3 as RevisionPatchSetNum,
};
element.handleDiffAgainstLatest();
- assert(navigateToChangeStub.called);
- const args = navigateToChangeStub.getCall(0).args;
- assert.equal(args[0], element.change);
- assert.equal(args[1]!.patchNum, 10 as RevisionPatchSetNum);
- assert.equal(args[1]!.basePatchNum, 1 as BasePatchSetNum);
+ assert.isTrue(setUrlStub.called);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/1..10');
});
test('handleDiffBaseAgainstLeft', () => {
@@ -618,10 +609,8 @@
basePatchNum: 1 as BasePatchSetNum,
};
element.handleDiffBaseAgainstLeft();
- assert(navigateToChangeStub.called);
- const args = navigateToChangeStub.getCall(0).args;
- assert.equal(args[0], element.change);
- assert.equal(args[1]!.patchNum, 1 as RevisionPatchSetNum);
+ assert.isTrue(setUrlStub.called);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/1');
});
test('handleDiffRightAgainstLatest', () => {
@@ -634,10 +623,8 @@
patchNum: 3 as RevisionPatchSetNum,
};
element.handleDiffRightAgainstLatest();
- assert(navigateToChangeStub.called);
- const args = navigateToChangeStub.getCall(0).args;
- assert.equal(args[1]!.patchNum, 10 as RevisionPatchSetNum);
- assert.equal(args[1]!.basePatchNum, 3 as BasePatchSetNum);
+ assert.isTrue(setUrlStub.called);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/3..10');
});
test('handleDiffBaseAgainstLatest', () => {
@@ -650,10 +637,8 @@
patchNum: 3 as RevisionPatchSetNum,
};
element.handleDiffBaseAgainstLatest();
- assert(navigateToChangeStub.called);
- const args = navigateToChangeStub.getCall(0).args;
- assert.equal(args[1]!.patchNum, 10 as RevisionPatchSetNum);
- assert.isNotOk(args[1]!.basePatchNum);
+ assert.isTrue(setUrlStub.called);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/10');
});
test('toggle attention set status', async () => {
@@ -807,20 +792,16 @@
});
test('U should navigate to root if no backPage set', () => {
- const relativeNavStub = sinon.stub(GerritNav, 'navigateToRelativeUrl');
pressKey(element, 'u');
- assert.isTrue(relativeNavStub.called);
- assert.isTrue(relativeNavStub.lastCall.calledWithExactly(rootUrl()));
+ assert.isTrue(setUrlStub.called);
+ assert.isTrue(setUrlStub.lastCall.calledWithExactly(rootUrl()));
});
test('U should navigate to backPage if set', () => {
- const relativeNavStub = sinon.stub(GerritNav, 'navigateToRelativeUrl');
element.backPage = '/dashboard/self';
pressKey(element, 'u');
- assert.isTrue(relativeNavStub.called);
- assert.isTrue(
- relativeNavStub.lastCall.calledWithExactly('/dashboard/self')
- );
+ assert.isTrue(setUrlStub.called);
+ assert.isTrue(setUrlStub.lastCall.calledWithExactly('/dashboard/self'));
});
test('A fires an error event when not logged in', async () => {
@@ -1426,20 +1407,6 @@
assert.isTrue(overlayOpenStub.called);
});
- test('prepareCommitMsgForLinkify', () => {
- let commitMessage = 'R=test@google.com';
- let result = element.prepareCommitMsgForLinkify(commitMessage);
- assert.equal(result, 'R=\u200Btest@google.com');
-
- commitMessage = 'R=test@google.com\nR=test@google.com';
- result = element.prepareCommitMsgForLinkify(commitMessage);
- assert.equal(result, 'R=\u200Btest@google.com\nR=\u200Btest@google.com');
-
- commitMessage = 'CC=test@google.com';
- result = element.prepareCommitMsgForLinkify(commitMessage);
- assert.equal(result, 'CC=\u200Btest@google.com');
- });
-
test('_isSubmitEnabled', () => {
assert.isFalse(element.isSubmitEnabled());
element.currentRevisionActions = {submit: {}};
@@ -1710,7 +1677,7 @@
});
await element.loadData(true);
- assert.isFalse(navigateToChangeStub.called);
+ assert.isFalse(setUrlStub.called);
assert.isTrue(reloadStub.called);
});
@@ -2124,10 +2091,6 @@
const openDeleteDialogStub = sinon.stub(controls, 'openDeleteDialog');
const openRenameDialogStub = sinon.stub(controls, 'openRenameDialog');
const openRestoreDialogStub = sinon.stub(controls, 'openRestoreDialog');
- const navigateToRelativeUrlStub = sinon.stub(
- GerritNav,
- 'navigateToRelativeUrl'
- );
// Delete
fileList.dispatchEvent(
@@ -2178,7 +2141,7 @@
);
await element.updateComplete;
- assert.isTrue(navigateToRelativeUrlStub.called);
+ assert.isTrue(setUrlStub.called);
});
test('selectedRevision updates when patchNum is changed', async () => {
@@ -2290,7 +2253,6 @@
assertIsDefined(element.actions);
element.actions.dispatchEvent(new CustomEvent('edit-tap'));
};
- navigateToChangeStub.restore();
element.change = {
...createChangeViewChange(),
@@ -2299,13 +2261,6 @@
});
test('edit exists in revisions', async () => {
- const promise = mockPromise();
- sinon.stub(GerritNav, 'navigateToChange').callsFake((...args) => {
- assert.equal(args.length, 2);
- assert.equal(args[1]!.patchNum, EDIT); // patchNum
- promise.resolve();
- });
-
assertIsDefined(element.change);
const newChange = {...element.change};
newChange.revisions.rev2 = createRevision(EDIT);
@@ -2313,18 +2268,11 @@
await element.updateComplete;
fireEdit();
- await promise;
+ assert.isTrue(setUrlStub.called);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/edit');
});
test('no edit exists in revisions, non-latest patchset', async () => {
- const promise = mockPromise();
- sinon.stub(GerritNav, 'navigateToChange').callsFake((...args) => {
- assert.equal(args.length, 2);
- assert.equal(args[1]!.patchNum, 1 as RevisionPatchSetNum); // patchNum
- assert.equal(args[1]!.isEdit, true); // opt_isEdit
- promise.resolve();
- });
-
assertIsDefined(element.change);
const newChange = {...element.change};
newChange.revisions.rev2 = createRevision(2);
@@ -2333,19 +2281,14 @@
await element.updateComplete;
fireEdit();
- await promise;
+ assert.isTrue(setUrlStub.called);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/1,edit?forceReload=true'
+ );
});
test('no edit exists in revisions, latest patchset', async () => {
- const promise = mockPromise();
- sinon.stub(GerritNav, 'navigateToChange').callsFake((...args) => {
- assert.equal(args.length, 2);
- // No patch should be specified when patchNum == latest.
- assert.isNotOk(args[1]!.patchNum); // patchNum
- assert.equal(args[1]!.isEdit, true); // opt_isEdit
- promise.resolve();
- });
-
assertIsDefined(element.change);
const newChange = {...element.change};
newChange.revisions.rev2 = createRevision(2);
@@ -2354,7 +2297,11 @@
await element.updateComplete;
fireEdit();
- await promise;
+ assert.isTrue(setUrlStub.called);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42,edit?forceReload=true'
+ );
});
});
@@ -2366,19 +2313,17 @@
assertIsDefined(element.metadata);
assertIsDefined(element.actions);
sinon.stub(element.metadata, 'computeLabelNames');
- navigateToChangeStub.restore();
- const promise = mockPromise();
- sinon.stub(GerritNav, 'navigateToChange').callsFake((...args) => {
- assert.equal(args.length, 2);
- assert.equal(args[1]!.patchNum, 1 as RevisionPatchSetNum); // patchNum
- promise.resolve();
- });
element.patchRange = {patchNum: 1 as RevisionPatchSetNum};
element.actions.dispatchEvent(
new CustomEvent('stop-edit-tap', {bubbles: false})
);
- await promise;
+
+ assert.isTrue(setUrlStub.called);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/1?forceReload=true'
+ );
});
suite('plugin endpoints', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
index bbebf10..832738b 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
@@ -11,7 +11,7 @@
import '../../shared/gr-icon/gr-icon';
import '../gr-commit-info/gr-commit-info';
import {FilesExpandedState} from '../gr-file-list-constants';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {computeLatestPatchNum, PatchSet} from '../../../utils/patch-set-util';
import {property, customElement, query, state} from 'lit/decorators.js';
import {
@@ -41,6 +41,7 @@
import {getAppContext} from '../../../services/app-context';
import {subscribe} from '../../lit/subscription-controller';
import {configModelToken} from '../../../models/config/config-model';
+import {createChangeUrl} from '../../../models/views/change';
@customElement('gr-file-list-header')
export class GrFileListHeader extends LitElement {
@@ -124,6 +125,8 @@
private readonly userModel = getAppContext().userModel;
+ private readonly getNavigation = resolve(this, navigationToken);
+
constructor() {
super();
subscribe(
@@ -421,7 +424,9 @@
) {
return;
}
- GerritNav.navigateToChange(this.change, {patchNum, basePatchNum});
+ this.getNavigation().setUrl(
+ createChangeUrl({change: this.change, patchNum, basePatchNum})
+ );
}
private handlePrefsTap(e: Event) {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.ts
index 030934e..7b79893 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.ts
@@ -6,7 +6,7 @@
import '../../../test/common-test-setup';
import './gr-file-list-header';
import {FilesExpandedState} from '../gr-file-list-constants';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {createChange, createRevision} from '../../../test/test-data-generators';
import {
isVisible,
@@ -22,13 +22,13 @@
PARENT,
PatchSetNum,
PatchSetNumber,
- RevisionPatchSetNum,
} from '../../../types/common';
import {ChangeInfo, ChangeStatus} from '../../../api/rest-api';
import {PatchSet} from '../../../utils/patch-set-util';
import {createDefaultDiffPrefs} from '../../../constants/constants';
import {fixture, html, assert} from '@open-wc/testing';
import {GrButton} from '../../shared/gr-button/gr-button';
+import {testResolver} from '../../../test/common-test-setup';
suite('gr-file-list-header tests', () => {
let element: GrFileListHeader;
@@ -238,8 +238,8 @@
assert.equal(getComputedStyle(collapseBtn).display, 'none');
});
- test('navigateToChange called when range select changes', async () => {
- const navigateToChangeStub = sinon.stub(GerritNav, 'navigateToChange');
+ test('setUrl called when range select changes', async () => {
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
element.basePatchNum = 1 as BasePatchSetNum;
element.patchNum = 2 as PatchSetNum;
await element.updateComplete;
@@ -249,13 +249,8 @@
} as CustomEvent);
await element.updateComplete;
- assert.equal(navigateToChangeStub.callCount, 1);
- assert.isTrue(
- navigateToChangeStub.lastCall.calledWithExactly(change, {
- patchNum: 3 as RevisionPatchSetNum,
- basePatchNum: 1 as BasePatchSetNum,
- })
- );
+ assert.equal(setUrlStub.callCount, 1);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/1..3');
});
test('class is applied to file list on old patch set', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index d2b840f..731f227 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -12,7 +12,6 @@
import '../../shared/gr-button/gr-button';
import '../../shared/gr-cursor-manager/gr-cursor-manager';
import '../../shared/gr-icon/gr-icon';
-import '../../shared/gr-linked-text/gr-linked-text';
import '../../shared/gr-select/gr-select';
import '../../shared/gr-tooltip-content/gr-tooltip-content';
import '../../shared/gr-copy-clipboard/gr-copy-clipboard';
@@ -21,7 +20,7 @@
import {asyncForeach} from '../../../utils/async-util';
import {FilesExpandedState} from '../gr-file-list-constants';
import {diffFilePaths, pluralize} from '../../../utils/string-util';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {getAppContext} from '../../../services/app-context';
@@ -76,6 +75,7 @@
import {HtmlPatched} from '../../../utils/lit-util';
import {createDiffUrl} from '../../../models/views/diff';
import {createEditUrl} from '../../../models/views/edit';
+import {createChangeUrl} from '../../../models/views/change';
export const DEFAULT_NUM_FILES_SHOWN = 200;
@@ -304,6 +304,8 @@
shortcutsController = new ShortcutController(this);
+ private readonly getNavigation = resolve(this, navigationToken);
+
// private but used in test
fileCursor = new GrCursorManager();
@@ -2040,11 +2042,13 @@
if (!this.change || !diff || !this.patchRange || !diff.path) {
throw new Error('change, diff and patchRange must be all set and valid');
}
- GerritNav.navigateToDiff(
- this.change,
- diff.path,
- this.patchRange.patchNum,
- this.patchRange.basePatchNum
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: diff.path,
+ patchNum: this.patchRange.patchNum,
+ basePatchNum: this.patchRange.basePatchNum,
+ })
);
}
@@ -2059,11 +2063,13 @@
if (!this.change || !this.patchRange) {
throw new Error('change and patchRange must be set');
}
- GerritNav.navigateToDiff(
- this.change,
- this.files[this.fileCursor.index].__path,
- this.patchRange.patchNum,
- this.patchRange.basePatchNum
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: this.files[this.fileCursor.index].__path,
+ patchNum: this.patchRange.patchNum,
+ basePatchNum: this.patchRange.basePatchNum,
+ })
);
}
@@ -2169,10 +2175,13 @@
private handleShowParent1(): void {
if (!this.change || !this.patchRange) return;
- GerritNav.navigateToChange(this.change, {
- patchNum: this.patchRange.patchNum,
- basePatchNum: -1 as BasePatchSetNum, // Parent 1
- });
+ this.getNavigation().setUrl(
+ createChangeUrl({
+ change: this.change,
+ patchNum: this.patchRange.patchNum,
+ basePatchNum: -1 as BasePatchSetNum, // Parent 1
+ })
+ );
}
private computeFilesShown(): NormalizedFileInfo[] {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
index c693d74..76335cb 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
@@ -7,7 +7,7 @@
import '../../shared/gr-date-formatter/gr-date-formatter';
import './gr-file-list';
import {FilesExpandedState} from '../gr-file-list-constants';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {
mockPromise,
query,
@@ -55,6 +55,7 @@
import {GrIcon} from '../../shared/gr-icon/gr-icon';
import {fixture, html, assert} from '@open-wc/testing';
import {Modifier} from '../../../utils/dom-util';
+import {testResolver} from '../../../test/common-test-setup';
suite('gr-diff a11y test', () => {
test('audit', async () => {
@@ -928,7 +929,10 @@
basePatchNum: PARENT,
patchNum: 2 as RevisionPatchSetNum,
};
- element.change = {_number: 42 as NumericChangeId} as ParsedChangeInfo;
+ element.change = {
+ _number: 42 as NumericChangeId,
+ project: 'test-project',
+ } as ParsedChangeInfo;
element.fileCursor.setCursorAtIndex(0);
await element.updateComplete;
await waitEventLoop();
@@ -966,7 +970,7 @@
assert.equal(element.selectedIndex, 1);
pressKey(element, 'j');
- const navStub = sinon.stub(GerritNav, 'navigateToDiff');
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
assert.equal(element.fileCursor.index, 2);
assert.equal(element.selectedIndex, 2);
@@ -983,13 +987,10 @@
assert.equal(element.selectedIndex, 1);
pressKey(element, 'o');
- assert(
- navStub.lastCall.calledWith(
- element.change,
- 'file_added_in_rev2.txt',
- 2 as RevisionPatchSetNum
- ),
- 'Should navigate to /c/42/2/file_added_in_rev2.txt'
+ assert.equal(setUrlStub.callCount, 1);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/2/file_added_in_rev2.txt'
);
pressKey(element, 'k');
@@ -2241,16 +2242,16 @@
const files = element.files;
element.files = [];
await element.updateComplete;
- const navStub = sinon.stub(GerritNav, 'navigateToDiff');
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
// Noop when there are no files.
element.openSelectedFile();
- assert.isFalse(navStub.called);
+ assert.isFalse(setUrlStub.calledOnce);
element.files = files;
await element.updateComplete;
// Navigates when a file is selected.
element.openSelectedFile();
- assert.isTrue(navStub.called);
+ assert.isTrue(setUrlStub.calledOnce);
});
test('displayLine', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.ts b/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
index 04766bd..e11822f 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
@@ -38,7 +38,7 @@
import {LABEL_TITLE_SCORE_PATTERN} from '../gr-message-scores/gr-message-scores';
import {getAppContext} from '../../../services/app-context';
import {pluralize} from '../../../utils/string-util';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {
computeAllPatchSets,
computeLatestPatchNum,
@@ -48,6 +48,8 @@
import {assertIsDefined} from '../../../utils/common-util';
import {when} from 'lit/directives/when.js';
import {FormattedReviewerUpdateInfo} from '../../../types/types';
+import {resolve} from '../../../models/dependency';
+import {createChangeUrl} from '../../../models/views/change';
const UPLOADED_NEW_PATCHSET_PATTERN = /Uploaded patch set (\d+)./;
const MERGED_PATCHSET_PATTERN = /(\d+) is the latest approved patch-set/;
@@ -127,6 +129,8 @@
private readonly restApiService = getAppContext().restApiService;
+ private readonly getNavigation = resolve(this, navigationToken);
+
// for COMMENTS_AUTOCLOSE logging purposes only
readonly uid = performance.now().toString(36) + Math.random().toString(36);
@@ -623,7 +627,9 @@
patchNum = computeLatestPatchNum(computeAllPatchSets(this.change))!;
basePatchNum = computePredecessor(patchNum)!;
}
- GerritNav.navigateToChange(this.change, {patchNum, basePatchNum});
+ this.getNavigation().setUrl(
+ createChangeUrl({change: this.change, patchNum, basePatchNum})
+ );
// stop propagation to stop message expansion
e.stopPropagation();
}
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.ts b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.ts
index c55f380..34292d6 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.ts
@@ -5,7 +5,7 @@
*/
import '../../../test/common-test-setup';
import './gr-message';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {
createAccountWithIdNameAndEmail,
createChange,
@@ -25,11 +25,9 @@
import {GrMessage} from './gr-message';
import {
AccountId,
- BasePatchSetNum,
ChangeMessageId,
EmailAddress,
NumericChangeId,
- PARENT,
RevisionPatchSetNum,
ReviewInputTag,
Timestamp,
@@ -41,9 +39,10 @@
} from '../../../types/events';
import {GrButton} from '../../shared/gr-button/gr-button';
import {CommentSide} from '../../../constants/constants';
-import {SinonStubbedMember} from 'sinon';
+import {SinonStub} from 'sinon';
import {html} from 'lit';
import {fixture, assert} from '@open-wc/testing';
+import {testResolver} from '../../../test/common-test-setup';
suite('gr-message tests', () => {
let element: GrMessage;
@@ -424,10 +423,10 @@
});
suite('uploaded patchset X message navigates to X - 1 vs X', () => {
- let navStub: SinonStubbedMember<typeof GerritNav.navigateToChange>;
+ let setUrlStub: SinonStub;
setup(() => {
element.change = {...createChange(), revisions: createRevisions(4)};
- navStub = sinon.stub(GerritNav, 'navigateToChange');
+ setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
});
test('Patchset 1 navigates to Base', () => {
@@ -436,12 +435,9 @@
message: 'Uploaded patch set 1.',
};
element.handleViewPatchsetDiff(new MouseEvent('click'));
- assert.isTrue(
- navStub.calledWithExactly(element.change!, {
- patchNum: 1 as RevisionPatchSetNum,
- basePatchNum: PARENT,
- })
- );
+
+ assert.isTrue(setUrlStub.calledOnce);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/1');
});
test('Patchset X navigates to X vs X - 1', () => {
@@ -450,23 +446,20 @@
message: 'Uploaded patch set 2.',
};
element.handleViewPatchsetDiff(new MouseEvent('click'));
- assert.isTrue(
- navStub.calledWithExactly(element.change!, {
- patchNum: 2 as RevisionPatchSetNum,
- basePatchNum: 1 as BasePatchSetNum,
- })
- );
+
+ assert.isTrue(setUrlStub.calledOnce);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/1..2');
element.message = {
...createChangeMessage(),
message: 'Uploaded patch set 200.',
};
element.handleViewPatchsetDiff(new MouseEvent('click'));
- assert.isTrue(
- navStub.calledWithExactly(element.change!, {
- patchNum: 200 as RevisionPatchSetNum,
- basePatchNum: 199 as BasePatchSetNum,
- })
+
+ assert.isTrue(setUrlStub.calledTwice);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/199..200'
);
});
@@ -476,12 +469,9 @@
message: 'Commit message updated.',
};
element.handleViewPatchsetDiff(new MouseEvent('click'));
- assert.isTrue(
- navStub.calledWithExactly(element.change!, {
- patchNum: 4 as RevisionPatchSetNum,
- basePatchNum: 3 as BasePatchSetNum,
- })
- );
+
+ assert.isTrue(setUrlStub.calledOnce);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/3..4');
});
test('Merged patchset change message', () => {
@@ -490,12 +480,9 @@
message: 'abcd↵3 is the latest approved patch-set.↵abc',
};
element.handleViewPatchsetDiff(new MouseEvent('click'));
- assert.isTrue(
- navStub.calledWithExactly(element.change!, {
- patchNum: 4 as RevisionPatchSetNum,
- basePatchNum: 3 as BasePatchSetNum,
- })
- );
+
+ assert.isTrue(setUrlStub.calledOnce);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/3..4');
});
});
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
index fb0a99b..258ed9b 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
@@ -296,11 +296,7 @@
<gr-related-change
.label=${this.renderChangeTitle(change)}
.change=${change}
- .href=${createChangeUrl({
- changeNum: change._number,
- project: change.project,
- usp: 'submitted-together',
- })}
+ .href=${createChangeUrl({change, usp: 'submitted-together'})}
?show-submittable-check=${showSubmittabilityCheck}
>${change.subject}</gr-related-change
>
@@ -377,11 +373,7 @@
mergeConflictsMarkersPredicate(index)
)}<gr-related-change
.change=${change}
- .href=${createChangeUrl({
- changeNum: change._number,
- project: change.project,
- usp: 'merge-conflict',
- })}
+ .href=${createChangeUrl({change, usp: 'merge-conflict'})}
>${change.subject}</gr-related-change
>
</div>`
@@ -422,11 +414,7 @@
cherryPicksMarkersPredicate(index)
)}<gr-related-change
.change=${change}
- .href=${createChangeUrl({
- changeNum: change._number,
- project: change.project,
- usp: 'cherry-pick',
- })}
+ .href=${createChangeUrl({change, usp: 'cherry-pick'})}
>${change.branch}: ${change.subject}</gr-related-change
>
</div>`
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.ts
index 0eb0227..6fba4e4 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.ts
@@ -21,7 +21,6 @@
Timestamp,
} from '../../../types/common';
import {createChange} from '../../../test/test-data-generators';
-import {GrTextarea} from '../../shared/gr-textarea/gr-textarea';
import {GrButton} from '../../shared/gr-button/gr-button';
suite('gr-reply-dialog-it tests', () => {
@@ -120,14 +119,6 @@
getPluginLoader().loadPlugins([]);
await getPluginLoader().awaitPluginsLoaded();
await waitEventLoop();
- const textarea = queryAndAssert<GrTextarea>(
- element,
- 'gr-textarea'
- ).getNativeTextarea();
- textarea.value = 'LGTM';
- textarea.dispatchEvent(
- new CustomEvent('input', {bubbles: true, composed: true})
- );
await waitEventLoop();
const labelScoreRows = queryAndAssert(
element.getLabelScores(),
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index 13de1cd..b540c89 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -3,12 +3,10 @@
* Copyright 2015 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import '@polymer/iron-autogrow-textarea/iron-autogrow-textarea';
import '../../plugins/gr-endpoint-decorator/gr-endpoint-decorator';
import '../../plugins/gr-endpoint-param/gr-endpoint-param';
import '../../plugins/gr-endpoint-slot/gr-endpoint-slot';
import '../../shared/gr-account-chip/gr-account-chip';
-import '../../shared/gr-textarea/gr-textarea';
import '../../shared/gr-button/gr-button';
import '../../shared/gr-icon/gr-icon';
import '../../shared/gr-formatted-text/gr-formatted-text';
@@ -51,7 +49,6 @@
AccountInfo,
AttentionSetInput,
ChangeInfo,
- CommentInput,
GroupInfo,
isAccount,
isDetailedLabelInfo,
@@ -86,7 +83,6 @@
isUnresolved,
UnsavedInfo,
} from '../../../utils/comment-util';
-import {GrTextarea} from '../../shared/gr-textarea/gr-textarea';
import {GrAccountChip} from '../../shared/gr-account-chip/gr-account-chip';
import {GrOverlay, GrOverlayStops} from '../../shared/gr-overlay/gr-overlay';
import {
@@ -117,11 +113,11 @@
LabelNameToValuesMap,
PatchSetNumber,
} from '../../../api/rest-api';
-import {css, html, PropertyValues, LitElement, nothing} from 'lit';
+import {css, html, PropertyValues, LitElement} from 'lit';
import {sharedStyles} from '../../../styles/shared-styles';
import {when} from 'lit/directives/when.js';
import {classMap} from 'lit/directives/class-map.js';
-import {BindValueChangeEvent, ValueChangedEvent} from '../../../types/events';
+import {ValueChangedEvent} from '../../../types/events';
import {customElement, property, state, query} from 'lit/decorators.js';
import {subscribe} from '../../lit/subscription-controller';
import {configModelToken} from '../../../models/config/config-model';
@@ -255,8 +251,6 @@
@query('#labelScores') labelScores?: GrLabelScores;
- @query('#textarea') textarea?: GrTextarea;
-
@query('#reviewerConfirmationOverlay')
reviewerConfirmationOverlay?: GrOverlay;
@@ -496,28 +490,13 @@
min-height: unset;
}
textareaContainer,
- #textarea,
gr-endpoint-decorator[name='reply-text'] {
display: flex;
width: 100%;
}
- #textarea {
- display: block;
- width: unset;
- font-family: var(--monospace-font-family);
- font-size: var(--font-size-code);
- line-height: calc(var(--font-size-code) + var(--spacing-s));
- font-weight: var(--font-weight-normal);
- }
- .newReplyDialog#textarea {
- padding: var(--spacing-m);
- }
gr-endpoint-decorator[name='reply-text'] {
flex-direction: column;
}
- #textarea {
- flex: 1;
- }
#checkingStatusLabel,
#notLatestLabel {
margin-left: var(--spacing-l);
@@ -948,14 +927,6 @@
}
private renderPatchsetLevelComment() {
- if (
- !this.flagsService.isEnabled(
- KnownExperimentId.PATCHSET_LEVEL_COMMENT_USES_GRCOMMENT
- )
- ) {
- return nothing;
- }
-
if (!this.patchsetLevelComment)
this.patchsetLevelComment = this.createDraft();
return html`
@@ -976,30 +947,6 @@
`;
}
- private renderPatchsetLevelTextarea() {
- if (
- this.flagsService.isEnabled(
- KnownExperimentId.PATCHSET_LEVEL_COMMENT_USES_GRCOMMENT
- )
- )
- return nothing;
- return html` <gr-textarea
- id="textarea"
- class="message newReplyDialog"
- .autocomplete=${'on'}
- .placeholder=${this.messagePlaceholder}
- monospace
- ?disabled=${this.disabled}
- .rows=${4}
- .text=${this.patchsetLevelDraftMessage}
- @bind-value-changed=${(e: BindValueChangeEvent) => {
- this.patchsetLevelDraftMessage = e.detail.value ?? '';
- this.handleHeightChanged();
- }}
- >
- </gr-textarea>`;
- }
-
private renderReplyText() {
if (!this.change) return;
return html`
@@ -1013,7 +960,6 @@
>
<gr-endpoint-decorator name="reply-text">
${this.renderPatchsetLevelComment()}
- ${this.renderPatchsetLevelTextarea()}
<gr-endpoint-param name="change" .value=${this.change}>
</gr-endpoint-param>
</gr-endpoint-decorator>
@@ -1501,32 +1447,11 @@
reviewInput.remove_from_attention_set
);
- if (
- this.flagsService.isEnabled(
- KnownExperimentId.PATCHSET_LEVEL_COMMENT_USES_GRCOMMENT
- )
- ) {
- const patchsetLevelComment = queryAndAssert<GrComment>(
- this,
- '#patchsetLevelComment'
- );
- await patchsetLevelComment.save();
- }
-
- if (
- this.patchsetLevelDraftMessage &&
- !this.flagsService.isEnabled(
- KnownExperimentId.PATCHSET_LEVEL_COMMENT_USES_GRCOMMENT
- )
- ) {
- const comment: CommentInput = {
- message: this.patchsetLevelDraftMessage,
- unresolved: !this.patchsetLevelDraftIsResolved,
- };
- reviewInput.comments = {
- [SpecialFilePath.PATCHSET_LEVEL_COMMENTS]: [comment],
- };
- }
+ const patchsetLevelComment = queryAndAssert<GrComment>(
+ this,
+ '#patchsetLevelComment'
+ );
+ await patchsetLevelComment.save();
assertIsDefined(this.change, 'change');
reviewInput.reviewers = this.computeReviewers();
@@ -1570,10 +1495,7 @@
if (!section || section === FocusTarget.ANY) {
section = this.chooseFocusTarget();
}
- if (section === FocusTarget.BODY) {
- const textarea = queryAndAssert<GrTextarea>(this, 'gr-textarea');
- setTimeout(() => textarea.getNativeTextarea().focus());
- } else if (section === FocusTarget.REVIEWERS) {
+ if (section === FocusTarget.REVIEWERS) {
const reviewerEntry = this.reviewersList?.focusStart;
setTimeout(() => reviewerEntry?.focus());
} else if (section === FocusTarget.CCS) {
@@ -1937,19 +1859,11 @@
bubbles: false,
})
);
- if (
- this.flagsService.isEnabled(
- KnownExperimentId.PATCHSET_LEVEL_COMMENT_USES_GRCOMMENT
- )
- ) {
- const patchsetLevelComment = queryAndAssert<GrComment>(
- this,
- '#patchsetLevelComment'
- );
- await patchsetLevelComment.save();
- } else {
- queryAndAssert<GrTextarea>(this, 'gr-textarea').closeDropdown();
- }
+ const patchsetLevelComment = queryAndAssert<GrComment>(
+ this,
+ '#patchsetLevelComment'
+ );
+ await patchsetLevelComment.save();
this.rebuildReviewerArrays();
}
@@ -2139,14 +2053,9 @@
isDetailedLabelInfo(label) && getApprovalInfo(label, this.account!)
);
const revotingOrNewVote = this.labelsChanged || existingVote;
- let hasDrafts = this.includeComments && this.draftCommentThreads.length > 0;
- if (
- this.flagsService.isEnabled(
- KnownExperimentId.PATCHSET_LEVEL_COMMENT_USES_GRCOMMENT
- )
- ) {
- hasDrafts = hasDrafts || this.patchsetLevelDraftMessage.length > 0;
- }
+ const hasDrafts =
+ (this.includeComments && this.draftCommentThreads.length > 0) ||
+ this.patchsetLevelDraftMessage.length > 0;
return (
!hasDrafts &&
!this.patchsetLevelDraftMessage.length &&
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
index cfa4145..2001d6d 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.ts
@@ -16,11 +16,7 @@
stubFlags,
stubRestApi,
} from '../../../test/test-utils';
-import {
- ChangeStatus,
- ReviewerState,
- SpecialFilePath,
-} from '../../../constants/constants';
+import {ChangeStatus, ReviewerState} from '../../../constants/constants';
import {JSON_PREFIX} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
import {StandardLabels} from '../../../utils/label-util';
import {
@@ -237,12 +233,12 @@
<section class="newReplyDialog textareaContainer">
<div class="patchsetLevelContainer resolved">
<gr-endpoint-decorator name="reply-text">
- <gr-textarea
- class="message monospace newReplyDialog"
- id="textarea"
- monospace=""
+ <gr-comment
+ hide-header=""
+ id="patchsetLevelComment"
+ permanent-editing-mode=""
>
- </gr-textarea>
+ </gr-comment>
<gr-endpoint-param name="change"> </gr-endpoint-param>
</gr-endpoint-decorator>
</div>
@@ -346,14 +342,6 @@
'Code-Review': 0,
Verified: 0,
},
- comments: {
- [SpecialFilePath.PATCHSET_LEVEL_COMMENTS]: [
- {
- message: 'I wholeheartedly disapprove',
- unresolved: false,
- },
- ],
- },
reviewers: [],
add_to_attention_set: [
{reason: '<GERRIT_ACCOUNT_1> replied on the change', user: 999},
@@ -1090,14 +1078,6 @@
'Code-Review': -1,
Verified: -1,
},
- comments: {
- [SpecialFilePath.PATCHSET_LEVEL_COMMENTS]: [
- {
- message: 'I wholeheartedly disapprove',
- unresolved: false,
- },
- ],
- },
reviewers: [],
add_to_attention_set: [
{user: 999, reason: '<GERRIT_ACCOUNT_1> replied on the change'},
@@ -1134,14 +1114,6 @@
'Code-Review': 0,
Verified: 0,
},
- comments: {
- [SpecialFilePath.PATCHSET_LEVEL_COMMENTS]: [
- {
- message: 'I wholeheartedly disapprove',
- unresolved: false,
- },
- ],
- },
reviewers: [],
add_to_attention_set: [
{reason: '<GERRIT_ACCOUNT_1> replied on the change', user: 999},
@@ -1477,20 +1449,29 @@
// explicitly instead
clock.tick(1);
assert.equal(chooseFocusTargetSpy.callCount, 1);
- assert.equal(element?.shadowRoot?.activeElement?.tagName, 'GR-TEXTAREA');
- assert.equal(element?.shadowRoot?.activeElement?.id, 'textarea');
+ assert.equal(element?.shadowRoot?.activeElement?.tagName, 'GR-COMMENT');
+ assert.equal(
+ element?.shadowRoot?.activeElement?.id,
+ 'patchsetLevelComment'
+ );
element.focusOn(element.FocusTarget.ANY);
clock.tick(1);
assert.equal(chooseFocusTargetSpy.callCount, 2);
- assert.equal(element?.shadowRoot?.activeElement?.tagName, 'GR-TEXTAREA');
- assert.equal(element?.shadowRoot?.activeElement?.id, 'textarea');
+ assert.equal(element?.shadowRoot?.activeElement?.tagName, 'GR-COMMENT');
+ assert.equal(
+ element?.shadowRoot?.activeElement?.id,
+ 'patchsetLevelComment'
+ );
element.focusOn(element.FocusTarget.BODY);
clock.tick(1);
assert.equal(chooseFocusTargetSpy.callCount, 2);
- assert.equal(element?.shadowRoot?.activeElement?.tagName, 'GR-TEXTAREA');
- assert.equal(element?.shadowRoot?.activeElement?.id, 'textarea');
+ assert.equal(element?.shadowRoot?.activeElement?.tagName, 'GR-COMMENT');
+ assert.equal(
+ element?.shadowRoot?.activeElement?.id,
+ 'patchsetLevelComment'
+ );
element.focusOn(element.FocusTarget.REVIEWERS);
clock.tick(1);
@@ -2291,9 +2272,6 @@
suite('patchset level comment using GrComment', () => {
setup(async () => {
- stubFlags('isEnabled')
- .withArgs(KnownExperimentId.PATCHSET_LEVEL_COMMENT_USES_GRCOMMENT)
- .returns(true);
element.account = createAccountWithId(1);
element.requestUpdate();
await element.updateComplete;
diff --git a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
index 4fe0f0b..4af24cc 100644
--- a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
+++ b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
@@ -3,38 +3,8 @@
* Copyright 2017 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {
- BasePatchSetNum,
- ChangeInfo,
- RepoName,
- RevisionPatchSetNum,
-} from '../../../types/common';
-import {ParsedChangeInfo} from '../../../types/types';
-import {createRepoUrl} from '../../../models/views/repo';
-import {createDiffUrl} from '../../../models/views/diff';
-import {createChangeUrl} from '../../../models/views/change';
import {define} from '../../../models/dependency';
-const uninitialized = () => {
- console.warn('Use of uninitialized routing');
-};
-
-const uninitializedNavigate: NavigateCallback = () => {
- uninitialized();
- return '';
-};
-
-export type NavigateCallback = (target: string, redirect?: boolean) => void;
-
-interface NavigateToChangeParams {
- patchNum?: RevisionPatchSetNum;
- basePatchNum?: BasePatchSetNum;
- isEdit?: boolean;
- redirect?: boolean;
- forceReload?: boolean;
- openReplyDialog?: boolean;
-}
-
export const navigationToken = define<NavigationService>('navigation');
export interface NavigationService {
@@ -57,99 +27,3 @@
*/
replaceUrl(url: string): void;
}
-
-// TODO(dmfilippov) Convert to class, extract consts, give better name and
-// expose as a service from appContext
-export const GerritNav = {
- _navigate: uninitializedNavigate,
-
- /**
- * Setup router implementation.
- *
- * @param navigate the router-abstracted equivalent of
- * `window.location.href = ...` or window.location.replace(...). The
- * string is a new location and boolean defines is it redirect or not
- * (true means redirect, i.e. equivalent of window.location.replace).
- */
- setup(navigate: NavigateCallback) {
- this._navigate = navigate;
- },
-
- destroy() {
- this._navigate = uninitializedNavigate;
- },
-
- /**
- * @param basePatchNum The string PARENT can be used for none.
- * @param redirect redirect to a change - if true, the current
- * location (i.e. page which makes redirect) is not added to a history.
- * I.e. back/forward buttons skip current location
- * @param forceReload Some views are smart about how to handle the reload
- * of the view. In certain cases we want to force the view to reload
- * and re-render everything.
- */
- navigateToChange(
- change: Pick<ChangeInfo, '_number' | 'project'>,
- options: NavigateToChangeParams = {}
- ) {
- const {
- patchNum,
- basePatchNum,
- isEdit,
- forceReload,
- redirect,
- openReplyDialog,
- } = options;
- this._navigate(
- createChangeUrl({
- changeNum: change._number,
- project: change.project,
- patchNum,
- basePatchNum,
- edit: isEdit,
- forceReload,
- openReplyDialog,
- }),
- redirect
- );
- },
-
- /**
- * @param basePatchNum The string PARENT can be used for none.
- */
- navigateToDiff(
- change: ChangeInfo | ParsedChangeInfo,
- filePath: string,
- patchNum?: RevisionPatchSetNum,
- basePatchNum?: BasePatchSetNum,
- lineNum?: number
- ) {
- this._navigate(
- createDiffUrl({
- changeNum: change._number,
- project: change.project,
- path: filePath,
- patchNum,
- basePatchNum,
- lineNum,
- })
- );
- },
-
- /**
- * Navigate to an arbitrary relative URL.
- */
- navigateToRelativeUrl(relativeUrl: string) {
- if (!relativeUrl.startsWith('/')) {
- throw new Error('navigateToRelativeUrl with non-relative URL');
- }
- this._navigate(relativeUrl);
- },
-
- /**
- * Navigate to a repo settings page.
- */
- navigateToRepo(repo: RepoName) {
- this._navigate(createRepoUrl({repo}));
- },
-};
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 c1728a2..762f6b7 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -8,7 +8,7 @@
PageContext,
PageNextCallback,
} from '../../../utils/page-wrapper-utils';
-import {GerritNav, NavigationService} from '../gr-navigation/gr-navigation';
+import {NavigationService} from '../gr-navigation/gr-navigation';
import {getAppContext} from '../../../services/app-context';
import {convertToPatchSetNum} from '../../../utils/patch-set-util';
import {assertIsDefined} from '../../../utils/common-util';
@@ -477,14 +477,6 @@
page.base(base);
}
- GerritNav.setup((url, redirect?) => {
- if (redirect) {
- page.redirect(url);
- } else {
- page.show(url);
- }
- });
-
page.exit('*', (_, next) => {
if (!this._isRedirecting) {
this.reporting.beforeLocationChanged();
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 47461eb..ce3ed0d 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
@@ -6,7 +6,6 @@
import '../../../test/common-test-setup';
import './gr-router';
import {page, PageContext} from '../../../utils/page-wrapper-utils';
-import {GerritNav} from '../gr-navigation/gr-navigation';
import {
stubBaseUrl,
stubRestApi,
@@ -104,7 +103,6 @@
const requiresAuth: any = {};
const doesNotRequireAuth: any = {};
- sinon.stub(GerritNav, 'setup');
sinon.stub(page, 'start');
sinon.stub(page, 'base');
sinon
@@ -368,7 +366,6 @@
onExit = _onExit;
};
sinon.stub(page, 'exit').callsFake(onRegisteringExit);
- sinon.stub(GerritNav, 'setup');
sinon.stub(page, 'start');
sinon.stub(page, 'base');
router.startRouter();
diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
index ee6948a..17edc19 100644
--- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
+++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
@@ -21,7 +21,6 @@
query as queryDec,
} from 'lit/decorators.js';
import {Shortcut, ShortcutController} from '../../lit/shortcut-controller';
-import {query as queryUtil} from '../../../utils/common-util';
import {assertIsDefined} from '../../../utils/common-util';
import {configModelToken} from '../../../models/config/config-model';
import {resolve} from '../../../models/dependency';
@@ -295,14 +294,6 @@
*/
private preventDefaultAndNavigateToInputVal(e: Event) {
e.preventDefault();
- const target = e.composedPath()[0] as HTMLElement;
- // If the target is the #searchInput or has a sub-input component, that
- // is what holds the focus as opposed to the target from the DOM event.
- if (queryUtil(target, '#input')) {
- queryUtil<HTMLElement>(target, '#input')!.blur();
- } else {
- target.blur();
- }
if (!this.inputVal) return;
const trimmedInput = this.inputVal.trim();
if (trimmedInput) {
diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.ts b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.ts
index f7e8b8e..dbb3db9 100644
--- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.ts
@@ -134,26 +134,6 @@
await promise;
});
- test('input blurred after commit', async () => {
- const blurSpy = sinon.spy(
- queryAndAssert<PaperInputElement>(
- queryAndAssert<GrAutocomplete>(element, '#searchInput'),
- '#input'
- ),
- 'blur'
- );
- queryAndAssert<GrAutocomplete>(element, '#searchInput').text = 'fate/stay';
- await element.updateComplete;
- pressKey(
- queryAndAssert<PaperInputElement>(
- queryAndAssert<GrAutocomplete>(element, '#searchInput'),
- '#input'
- ),
- Key.ENTER
- );
- await waitUntil(() => blurSpy.called);
- });
-
test('empty search query does not trigger nav', async () => {
const searchSpy = sinon.spy();
element.addEventListener('handle-search', searchSpy);
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
index f2fa44c..262f1fd 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
@@ -8,7 +8,7 @@
import '../../shared/gr-icon/gr-icon';
import '../../shared/gr-overlay/gr-overlay';
import '../../../embed/diff/gr-diff/gr-diff';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {
NumericChangeId,
EDIT,
@@ -31,6 +31,8 @@
import {sharedStyles} from '../../../styles/shared-styles';
import {subscribe} from '../../lit/subscription-controller';
import {assert} from '../../../utils/common-util';
+import {resolve} from '../../../models/dependency';
+import {createChangeUrl} from '../../../models/views/change';
interface FilePreview {
filepath: string;
@@ -79,6 +81,8 @@
private readonly userModel = getAppContext().userModel;
+ private readonly getNavigation = resolve(this, navigationToken);
+
constructor() {
super();
subscribe(
@@ -338,10 +342,13 @@
);
}
if (res && res.ok) {
- GerritNav.navigateToChange(change, {
- patchNum: EDIT,
- basePatchNum: patchNum as BasePatchSetNum,
- });
+ this.getNavigation().setUrl(
+ createChangeUrl({
+ change,
+ patchNum: EDIT,
+ basePatchNum: patchNum as BasePatchSetNum,
+ })
+ );
this.close(true);
}
this.isApplyFixLoading = false;
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts
index 47ac64e..b6e4a95 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts
@@ -5,10 +5,10 @@
*/
import '../../../test/common-test-setup';
import './gr-apply-fix-dialog';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {queryAndAssert, stubRestApi} from '../../../test/test-utils';
import {GrApplyFixDialog} from './gr-apply-fix-dialog';
-import {BasePatchSetNum, EDIT, PatchSetNum} from '../../../types/common';
+import {PatchSetNum} from '../../../types/common';
import {
createFixSuggestionInfo,
createParsedChange,
@@ -24,9 +24,12 @@
} from '../../../types/events';
import {GrButton} from '../../shared/gr-button/gr-button';
import {fixture, html, assert} from '@open-wc/testing';
+import {SinonStub} from 'sinon';
+import {testResolver} from '../../../test/common-test-setup';
suite('gr-apply-fix-dialog tests', () => {
let element: GrApplyFixDialog;
+ let setUrlStub: SinonStub;
const TWO_FIXES: OpenFixPreviewEventDetail = {
patchNum: 2 as PatchSetNum,
@@ -58,6 +61,7 @@
}
setup(async () => {
+ setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
element = await fixture<GrApplyFixDialog>(
html`<gr-apply-fix-dialog></gr-apply-fix-dialog>`
);
@@ -238,7 +242,6 @@
const applyRobotFixSuggestionStub = stubRestApi(
'applyRobotFixSuggestion'
).returns(Promise.resolve(new Response(null, {status: 200})));
- const navigateToChangeStub = sinon.stub(GerritNav, 'navigateToChange');
element.currentFix = createFixSuggestionInfo('123');
const closeFixPreviewEventSpy = sinon.spy();
@@ -247,6 +250,7 @@
EventType.CLOSE_FIX_PREVIEW,
closeFixPreviewEventSpy
);
+
await element.handleApplyFix(new CustomEvent('confirm'));
sinon.assert.calledOnceWithExactly(
@@ -255,10 +259,8 @@
2 as PatchSetNum,
'123'
);
- sinon.assert.calledWithExactly(navigateToChangeStub, element.change!, {
- patchNum: EDIT,
- basePatchNum: element.change!.revisions[2]._number as BasePatchSetNum,
- });
+ assert.isTrue(setUrlStub.called);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/2..edit');
sinon.assert.calledOnceWithExactly(
closeFixPreviewEventSpy,
@@ -268,7 +270,6 @@
},
})
);
-
// reset gr-apply-fix-dialog and close
assert.equal(element.currentFix, undefined);
assert.equal(element.currentPreviews.length, 0);
@@ -278,18 +279,17 @@
const applyRobotFixSuggestionStub = stubRestApi(
'applyRobotFixSuggestion'
).returns(Promise.resolve(new Response(null, {status: 500})));
- const navigateToChangeStub = sinon.stub(GerritNav, 'navigateToChange');
element.currentFix = createFixSuggestionInfo('fix_123');
await element.handleApplyFix(new CustomEvent('confirm'));
+
sinon.assert.calledWithExactly(
applyRobotFixSuggestionStub,
element.change!._number,
2 as PatchSetNum,
'fix_123'
);
- assert.isTrue(navigateToChangeStub.notCalled);
-
+ assert.isFalse(setUrlStub.called);
assert.equal(element.isApplyFixLoading, false);
});
@@ -307,7 +307,6 @@
stubRestApi('applyRobotFixSuggestion').returns(
Promise.reject(new Error('backend error'))
);
- const navigateToChangeStub = sinon.stub(GerritNav, 'navigateToChange');
element.currentFix = createFixSuggestionInfo('fix_123');
const closeFixPreviewEventSpy = sinon.spy();
@@ -322,7 +321,7 @@
expectedError = e;
});
assert.isOk(expectedError);
- assert.isFalse(navigateToChangeStub.called);
+ assert.isFalse(setUrlStub.called);
sinon.assert.notCalled(closeFixPreviewEventSpy);
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 5134d46..18322fe 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -21,7 +21,7 @@
import '../gr-patch-range-select/gr-patch-range-select';
import '../../change/gr-download-dialog/gr-download-dialog';
import '../../shared/gr-overlay/gr-overlay';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getAppContext} from '../../../services/app-context';
import {
computeAllPatchSets,
@@ -318,6 +318,8 @@
private readonly shortcutsController = new ShortcutController(this);
+ private readonly getNavigation = resolve(this, navigationToken);
+
constructor() {
super();
this.setupKeyboardShortcuts();
@@ -1158,11 +1160,13 @@
return;
}
- GerritNav.navigateToDiff(
- this.change,
- this.commentSkips.previous,
- this.patchRange.patchNum,
- this.patchRange.basePatchNum
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: this.commentSkips.previous,
+ patchNum: this.patchRange.patchNum,
+ basePatchNum: this.patchRange.basePatchNum,
+ })
);
}
@@ -1178,11 +1182,13 @@
return;
}
- GerritNav.navigateToDiff(
- this.change,
- this.commentSkips.next,
- this.patchRange.patchNum,
- this.patchRange.basePatchNum
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: this.commentSkips.next,
+ patchNum: this.patchRange.patchNum,
+ basePatchNum: this.patchRange.basePatchNum,
+ })
);
}
@@ -1357,12 +1363,14 @@
newPath.path,
this.patchRange
)?.[0].line;
- GerritNav.navigateToDiff(
- this.change,
- newPath.path,
- this.patchRange.patchNum,
- this.patchRange.basePatchNum,
- lineNum
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: newPath.path,
+ patchNum: this.patchRange.patchNum,
+ basePatchNum: this.patchRange.basePatchNum,
+ lineNum,
+ })
);
}
@@ -1402,7 +1410,7 @@
patchNum: this.patchRange.patchNum,
lineNum: cursorAddress?.number,
});
- GerritNav.navigateToRelativeUrl(editUrl);
+ this.getNavigation().setUrl(editUrl);
}
/**
@@ -1536,7 +1544,7 @@
);
if (!comment) {
fireAlert(this, 'comment not found');
- GerritNav.navigateToChange(this.change);
+ this.getNavigation().setUrl(createChangeUrl({change: this.change}));
return;
}
this.getChangeModel().updatePath(comment.path);
@@ -1707,12 +1715,14 @@
${this.patchRange.patchNum}. Showing diff of Base vs
${this.patchRange.basePatchNum}`
);
- GerritNav.navigateToDiff(
- this.change,
- this.path,
- this.patchRange.basePatchNum as RevisionPatchSetNum,
- PARENT,
- this.focusLineNum
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: this.path,
+ patchNum: this.patchRange.basePatchNum as RevisionPatchSetNum,
+ basePatchNum: PARENT,
+ lineNum: this.focusLineNum,
+ })
);
return;
}
@@ -1819,8 +1829,7 @@
this.change.revisions
);
return createChangeUrl({
- changeNum: this.change._number,
- project: this.change.project,
+ change: this.change,
patchNum: range.patchNum,
basePatchNum: range.basePatchNum,
});
@@ -1835,11 +1844,14 @@
) {
if (!change) return;
const range = this.getChangeUrlRange(patchRange, revisions);
- GerritNav.navigateToChange(change, {
- patchNum: range.patchNum,
- basePatchNum: range.basePatchNum,
- openReplyDialog: !!openReplyDialog,
- });
+ this.getNavigation().setUrl(
+ createChangeUrl({
+ change,
+ patchNum: range.patchNum,
+ basePatchNum: range.basePatchNum,
+ openReplyDialog: !!openReplyDialog,
+ })
+ );
}
// Private but used in tests
@@ -1877,11 +1889,13 @@
return;
}
- GerritNav.navigateToDiff(
- this.change,
- path,
- this.patchRange.patchNum,
- this.patchRange.basePatchNum
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path,
+ patchNum: this.patchRange.patchNum,
+ basePatchNum: this.patchRange.basePatchNum,
+ })
);
}
@@ -1898,7 +1912,14 @@
) {
return;
}
- GerritNav.navigateToDiff(this.change, this.path, patchNum, basePatchNum);
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: this.path,
+ patchNum,
+ basePatchNum,
+ })
+ );
}
// Private but used in tests.
@@ -2104,7 +2125,13 @@
fireAlert(this, 'Base is already selected.');
return;
}
- GerritNav.navigateToDiff(this.change, this.path, this.patchRange.patchNum);
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: this.path,
+ patchNum: this.patchRange.patchNum,
+ })
+ );
}
// Private but used in tests.
@@ -2117,14 +2144,18 @@
fireAlert(this, 'Left is already base.');
return;
}
- GerritNav.navigateToDiff(
- this.change,
- this.path,
- this.patchRange.basePatchNum as RevisionPatchSetNum,
- PARENT,
+ const lineNum =
this.viewState?.view === GerritView.DIFF && this.viewState?.commentLink
? this.focusLineNum
- : undefined
+ : undefined;
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: this.path,
+ patchNum: this.patchRange.basePatchNum as RevisionPatchSetNum,
+ basePatchNum: PARENT,
+ lineNum,
+ })
);
}
@@ -2140,11 +2171,13 @@
return;
}
- GerritNav.navigateToDiff(
- this.change,
- this.path,
- latestPatchNum,
- this.patchRange.basePatchNum
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: this.path,
+ patchNum: latestPatchNum,
+ basePatchNum: this.patchRange.basePatchNum,
+ })
);
}
@@ -2159,11 +2192,13 @@
fireAlert(this, 'Right is already latest.');
return;
}
- GerritNav.navigateToDiff(
- this.change,
- this.path,
- latestPatchNum,
- this.patchRange.patchNum as BasePatchSetNum
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: this.path,
+ patchNum: latestPatchNum,
+ basePatchNum: this.patchRange.patchNum as BasePatchSetNum,
+ })
);
}
@@ -2181,7 +2216,13 @@
fireAlert(this, 'Already diffing base against latest.');
return;
}
- GerritNav.navigateToDiff(this.change, this.path, latestPatchNum);
+ this.getNavigation().setUrl(
+ createDiffUrl({
+ change: this.change,
+ path: this.path,
+ patchNum: latestPatchNum,
+ })
+ );
}
// Private but used in tests.
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
index 1b93560..b6e26ab 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
@@ -5,7 +5,7 @@
*/
import '../../../test/common-test-setup';
import './gr-diff-view';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {
ChangeStatus,
DiffViewMode,
@@ -69,7 +69,7 @@
import {EventType} from '../../../types/events';
import {Key} from '../../../utils/dom-util';
import {GrButton} from '../../shared/gr-button/gr-button';
-import {createEditUrl} from '../../../models/views/edit';
+import {testResolver} from '../../../test/common-test-setup';
function createComment(
id: string,
@@ -92,6 +92,7 @@
let clock: SinonFakeTimers;
let diffCommentsStub;
let getDiffRestApiStub: SinonStub;
+ let setUrlStub: SinonStub;
function getFilesFromFileList(fileList: string[]): Files {
const changeFilesByPath = fileList.reduce((files, path) => {
@@ -105,6 +106,7 @@
}
setup(async () => {
+ setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
stubRestApi('getConfig').returns(Promise.resolve(createServerInfo()));
stubRestApi('getLoggedIn').returns(Promise.resolve(false));
stubRestApi('getProjectConfig').returns(Promise.resolve(createConfig()));
@@ -262,8 +264,7 @@
});
});
- test('unchanged diff X vs latest from comment links navigates to base vs X', () => {
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
+ test('unchanged diff X vs latest from comment links navigates to base vs X', async () => {
element.getCommentsModel().setState({
comments: {
'/COMMIT_MSG': [
@@ -301,21 +302,15 @@
...createParsedChange(),
revisions: createRevisions(11),
};
- return viewStateChangedSpy.returnValues[0]?.then(() => {
- assert.isTrue(
- diffNavStub.lastCall.calledWithExactly(
- element.change!,
- '/COMMIT_MSG',
- 2 as RevisionPatchSetNum,
- PARENT,
- 10
- )
- );
- });
+ await viewStateChangedSpy.returnValues[0];
+ assert.isTrue(setUrlStub.calledOnce);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/2//COMMIT_MSG#10'
+ );
});
- test('unchanged diff Base vs latest from comment does not navigate', () => {
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
+ test('unchanged diff Base vs latest from comment does not navigate', async () => {
element.getCommentsModel().setState({
comments: {
'/COMMIT_MSG': [
@@ -353,9 +348,8 @@
...createParsedChange(),
revisions: createRevisions(11),
};
- return viewStateChangedSpy.returnValues[0]!.then(() => {
- assert.isFalse(diffNavStub.called);
- });
+ await viewStateChangedSpy.returnValues[0];
+ assert.isFalse(setUrlStub.calledOnce);
});
test('isFileUnchanged', () => {
@@ -649,26 +643,18 @@
element.path = 'glados.txt';
element.loggedIn = true;
await element.updateComplete;
-
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
- const changeNavStub = sinon.stub(GerritNav, 'navigateToChange');
+ setUrlStub.reset();
pressKey(element, 'u');
- assert(
- changeNavStub.lastCall.calledWith(element.change),
- 'Should navigate to /c/42/'
- );
+ assert.equal(setUrlStub.callCount, 1);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42');
await element.updateComplete;
pressKey(element, ']');
- assert(
- diffNavStub.lastCall.calledWith(
- element.change,
- 'wheatley.md',
- 10 as RevisionPatchSetNum,
- PARENT
- ),
- 'Should navigate to /c/42/10/wheatley.md'
+ assert.equal(setUrlStub.callCount, 2);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/10/wheatley.md'
);
element.path = 'wheatley.md';
await element.updateComplete;
@@ -676,14 +662,10 @@
assert.isTrue(element.loading);
pressKey(element, '[');
- assert(
- diffNavStub.lastCall.calledWith(
- element.change,
- 'glados.txt',
- 10 as RevisionPatchSetNum,
- PARENT
- ),
- 'Should navigate to /c/42/10/glados.txt'
+ assert.equal(setUrlStub.callCount, 3);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/10/glados.txt'
);
element.path = 'glados.txt';
await element.updateComplete;
@@ -691,14 +673,10 @@
assert.isTrue(element.loading);
pressKey(element, '[');
- assert(
- diffNavStub.lastCall.calledWith(
- element.change,
- 'chell.go',
- 10 as RevisionPatchSetNum,
- PARENT
- ),
- 'Should navigate to /c/42/10/chell.go'
+ assert.equal(setUrlStub.callCount, 4);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/10/chell.go'
);
element.path = 'chell.go';
await element.updateComplete;
@@ -706,10 +684,8 @@
assert.isTrue(element.loading);
pressKey(element, '[');
- assert(
- changeNavStub.lastCall.calledWith(element.change),
- 'Should navigate to /c/42/'
- );
+ assert.equal(setUrlStub.callCount, 5);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42');
await element.updateComplete;
assert.isTrue(element.loading);
@@ -789,8 +765,6 @@
});
test('moveToNextCommentThread navigates to next file', async () => {
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
- const diffChangeStub = sinon.stub(element, 'navigateToChange');
assertIsDefined(element.cursor);
sinon.stub(element.cursor, 'isAtEnd').returns(true);
element.changeNum = 42 as NumericChangeId;
@@ -816,17 +790,15 @@
]);
element.path = 'glados.txt';
element.loggedIn = true;
+ await element.updateComplete;
+ setUrlStub.reset();
pressKey(element, 'N');
await element.updateComplete;
- assert.isTrue(
- diffNavStub.calledWithExactly(
- element.change,
- 'wheatley.md',
- 10 as RevisionPatchSetNum,
- PARENT,
- 21
- )
+ assert.equal(setUrlStub.callCount, 1);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/10/wheatley.md#21'
);
element.path = 'wheatley.md'; // navigated to next file
@@ -834,7 +806,8 @@
pressKey(element, 'N');
await element.updateComplete;
- assert.isTrue(diffChangeStub.called);
+ assert.equal(setUrlStub.callCount, 2);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42');
});
test('shift+x shortcut toggles all diff context', async () => {
@@ -851,11 +824,11 @@
patchNum: 10 as RevisionPatchSetNum,
};
await element.updateComplete;
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element.handleDiffAgainstBase();
- const args = diffNavStub.getCall(0).args;
- assert.equal(args[2], 10 as RevisionPatchSetNum);
- assert.isNotOk(args[3]);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/10/some/path.txt'
+ );
});
test('diff against latest', async () => {
@@ -869,11 +842,11 @@
patchNum: 10 as RevisionPatchSetNum,
};
await element.updateComplete;
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element.handleDiffAgainstLatest();
- const args = diffNavStub.getCall(0).args;
- assert.equal(args[2], 12 as RevisionPatchSetNum);
- assert.equal(args[3], 5 as BasePatchSetNum);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/5..12/foo'
+ );
});
test('handleDiffBaseAgainstLeft', async () => {
@@ -894,13 +867,8 @@
path: 'foo',
};
await element.updateComplete;
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element.handleDiffBaseAgainstLeft();
- assert(diffNavStub.called);
- const args = diffNavStub.getCall(0).args;
- assert.equal(args[2], 1 as RevisionPatchSetNum);
- assert.equal(args[3], PARENT);
- assert.isNotOk(args[4]);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/1/foo');
});
test('handleDiffBaseAgainstLeft when initially navigating to a comment', () => {
@@ -919,13 +887,11 @@
changeNum: 42 as NumericChangeId,
};
element.focusLineNum = 10;
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element.handleDiffBaseAgainstLeft();
- assert(diffNavStub.called);
- const args = diffNavStub.getCall(0).args;
- assert.equal(args[2], 1 as RevisionPatchSetNum);
- assert.equal(args[3], PARENT);
- assert.equal(args[4], 10);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/1/some/path.txt#10'
+ );
});
test('handleDiffRightAgainstLatest', async () => {
@@ -939,12 +905,11 @@
patchNum: 3 as RevisionPatchSetNum,
};
await element.updateComplete;
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element.handleDiffRightAgainstLatest();
- assert(diffNavStub.called);
- const args = diffNavStub.getCall(0).args;
- assert.equal(args[2], 10 as RevisionPatchSetNum);
- assert.equal(args[3], 3 as BasePatchSetNum);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/3..10/foo'
+ );
});
test('handleDiffBaseAgainstLatest', async () => {
@@ -957,26 +922,20 @@
patchNum: 3 as RevisionPatchSetNum,
};
await element.updateComplete;
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element.handleDiffBaseAgainstLatest();
- assert(diffNavStub.called);
- const args = diffNavStub.getCall(0).args;
- assert.equal(args[2], 10 as RevisionPatchSetNum);
- assert.isNotOk(args[3]);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/10/some/path.txt'
+ );
});
test('A fires an error event when not logged in', async () => {
- const changeNavStub = sinon.stub(GerritNav, 'navigateToChange');
element.loggedIn = false;
const loggedInErrorSpy = sinon.spy();
element.addEventListener('show-auth-required', loggedInErrorSpy);
pressKey(element, 'a');
await element.updateComplete;
- assert.isTrue(
- changeNavStub.notCalled,
- 'The `a` keyboard shortcut ' +
- 'should only work when the user is logged in.'
- );
+ assert.isFalse(setUrlStub.calledOnce);
assert.isTrue(loggedInErrorSpy.called);
});
@@ -994,19 +953,19 @@
b: createRevision(5),
},
};
- const changeNavStub = sinon.stub(GerritNav, 'navigateToChange');
element.loggedIn = true;
+ await element.updateComplete;
const loggedInErrorSpy = sinon.spy();
element.addEventListener('show-auth-required', loggedInErrorSpy);
+ setUrlStub.reset();
+
pressKey(element, 'a');
+
await element.updateComplete;
- assert(
- changeNavStub.lastCall.calledWithExactly(element.change, {
- patchNum: 10 as RevisionPatchSetNum,
- basePatchNum: 5 as BasePatchSetNum,
- openReplyDialog: true,
- }),
- 'Should navigate to /c/42/5..10'
+ assert.equal(setUrlStub.callCount, 1);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/5..10?openReplyDialog=true'
);
assert.isFalse(loggedInErrorSpy.called);
});
@@ -1025,19 +984,15 @@
b: createRevision(2),
},
};
- const changeNavStub = sinon.stub(GerritNav, 'navigateToChange');
element.loggedIn = true;
const loggedInErrorSpy = sinon.spy();
element.addEventListener('show-auth-required', loggedInErrorSpy);
pressKey(element, 'a');
await element.updateComplete;
- assert(
- changeNavStub.lastCall.calledWithExactly(element.change, {
- patchNum: 1 as RevisionPatchSetNum,
- basePatchNum: PARENT,
- openReplyDialog: true,
- }),
- 'Should navigate to /c/42/1'
+ assert.isTrue(setUrlStub.calledOnce);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/1?openReplyDialog=true'
);
assert.isFalse(loggedInErrorSpy.called);
});
@@ -1063,71 +1018,41 @@
]);
element.path = 'glados.txt';
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
- const changeNavStub = sinon.stub(GerritNav, 'navigateToChange');
-
pressKey(element, 'u');
- assert(
- changeNavStub.lastCall.calledWithExactly(element.change, {
- patchNum: 10 as RevisionPatchSetNum,
- basePatchNum: 5 as BasePatchSetNum,
- openReplyDialog: false,
- }),
- 'Should navigate to /c/42/5..10'
- );
+ assert.equal(setUrlStub.callCount, 1);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/5..10');
pressKey(element, ']');
assert.isTrue(element.loading);
- assert(
- diffNavStub.lastCall.calledWithExactly(
- element.change,
- 'wheatley.md',
- 10 as RevisionPatchSetNum,
- 5 as BasePatchSetNum,
- undefined
- ),
- 'Should navigate to /c/42/5..10/wheatley.md'
+ assert.equal(setUrlStub.callCount, 2);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/5..10/wheatley.md'
);
element.path = 'wheatley.md';
pressKey(element, '[');
assert.isTrue(element.loading);
- assert(
- diffNavStub.lastCall.calledWithExactly(
- element.change,
- 'glados.txt',
- 10 as RevisionPatchSetNum,
- 5 as BasePatchSetNum,
- undefined
- ),
- 'Should navigate to /c/42/5..10/glados.txt'
+ assert.equal(setUrlStub.callCount, 3);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/5..10/glados.txt'
);
element.path = 'glados.txt';
pressKey(element, '[');
assert.isTrue(element.loading);
- assert(
- diffNavStub.lastCall.calledWithExactly(
- element.change,
- 'chell.go',
- 10 as RevisionPatchSetNum,
- 5 as BasePatchSetNum,
- undefined
- ),
- 'Should navigate to /c/42/5..10/chell.go'
+ assert.equal(setUrlStub.callCount, 4);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/5..10/chell.go'
);
element.path = 'chell.go';
pressKey(element, '[');
assert.isTrue(element.loading);
- assert(
- changeNavStub.lastCall.calledWithExactly(element.change, {
- patchNum: 10 as RevisionPatchSetNum,
- basePatchNum: 5 as BasePatchSetNum,
- openReplyDialog: false,
- }),
- 'Should navigate to /c/42/5..10'
- );
+ assert.equal(setUrlStub.callCount, 5);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/5..10');
assertIsDefined(element.downloadOverlay);
const downloadOverlayStub = sinon
@@ -1158,69 +1083,35 @@
]);
element.path = 'glados.txt';
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
- const changeNavStub = sinon.stub(GerritNav, 'navigateToChange');
-
pressKey(element, 'u');
- assert(
- changeNavStub.lastCall.calledWithExactly(element.change, {
- patchNum: 1 as RevisionPatchSetNum,
- basePatchNum: PARENT,
- openReplyDialog: false,
- }),
- 'Should navigate to /c/42/1'
- );
+ assert.isTrue(setUrlStub.calledOnce);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/1');
pressKey(element, ']');
- assert(
- diffNavStub.lastCall.calledWithExactly(
- element.change,
- 'wheatley.md',
- 1 as RevisionPatchSetNum,
- PARENT,
- undefined
- ),
- 'Should navigate to /c/42/1/wheatley.md'
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/1/wheatley.md'
);
element.path = 'wheatley.md';
pressKey(element, '[');
- assert(
- diffNavStub.lastCall.calledWithExactly(
- element.change,
- 'glados.txt',
- 1 as RevisionPatchSetNum,
- PARENT,
- undefined
- ),
- 'Should navigate to /c/42/1/glados.txt'
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/1/glados.txt'
);
element.path = 'glados.txt';
pressKey(element, '[');
- assert(
- diffNavStub.lastCall.calledWithExactly(
- element.change,
- 'chell.go',
- 1 as RevisionPatchSetNum,
- PARENT,
- undefined
- ),
- 'Should navigate to /c/42/1/chell.go'
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42/1/chell.go'
);
element.path = 'chell.go';
- changeNavStub.reset();
+ setUrlStub.reset();
pressKey(element, '[');
- assert(
- changeNavStub.lastCall.calledWithExactly(element.change, {
- patchNum: 1 as RevisionPatchSetNum,
- basePatchNum: PARENT,
- openReplyDialog: false,
- }),
- 'Should navigate to /c/42/1'
- );
- assert.isTrue(changeNavStub.calledOnce);
+ assert.isTrue(setUrlStub.calledOnce);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/1');
});
test('edit should redirect to edit page', async () => {
@@ -1240,7 +1131,6 @@
b: createRevision(2),
},
};
- const redirectStub = sinon.stub(GerritNav, 'navigateToRelativeUrl');
await element.updateComplete;
const editBtn = queryAndAssert<GrButton>(
element,
@@ -1248,17 +1138,8 @@
);
assert.isTrue(!!editBtn);
editBtn.click();
- assert.isTrue(redirectStub.called);
- assert.isTrue(
- redirectStub.lastCall.calledWithExactly(
- createEditUrl({
- changeNum: element.change._number,
- project: element.change.project,
- path: element.path,
- patchNum: element.patchRange.patchNum,
- })
- )
- );
+ assert.equal(setUrlStub.callCount, 1);
+ assert.equal(setUrlStub.lastCall.firstArg, '/c/gerrit/+/42/1/t.txt,edit');
});
test('edit should redirect to edit page with line number', async () => {
@@ -1283,7 +1164,6 @@
sinon
.stub(element.cursor, 'getAddress')
.returns({number: lineNumber, leftSide: false});
- const redirectStub = sinon.stub(GerritNav, 'navigateToRelativeUrl');
await element.updateComplete;
const editBtn = queryAndAssert<GrButton>(
element,
@@ -1291,17 +1171,10 @@
);
assert.isTrue(!!editBtn);
editBtn.click();
- assert.isTrue(redirectStub.called);
- assert.isTrue(
- redirectStub.lastCall.calledWithExactly(
- createEditUrl({
- changeNum: element.change._number,
- project: element.change.project,
- path: element.path,
- patchNum: element.patchRange.patchNum,
- lineNum: lineNumber,
- })
- )
+ assert.equal(setUrlStub.callCount, 1);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/gerrit/+/42/1/t.txt,edit#42'
);
});
@@ -1622,8 +1495,7 @@
});
});
- test('handlePatchChange calls navigateToDiff correctly', async () => {
- const navigateStub = sinon.stub(GerritNav, 'navigateToDiff');
+ test('handlePatchChange calls setUrl correctly', async () => {
element.change = {
...createParsedChange(),
_number: 321 as NumericChangeId,
@@ -1646,13 +1518,9 @@
new CustomEvent('patch-range-change', {detail, bubbles: false})
);
- assert(
- navigateStub.lastCall.calledWithExactly(
- element.change,
- element.path,
- 1 as RevisionPatchSetNum,
- PARENT
- )
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/foo/bar/+/321/1/path/to/file.txt'
);
});
@@ -2166,11 +2034,9 @@
suite('skip next/previous', () => {
let navToChangeStub: SinonStub;
- let navToDiffStub: SinonStub;
setup(() => {
navToChangeStub = sinon.stub(element, 'navToChangeView');
- navToDiffStub = sinon.stub(GerritNav, 'navigateToDiff');
element.files = getFilesFromFileList([
'path/one.jpg',
'path/two.m4v',
@@ -2186,7 +2052,7 @@
test('no skips', () => {
element.moveToPreviousFileWithComment();
assert.isFalse(navToChangeStub.called);
- assert.isFalse(navToDiffStub.called);
+ assert.isFalse(setUrlStub.called);
});
test('no previous', async () => {
@@ -2200,7 +2066,7 @@
element.moveToPreviousFileWithComment();
assert.isTrue(navToChangeStub.calledOnce);
- assert.isFalse(navToDiffStub.called);
+ assert.isFalse(setUrlStub.called);
});
test('w/ previous', async () => {
@@ -2214,7 +2080,7 @@
element.moveToPreviousFileWithComment();
assert.isFalse(navToChangeStub.called);
- assert.isTrue(navToDiffStub.calledOnce);
+ assert.isTrue(setUrlStub.calledOnce);
});
});
@@ -2222,7 +2088,7 @@
test('no skips', () => {
element.moveToNextFileWithComment();
assert.isFalse(navToChangeStub.called);
- assert.isFalse(navToDiffStub.called);
+ assert.isFalse(setUrlStub.called);
});
test('no previous', async () => {
@@ -2236,7 +2102,7 @@
element.moveToNextFileWithComment();
assert.isTrue(navToChangeStub.calledOnce);
- assert.isFalse(navToDiffStub.called);
+ assert.isFalse(setUrlStub.called);
});
test('w/ previous', async () => {
@@ -2250,7 +2116,7 @@
element.moveToNextFileWithComment();
assert.isFalse(navToChangeStub.called);
- assert.isTrue(navToDiffStub.calledOnce);
+ assert.isTrue(setUrlStub.calledOnce);
});
});
});
@@ -2492,10 +2358,9 @@
assert.deepEqual(navStub.lastCall.args, [['file1', 'file3'], 1]);
});
- test('File change should trigger navigateToDiff once', async () => {
+ test('File change should trigger setUrl once', async () => {
element.files = getFilesFromFileList(['file1', 'file2', 'file3']);
sinon.stub(element, 'initLineOfInterestAndCursor');
- const navigateToDiffStub = sinon.stub(GerritNav, 'navigateToDiff');
// Load file1
element.viewState = {
@@ -2514,13 +2379,13 @@
revisions: createRevisions(1),
};
await element.updateComplete;
- assert.isTrue(navigateToDiffStub.notCalled);
+ assert.isFalse(setUrlStub.called);
// Switch to file2
element.handleFileChange(
new CustomEvent('value-change', {detail: {value: 'file2'}})
);
- assert.isTrue(navigateToDiffStub.calledOnce);
+ assert.isTrue(setUrlStub.calledOnce);
// This is to mock the param change triggered by above navigate
element.viewState = {
@@ -2536,7 +2401,7 @@
};
// No extra call
- assert.isTrue(navigateToDiffStub.calledOnce);
+ assert.isTrue(setUrlStub.calledOnce);
});
test('_computeDownloadDropdownLinks', () => {
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
index 6ef5406..a273a3e 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
@@ -10,7 +10,7 @@
import '../../shared/gr-dropdown/gr-dropdown';
import '../../shared/gr-overlay/gr-overlay';
import {GrEditAction, GrEditConstants} from '../gr-edit-constants';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {ChangeInfo, RevisionPatchSetNum} from '../../../types/common';
import {GrDialog} from '../../shared/gr-dialog/gr-dialog';
import {
@@ -32,6 +32,7 @@
import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
import {IronInputElement} from '@polymer/iron-input/iron-input';
import {createEditUrl} from '../../../models/views/edit';
+import {resolve} from '../../../models/dependency';
@customElement('gr-edit-controls')
export class GrEditControls extends LitElement {
@@ -75,6 +76,8 @@
private readonly restApiService = getAppContext().restApiService;
+ private readonly getNavigation = resolve(this, navigationToken);
+
static override get styles() {
return [
sharedStyles,
@@ -431,7 +434,7 @@
patchNum: this.patchNum,
});
- GerritNav.navigateToRelativeUrl(url);
+ this.getNavigation().setUrl(url);
this.closeDialog(this.getDialogFromEvent(e));
};
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
index 71a2c91..b4469db 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
@@ -6,7 +6,7 @@
import '../../../test/common-test-setup';
import './gr-edit-controls';
import {GrEditControls} from './gr-edit-controls';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {queryAll, stubRestApi, waitUntil} from '../../../test/test-utils';
import {createChange, createRevision} from '../../../test/test-data-generators';
import {GrAutocomplete} from '../../shared/gr-autocomplete/gr-autocomplete';
@@ -21,6 +21,8 @@
import {fixture, html, assert} from '@open-wc/testing';
import {GrButton} from '../../shared/gr-button/gr-button';
import '../../shared/gr-dialog/gr-dialog';
+import {waitForEventOnce} from '../../../utils/event-util';
+import {testResolver} from '../../../test/common-test-setup';
suite('gr-edit-controls tests', () => {
let element: GrEditControls;
@@ -193,11 +195,11 @@
});
suite('edit button CUJ', () => {
- let navStub: sinon.SinonStub;
+ let setUrlStub: sinon.SinonStub;
let openAutoComplete: GrAutocomplete;
setup(() => {
- navStub = sinon.stub(GerritNav, 'navigateToRelativeUrl');
+ setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
openAutoComplete = queryAndAssert<GrAutocomplete>(
element.openDialog,
'gr-autocomplete'
@@ -233,7 +235,7 @@
'gr-button[primary]'
).click();
- assert.isTrue(navStub.called);
+ assert.isTrue(setUrlStub.called);
assert.isTrue(closeDialogSpy.called);
});
@@ -246,7 +248,7 @@
await element.updateComplete;
await waitUntil(() => !element.openDialog!.disabled);
queryAndAssert<GrButton>(element.openDialog, 'gr-button').click();
- assert.isFalse(navStub.called);
+ assert.isFalse(setUrlStub.called);
await waitUntil(() => closeDialogSpy.called);
assert.equal(element.path, '');
});
@@ -507,15 +509,13 @@
});
suite('save file upload', () => {
- let navStub: sinon.SinonStub;
let fileStub: sinon.SinonStub;
setup(() => {
- navStub = sinon.stub(GerritNav, 'navigateToChange');
fileStub = stubRestApi('saveFileUploadChangeEdit');
});
- test('handleUploadConfirm', () => {
+ test('handleUploadConfirm', async () => {
fileStub.returns(Promise.resolve({ok: true}));
element.change = {
@@ -535,9 +535,13 @@
current_revision: 'efgh' as CommitId,
};
- element.handleUploadConfirm('test.php', 'base64').then(() => {
- assert.isTrue(navStub.calledWithExactly(1 as NumericChangeId));
- });
+ element.handleUploadConfirm('test.php', 'base64');
+
+ assert.isTrue(fileStub.calledOnce);
+ assert.equal(fileStub.lastCall.args[0], 1);
+ assert.equal(fileStub.lastCall.args[1], 'test.php');
+ assert.equal(fileStub.lastCall.args[2], 'base64');
+ await waitForEventOnce(element, 'reload');
});
});
diff --git a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
index 4cbde8f..0b1a2de 100644
--- a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
+++ b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
@@ -8,7 +8,7 @@
import '../../shared/gr-button/gr-button';
import '../../shared/gr-editable-label/gr-editable-label';
import '../gr-default-editor/gr-default-editor';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {computeTruncatedPath} from '../../../utils/path-list-util';
import {
EditPreferencesInfo,
@@ -32,6 +32,7 @@
import {changeModelToken} from '../../../models/change/change-model';
import {ShortcutController} from '../../lit/shortcut-controller';
import {editViewModelToken, EditViewState} from '../../../models/views/edit';
+import {createChangeUrl} from '../../../models/views/change';
const RESTORED_MESSAGE = 'Content restored from a previous edit.';
const SAVING_MESSAGE = 'Saving changes...';
@@ -94,6 +95,8 @@
private readonly getEditViewModel = resolve(this, editViewModelToken);
+ private readonly getNavigation = resolve(this, navigationToken);
+
private readonly shortcuts = new ShortcutController(this);
// Tests use this so needs to be non private
@@ -328,7 +331,7 @@
this,
'Change edits cannot be created if change is merged or abandoned. Redirected to non edit mode.'
);
- GerritNav.navigateToChange(this.change);
+ this.getNavigation().setUrl(createChangeUrl({change: this.change}));
}
private navigateToChangeIfEditType() {
@@ -336,7 +339,7 @@
// Prevent editing binary files
fireAlert(this, 'You cannot edit binary files within the inline editor.');
- GerritNav.navigateToChange(this.change);
+ this.getNavigation().setUrl(createChangeUrl({change: this.change}));
}
// private but used in test
@@ -362,10 +365,9 @@
// private but used in test
viewEditInChangeView() {
if (!this.change) return;
- GerritNav.navigateToChange(this.change, {
- isEdit: true,
- forceReload: true,
- });
+ this.getNavigation().setUrl(
+ createChangeUrl({change: this.change, edit: true, forceReload: true})
+ );
}
// private but used in test
@@ -483,7 +485,9 @@
)
.then(() => {
assertIsDefined(this.change, 'change');
- GerritNav.navigateToChange(this.change, {forceReload: true});
+ this.getNavigation().setUrl(
+ createChangeUrl({change: this.change, forceReload: true})
+ );
});
});
};
diff --git a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.ts b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.ts
index 8bc788c..52581ed 100644
--- a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.ts
+++ b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.ts
@@ -6,7 +6,7 @@
import '../../../test/common-test-setup';
import './gr-editor-view';
import {GrEditorView} from './gr-editor-view';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {HttpMethod} from '../../../constants/constants';
import {
mockPromise,
@@ -31,6 +31,7 @@
import {fixture, html, assert} from '@open-wc/testing';
import {EventType} from '../../../types/events';
import {Modifier} from '../../../utils/dom-util';
+import {testResolver} from '../../../test/common-test-setup';
suite('gr-editor-view tests', () => {
let element: GrEditorView;
@@ -455,10 +456,15 @@
test('viewEditInChangeView', () => {
element.change = createChangeViewChange();
navigateStub.restore();
- const navStub = sinon.stub(GerritNav, 'navigateToChange');
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
+
element.viewEditInChangeView();
- assert.equal(navStub.lastCall.args[1]!.patchNum, undefined);
- assert.equal(navStub.lastCall.args[1]!.isEdit, true);
+
+ assert.isTrue(setUrlStub.called);
+ assert.equal(
+ setUrlStub.lastCall.firstArg,
+ '/c/test-project/+/42,edit?forceReload=true'
+ );
});
suite('keyboard shortcuts', () => {
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
index d6a4d94..8b651f1 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
@@ -370,10 +370,7 @@
content = this.content || '';
}
- // TODO(wyatta) switch linkify sequence, see issue 5526.
- this.newContent = this.removeZeroWidthSpace
- ? content.replace(/^R=\u200B/gm, 'R=')
- : content;
+ this.newContent = content;
}
computeSaveDisabled(): boolean {
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.ts b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.ts
index 6389955..fec347c6 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.ts
@@ -140,25 +140,6 @@
assert.equal(element.newContent, 'stale content');
});
- test('zero width spaces are removed properly', async () => {
- element.removeZeroWidthSpace = true;
- element.content = 'R=\u200Btest@google.com';
-
- // Needed because contentChanged resets newContent
- // We want contentChanged observer to finish before editingChanged is
- // called
-
- await element.updateComplete;
-
- element.editing = true;
-
- // editingChanged updates newContent so wait for it's observer
- // to finish
- await element.updateComplete;
-
- assert.equal(element.newContent, 'R=test@google.com');
- });
-
suite('editing', () => {
setup(async () => {
element.content = 'current content';
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
index 23b4e81..11e7152 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
@@ -3,7 +3,6 @@
* Copyright 2016 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import '../gr-linked-text/gr-linked-text';
import '../gr-markdown/gr-markdown';
import {CommentLinks} from '../../../types/common';
import {LitElement, css, html, TemplateResult} from 'lit';
@@ -91,7 +90,7 @@
ul,
code,
blockquote,
- gr-linked-text.pre {
+ gr-markdown.pre {
margin: 0 0 var(--spacing-m) 0;
}
p,
@@ -103,7 +102,7 @@
:host([noTrailingMargin]) p:last-child,
:host([noTrailingMargin]) ul:last-child,
:host([noTrailingMargin]) blockquote:last-child,
- :host([noTrailingMargin]) gr-linked-text.pre:last-child {
+ :host([noTrailingMargin]) gr-markdown.pre:last-child {
margin: 0;
}
blockquote {
@@ -142,7 +141,10 @@
if (!this.content) return;
if (this.flagsService.isEnabled(KnownExperimentId.RENDER_MARKDOWN)) {
- return html`<gr-markdown .markdown=${this.content}></gr-markdown>`;
+ return html`<gr-markdown
+ .markdown=${true}
+ .content=${this.content}
+ ></gr-markdown>`;
} else {
const blocks = this._computeBlocks(this.content);
return html`${blocks.map(block => this.renderBlock(block))}`;
@@ -354,14 +356,7 @@
}
private renderInlineText(content: string): TemplateResult {
- return html`
- <gr-linked-text
- .config=${this.config}
- content=${content}
- pre
- inline
- ></gr-linked-text>
- `;
+ return html`<gr-markdown .content=${content}></gr-markdown>`;
}
private renderLink(text: string, url: string): TemplateResult {
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
index 191886e..62cb7c8 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
@@ -103,9 +103,7 @@
element,
/* HTML */ `
<p>
- <gr-linked-text content="text " inline="" pre="">
- <span id="output" slot="insert"> text </span>
- </gr-linked-text>
+ <gr-markdown></gr-markdown>
<span class="inline-code"> code </span>
</p>
`
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.ts
index 655c538..4900ed5 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.ts
@@ -21,7 +21,6 @@
EventCallback,
EventEmitterService,
} from '../../../services/gr-event-interface/gr-event-interface';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {Gerrit} from '../../../api/gerrit';
import {fontStyles} from '../../../styles/gr-font-styles';
import {formStyles} from '../../../styles/gr-form-styles';
@@ -67,7 +66,6 @@
_customStyleSheet?: CSSStyleSheet;
// exposed methods
- Nav: typeof GerritNav;
Auth: AuthService;
}
@@ -112,8 +110,6 @@
class GerritImpl implements GerritInternal {
_customStyleSheet?: CSSStyleSheet;
- public readonly Nav = GerritNav;
-
public readonly Auth: AuthService;
private readonly reportingService: ReportingService;
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.ts b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.ts
deleted file mode 100644
index 16a60e7..0000000
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.ts
+++ /dev/null
@@ -1,178 +0,0 @@
-/**
- * @license
- * Copyright 2015 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import '../../../styles/shared-styles';
-import {GrLinkTextParser, LinkTextParserConfig} from './link-text-parser';
-import {LitElement, css, html, PropertyValues} from 'lit';
-import {customElement, property} from 'lit/decorators.js';
-import {assertIsDefined} from '../../../utils/common-util';
-
-declare global {
- interface HTMLElementTagNameMap {
- 'gr-linked-text': GrLinkedText;
- }
-}
-
-@customElement('gr-linked-text')
-export class GrLinkedText extends LitElement {
- private outputElement?: HTMLSpanElement;
-
- @property({type: Boolean, attribute: 'remove-zero-width-space'})
- removeZeroWidthSpace?: boolean;
-
- @property({type: String})
- content = '';
-
- @property({type: Boolean, attribute: true})
- pre = false;
-
- @property({type: Boolean, attribute: true})
- disabled = false;
-
- @property({type: Boolean, attribute: true})
- inline = false;
-
- @property({type: Object})
- config?: LinkTextParserConfig;
-
- static override get styles() {
- return css`
- :host {
- display: block;
- }
- :host([inline]) {
- display: inline;
- }
- :host([pre]) ::slotted(span) {
- white-space: var(--linked-text-white-space, pre-wrap);
- word-wrap: var(--linked-text-word-wrap, break-word);
- }
- `;
- }
-
- override render() {
- return html`<slot name="insert"></slot>`;
- }
-
- // NOTE: LinkTextParser dynamically creates HTML fragments based on backend
- // configuration commentLinks. These commentLinks can contain arbitrary HTML
- // fragments. This means that arbitrary HTML needs to be injected into the
- // DOM-tree, where this HTML is is controlled on the server-side in the
- // server-configuration rather than by arbitrary users.
- // To enable this injection of 'unsafe' HTML, LinkTextParser generates
- // HTML fragments. Lit does not support inserting html fragments directly
- // into its DOM-tree as it controls the DOM-tree that it generates.
- // Therefore, to get around this we create a single element that we slot into
- // the Lit-owned DOM. This element will not be part of this LitElement as
- // it's slotted in and thus can be modified on the fly by handleParseResult.
- override firstUpdated(_changedProperties: PropertyValues): void {
- this.outputElement = document.createElement('span');
- this.outputElement.id = 'output';
- this.outputElement.slot = 'insert';
- this.append(this.outputElement);
- }
-
- override updated(changedProperties: PropertyValues): void {
- if (changedProperties.has('content') || changedProperties.has('config')) {
- this._contentOrConfigChanged();
- } else if (changedProperties.has('disabled')) {
- this.styleLinks();
- }
- }
-
- /**
- * Because either the source text or the linkification config has changed,
- * the content should be re-parsed.
- * Private but used in tests.
- *
- * @param content The raw, un-linkified source string to parse.
- * @param config The server config specifying commentLink patterns
- */
- _contentOrConfigChanged() {
- if (!this.config) {
- assertIsDefined(this.outputElement);
- this.outputElement.textContent = this.content;
- return;
- }
-
- assertIsDefined(this.outputElement);
- this.outputElement.textContent = '';
- const parser = new GrLinkTextParser(
- this.config,
- (text: string | null, href: string | null, fragment?: DocumentFragment) =>
- this.handleParseResult(text, href, fragment),
- this.removeZeroWidthSpace
- );
- parser.parse(this.content);
-
- // Ensure that external links originating from HTML commentlink configs
- // open in a new tab. @see Issue 5567
- // Ensure links to the same host originating from commentlink configs
- // open in the same tab. When target is not set - default is _self
- // @see Issue 4616
- this.outputElement.querySelectorAll('a').forEach(anchor => {
- if (anchor.hostname === window.location.hostname) {
- anchor.removeAttribute('target');
- } else {
- anchor.setAttribute('target', '_blank');
- }
- anchor.setAttribute('rel', 'noopener');
- });
-
- this.styleLinks();
- }
-
- /**
- * Styles the links based on whether gr-linked-text is disabled or not
- */
- private styleLinks() {
- assertIsDefined(this.outputElement);
- this.outputElement.querySelectorAll('a').forEach(anchor => {
- anchor.setAttribute('style', this.computeLinkStyle());
- });
- }
-
- private computeLinkStyle() {
- if (this.disabled) {
- return `
- color: inherit;
- text-decoration: none;
- pointer-events: none;
- `;
- } else {
- return 'color: var(--link-color)';
- }
- }
-
- /**
- * This method is called when the GrLikTextParser emits a partial result
- * (used as the "callback" parameter). It will be called in either of two
- * ways:
- * - To create a link: when called with `text` and `href` arguments, a link
- * element should be created and attached to the resulting DOM.
- * - To attach an arbitrary fragment: when called with only the `fragment`
- * argument, the fragment should be attached to the resulting DOM as is.
- */
- private handleParseResult(
- text: string | null,
- href: string | null,
- fragment?: DocumentFragment
- ) {
- assertIsDefined(this.outputElement);
- const output = this.outputElement;
- if (href) {
- const a = document.createElement('a');
- a.setAttribute('href', href);
- // GrLinkTextParser either pass text and href together or
- // only DocumentFragment - see LinkTextParserCallback
- a.textContent = text!;
- a.target = '_blank';
- a.setAttribute('rel', 'noopener');
- output.appendChild(a);
- } else if (fragment) {
- output.appendChild(fragment);
- }
- }
-}
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.ts b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.ts
deleted file mode 100644
index 00e0313..0000000
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.ts
+++ /dev/null
@@ -1,471 +0,0 @@
-/**
- * @license
- * Copyright 2015 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import '../../../test/common-test-setup';
-import './gr-linked-text';
-import {fixture, html, assert} from '@open-wc/testing';
-import {GrLinkedText} from './gr-linked-text';
-import {queryAndAssert} from '../../../test/test-utils';
-
-suite('gr-linked-text tests', () => {
- let element: GrLinkedText;
-
- let originalCanonicalPath: string | undefined;
-
- setup(async () => {
- originalCanonicalPath = window.CANONICAL_PATH;
- element = await fixture<GrLinkedText>(html`
- <gr-linked-text>
- <div id="output"></div>
- </gr-linked-text>
- `);
-
- element.config = {
- ph: {
- match: '([Bb]ug|[Ii]ssue)\\s*#?(\\d+)',
- link: 'https://bugs.chromium.org/p/gerrit/issues/detail?id=$2',
- },
- prefixsameinlinkandpattern: {
- match: '([Hh][Tt][Tt][Pp]example)\\s*#?(\\d+)',
- link: 'https://bugs.chromium.org/p/gerrit/issues/detail?id=$2',
- },
- changeid: {
- match: '(I[0-9a-f]{8,40})',
- link: '#/q/$1',
- },
- changeid2: {
- match: 'Change-Id: +(I[0-9a-f]{8,40})',
- link: '#/q/$1',
- },
- googlesearch: {
- match: 'google:(.+)',
- link: 'https://bing.com/search?q=$1', // html should supersede link.
- html: '<a href="https://google.com/search?q=$1">$1</a>',
- },
- hashedhtml: {
- match: 'hash:(.+)',
- html: '<a href="#/awesomesauce">$1</a>',
- },
- baseurl: {
- match: 'test (.+)',
- html: '<a href="/r/awesomesauce">$1</a>',
- },
- anotatstartwithbaseurl: {
- match: 'a test (.+)',
- html: '[Lookup: <a href="/r/awesomesauce">$1</a>]',
- },
- disabledconfig: {
- match: 'foo:(.+)',
- link: 'https://google.com/search?q=$1',
- enabled: false,
- },
- };
- });
-
- teardown(() => {
- window.CANONICAL_PATH = originalCanonicalPath;
- });
-
- test('render', async () => {
- element.content =
- 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
- await element.updateComplete;
- assert.lightDom.equal(
- element,
- /* HTML */ `
- <div id="output"></div>
- <span id="output" slot="insert">
- <a
- href="https://bugs.chromium.org/p/gerrit/issues/detail?id=3650"
- rel="noopener"
- style="color: var(--link-color)"
- target="_blank"
- >
- https://bugs.chromium.org/p/gerrit/issues/detail?id=3650
- </a>
- </span>
- `
- );
- });
-
- test('URL pattern was parsed and linked.', async () => {
- // Regular inline link.
- const url = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
- element.content = url;
- await element.updateComplete;
-
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- assert.equal(linkEl.target, '_blank');
- assert.equal(linkEl.rel, 'noopener');
- assert.equal(linkEl.href, url);
- assert.equal(linkEl.textContent, url);
- });
-
- test('Bug pattern was parsed and linked', async () => {
- // "Issue/Bug" pattern.
- element.content = 'Issue 3650';
- await element.updateComplete;
-
- let linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- const url = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
- assert.equal(linkEl.target, '_blank');
- assert.equal(linkEl.href, url);
- assert.equal(linkEl.textContent, 'Issue 3650');
-
- element.content = 'Bug 3650';
- await element.updateComplete;
-
- linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- assert.equal(linkEl.target, '_blank');
- assert.equal(linkEl.rel, 'noopener');
- assert.equal(linkEl.href, url);
- assert.equal(linkEl.textContent, 'Bug 3650');
- });
-
- test('Pattern with same prefix as link was correctly parsed', async () => {
- // Pattern starts with the same prefix (`http`) as the url.
- element.content = 'httpexample 3650';
- await element.updateComplete;
-
- assert.equal(queryAndAssert(element, 'span#output').childNodes.length, 1);
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- const url = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
- assert.equal(linkEl.target, '_blank');
- assert.equal(linkEl.href, url);
- assert.equal(linkEl.textContent, 'httpexample 3650');
- });
-
- test('Change-Id pattern was parsed and linked', async () => {
- // "Change-Id:" pattern.
- const changeID = 'I11d6a37f5e9b5df0486f6c922d8836dfa780e03e';
- const prefix = 'Change-Id: ';
- element.content = prefix + changeID;
- await element.updateComplete;
-
- const textNode = queryAndAssert(element, 'span#output').childNodes[0];
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[1] as HTMLAnchorElement;
- assert.equal(textNode.textContent, prefix);
- const url = '/q/' + changeID;
- assert.isFalse(linkEl.hasAttribute('target'));
- // Since url is a path, the host is added automatically.
- assert.isTrue(linkEl.href.endsWith(url));
- assert.equal(linkEl.textContent, changeID);
- });
-
- test('Change-Id pattern was parsed and linked with base url', async () => {
- window.CANONICAL_PATH = '/r';
-
- // "Change-Id:" pattern.
- const changeID = 'I11d6a37f5e9b5df0486f6c922d8836dfa780e03e';
- const prefix = 'Change-Id: ';
- element.content = prefix + changeID;
- await element.updateComplete;
-
- const textNode = queryAndAssert(element, 'span#output').childNodes[0];
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[1] as HTMLAnchorElement;
- assert.equal(textNode.textContent, prefix);
- const url = '/r/q/' + changeID;
- assert.isFalse(linkEl.hasAttribute('target'));
- // Since url is a path, the host is added automatically.
- assert.isTrue(linkEl.href.endsWith(url));
- assert.equal(linkEl.textContent, changeID);
- });
-
- test('Multiple matches', async () => {
- element.content = 'Issue 3650\nIssue 3450';
- await element.updateComplete;
-
- const linkEl1 = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- const linkEl2 = queryAndAssert(element, 'span#output')
- .childNodes[2] as HTMLAnchorElement;
-
- assert.equal(linkEl1.target, '_blank');
- assert.equal(
- linkEl1.href,
- 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650'
- );
- assert.equal(linkEl1.textContent, 'Issue 3650');
-
- assert.equal(linkEl2.target, '_blank');
- assert.equal(
- linkEl2.href,
- 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3450'
- );
- assert.equal(linkEl2.textContent, 'Issue 3450');
- });
-
- test('Change-Id pattern parsed before bug pattern', async () => {
- // "Change-Id:" pattern.
- const changeID = 'I11d6a37f5e9b5df0486f6c922d8836dfa780e03e';
- const prefix = 'Change-Id: ';
-
- // "Issue/Bug" pattern.
- const bug = 'Issue 3650';
-
- const changeUrl = '/q/' + changeID;
- const bugUrl = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
-
- element.content = prefix + changeID + bug;
- await element.updateComplete;
-
- const textNode = queryAndAssert(element, 'span#output').childNodes[0];
- const changeLinkEl = queryAndAssert(element, 'span#output')
- .childNodes[1] as HTMLAnchorElement;
- const bugLinkEl = queryAndAssert(element, 'span#output')
- .childNodes[2] as HTMLAnchorElement;
-
- assert.equal(textNode.textContent, prefix);
-
- assert.isFalse(changeLinkEl.hasAttribute('target'));
- assert.isTrue(changeLinkEl.href.endsWith(changeUrl));
- assert.equal(changeLinkEl.textContent, changeID);
-
- assert.equal(bugLinkEl.target, '_blank');
- assert.equal(bugLinkEl.href, bugUrl);
- assert.equal(bugLinkEl.textContent, 'Issue 3650');
- });
-
- test('html field in link config', async () => {
- element.content = 'google:do a barrel roll';
- await element.updateComplete;
-
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- assert.equal(
- linkEl.getAttribute('href'),
- 'https://google.com/search?q=do a barrel roll'
- );
- assert.equal(linkEl.textContent, 'do a barrel roll');
- });
-
- test('removing hash from links', async () => {
- element.content = 'hash:foo';
- await element.updateComplete;
-
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- assert.isTrue(linkEl.href.endsWith('/awesomesauce'));
- assert.equal(linkEl.textContent, 'foo');
- });
-
- test('html with base url', async () => {
- window.CANONICAL_PATH = '/r';
-
- element.content = 'test foo';
- await element.updateComplete;
-
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- assert.isTrue(linkEl.href.endsWith('/r/awesomesauce'));
- assert.equal(linkEl.textContent, 'foo');
- });
-
- test('a is not at start', async () => {
- window.CANONICAL_PATH = '/r';
-
- element.content = 'a test foo';
- await element.updateComplete;
-
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[1] as HTMLAnchorElement;
- assert.isTrue(linkEl.href.endsWith('/r/awesomesauce'));
- assert.equal(linkEl.textContent, 'foo');
- });
-
- test('hash html with base url', async () => {
- window.CANONICAL_PATH = '/r';
-
- element.content = 'hash:foo';
- await element.updateComplete;
-
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- assert.isTrue(linkEl.href.endsWith('/r/awesomesauce'));
- assert.equal(linkEl.textContent, 'foo');
- });
-
- test('disabled config', async () => {
- element.content = 'foo:baz';
- await element.updateComplete;
-
- assert.equal(queryAndAssert(element, 'span#output').innerHTML, 'foo:baz');
- });
-
- test('R=email labels link correctly', async () => {
- element.removeZeroWidthSpace = true;
- element.content = 'R=\u200Btest@google.com';
- await element.updateComplete;
-
- assert.equal(
- queryAndAssert(element, 'span#output').textContent,
- 'R=test@google.com'
- );
- assert.equal(
- queryAndAssert(element, 'span#output').innerHTML.match(/(R=<a)/g)!.length,
- 1
- );
- });
-
- test('CC=email labels link correctly', async () => {
- element.removeZeroWidthSpace = true;
- element.content = 'CC=\u200Btest@google.com';
- await element.updateComplete;
-
- assert.equal(
- queryAndAssert(element, 'span#output').textContent,
- 'CC=test@google.com'
- );
- assert.equal(
- queryAndAssert(element, 'span#output').innerHTML.match(/(CC=<a)/g)!
- .length,
- 1
- );
- });
-
- test('only {http,https,mailto} protocols are linkified', async () => {
- element.content = 'xx mailto:test@google.com yy';
- await element.updateComplete;
-
- let links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'mailto:test@google.com');
- assert.equal(links[0].innerHTML, 'mailto:test@google.com');
-
- element.content = 'xx http://google.com yy';
- await element.updateComplete;
-
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'http://google.com');
- assert.equal(links[0].innerHTML, 'http://google.com');
-
- element.content = 'xx https://google.com yy';
- await element.updateComplete;
-
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'https://google.com');
- assert.equal(links[0].innerHTML, 'https://google.com');
-
- element.content = 'xx ssh://google.com yy';
- await element.updateComplete;
-
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 0);
-
- element.content = 'xx ftp://google.com yy';
- await element.updateComplete;
-
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 0);
- });
-
- test('links without leading whitespace are linkified', async () => {
- element.content = 'xx abcmailto:test@google.com yy';
- await element.updateComplete;
-
- assert.equal(
- queryAndAssert(element, 'span#output').innerHTML.substr(0, 6),
- 'xx abc'
- );
- let links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'mailto:test@google.com');
- assert.equal(links[0].innerHTML, 'mailto:test@google.com');
-
- element.content = 'xx defhttp://google.com yy';
- await element.updateComplete;
-
- assert.equal(
- queryAndAssert(element, 'span#output').innerHTML.substr(0, 6),
- 'xx def'
- );
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'http://google.com');
- assert.equal(links[0].innerHTML, 'http://google.com');
-
- element.content = 'xx qwehttps://google.com yy';
- await element.updateComplete;
-
- assert.equal(
- queryAndAssert(element, 'span#output').innerHTML.substr(0, 6),
- 'xx qwe'
- );
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'https://google.com');
- assert.equal(links[0].innerHTML, 'https://google.com');
-
- // Non-latin character
- element.content = 'xx абвhttps://google.com yy';
- await element.updateComplete;
-
- assert.equal(
- queryAndAssert(element, 'span#output').innerHTML.substr(0, 6),
- 'xx абв'
- );
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'https://google.com');
- assert.equal(links[0].innerHTML, 'https://google.com');
-
- element.content = 'xx ssh://google.com yy';
- await element.updateComplete;
-
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 0);
-
- element.content = 'xx ftp://google.com yy';
- await element.updateComplete;
-
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 0);
- });
-
- test('overlapping links', async () => {
- element.config = {
- b1: {
- match: '(B:\\s*)(\\d+)',
- html: '$1<a href="ftp://foo/$2">$2</a>',
- },
- b2: {
- match: '(B:\\s*\\d+\\s*,\\s*)(\\d+)',
- html: '$1<a href="ftp://foo/$2">$2</a>',
- },
- };
- element.content = '- B: 123, 45';
- await element.updateComplete;
-
- const links = element.querySelectorAll('a');
-
- assert.equal(links.length, 2);
- assert.equal(
- queryAndAssert<HTMLSpanElement>(element, 'span').textContent,
- '- B: 123, 45'
- );
-
- assert.equal(links[0].href, 'ftp://foo/123');
- assert.equal(links[0].textContent, '123');
-
- assert.equal(links[1].href, 'ftp://foo/45');
- assert.equal(links[1].textContent, '45');
- });
-
- test('_contentOrConfigChanged called with config', async () => {
- const contentConfigStub = sinon.stub(element, '_contentOrConfigChanged');
- element.content = 'some text';
- await element.updateComplete;
-
- assert.isTrue(contentConfigStub.called);
- });
-});
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.ts b/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.ts
deleted file mode 100644
index 73cf58b..0000000
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.ts
+++ /dev/null
@@ -1,415 +0,0 @@
-/**
- * @license
- * Copyright 2015 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import 'ba-linkify/ba-linkify';
-import {getBaseUrl} from '../../../utils/url-util';
-import {CommentLinkInfo} from '../../../types/common';
-
-/**
- * Pattern describing URLs with supported protocols.
- */
-const URL_PROTOCOL_PATTERN = /^(.*)(https?:\/\/|mailto:)/;
-
-export type LinkTextParserCallback = ((text: string, href: string) => void) &
- ((text: null, href: null, fragment: DocumentFragment) => void);
-
-export interface CommentLinkItem {
- position: number;
- length: number;
- html: HTMLAnchorElement | DocumentFragment;
-}
-
-export type LinkTextParserConfig = {[name: string]: CommentLinkInfo};
-
-export class GrLinkTextParser {
- private readonly baseUrl = getBaseUrl();
-
- /**
- * Construct a parser for linkifying text. Will linkify plain URLs that appear
- * in the text as well as custom links if any are specified in the linkConfig
- * parameter.
- *
- * @param linkConfig Comment links as specified by the commentlinks field on a
- * project config.
- * @param callback The callback to be fired when an intermediate parse result
- * is emitted. The callback is passed text and href strings if a link is to
- * be created, or a document fragment otherwise.
- * @param removeZeroWidthSpace If true, zero-width spaces will be removed from
- * R=<email> and CC=<email> expressions.
- */
- constructor(
- private readonly linkConfig: LinkTextParserConfig,
- private readonly callback: LinkTextParserCallback,
- private readonly removeZeroWidthSpace?: boolean
- ) {
- Object.preventExtensions(this);
- }
-
- /**
- * Emit a callback to create a link element.
- *
- * @param text The text of the link.
- * @param href The URL to use as the href of the link.
- */
- addText(text: string, href: string) {
- if (!text) {
- return;
- }
- this.callback(text, href);
- }
-
- /**
- * Given the source text and a list of CommentLinkItem objects that were
- * generated by the commentlinks config, emit parsing callbacks.
- *
- * @param text The chuml of source text over which the outputArray items range.
- * @param outputArray The list of items to add resulting from commentlink
- * matches.
- */
- processLinks(text: string, outputArray: CommentLinkItem[]) {
- this.sortArrayReverse(outputArray);
- const fragment = document.createDocumentFragment();
- let cursor = text.length;
-
- // Start inserting linkified URLs from the end of the String. That way, the
- // string positions of the items don't change as we iterate through.
- outputArray.forEach(item => {
- // Add any text between the current linkified item and the item added
- // before if it exists.
- if (item.position + item.length !== cursor) {
- fragment.insertBefore(
- document.createTextNode(
- text.slice(item.position + item.length, cursor)
- ),
- fragment.firstChild
- );
- }
- fragment.insertBefore(item.html, fragment.firstChild);
- cursor = item.position;
- });
-
- // Add the beginning portion at the end.
- if (cursor !== 0) {
- fragment.insertBefore(
- document.createTextNode(text.slice(0, cursor)),
- fragment.firstChild
- );
- }
-
- this.callback(null, null, fragment);
- }
-
- /**
- * Sort the given array of CommentLinkItems such that the positions are in
- * reverse order.
- */
- sortArrayReverse(outputArray: CommentLinkItem[]) {
- outputArray.sort((a, b) => b.position - a.position);
- }
-
- addItem(
- text: string,
- href: string,
- html: null,
- position: number,
- length: number,
- outputArray: CommentLinkItem[]
- ): void;
-
- addItem(
- text: null,
- href: null,
- html: string,
- position: number,
- length: number,
- outputArray: CommentLinkItem[]
- ): void;
-
- /**
- * Create a CommentLinkItem and append it to the given output array. This
- * method can be called in either of two ways:
- * - With `text` and `href` parameters provided, and the `html` parameter
- * passed as `null`. In this case, the new CommentLinkItem will be a link
- * element with the given text and href value.
- * - With the `html` paremeter provided, and the `text` and `href` parameters
- * passed as `null`. In this case, the string of HTML will be parsed and the
- * first resulting node will be used as the resulting content.
- *
- * @param text The text to use if creating a link.
- * @param href The href to use as the URL if creating a link.
- * @param html The html to parse and use as the result.
- * @param position The position inside the source text where the item
- * starts.
- * @param length The number of characters in the source text
- * represented by the item.
- * @param outputArray The array to which the
- * new item is to be appended.
- */
- addItem(
- text: string | null,
- href: string | null,
- html: string | null,
- position: number,
- length: number,
- outputArray: CommentLinkItem[]
- ): void {
- if (href) {
- const a = document.createElement('a');
- a.setAttribute('href', href);
- a.textContent = text;
- a.target = '_blank';
- a.rel = 'noopener';
- outputArray.push({
- html: a,
- position,
- length,
- });
- } else if (html) {
- // addItem has 2 overloads. If href is null, then html
- // can't be null.
- // TODO(TS): remove if(html) and keep else block without condition
- const fragment = document.createDocumentFragment();
- // Create temporary div to hold the nodes in.
- const div = document.createElement('div');
- div.innerHTML = html;
- while (div.firstChild) {
- fragment.appendChild(div.firstChild);
- }
- outputArray.push({
- html: fragment,
- position,
- length,
- });
- }
- }
-
- /**
- * Create a CommentLinkItem for a link and append it to the given output
- * array.
- *
- * @param text The text for the link.
- * @param href The href to use as the URL of the link.
- * @param position The position inside the source text where the link
- * starts.
- * @param length The number of characters in the source text
- * represented by the link.
- * @param outputArray The array to which the
- * new item is to be appended.
- */
- addLink(
- text: string,
- href: string,
- position: number,
- length: number,
- outputArray: CommentLinkItem[]
- ) {
- // TODO(TS): remove !test condition
- if (!text || this.hasOverlap(position, length, outputArray)) {
- return;
- }
- if (
- !!this.baseUrl &&
- href.startsWith('/') &&
- !href.startsWith(this.baseUrl)
- ) {
- href = this.baseUrl + href;
- }
- this.addItem(text, href, null, position, length, outputArray);
- }
-
- /**
- * Create a CommentLinkItem specified by an HTMl string and append it to the
- * given output array.
- *
- * @param html The html to parse and use as the result.
- * @param position The position inside the source text where the item
- * starts.
- * @param length The number of characters in the source text
- * represented by the item.
- * @param outputArray The array to which the
- * new item is to be appended.
- */
- addHTML(
- html: string,
- position: number,
- length: number,
- outputArray: CommentLinkItem[]
- ) {
- if (this.hasOverlap(position, length, outputArray)) {
- return;
- }
- if (
- !!this.baseUrl &&
- html.match(/<a href="\//g) &&
- !new RegExp(`<a href="${this.baseUrl}`, 'g').test(html)
- ) {
- html = html.replace(/<a href="\//g, `<a href="${this.baseUrl}/`);
- }
- this.addItem(null, null, html, position, length, outputArray);
- }
-
- /**
- * Does the given range overlap with anything already in the item list.
- */
- hasOverlap(position: number, length: number, outputArray: CommentLinkItem[]) {
- const endPosition = position + length;
- for (let i = 0; i < outputArray.length; i++) {
- const arrayItemStart = outputArray[i].position;
- const arrayItemEnd = outputArray[i].position + outputArray[i].length;
- if (
- (position >= arrayItemStart && position < arrayItemEnd) ||
- (endPosition > arrayItemStart && endPosition <= arrayItemEnd) ||
- (position === arrayItemStart && position === arrayItemEnd)
- ) {
- return true;
- }
- }
- return false;
- }
-
- /**
- * Parse the given source text and emit callbacks for the items that are
- * parsed.
- */
- parse(text?: string | null) {
- if (text) {
- window.linkify(text, {
- callback: (text: string, href?: string) => this.parseChunk(text, href),
- });
- }
- }
-
- /**
- * Callback that is pased into the linkify function. ba-linkify will call this
- * method in either of two ways:
- * - With both a `text` and `href` parameter provided: this indicates that
- * ba-linkify has found a plain URL and wants it linkified.
- * - With only a `text` parameter provided: this represents the non-link
- * content that lies between the links the library has found.
- *
- */
- parseChunk(text: string, href?: string) {
- // TODO(wyatta) switch linkify sequence, see issue 5526.
- if (this.removeZeroWidthSpace) {
- // Remove the zero-width space added in gr-change-view.
- text = text.replace(/^(CC|R)=\u200B/gm, '$1=');
- }
-
- // If the href is provided then ba-linkify has recognized it as a URL. If
- // the source text does not include a protocol, the protocol will be added
- // by ba-linkify. Create the link if the href is provided and its protocol
- // matches the expected pattern.
- if (href) {
- const result = URL_PROTOCOL_PATTERN.exec(href);
- if (result) {
- const prefixText = result[1];
- if (prefixText.length > 0) {
- // Fix for simple cases from
- // https://bugs.chromium.org/p/gerrit/issues/detail?id=11697
- // When leading whitespace is missed before link,
- // linkify add this text before link as a schema name to href.
- // We suppose, that prefixText just a single word
- // before link and add this word as is, without processing
- // any patterns in it.
- this.parseLinks(prefixText, {});
- text = text.substring(prefixText.length);
- href = href.substring(prefixText.length);
- }
- this.addText(text, href);
- return;
- }
- }
- // For the sections of text that lie between the links found by
- // ba-linkify, we search for the project-config-specified link patterns.
- this.parseLinks(text, this.linkConfig);
- }
-
- /**
- * Walk over the given source text to find matches for comemntlink patterns
- * and emit parse result callbacks.
- *
- * @param text The raw source text.
- * @param config A comment links specification object.
- */
- parseLinks(text: string, config: LinkTextParserConfig) {
- // The outputArray is used to store all of the matches found for all
- // patterns.
- const outputArray: CommentLinkItem[] = [];
- for (const [configName, linkInfo] of Object.entries(config)) {
- // TODO(TS): it seems, the following line can be rewritten as:
- // if(enabled === false || enabled === 0 || enabled === '')
- // Should be double-checked before update
- // eslint-disable-next-line eqeqeq
- if (linkInfo.enabled != null && linkInfo.enabled == false) {
- continue;
- }
- // PolyGerrit doesn't use hash-based navigation like the GWT UI.
- // Account for this.
- const html = linkInfo.html;
- const link = linkInfo.link;
- if (html) {
- linkInfo.html = html.replace(/<a href="#\//g, '<a href="/');
- } else if (link) {
- if (link[0] === '#') {
- linkInfo.link = link.substr(1);
- }
- }
-
- const pattern = new RegExp(linkInfo.match, 'g');
-
- let match;
- let textToCheck = text;
- let susbtrIndex = 0;
-
- while ((match = pattern.exec(textToCheck))) {
- textToCheck = textToCheck.substr(match.index + match[0].length);
- let result = match[0].replace(
- pattern,
- // Either html or link has a value. Otherwise an exception is thrown
- // in the code below.
- (linkInfo.html || linkInfo.link)!
- );
-
- if (linkInfo.html) {
- let i;
- // Skip portion of replacement string that is equal to original to
- // allow overlapping patterns.
- for (i = 0; i < result.length; i++) {
- if (result[i] !== match[0][i]) {
- break;
- }
- }
- result = result.slice(i);
-
- this.addHTML(
- result,
- susbtrIndex + match.index + i,
- match[0].length - i,
- outputArray
- );
- } else if (linkInfo.link) {
- this.addLink(
- match[0],
- result,
- susbtrIndex + match.index,
- match[0].length,
- outputArray
- );
- } else {
- throw Error(
- 'linkconfig entry ' +
- configName +
- ' doesn’t contain a link or html attribute.'
- );
- }
-
- // Update the substring location so we know where we are in relation to
- // the initial full text string.
- susbtrIndex = susbtrIndex + match.index + match[0].length;
- }
- }
- this.processLinks(text, outputArray);
- }
-}
diff --git a/polygerrit-ui/app/elements/shared/gr-markdown/gr-markdown.ts b/polygerrit-ui/app/elements/shared/gr-markdown/gr-markdown.ts
index ae4b004..50f0602 100644
--- a/polygerrit-ui/app/elements/shared/gr-markdown/gr-markdown.ts
+++ b/polygerrit-ui/app/elements/shared/gr-markdown/gr-markdown.ts
@@ -5,7 +5,11 @@
*/
import {css, html, LitElement} from 'lit';
import {customElement, property, state} from 'lit/decorators.js';
-import {htmlEscape} from '../../../utils/inner-html-util';
+import {
+ htmlEscape,
+ sanitizeHtml,
+ sanitizeHtmlToFragment,
+} from '../../../utils/inner-html-util';
import {unescapeHTML} from '../../../utils/syntax-util';
import '@polymer/marked-element';
import {resolve} from '../../../models/dependency';
@@ -22,12 +26,15 @@
* This element renders markdown and also applies some regex replacements to
* linkify key parts of the text defined by the host's config.
*
- * TODO: Remove gr-formatted-text once this is rolled out.
+ * TODO: Replace gr-formatted-text with this once markdown flag is rolled out.
*/
@customElement('gr-markdown')
export class GrMarkdown extends LitElement {
@property({type: String})
- markdown?: string;
+ content = '';
+
+ @property({type: Boolean})
+ markdown = false;
@state()
private repoCommentLinks: CommentLinks = {};
@@ -86,6 +93,11 @@
li {
margin-left: var(--spacing-xl);
}
+ .plaintext {
+ font: inherit;
+ white-space: var(--linked-text-white-space, pre-wrap);
+ word-wrap: var(--linked-text-word-wrap, break-word);
+ }
`,
];
@@ -99,9 +111,25 @@
}
override render() {
- // Note: Handling \u200B added in gr-change-view.ts is not needed here
- // because the commit message is not markdown formatted.
+ if (this.markdown) {
+ return this.renderAsMarkdown();
+ } else {
+ return this.renderAsPlaintext();
+ }
+ }
+ private renderAsPlaintext() {
+ const linkedText = this.rewriteText(
+ htmlEscape(this.content).toString(),
+ this.repoCommentLinks
+ );
+
+ return html`
+ <pre class="plaintext">${sanitizeHtmlToFragment(linkedText)}</pre>
+ `;
+ }
+
+ private renderAsMarkdown() {
// <marked-element> internals will be in charge of calling our custom
// renderer so we wrap 'this.rewriteText' so that 'this' is preserved via
// closure.
@@ -130,11 +158,16 @@
}
// The child with slot is optional but allows us control over the styling.
+ // The `callback` property lets us do a final sanitization of the output
+ // HTML string before it is rendered by `<marked-element>` in case any
+ // rewrites have been abused to attempt an XSS attack.
return html`
<marked-element
- .markdown=${this.escapeAllButBlockQuotes(this.markdown ?? '')}
+ .markdown=${this.escapeAllButBlockQuotes(this.content)}
.breaks=${true}
.renderer=${customRenderer}
+ .callback=${(_error: string | null, contents: string) =>
+ sanitizeHtml(contents)}
>
<div slot="markdown-html"></div>
</marked-element>
diff --git a/polygerrit-ui/app/elements/shared/gr-markdown/gr-markdown_test.ts b/polygerrit-ui/app/elements/shared/gr-markdown/gr-markdown_test.ts
index 1128b3a..2327dfe 100644
--- a/polygerrit-ui/app/elements/shared/gr-markdown/gr-markdown_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-markdown/gr-markdown_test.ts
@@ -57,212 +57,89 @@
).querySelector('gr-markdown')!;
});
- test('renders plain text with links and rewrites', async () => {
- element.markdown = `text
- \ntext with plain link: google.com
- \ntext with config link: LinkRewriteMe
- \ntext with config html: HTMLRewriteMe`;
- await element.updateComplete;
+ suite('as plaintext', () => {
+ setup(async () => {
+ element.markdown = false;
+ await element.updateComplete;
+ });
- assert.shadowDom.equal(
- element,
- /* HTML */ `
- <marked-element>
- <div slot="markdown-html">
- <p>text</p>
- <p>
- text with plain link:
- <a href="http://google.com" rel="noopener" target="_blank">
- google.com
- </a>
- </p>
- <p>
- text with config link:
- <a
- href="http://google.com/LinkRewriteMe"
- rel="noopener"
- target="_blank"
- >
- LinkRewriteMe
- </a>
- </p>
- <p>text with config html:</p>
+ test('renders text with links and rewrites', async () => {
+ element.content = `text with plain link: google.com
+ \ntext with config link: LinkRewriteMe
+ \ntext with config html: HTMLRewriteMe`;
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <pre class="plaintext">
+ text with plain link:
+ <a href="http://google.com" rel="noopener" target="_blank">
+ google.com
+ </a>
+ text with config link:
+ <a
+ href="http://google.com/LinkRewriteMe"
+ rel="noopener"
+ target="_blank"
+ >
+ LinkRewriteMe
+ </a>
+ text with config html:
<div>HTMLRewritten</div>
- <p></p>
- </div>
- </marked-element>
- `
- );
+ </pre>
+ `
+ );
+ });
+
+ test('does not render typed html', async () => {
+ element.content = 'plain text <div>foo</div>';
+ await element.updateComplete;
+
+ const escapedDiv = '<div>foo</div>';
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `<pre class="plaintext">plain text ${escapedDiv}</pre>`
+ );
+ });
+
+ test('does not render markdown', async () => {
+ element.content = '# A Markdown Heading';
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ '<pre class="plaintext"># A Markdown Heading</pre>'
+ );
+ });
});
- test('renders headings with links and rewrites', async () => {
- element.markdown = `# h1-heading
- \n## h2-heading
- \n### h3-heading
- \n#### h4-heading
- \n##### h5-heading
- \n###### h6-heading
- \n# heading with plain link: google.com
- \n# heading with config link: LinkRewriteMe
- \n# heading with config html: HTMLRewriteMe`;
- await element.updateComplete;
+ suite('as markdown', () => {
+ setup(async () => {
+ element.markdown = true;
+ await element.updateComplete;
+ });
+ test('renders text with links and rewrites', async () => {
+ element.content = `text
+ \ntext with plain link: google.com
+ \ntext with config link: LinkRewriteMe
+ \ntext with config html: HTMLRewriteMe`;
+ await element.updateComplete;
- assert.shadowDom.equal(
- element,
- /* HTML */ `
- <marked-element>
- <div slot="markdown-html">
- <h1 id="h1-heading">h1-heading</h1>
- <h2 id="h2-heading">h2-heading</h2>
- <h3 id="h3-heading">h3-heading</h3>
- <h4 id="h4-heading">h4-heading</h4>
- <h5 id="h5-heading">h5-heading</h5>
- <h6 id="h6-heading">h6-heading</h6>
- <h1 id="heading-with-plain-link-google-com">
- heading with plain link:
- <a href="http://google.com" rel="noopener" target="_blank">
- google.com
- </a>
- </h1>
- <h1 id="heading-with-config-link-linkrewriteme">
- heading with config link:
- <a
- href="http://google.com/LinkRewriteMe"
- rel="noopener"
- target="_blank"
- >
- LinkRewriteMe
- </a>
- </h1>
- <h1 id="heading-with-config-html-htmlrewriteme">
- heading with config html:
- <div>HTMLRewritten</div>
- </h1>
- </div>
- </marked-element>
- `
- );
- });
-
- test('renders inline-code without linking or rewriting', async () => {
- element.markdown = `\`inline code\`
- \n\`inline code with plain link: google.com\`
- \n\`inline code with config link: LinkRewriteMe\`
- \n\`inline code with config html: HTMLRewriteMe\``;
- await element.updateComplete;
-
- assert.shadowDom.equal(
- element,
- /* HTML */ `
- <marked-element>
- <div slot="markdown-html">
- <p>
- <code> inline code </code>
- </p>
- <p>
- <code> inline code with plain link: google.com </code>
- </p>
- <p>
- <code> inline code with config link: LinkRewriteMe </code>
- </p>
- <p>
- <code> inline code with config html: HTMLRewriteMe </code>
- </p>
- </div>
- </marked-element>
- `
- );
- });
- test('renders multiline-code without linking or rewriting', async () => {
- element.markdown = `\`\`\`\nmultiline code\n\`\`\`
- \n\`\`\`\nmultiline code with plain link: google.com\n\`\`\`
- \n\`\`\`\nmultiline code with config link: LinkRewriteMe\n\`\`\`
- \n\`\`\`\nmultiline code with config html: HTMLRewriteMe\n\`\`\``;
- await element.updateComplete;
-
- assert.shadowDom.equal(
- element,
- /* HTML */ `
- <marked-element>
- <div slot="markdown-html">
- <pre>
- <code> multiline code </code>
- </pre>
- <pre>
- <code> multiline code with plain link: google.com </code>
- </pre>
- <pre>
- <code> multiline code with config link: LinkRewriteMe </code>
- </pre>
- <pre>
- <code> multiline code with config html: HTMLRewriteMe </code>
- </pre>
- </div>
- </marked-element>
- `
- );
- });
-
- test('does not render inline images into <img> tags', async () => {
- element.markdown = '';
- await element.updateComplete;
-
- assert.shadowDom.equal(
- element,
- /* HTML */ `
- <marked-element>
- <div slot="markdown-html">
- <p></p>
- </div>
- </marked-element>
- `
- );
- });
-
- test('renders inline links into <a> tags', async () => {
- element.markdown = '[myLink](https://www.google.com)';
- await element.updateComplete;
-
- assert.shadowDom.equal(
- element,
- /* HTML */ `
- <marked-element>
- <div slot="markdown-html">
- <p>
- <a href="https://www.google.com">myLink</a>
- </p>
- </div>
- </marked-element>
- `
- );
- });
-
- test('renders block quotes with links and rewrites', async () => {
- element.markdown = `> block quote
- \n> block quote with plain link: google.com
- \n> block quote with config link: LinkRewriteMe
- \n> block quote with config html: HTMLRewriteMe`;
- await element.updateComplete;
-
- assert.shadowDom.equal(
- element,
- /* HTML */ `
- <marked-element>
- <div slot="markdown-html">
- <blockquote>
- <p>block quote</p>
- </blockquote>
- <blockquote>
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <marked-element>
+ <div slot="markdown-html">
+ <p>text</p>
<p>
- block quote with plain link:
+ text with plain link:
<a href="http://google.com" rel="noopener" target="_blank">
google.com
</a>
</p>
- </blockquote>
- <blockquote>
<p>
- block quote with config link:
+ text with config link:
<a
href="http://google.com/LinkRewriteMe"
rel="noopener"
@@ -271,50 +148,236 @@
LinkRewriteMe
</a>
</p>
- </blockquote>
- <blockquote>
- <p>block quote with config html:</p>
+ <p>text with config html:</p>
<div>HTMLRewritten</div>
<p></p>
- </blockquote>
- </div>
- </marked-element>
- `
- );
- });
+ </div>
+ </marked-element>
+ `
+ );
+ });
- test('never renders typed html', async () => {
- element.markdown = `plain text <div>foo</div>
- \n\`inline code <div>foo</div>\`
- \n\`\`\`\nmultiline code <div>foo</div>\`\`\`
- \n> block quote <div>foo</div>
- \n[inline link <div>foo</div>](http://google.com)`;
- await element.updateComplete;
+ test('renders headings with links and rewrites', async () => {
+ element.content = `# h1-heading
+ \n## h2-heading
+ \n### h3-heading
+ \n#### h4-heading
+ \n##### h5-heading
+ \n###### h6-heading
+ \n# heading with plain link: google.com
+ \n# heading with config link: LinkRewriteMe
+ \n# heading with config html: HTMLRewriteMe`;
+ await element.updateComplete;
- const escapedDiv = '<div>foo</div>';
- assert.shadowDom.equal(
- element,
- /* HTML */ `
- <marked-element>
- <div slot="markdown-html">
- <p>plain text ${escapedDiv}</p>
- <p>
- <code> inline code ${escapedDiv} </code>
- </p>
- <pre>
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <marked-element>
+ <div slot="markdown-html">
+ <h1>h1-heading</h1>
+ <h2>h2-heading</h2>
+ <h3>h3-heading</h3>
+ <h4>h4-heading</h4>
+ <h5>h5-heading</h5>
+ <h6>h6-heading</h6>
+ <h1>
+ heading with plain link:
+ <a href="http://google.com" rel="noopener" target="_blank">
+ google.com
+ </a>
+ </h1>
+ <h1>
+ heading with config link:
+ <a
+ href="http://google.com/LinkRewriteMe"
+ rel="noopener"
+ target="_blank"
+ >
+ LinkRewriteMe
+ </a>
+ </h1>
+ <h1>
+ heading with config html:
+ <div>HTMLRewritten</div>
+ </h1>
+ </div>
+ </marked-element>
+ `
+ );
+ });
+
+ test('renders inline-code without linking or rewriting', async () => {
+ element.content = `\`inline code\`
+ \n\`inline code with plain link: google.com\`
+ \n\`inline code with config link: LinkRewriteMe\`
+ \n\`inline code with config html: HTMLRewriteMe\``;
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <marked-element>
+ <div slot="markdown-html">
+ <p>
+ <code>inline code</code>
+ </p>
+ <p>
+ <code>inline code with plain link: google.com</code>
+ </p>
+ <p>
+ <code>inline code with config link: LinkRewriteMe</code>
+ </p>
+ <p>
+ <code>inline code with config html: HTMLRewriteMe</code>
+ </p>
+ </div>
+ </marked-element>
+ `
+ );
+ });
+ test('renders multiline-code without linking or rewriting', async () => {
+ element.content = `\`\`\`\nmultiline code\n\`\`\`
+ \n\`\`\`\nmultiline code with plain link: google.com\n\`\`\`
+ \n\`\`\`\nmultiline code with config link: LinkRewriteMe\n\`\`\`
+ \n\`\`\`\nmultiline code with config html: HTMLRewriteMe\n\`\`\``;
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <marked-element>
+ <div slot="markdown-html">
+ <pre>
+ <code>multiline code</code>
+ </pre>
+ <pre>
+ <code>multiline code with plain link: google.com</code>
+ </pre>
+ <pre>
+ <code>multiline code with config link: LinkRewriteMe</code>
+ </pre>
+ <pre>
+ <code>multiline code with config html: HTMLRewriteMe</code>
+ </pre>
+ </div>
+ </marked-element>
+ `
+ );
+ });
+
+ test('does not render inline images into <img> tags', async () => {
+ element.content = '';
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <marked-element>
+ <div slot="markdown-html">
+ <p></p>
+ </div>
+ </marked-element>
+ `
+ );
+ });
+
+ test('renders inline links into <a> tags', async () => {
+ element.content = '[myLink](https://www.google.com)';
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <marked-element>
+ <div slot="markdown-html">
+ <p>
+ <a href="https://www.google.com">myLink</a>
+ </p>
+ </div>
+ </marked-element>
+ `
+ );
+ });
+
+ test('renders block quotes with links and rewrites', async () => {
+ element.content = `> block quote
+ \n> block quote with plain link: google.com
+ \n> block quote with config link: LinkRewriteMe
+ \n> block quote with config html: HTMLRewriteMe`;
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <marked-element>
+ <div slot="markdown-html">
+ <blockquote>
+ <p>block quote</p>
+ </blockquote>
+ <blockquote>
+ <p>
+ block quote with plain link:
+ <a href="http://google.com" rel="noopener" target="_blank">
+ google.com
+ </a>
+ </p>
+ </blockquote>
+ <blockquote>
+ <p>
+ block quote with config link:
+ <a
+ href="http://google.com/LinkRewriteMe"
+ rel="noopener"
+ target="_blank"
+ >
+ LinkRewriteMe
+ </a>
+ </p>
+ </blockquote>
+ <blockquote>
+ <p>block quote with config html:</p>
+ <div>HTMLRewritten</div>
+ <p></p>
+ </blockquote>
+ </div>
+ </marked-element>
+ `
+ );
+ });
+
+ test('never renders typed html', async () => {
+ element.content = `plain text <div>foo</div>
+ \n\`inline code <div>foo</div>\`
+ \n\`\`\`\nmultiline code <div>foo</div>\`\`\`
+ \n> block quote <div>foo</div>
+ \n[inline link <div>foo</div>](http://google.com)`;
+ await element.updateComplete;
+
+ const escapedDiv = '<div>foo</div>';
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <marked-element>
+ <div slot="markdown-html">
+ <p>plain text ${escapedDiv}</p>
+ <p>
+ <code>inline code ${escapedDiv}</code>
+ </p>
+ <pre>
<code>
multiline code ${escapedDiv}
</code>
</pre>
- <blockquote>
- <p>block quote ${escapedDiv}</p>
- </blockquote>
- <p>
- <a href="http://google.com"> inline link ${escapedDiv} </a>
- </p>
- </div>
- </marked-element>
- `
- );
+ <blockquote>
+ <p>block quote ${escapedDiv}</p>
+ </blockquote>
+ <p>
+ <a href="http://google.com">inline link ${escapedDiv}</a>
+ </p>
+ </div>
+ </marked-element>
+ `
+ );
+ });
});
});
diff --git a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
index 3ea9c5d..7b5ce15 100644
--- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
+++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
@@ -24,12 +24,14 @@
import {PropertyValues} from 'lit';
import {classMap} from 'lit/directives/class-map.js';
import {KnownExperimentId} from '../../../services/flags/flags';
-import {NumericChangeId} from '../../../api/rest-api';
+import {NumericChangeId, ServerInfo} from '../../../api/rest-api';
import {subscribe} from '../../lit/subscription-controller';
import {resolve} from '../../../models/dependency';
import {changeModelToken} from '../../../models/change/change-model';
import {assert} from '../../../utils/common-util';
import {ShortcutController} from '../../lit/shortcut-controller';
+import {getAccountDisplayName} from '../../../utils/display-name-util';
+import {configModelToken} from '../../../models/config/config-model';
const MAX_ITEMS_DROPDOWN = 10;
@@ -118,6 +120,10 @@
private readonly restApiService = getAppContext().restApiService;
+ private readonly getConfigModel = resolve(this, configModelToken);
+
+ private serverConfig?: ServerInfo;
+
private changeNum?: NumericChangeId;
// private but used in tests
@@ -135,6 +141,13 @@
() => this.getChangeModel().changeNum$,
x => (this.changeNum = x)
);
+ subscribe(
+ this,
+ () => this.getConfigModel().serverConfig$,
+ config => {
+ this.serverConfig = config;
+ }
+ );
this.shortcuts.addLocal({key: Key.UP}, e => this.handleUpKey(e), {
preventDefault: false,
});
@@ -600,6 +613,7 @@
}
}
+ // TODO(dhruvsri): merge with getAccountSuggestions in account-util
async computeReviewerSuggestions() {
this.suggestions = (
(await this.restApiService.getSuggestedAccounts(
@@ -612,7 +626,7 @@
.filter(account => account.email)
.map(account => {
return {
- text: `${account.name ?? ''} <${account.email}>`,
+ text: `${getAccountDisplayName(this.serverConfig, account)}`,
dataValue: account.email,
};
});
diff --git a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts
index 76c5033..6837a71 100644
--- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts
@@ -137,7 +137,11 @@
test('mention selector opens when previous char is \n', async () => {
stubRestApi('getSuggestedAccounts').returns(
Promise.resolve([
- {...createAccountWithEmail('abc@google.com'), name: 'A'},
+ {
+ ...createAccountWithEmail('abc@google.com'),
+ name: 'A',
+ display_name: 'display A',
+ },
{...createAccountWithEmail('abcdef@google.com'), name: 'B'},
])
);
@@ -154,7 +158,7 @@
assert.deepEqual(element.suggestions, [
{
dataValue: 'abc@google.com',
- text: 'A <abc@google.com>',
+ text: 'display A <abc@google.com>',
},
{
dataValue: 'abcdef@google.com',
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor_test.ts
index d958ef4..1637296 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor_test.ts
@@ -637,8 +637,8 @@
// REST API.
let content = [
' <section class="summary">',
- ' <gr-linked-text content="' +
- '[[_computeCurrentRevisionMessage(change)]]"></gr-linked-text>',
+ ' <gr-markdown content="' +
+ '[[_computeCurrentRevisionMessage(change)]]"></gr-markdown>',
' </section>',
];
let highlights = [
@@ -664,7 +664,7 @@
{
contentIndex: 2,
startIndex: 0,
- endIndex: 6,
+ endIndex: 12,
},
]);
const lines = element.linesFromRows(
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection_test.ts
index cb3eed5..89a0756 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection_test.ts
@@ -22,7 +22,7 @@
<div data-side="left">
<div class="comment-thread">
<div class="gr-formatted-text message">
- <span id="output" class="gr-linked-text">This is a comment</span>
+ <span id="output" class="gr-markdown">This is a comment</span>
</div>
</div>
</div>
@@ -44,7 +44,7 @@
<div data-side="right">
<div class="comment-thread">
<div class="gr-formatted-text message">
- <span id="output" class="gr-linked-text"
+ <span id="output" class="gr-markdown"
>This is a comment on the right</span
>
</div>
@@ -60,7 +60,7 @@
<div data-side="left">
<div class="comment-thread">
<div class="gr-formatted-text message">
- <span id="output" class="gr-linked-text"
+ <span id="output" class="gr-markdown"
>This is <a>a</a> different comment 💩 unicode is fun</span
>
</div>
diff --git a/polygerrit-ui/app/models/change/change-model.ts b/polygerrit-ui/app/models/change/change-model.ts
index 5d46210..5cfe670 100644
--- a/polygerrit-ui/app/models/change/change-model.ts
+++ b/polygerrit-ui/app/models/change/change-model.ts
@@ -139,7 +139,7 @@
// but we are not sure whether this was ever 100% working correctly. A
// major challenge is being able to select PARENT explicitly even if your
// preference for the default choice is FIRST_PARENT. <gr-file-list-header>
- // just uses `GerritNav.navigateToChange()` and the router does not have any
+ // just uses `navigation.setUrl()` and the router does not have any
// way of forcing the basePatchSetNum to stick to PARENT without being
// altered back to FIRST_PARENT here.
// See also corresponding TODO in gr-settings-view.
diff --git a/polygerrit-ui/app/models/views/change.ts b/polygerrit-ui/app/models/views/change.ts
index 7f75d83..b8451a0 100644
--- a/polygerrit-ui/app/models/views/change.ts
+++ b/polygerrit-ui/app/models/views/change.ts
@@ -8,6 +8,7 @@
RepoName,
RevisionPatchSetNum,
BasePatchSetNum,
+ ChangeInfo,
} from '../../api/rest-api';
import {GerritView} from '../../services/router/router-model';
import {UrlEncodedCommentId} from '../../types/common';
@@ -23,8 +24,8 @@
export interface ChangeViewState extends ViewState {
view: GerritView.CHANGE;
- changeNum?: NumericChangeId;
- project?: RepoName;
+ changeNum: NumericChangeId;
+ project: RepoName;
edit?: boolean;
patchNum?: RevisionPatchSetNum;
basePatchNum?: BasePatchSetNum;
@@ -41,7 +42,42 @@
usp?: string;
}
-export function createChangeUrl(state: Omit<ChangeViewState, 'view'>) {
+/**
+ * This is a convenience type such that you can pass a `ChangeInfo` object
+ * as the `change` property instead of having to set both the `changeNum` and
+ * `project` properties explicitly.
+ */
+export type CreateChangeUrlObject = Omit<
+ ChangeViewState,
+ 'view' | 'changeNum' | 'project'
+> & {
+ change: Pick<ChangeInfo, '_number' | 'project'>;
+};
+
+export function isCreateChangeUrlObject(
+ state: CreateChangeUrlObject | Omit<ChangeViewState, 'view'>
+): state is CreateChangeUrlObject {
+ return !!(state as CreateChangeUrlObject).change;
+}
+
+export function objToState(
+ obj: CreateChangeUrlObject | Omit<ChangeViewState, 'view'>
+): ChangeViewState {
+ if (isCreateChangeUrlObject(obj)) {
+ return {
+ ...obj,
+ view: GerritView.CHANGE,
+ changeNum: obj.change._number,
+ project: obj.change.project,
+ };
+ }
+ return {...obj, view: GerritView.CHANGE};
+}
+
+export function createChangeUrl(
+ obj: CreateChangeUrlObject | Omit<ChangeViewState, 'view'>
+) {
+ const state: ChangeViewState = objToState(obj);
let range = getPatchRangeExpression(state);
if (range.length) {
range = '/' + range;
diff --git a/polygerrit-ui/app/models/views/diff.ts b/polygerrit-ui/app/models/views/diff.ts
index 85fa081..63df521 100644
--- a/polygerrit-ui/app/models/views/diff.ts
+++ b/polygerrit-ui/app/models/views/diff.ts
@@ -8,6 +8,7 @@
RepoName,
RevisionPatchSetNum,
BasePatchSetNum,
+ ChangeInfo,
} from '../../api/rest-api';
import {GerritView} from '../../services/router/router-model';
import {UrlEncodedCommentId} from '../../types/common';
@@ -29,7 +30,42 @@
commentLink?: boolean;
}
-export function createDiffUrl(state: Omit<DiffViewState, 'view'>): string {
+/**
+ * This is a convenience type such that you can pass a `ChangeInfo` object
+ * as the `change` property instead of having to set both the `changeNum` and
+ * `project` properties explicitly.
+ */
+export type CreateChangeUrlObject = Omit<
+ DiffViewState,
+ 'view' | 'changeNum' | 'project'
+> & {
+ change: Pick<ChangeInfo, '_number' | 'project'>;
+};
+
+export function isCreateChangeUrlObject(
+ state: CreateChangeUrlObject | Omit<DiffViewState, 'view'>
+): state is CreateChangeUrlObject {
+ return !!(state as CreateChangeUrlObject).change;
+}
+
+export function objToState(
+ obj: CreateChangeUrlObject | Omit<DiffViewState, 'view'>
+): DiffViewState {
+ if (isCreateChangeUrlObject(obj)) {
+ return {
+ ...obj,
+ view: GerritView.DIFF,
+ changeNum: obj.change._number,
+ project: obj.change.project,
+ };
+ }
+ return {...obj, view: GerritView.DIFF};
+}
+
+export function createDiffUrl(
+ obj: CreateChangeUrlObject | Omit<DiffViewState, 'view'>
+) {
+ const state: DiffViewState = objToState(obj);
let range = getPatchRangeExpression(state);
if (range.length) range = '/' + range;
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index e142b96..0bf1522 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -16,12 +16,12 @@
export enum KnownExperimentId {
NEW_IMAGE_DIFF_UI = 'UiFeature__new_image_diff_ui',
CHECKS_DEVELOPER = 'UiFeature__checks_developer',
+ PUSH_NOTIFICATIONS_DEVELOPER = 'UiFeature__push_notifications_developer',
DIFF_RENDERING_LIT = 'UiFeature__diff_rendering_lit',
PUSH_NOTIFICATIONS = 'UiFeature__push_notifications',
SUGGEST_EDIT = 'UiFeature__suggest_edit',
CHECKS_FIXES = 'UiFeature__checks_fixes',
MENTION_USERS = 'UiFeature__mention_users',
- PATCHSET_LEVEL_COMMENT_USES_GRCOMMENT = 'UiFeature__patchset_level_comment_uses_GrComment',
RENDER_MARKDOWN = 'UiFeature__render_markdown',
AUTO_APP_THEME = 'UiFeature__auto_app_theme',
COPY_LINK_DIALOG = 'UiFeature__copy_link_dialog',
diff --git a/polygerrit-ui/app/services/service-worker-installer.ts b/polygerrit-ui/app/services/service-worker-installer.ts
index 3af79a3..0eec23a 100644
--- a/polygerrit-ui/app/services/service-worker-installer.ts
+++ b/polygerrit-ui/app/services/service-worker-installer.ts
@@ -5,9 +5,13 @@
*/
import {FlagsService, KnownExperimentId} from './flags/flags';
-import {registerServiceWorker} from '../utils/worker-util';
+import {
+ areNotificationsEnabled,
+ registerServiceWorker,
+} from '../utils/worker-util';
import {UserModel} from '../models/user/user-model';
import {AccountDetailInfo} from '../api/rest-api';
+import {until} from '../utils/async-util';
/** Type of incoming messages for ServiceWorker. */
export enum ServiceWorkerMessageType {
@@ -30,8 +34,23 @@
async init() {
if (this.initialized) return;
- if (!this.flagsService.isEnabled(KnownExperimentId.PUSH_NOTIFICATIONS)) {
- return;
+ if (
+ !this.flagsService.isEnabled(
+ KnownExperimentId.PUSH_NOTIFICATIONS_DEVELOPER
+ )
+ ) {
+ if (!this.flagsService.isEnabled(KnownExperimentId.PUSH_NOTIFICATIONS)) {
+ return;
+ }
+ const timeout1s = new Promise(resolve => {
+ setTimeout(resolve, 1000);
+ });
+ // We wait for account to be defined, if its not defined in 1s, it's guest
+ await Promise.race([
+ timeout1s,
+ until(this.userModel.account$, account => !!account),
+ ]);
+ if (!areNotificationsEnabled(this.account)) return;
}
if (!('serviceWorker' in navigator)) {
console.error('Service worker API not available');
diff --git a/polygerrit-ui/app/test/common-test-setup.ts b/polygerrit-ui/app/test/common-test-setup.ts
index d9d98b9..6f6fe73 100644
--- a/polygerrit-ui/app/test/common-test-setup.ts
+++ b/polygerrit-ui/app/test/common-test-setup.ts
@@ -6,7 +6,6 @@
// TODO(dmfilippov): remove bundled-polymer.js imports when the following issue
// https://github.com/Polymer/polymer-resin/issues/9 is resolved.
import '../scripts/bundled-polymer';
-import './test-router';
import {AppContext, injectAppContext} from '../services/app-context';
import {Finalizable} from '../services/registry';
import {
diff --git a/polygerrit-ui/app/test/test-router.ts b/polygerrit-ui/app/test/test-router.ts
deleted file mode 100644
index cb38f55..0000000
--- a/polygerrit-ui/app/test/test-router.ts
+++ /dev/null
@@ -1,10 +0,0 @@
-/**
- * @license
- * Copyright 2017 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import {GerritNav} from '../elements/core/gr-navigation/gr-navigation';
-
-GerritNav.setup(() => {
- /* noop */
-});
diff --git a/polygerrit-ui/app/utils/account-util.ts b/polygerrit-ui/app/utils/account-util.ts
index 0e2317f..6cc2e59 100644
--- a/polygerrit-ui/app/utils/account-util.ts
+++ b/polygerrit-ui/app/utils/account-util.ts
@@ -20,7 +20,7 @@
} from '../types/common';
import {AccountTag, ReviewerState} from '../constants/constants';
import {assertNever, hasOwnProperty} from './common-util';
-import {getDisplayName} from './display-name-util';
+import {getAccountDisplayName, getDisplayName} from './display-name-util';
import {getApprovalInfo} from './label-util';
import {RestApiService} from '../services/gr-rest-api/gr-rest-api';
import {ParsedChangeInfo} from '../types/types';
@@ -176,6 +176,7 @@
export function getAccountSuggestions(
input: string,
restApiService: RestApiService,
+ config?: ServerInfo,
canSee?: NumericChangeId,
filterActive = false
) {
@@ -185,14 +186,8 @@
if (!accounts) return [];
const accountSuggestions = [];
for (const account of accounts) {
- let nameAndEmail: string;
- if (account.email !== undefined) {
- nameAndEmail = `${account.name ?? ''} <${account.email}>`;
- } else {
- nameAndEmail = account.name ?? '';
- }
accountSuggestions.push({
- name: nameAndEmail,
+ name: getAccountDisplayName(config, account),
value: account._account_id?.toString(),
});
}
diff --git a/polygerrit-ui/app/utils/inner-html-util.ts b/polygerrit-ui/app/utils/inner-html-util.ts
index 8ac114a..ed9bfac 100644
--- a/polygerrit-ui/app/utils/inner-html-util.ts
+++ b/polygerrit-ui/app/utils/inner-html-util.ts
@@ -8,7 +8,7 @@
// Internally at Google it has different a implementation.
import {BrandType} from '../types/common';
-export {sanitizeHtml, htmlEscape} from 'safevalues';
+export {sanitizeHtml, htmlEscape, sanitizeHtmlToFragment} from 'safevalues';
export type SafeStyleSheet = BrandType<string, '_safeHtml'>;
diff --git a/polygerrit-ui/app/utils/link-util.ts b/polygerrit-ui/app/utils/link-util.ts
index 223f780..fd5965b 100644
--- a/polygerrit-ui/app/utils/link-util.ts
+++ b/polygerrit-ui/app/utils/link-util.ts
@@ -8,10 +8,20 @@
import {getBaseUrl} from './url-util';
export function linkifyNormalUrls(base: string): string {
+ // Some tools are known to look for reviewers/CCs by finding lines such as
+ // "R=foo@gmail.com, bar@gmail.com". However, "=" is technically a valid email
+ // character, so ba-linkify interprets the entire string "R=foo@gmail.com" as
+ // an email address. To fix this, we insert a zero width space character
+ // \u200B before linking that prevents ba-linkify from associating the prefix
+ // with the email. After linking we remove the zero width space.
+ const baseWithZeroWidthSpace = base.replace(/^(R=|CC=)/g, '$&\u200B');
const parts: string[] = [];
- window.linkify(base, {
- callback: (text, href) =>
- parts.push(href ? createLinkTemplate(text, href) : text),
+ window.linkify(baseWithZeroWidthSpace, {
+ callback: (text, href) => {
+ const result = href ? createLinkTemplate(text, href) : text;
+ const resultWithoutZeroWidthSpace = result.replace(/\u200B/g, '');
+ parts.push(resultWithoutZeroWidthSpace);
+ },
});
return parts.join('');
}
diff --git a/polygerrit-ui/app/utils/link-util_test.ts b/polygerrit-ui/app/utils/link-util_test.ts
index ecbcb61..c491e35 100644
--- a/polygerrit-ui/app/utils/link-util_test.ts
+++ b/polygerrit-ui/app/utils/link-util_test.ts
@@ -47,13 +47,45 @@
`${linkedNumber} ${linkedFoo}`
);
});
- test('linkifyNormalUrls', () => {
- const googleLink = link('google.com', 'http://google.com');
- const mapsLink = link('maps.google.com', 'http://maps.google.com');
- assert.equal(
- linkifyNormalUrls('google.com, maps.google.com'),
- `${googleLink}, ${mapsLink}`
- );
+ suite('linkifyNormalUrls', () => {
+ test('links urls', () => {
+ const googleLink = link('google.com', 'http://google.com');
+ const mapsLink = link('maps.google.com', 'http://maps.google.com');
+
+ assert.equal(
+ linkifyNormalUrls('google.com, maps.google.com'),
+ `${googleLink}, ${mapsLink}`
+ );
+ });
+
+ test('links emails without including R= prefix', () => {
+ const fooEmail = link('foo@gmail.com', 'mailto:foo@gmail.com');
+ const barEmail = link('bar@gmail.com', 'mailto:bar@gmail.com');
+ assert.equal(
+ linkifyNormalUrls('R=foo@gmail.com, bar@gmail.com'),
+ `R=${fooEmail}, ${barEmail}`
+ );
+ });
+
+ test('links emails without including CC= prefix', () => {
+ const fooEmail = link('foo@gmail.com', 'mailto:foo@gmail.com');
+ const barEmail = link('bar@gmail.com', 'mailto:bar@gmail.com');
+ assert.equal(
+ linkifyNormalUrls('CC=foo@gmail.com, bar@gmail.com'),
+ `CC=${fooEmail}, ${barEmail}`
+ );
+ });
+
+ test('links emails maintains R= and CC= within addresses', () => {
+ const fooBarBazEmail = link(
+ 'fooR=barCC=baz@gmail.com',
+ 'mailto:fooR=barCC=baz@gmail.com'
+ );
+ assert.equal(
+ linkifyNormalUrls('fooR=barCC=baz@gmail.com'),
+ fooBarBazEmail
+ );
+ });
});
});
diff --git a/polygerrit-ui/app/utils/worker-util.ts b/polygerrit-ui/app/utils/worker-util.ts
index c735a44..aeee537 100644
--- a/polygerrit-ui/app/utils/worker-util.ts
+++ b/polygerrit-ui/app/utils/worker-util.ts
@@ -4,6 +4,11 @@
* SPDX-License-Identifier: Apache-2.0
*/
+// This file adds some simple checks to match internal Google rules.
+// Internally at Google it has different a implementation.
+
+import {AccountDetailInfo} from '../api/rest-api';
+
/**
* We cannot import the worker script from cdn directly, because that is
* creating cross-origin issues. Instead we have to create a worker script on
@@ -25,6 +30,10 @@
return window.navigator.serviceWorker.register(workerUrl);
}
+export function areNotificationsEnabled(account?: AccountDetailInfo): boolean {
+ return !!account?._account_id;
+}
+
export function importScript(scope: WorkerGlobalScope, url: string): void {
scope.importScripts(url);
}
diff --git a/polygerrit-ui/app/workers/service-worker-class.ts b/polygerrit-ui/app/workers/service-worker-class.ts
index ff5da7f..c257d8d 100644
--- a/polygerrit-ui/app/workers/service-worker-class.ts
+++ b/polygerrit-ui/app/workers/service-worker-class.ts
@@ -105,8 +105,7 @@
) {
const body = getReason(undefined, account, change);
const changeUrl = createChangeUrl({
- changeNum: change._number,
- project: change.project,
+ change,
usp: 'service-worker-notification',
});
// We are adding origin because each notification can have different origin
