Merge "Merge gr-linked-text into gr-markdown"
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-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-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/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
index c5a007e..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';
@@ -1863,7 +1860,7 @@
}
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-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 203fe86..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
@@ -37,10 +37,7 @@
import {GrEditConstants} from '../../edit/gr-edit-constants';
import {pluralize} from '../../../utils/string-util';
import {querySelectorAll, windowLocationReload} from '../../../utils/dom-util';
-import {
- GerritNav,
- navigationToken,
-} 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';
@@ -2615,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(
@@ -3216,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,
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 3db03a1..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,10 +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,
- 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 {EventType, PluginApi} from '../../../api/plugin';
import {
@@ -795,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 () => {
@@ -2098,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(
@@ -2152,7 +2141,7 @@
);
await element.updateComplete;
- assert.isTrue(navigateToRelativeUrlStub.called);
+ assert.isTrue(setUrlStub.called);
});
test('selectedRevision updates when patchNum is changed', async () => {
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 0db0c96..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
@@ -20,10 +20,7 @@
import {asyncForeach} from '../../../utils/async-util';
import {FilesExpandedState} from '../gr-file-list-constants';
import {diffFilePaths, pluralize} from '../../../utils/string-util';
-import {
- GerritNav,
- navigationToken,
-} 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';
@@ -2045,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,
+ })
);
}
@@ -2064,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,
+ })
);
}
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/core/gr-navigation/gr-navigation.ts b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
index 69a7b1d..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,26 +3,8 @@
* Copyright 2017 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {
- BasePatchSetNum,
- ChangeInfo,
- RevisionPatchSetNum,
-} from '../../../types/common';
-import {ParsedChangeInfo} from '../../../types/types';
-import {createDiffUrl} from '../../../models/views/diff';
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;
-
export const navigationToken = define<NavigationService>('navigation');
export interface NavigationService {
@@ -45,57 +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.
- */
- 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);
- },
-};
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/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 295d4b9..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,10 +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,
- navigationToken,
-} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getAppContext} from '../../../services/app-context';
import {
computeAllPatchSets,
@@ -1163,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,
+ })
);
}
@@ -1183,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,
+ })
);
}
@@ -1362,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,
+ })
);
}
@@ -1407,7 +1410,7 @@
patchNum: this.patchRange.patchNum,
lineNum: cursorAddress?.number,
});
- GerritNav.navigateToRelativeUrl(editUrl);
+ this.getNavigation().setUrl(editUrl);
}
/**
@@ -1712,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;
}
@@ -1884,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,
+ })
);
}
@@ -1905,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.
@@ -2111,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.
@@ -2124,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,
+ })
);
}
@@ -2147,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,
+ })
);
}
@@ -2166,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,
+ })
);
}
@@ -2188,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 134f23e..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,10 +5,7 @@
*/
import '../../../test/common-test-setup';
import './gr-diff-view';
-import {
- GerritNav,
- navigationToken,
-} from '../../core/gr-navigation/gr-navigation';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {
ChangeStatus,
DiffViewMode,
@@ -72,7 +69,6 @@
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(
@@ -268,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': [
@@ -307,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': [
@@ -359,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', () => {
@@ -655,23 +643,18 @@
element.path = 'glados.txt';
element.loggedIn = true;
await element.updateComplete;
-
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
+ setUrlStub.reset();
pressKey(element, 'u');
- assert.isTrue(setUrlStub.calledOnce);
+ 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;
@@ -679,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;
@@ -694,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;
@@ -709,7 +684,7 @@
assert.isTrue(element.loading);
pressKey(element, '[');
- assert.isTrue(setUrlStub.calledTwice);
+ assert.equal(setUrlStub.callCount, 5);
assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42');
await element.updateComplete;
assert.isTrue(element.loading);
@@ -790,7 +765,6 @@
});
test('moveToNextCommentThread navigates to next file', async () => {
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
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(setUrlStub.calledOnce);
+ 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,12 +922,11 @@
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 () => {
@@ -990,11 +954,15 @@
},
};
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.isTrue(setUrlStub.calledOnce);
+ assert.equal(setUrlStub.callCount, 1);
assert.equal(
setUrlStub.lastCall.firstArg,
'/c/test-project/+/42/5..10?openReplyDialog=true'
@@ -1050,57 +1018,40 @@
]);
element.path = 'glados.txt';
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
-
pressKey(element, 'u');
- assert.isTrue(setUrlStub.calledOnce);
+ 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.isTrue(setUrlStub.calledTwice);
+ assert.equal(setUrlStub.callCount, 5);
assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/5..10');
assertIsDefined(element.downloadOverlay);
@@ -1132,48 +1083,28 @@
]);
element.path = 'glados.txt';
- const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
-
pressKey(element, 'u');
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';
@@ -1200,7 +1131,6 @@
b: createRevision(2),
},
};
- const redirectStub = sinon.stub(GerritNav, 'navigateToRelativeUrl');
await element.updateComplete;
const editBtn = queryAndAssert<GrButton>(
element,
@@ -1208,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 () => {
@@ -1243,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,
@@ -1251,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'
);
});
@@ -1582,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,
@@ -1606,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'
);
});
@@ -2126,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',
@@ -2146,7 +2052,7 @@
test('no skips', () => {
element.moveToPreviousFileWithComment();
assert.isFalse(navToChangeStub.called);
- assert.isFalse(navToDiffStub.called);
+ assert.isFalse(setUrlStub.called);
});
test('no previous', async () => {
@@ -2160,7 +2066,7 @@
element.moveToPreviousFileWithComment();
assert.isTrue(navToChangeStub.calledOnce);
- assert.isFalse(navToDiffStub.called);
+ assert.isFalse(setUrlStub.called);
});
test('w/ previous', async () => {
@@ -2174,7 +2080,7 @@
element.moveToPreviousFileWithComment();
assert.isFalse(navToChangeStub.called);
- assert.isTrue(navToDiffStub.calledOnce);
+ assert.isTrue(setUrlStub.calledOnce);
});
});
@@ -2182,7 +2088,7 @@
test('no skips', () => {
element.moveToNextFileWithComment();
assert.isFalse(navToChangeStub.called);
- assert.isFalse(navToDiffStub.called);
+ assert.isFalse(setUrlStub.called);
});
test('no previous', async () => {
@@ -2196,7 +2102,7 @@
element.moveToNextFileWithComment();
assert.isTrue(navToChangeStub.calledOnce);
- assert.isFalse(navToDiffStub.called);
+ assert.isFalse(setUrlStub.called);
});
test('w/ previous', async () => {
@@ -2210,7 +2116,7 @@
element.moveToNextFileWithComment();
assert.isFalse(navToChangeStub.called);
- assert.isTrue(navToDiffStub.calledOnce);
+ assert.isTrue(setUrlStub.calledOnce);
});
});
});
@@ -2452,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 = {
@@ -2474,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 = {
@@ -2496,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 ee83e80..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';
@@ -22,6 +22,7 @@
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;
@@ -194,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'
@@ -234,7 +235,7 @@
'gr-button[primary]'
).click();
- assert.isTrue(navStub.called);
+ assert.isTrue(setUrlStub.called);
assert.isTrue(closeDialogSpy.called);
});
@@ -247,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, '');
});
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/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/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(),
});
}
