Merge "Merge branch 'stable-3.6'"
diff --git a/Documentation/linux-quickstart.txt b/Documentation/linux-quickstart.txt
index c45de05..13873ed 100644
--- a/Documentation/linux-quickstart.txt
+++ b/Documentation/linux-quickstart.txt
@@ -29,10 +29,10 @@
. Download the desired Gerrit archive.
To view previous archives, see
-link:https://gerrit-releases.storage.googleapis.com/index.html[Gerrit Code Review: Releases,role=external,window=_blank]. The steps below install Gerrit 3.1.3:
+link:https://gerrit-releases.storage.googleapis.com/index.html[Gerrit Code Review: Releases,role=external,window=_blank]. The steps below install Gerrit 3.5.1:
....
-wget https://gerrit-releases.storage.googleapis.com/gerrit-3.1.3.war
+wget https://gerrit-releases.storage.googleapis.com/gerrit-3.5.1.war
....
NOTE: To build and install Gerrit from the source files, see
diff --git a/java/com/google/gerrit/server/permissions/GitVisibleChangeFilter.java b/java/com/google/gerrit/server/permissions/GitVisibleChangeFilter.java
index 5a7b3cb..506d292 100644
--- a/java/com/google/gerrit/server/permissions/GitVisibleChangeFilter.java
+++ b/java/com/google/gerrit/server/permissions/GitVisibleChangeFilter.java
@@ -100,7 +100,11 @@
.map(
id -> {
try {
- return changeDataFactory.create(projectName, id);
+ ChangeData cd = changeDataFactory.create(projectName, id);
+ cd.notes(); // Make sure notes are available. This will trigger loading notes and
+ // throw an exception in case the change is corrupt and can't be loaded. It will
+ // then be omitted from the result.
+ return cd;
} catch (Exception e) {
// We drop changes that we can't load. The repositories contain 'dead' change refs
// and we want to overall operation to continue.
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow.ts
index 58eec3b..95e9c87 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow.ts
@@ -34,13 +34,9 @@
import {AccountInputDetail} from '../../shared/gr-account-list/gr-account-list';
import '@polymer/iron-icon/iron-icon';
-const SUGGESTIONS_PROVIDERS_USERS_TYPES_BY_REVIEWER_STATE: Record<
- ReviewerState,
- SUGGESTIONS_PROVIDERS_USERS_TYPES
-> = {
+const SUGGESTIONS_PROVIDERS_USERS_TYPES_BY_REVIEWER_STATE = {
REVIEWER: SUGGESTIONS_PROVIDERS_USERS_TYPES.REVIEWER,
CC: SUGGESTIONS_PROVIDERS_USERS_TYPES.CC,
- REMOVED: SUGGESTIONS_PROVIDERS_USERS_TYPES.ANY,
};
@customElement('gr-change-list-reviewer-flow')
@@ -80,6 +76,8 @@
private restApiService = getAppContext().restApiService;
+ private isLoggedIn = false;
+
static override get styles() {
return css`
gr-dialog {
@@ -128,6 +126,11 @@
this.getConfigModel().serverConfig$,
serverConfig => (this.serverConfig = serverConfig)
);
+ subscribe(
+ this,
+ getAppContext().userModel.loggedIn$,
+ isLoggedIn => (this.isLoggedIn = isLoggedIn)
+ );
}
override render() {
@@ -275,7 +278,7 @@
ProgressStatus.NOT_STARTED,
])
);
- for (const state of [ReviewerState.REVIEWER, ReviewerState.CC]) {
+ for (const state of [ReviewerState.REVIEWER, ReviewerState.CC] as const) {
this.updatedAccountsByReviewerState.set(
state,
this.getCurrentAccounts(state)
@@ -396,15 +399,15 @@
}
private createSuggestionsProvider(
- state: ReviewerState
+ state: ReviewerState.CC | ReviewerState.REVIEWER
): ReviewerSuggestionsProvider {
- const suggestionsProvider = GrReviewerSuggestionsProvider.create(
+ const suggestionsProvider = new GrReviewerSuggestionsProvider(
this.restApiService,
- // TODO: fan out and get suggestions allowed by all changes
- this.selectedChanges[0]._number,
- SUGGESTIONS_PROVIDERS_USERS_TYPES_BY_REVIEWER_STATE[state]
+ SUGGESTIONS_PROVIDERS_USERS_TYPES_BY_REVIEWER_STATE[state],
+ this.serverConfig,
+ this.isLoggedIn,
+ ...this.selectedChanges.map(change => change._number)
);
- suggestionsProvider.init();
return suggestionsProvider;
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts
index 9f33990..c03121d 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts
@@ -306,7 +306,7 @@
super.disconnectedCallback();
}
- scrollToMessage(messageID: string) {
+ async scrollToMessage(messageID: string) {
const selector = `[data-message-id="${messageID}"]`;
const el = this.shadowRoot!.querySelector(selector) as
| GrMessage
@@ -325,6 +325,7 @@
}
el.message = {...el.message, expanded: true};
+ await el.updateComplete;
let top = el.offsetTop;
for (
let offsetParent = el.offsetParent as HTMLElement | null;
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.ts b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.ts
index 30dd257..b9cb616 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.ts
@@ -222,7 +222,7 @@
assert.isNotOk(query(element, '.showAllActivityToggle'));
});
- test('scroll to message', () => {
+ test('scroll to message', async () => {
const allMessageEls = getMessages();
for (const message of allMessageEls) {
assertIsDefined(message.message);
@@ -232,7 +232,7 @@
const scrollToStub = sinon.stub(window, 'scrollTo');
const highlightStub = sinon.stub(element, '_highlightEl');
- element.scrollToMessage('invalid');
+ await element.scrollToMessage('invalid');
for (const message of allMessageEls) {
assertIsDefined(message.message);
@@ -243,7 +243,7 @@
}
const messageID = messages[1].id;
- element.scrollToMessage(messageID);
+ await element.scrollToMessage(messageID);
assert.isTrue(
queryAndAssert<GrMessage>(element, `[data-message-id="${messageID}"]`)
.message?.expanded
@@ -253,16 +253,16 @@
assert.isTrue(highlightStub.calledOnce);
});
- test('scroll to message offscreen', () => {
+ test('scroll to message offscreen', async () => {
const scrollToStub = sinon.stub(window, 'scrollTo');
const highlightStub = sinon.stub(element, '_highlightEl');
element.messages = generateRandomMessages(25);
- flush();
+ await element.updateComplete;
assert.isFalse(scrollToStub.called);
assert.isFalse(highlightStub.called);
const messageID = element.messages[1].id;
- element.scrollToMessage(messageID);
+ await element.scrollToMessage(messageID);
assert.isTrue(scrollToStub.calledOnce);
assert.isTrue(highlightStub.calledOnce);
assert.isTrue(
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 95572a8..c35b6b3 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
@@ -118,6 +118,7 @@
import {classMap} from 'lit/directives/class-map';
import {BindValueChangeEvent, ValueChangedEvent} from '../../../types/events';
import {customElement, property, state, query} from 'lit/decorators';
+import {subscribe} from '../../lit/subscription-controller';
const STORAGE_DEBOUNCE_INTERVAL_MS = 400;
@@ -347,6 +348,8 @@
storeTask?: DelayedTask;
+ private isLoggedIn = false;
+
/** Called in disconnectedCallback. */
private cleanups: (() => void)[] = [];
@@ -687,6 +690,12 @@
this.addEventListener('remove-reviewer', e => {
this.reviewersList?.removeAccount((e as CustomEvent).detail.reviewer);
});
+
+ subscribe(
+ this,
+ getAppContext().userModel.loggedIn$,
+ isLoggedIn => (this.isLoggedIn = isLoggedIn)
+ );
}
override disconnectedCallback() {
@@ -2079,23 +2088,25 @@
getReviewerSuggestionsProvider(change?: ChangeInfo) {
if (!change) return;
- const provider = GrReviewerSuggestionsProvider.create(
+ const provider = new GrReviewerSuggestionsProvider(
this.restApiService,
- change._number,
- SUGGESTIONS_PROVIDERS_USERS_TYPES.REVIEWER
+ SUGGESTIONS_PROVIDERS_USERS_TYPES.REVIEWER,
+ this.serverConfig,
+ this.isLoggedIn,
+ change._number
);
- provider.init();
return provider;
}
getCcSuggestionsProvider(change?: ChangeInfo) {
if (!change) return;
- const provider = GrReviewerSuggestionsProvider.create(
+ const provider = new GrReviewerSuggestionsProvider(
this.restApiService,
- change._number,
- SUGGESTIONS_PROVIDERS_USERS_TYPES.CC
+ SUGGESTIONS_PROVIDERS_USERS_TYPES.CC,
+ this.serverConfig,
+ this.isLoggedIn,
+ change._number
);
- provider.init();
return provider;
}
diff --git a/polygerrit-ui/app/models/dependency.ts b/polygerrit-ui/app/models/dependency.ts
index e7ac242c..f39fa32 100644
--- a/polygerrit-ui/app/models/dependency.ts
+++ b/polygerrit-ui/app/models/dependency.ts
@@ -133,16 +133,35 @@
*/
export type Provider<T> = () => T;
+// Symbols to cache the providers and resolvers to avoid duplicate registration.
+const PROVIDERS_SYMBOL = Symbol('providers');
+const RESOLVERS_SYMBOL = Symbol('resolvers');
+
+interface Registrations {
+ [PROVIDERS_SYMBOL]?: Map<
+ DependencyToken<unknown>,
+ DependencyProvider<unknown>
+ >;
+ [RESOLVERS_SYMBOL]?: Map<DependencyToken<unknown>, Provider<unknown>>;
+}
/**
* A producer of a dependency expresses this as a need that results in a promise
* for the given dependency.
*/
export function provide<T>(
- host: ReactiveControllerHost & HTMLElement,
+ host: ReactiveControllerHost & HTMLElement & Registrations,
dependency: DependencyToken<T>,
provider: Provider<T>
) {
- host.addController(new DependencyProvider<T>(host, dependency, provider));
+ const hostProviders = (host[PROVIDERS_SYMBOL] ||= new Map());
+ const oldController = hostProviders.get(dependency);
+ if (oldController) {
+ host.removeController(oldController);
+ oldController.hostDisconnected();
+ }
+ const controller = new DependencyProvider<T>(host, dependency, provider);
+ hostProviders.set(dependency, provider);
+ host.addController(controller);
}
/**
@@ -151,12 +170,18 @@
* the injected value.
*/
export function resolve<T>(
- host: ReactiveControllerHost & HTMLElement,
+ host: ReactiveControllerHost & HTMLElement & Registrations,
dependency: DependencyToken<T>
): Provider<T> {
- const controller = new DependencySubscriber(host, dependency);
- host.addController(controller);
- return () => controller.get();
+ const hostResolvers = (host[RESOLVERS_SYMBOL] ||= new Map());
+ let resolver = hostResolvers.get(dependency);
+ if (!resolver) {
+ const controller = new DependencySubscriber(host, dependency);
+ host.addController(controller);
+ resolver = () => controller.get();
+ hostResolvers.set(dependency, resolver);
+ }
+ return resolver;
}
/**
diff --git a/polygerrit-ui/app/scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider.ts b/polygerrit-ui/app/scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider.ts
index d0f79f4..95e7f2a 100644
--- a/polygerrit-ui/app/scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider.ts
+++ b/polygerrit-ui/app/scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider.ts
@@ -28,25 +28,20 @@
SuggestedReviewerInfo,
Suggestion,
} from '../../types/common';
-import {assertNever} from '../../utils/common-util';
+import {assertNever, intersection} from '../../utils/common-util';
import {AutocompleteSuggestion} from '../../elements/shared/gr-autocomplete/gr-autocomplete';
+import {allSettled, isFulfilled} from '../../utils/async-util';
+import {notUndefined} from '../../types/types';
+import {accountKey} from '../../utils/account-util';
-// TODO(TS): enum name doesn't follow typescript style guid rules
+// TODO(TS): enum name doesn't follow typescript style guide rules
// Rename it
export enum SUGGESTIONS_PROVIDERS_USERS_TYPES {
REVIEWER = 'reviewers',
CC = 'ccs',
- ANY = 'any',
}
-export function isAccountSuggestions(s: Suggestion): s is AccountInfo {
- return (s as AccountInfo)._account_id !== undefined;
-}
-
-type ApiCallCallback = (input: string) => Promise<Suggestion[] | void>;
-
export interface ReviewerSuggestionsProvider {
- init(): void;
getSuggestions(input: string): Promise<Suggestion[]>;
makeSuggestionItem(
suggestion: Suggestion
@@ -56,66 +51,33 @@
export class GrReviewerSuggestionsProvider
implements ReviewerSuggestionsProvider
{
- static create(
- restApi: RestApiService,
- changeNumber: NumericChangeId,
- userType: SUGGESTIONS_PROVIDERS_USERS_TYPES
+ private changeNumbers: NumericChangeId[];
+
+ constructor(
+ private restApi: RestApiService,
+ private type: SUGGESTIONS_PROVIDERS_USERS_TYPES,
+ private config: ServerInfo | undefined,
+ private loggedIn: boolean,
+ ...changeNumbers: NumericChangeId[]
) {
- switch (userType) {
- case SUGGESTIONS_PROVIDERS_USERS_TYPES.REVIEWER:
- return new GrReviewerSuggestionsProvider(restApi, input =>
- restApi.getChangeSuggestedReviewers(changeNumber, input)
- );
- case SUGGESTIONS_PROVIDERS_USERS_TYPES.CC:
- return new GrReviewerSuggestionsProvider(restApi, input =>
- restApi.getChangeSuggestedCCs(changeNumber, input)
- );
- case SUGGESTIONS_PROVIDERS_USERS_TYPES.ANY:
- return new GrReviewerSuggestionsProvider(restApi, input =>
- restApi.getSuggestedAccounts(`cansee:${changeNumber} ${input}`)
- );
- default:
- throw new Error(`Unknown users type: ${userType}`);
- }
+ this.changeNumbers = changeNumbers;
}
- private initPromise?: Promise<void>;
+ async getSuggestions(input: string): Promise<Suggestion[]> {
+ if (!this.loggedIn) return [];
- config?: ServerInfo;
-
- loggedIn = false;
-
- private initialized = false;
-
- private constructor(
- private readonly _restAPI: RestApiService,
- private readonly _apiCall: ApiCallCallback
- ) {}
-
- init() {
- if (this.initPromise) {
- return this.initPromise;
- }
- const getConfigPromise = this._restAPI.getConfig().then(cfg => {
- this.config = cfg;
- });
- const getLoggedInPromise = this._restAPI.getLoggedIn().then(loggedIn => {
- this.loggedIn = loggedIn;
- });
- this.initPromise = Promise.all([getConfigPromise, getLoggedInPromise]).then(
- () => {
- this.initialized = true;
- }
+ const allResults = await allSettled(
+ this.changeNumbers.map(changeNumber =>
+ this.getSuggestionsForChange(changeNumber, input)
+ )
);
- return this.initPromise;
- }
-
- getSuggestions(input: string): Promise<Suggestion[]> {
- if (!this.initialized || !this.loggedIn) {
- return Promise.resolve([]);
- }
-
- return this._apiCall(input).then(reviewers => reviewers || []);
+ const allSuggestions = allResults
+ .filter(isFulfilled)
+ .map(result => result.value)
+ .filter(notUndefined);
+ return intersection(allSuggestions, (s1, s2) =>
+ this.areSameSuggestions(s1, s2)
+ );
}
makeSuggestionItem(
@@ -137,7 +99,7 @@
};
}
- if (isAccountSuggestions(suggestion)) {
+ if (this.isAccountSuggestion(suggestion)) {
// Reviewer is an account suggestion from getSuggestedAccounts.
return {
name: getAccountDisplayName(this.config, suggestion),
@@ -146,4 +108,28 @@
}
assertNever(suggestion, 'Received an incorrect suggestion');
}
+
+ private getSuggestionsForChange(
+ changeNumber: NumericChangeId,
+ input: string
+ ): Promise<Suggestion[] | undefined> {
+ return this.type === SUGGESTIONS_PROVIDERS_USERS_TYPES.REVIEWER
+ ? this.restApi.getChangeSuggestedReviewers(changeNumber, input)
+ : this.restApi.getChangeSuggestedCCs(changeNumber, input);
+ }
+
+ private areSameSuggestions(a: Suggestion, b: Suggestion): boolean {
+ if (isReviewerAccountSuggestion(a) && isReviewerAccountSuggestion(b)) {
+ return accountKey(a.account) === accountKey(b.account);
+ } else if (isReviewerGroupSuggestion(a) && isReviewerGroupSuggestion(b)) {
+ return a.group.id === b.group.id;
+ } else if (this.isAccountSuggestion(a) && this.isAccountSuggestion(b)) {
+ return accountKey(a) === accountKey(b);
+ }
+ return false;
+ }
+
+ private isAccountSuggestion(s: Suggestion): s is AccountInfo {
+ return (s as AccountInfo)._account_id !== undefined;
+ }
}
diff --git a/polygerrit-ui/app/scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider_test.ts b/polygerrit-ui/app/scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider_test.ts
index 757bcca..3d1a15f 100644
--- a/polygerrit-ui/app/scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider_test.ts
+++ b/polygerrit-ui/app/scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider_test.ts
@@ -23,237 +23,192 @@
import {getAppContext} from '../../services/app-context';
import {stubRestApi} from '../../test/test-utils';
import {
- AccountId,
- AccountInfo,
ChangeInfo,
- EmailAddress,
GroupId,
GroupName,
NumericChangeId,
} from '../../api/rest-api';
-import {SuggestedReviewerInfo} from '../../types/common';
-import {createChange, createServerInfo} from '../../test/test-data-generators';
+import {
+ SuggestedReviewerAccountInfo,
+ SuggestedReviewerGroupInfo,
+} from '../../types/common';
+import {
+ createAccountDetailWithIdNameAndEmail,
+ createChange,
+ createServerInfo,
+} from '../../test/test-data-generators';
suite('GrReviewerSuggestionsProvider tests', () => {
- let _nextAccountId = 0;
- function makeAccount(opt_status?: string): AccountInfo {
- const accountId = ++_nextAccountId;
- return {
- _account_id: accountId as AccountId,
- name: `name ${accountId}`,
- email: `email ${accountId}` as EmailAddress,
- status: opt_status,
- };
- }
- let _nextAccountId2 = 0;
- function makeAccount2(opt_status?: string): AccountInfo {
- const accountId2 = ++_nextAccountId2;
- return {
- _account_id: accountId2 as AccountId,
- name: `name ${accountId2}`,
- status: opt_status,
- };
- }
-
- let owner: AccountInfo;
- let existingReviewer1: AccountInfo;
- let existingReviewer2: AccountInfo;
- let suggestion1: SuggestedReviewerInfo;
- let suggestion2: SuggestedReviewerInfo;
- let suggestion3: SuggestedReviewerInfo;
+ const suggestion1: SuggestedReviewerAccountInfo = {
+ account: createAccountDetailWithIdNameAndEmail(3),
+ count: 1,
+ };
+ const suggestion2: SuggestedReviewerAccountInfo = {
+ account: createAccountDetailWithIdNameAndEmail(4),
+ count: 1,
+ };
+ const suggestion3: SuggestedReviewerGroupInfo = {
+ group: {
+ id: 'suggested group id' as GroupId,
+ name: 'suggested group' as GroupName,
+ },
+ count: 4,
+ };
+ const change: ChangeInfo = createChange();
+ let getChangeSuggestedReviewersStub: sinon.SinonStub;
+ let getChangeSuggestedCCsStub: sinon.SinonStub;
let provider: GrReviewerSuggestionsProvider;
- let redundantSuggestion1: SuggestedReviewerInfo;
- let redundantSuggestion2: SuggestedReviewerInfo;
- let redundantSuggestion3: SuggestedReviewerInfo;
- let change: ChangeInfo;
-
- setup(async () => {
- owner = makeAccount();
- existingReviewer1 = makeAccount();
- existingReviewer2 = makeAccount();
- suggestion1 = {account: makeAccount(), count: 1};
- suggestion2 = {account: makeAccount(), count: 1};
- suggestion3 = {
- group: {
- id: 'suggested group id' as GroupId,
- name: 'suggested group' as GroupName,
- },
- count: 1,
- };
-
- stubRestApi('getConfig').resolves(createServerInfo());
-
- change = {
- ...createChange(),
- _number: 42 as NumericChangeId,
- owner,
- reviewers: {
- CC: [existingReviewer1],
- REVIEWER: [existingReviewer2],
- },
- };
-
- await flush();
+ setup(() => {
+ getChangeSuggestedReviewersStub = stubRestApi(
+ 'getChangeSuggestedReviewers'
+ ).resolves([suggestion1, suggestion2, suggestion3]);
+ getChangeSuggestedCCsStub = stubRestApi('getChangeSuggestedCCs').resolves([
+ suggestion1,
+ suggestion2,
+ suggestion3,
+ ]);
+ provider = new GrReviewerSuggestionsProvider(
+ getAppContext().restApiService,
+ SUGGESTIONS_PROVIDERS_USERS_TYPES.REVIEWER,
+ createServerInfo(),
+ true,
+ change._number
+ );
});
- suite('allowAnyUser set to false', () => {
- setup(async () => {
- provider = GrReviewerSuggestionsProvider.create(
- getAppContext().restApiService,
- change._number,
- SUGGESTIONS_PROVIDERS_USERS_TYPES.REVIEWER
- );
- await provider.init();
- });
- suite('stubbed values for _getReviewerSuggestions', () => {
- let getChangeSuggestedReviewersStub: sinon.SinonStub;
- setup(() => {
- getChangeSuggestedReviewersStub = stubRestApi(
- 'getChangeSuggestedReviewers'
- ).callsFake(() => {
- redundantSuggestion1 = {account: existingReviewer1, count: 1};
- redundantSuggestion2 = {account: existingReviewer2, count: 1};
- redundantSuggestion3 = {account: owner, count: 1};
- return Promise.resolve([
- redundantSuggestion1,
- redundantSuggestion2,
- redundantSuggestion3,
- suggestion1,
- suggestion2,
- suggestion3,
- ]);
- });
- });
+ test('getSuggestions', async () => {
+ const reviewers = await provider.getSuggestions('');
- test('makeSuggestionItem formats account or group accordingly', () => {
- let account = makeAccount();
- const account3 = makeAccount2();
- let suggestion = provider.makeSuggestionItem({account, count: 1});
- assert.deepEqual(suggestion, {
- name: `${account.name} <${account.email}>`,
- value: {account, count: 1},
- });
-
- const group = {name: 'test' as GroupName, id: '5' as GroupId};
- suggestion = provider.makeSuggestionItem({group, count: 1});
- assert.deepEqual(suggestion, {
- name: `${group.name} (group)`,
- value: {group, count: 1},
- });
-
- suggestion = provider.makeSuggestionItem(account);
- assert.deepEqual(suggestion, {
- name: `${account.name} <${account.email}>`,
- value: {account, count: 1},
- });
-
- suggestion = provider.makeSuggestionItem({account: {}, count: 1});
- assert.deepEqual(suggestion, {
- name: 'Name of user not set',
- value: {account: {}, count: 1},
- });
-
- provider.config = {
- ...createServerInfo(),
- user: {
- anonymous_coward_name: 'Anonymous Coward Name',
- },
- };
-
- suggestion = provider.makeSuggestionItem({account: {}, count: 1});
- assert.deepEqual(suggestion, {
- name: 'Anonymous Coward Name',
- value: {account: {}, count: 1},
- });
-
- account = makeAccount('OOO');
-
- suggestion = provider.makeSuggestionItem({account, count: 1});
- assert.deepEqual(suggestion, {
- name: `${account.name} <${account.email}> (OOO)`,
- value: {account, count: 1},
- });
-
- suggestion = provider.makeSuggestionItem(account);
- assert.deepEqual(suggestion, {
- name: `${account.name} <${account.email}> (OOO)`,
- value: {account, count: 1},
- });
-
- account3.email = undefined;
-
- suggestion = provider.makeSuggestionItem(account3);
- assert.deepEqual(suggestion, {
- name: account3.name,
- value: {account: account3, count: 1},
- });
- });
-
- test('getSuggestions', async () => {
- const reviewers = await provider.getSuggestions('');
-
- // Default is no filtering.
- assert.equal(reviewers.length, 6);
- assert.deepEqual(reviewers, [
- redundantSuggestion1,
- redundantSuggestion2,
- redundantSuggestion3,
- suggestion1,
- suggestion2,
- suggestion3,
- ]);
- });
-
- test('getSuggestions short circuits when logged out', () => {
- provider.loggedIn = false;
- return provider.getSuggestions('').then(() => {
- assert.isFalse(getChangeSuggestedReviewersStub.called);
- provider.loggedIn = true;
- return provider.getSuggestions('').then(() => {
- assert.isTrue(getChangeSuggestedReviewersStub.called);
- });
- });
- });
- });
-
- test('getChangeSuggestedReviewers is used', async () => {
- const suggestReviewerStub = stubRestApi(
- 'getChangeSuggestedReviewers'
- ).returns(Promise.resolve([]));
- const suggestAccountStub = stubRestApi('getSuggestedAccounts').returns(
- Promise.resolve([])
- );
-
- await provider.getSuggestions('');
- assert.isTrue(suggestReviewerStub.calledOnce);
- assert.isTrue(suggestReviewerStub.calledWith(42 as NumericChangeId, ''));
- assert.isFalse(suggestAccountStub.called);
- });
+ assert.sameDeepMembers(reviewers, [suggestion1, suggestion2, suggestion3]);
});
- suite('allowAnyUser set to true', () => {
- setup(async () => {
- provider = GrReviewerSuggestionsProvider.create(
- getAppContext().restApiService,
- change._number,
- SUGGESTIONS_PROVIDERS_USERS_TYPES.ANY
- );
- await provider.init();
+ test('getSuggestions short circuits when logged out', async () => {
+ await provider.getSuggestions('');
+ assert.isTrue(getChangeSuggestedReviewersStub.calledOnce);
+
+ // not logged in
+ provider = new GrReviewerSuggestionsProvider(
+ getAppContext().restApiService,
+ SUGGESTIONS_PROVIDERS_USERS_TYPES.REVIEWER,
+ createServerInfo(),
+ false,
+ change._number
+ );
+
+ await provider.getSuggestions('');
+
+ // no additional call is made
+ assert.isTrue(getChangeSuggestedReviewersStub.calledOnce);
+ });
+
+ test('only returns REVIEWER suggestions shared by all changes', async () => {
+ getChangeSuggestedReviewersStub
+ .onSecondCall()
+ .resolves([suggestion2, suggestion3]);
+ provider = new GrReviewerSuggestionsProvider(
+ getAppContext().restApiService,
+ SUGGESTIONS_PROVIDERS_USERS_TYPES.REVIEWER,
+ createServerInfo(),
+ true,
+ ...[change._number, 43 as NumericChangeId]
+ );
+
+ // suggestion1 is excluded because it is not returned for the second
+ // change.
+ assert.sameDeepMembers(await provider.getSuggestions('s'), [
+ suggestion2,
+ suggestion3,
+ ]);
+ });
+
+ test('only returns CC suggestions shared by all changes', async () => {
+ getChangeSuggestedCCsStub
+ .onSecondCall()
+ .resolves([suggestion2, suggestion3]);
+ provider = new GrReviewerSuggestionsProvider(
+ getAppContext().restApiService,
+ SUGGESTIONS_PROVIDERS_USERS_TYPES.CC,
+ createServerInfo(),
+ true,
+ ...[change._number, 43 as NumericChangeId]
+ );
+
+ // suggestion1 is excluded because it is not returned for the second
+ // change.
+ assert.sameDeepMembers(await provider.getSuggestions('s'), [
+ suggestion2,
+ suggestion3,
+ ]);
+ });
+
+ test('makeSuggestionItem formats account or group accordingly', () => {
+ let account = createAccountDetailWithIdNameAndEmail(1);
+ const account3 = createAccountDetailWithIdNameAndEmail(2);
+ let suggestion = provider.makeSuggestionItem({account, count: 1});
+ assert.deepEqual(suggestion, {
+ name: `${account.name} <${account.email}>`,
+ value: {account, count: 1},
});
- test('getSuggestedAccounts is used', async () => {
- const suggestReviewerStub = stubRestApi(
- 'getChangeSuggestedReviewers'
- ).returns(Promise.resolve([]));
- const suggestAccountStub = stubRestApi('getSuggestedAccounts').returns(
- Promise.resolve([])
- );
+ const group = {name: 'test' as GroupName, id: '5' as GroupId};
+ suggestion = provider.makeSuggestionItem({group, count: 1});
+ assert.deepEqual(suggestion, {
+ name: `${group.name} (group)`,
+ value: {group, count: 1},
+ });
- await provider.getSuggestions('');
- assert.isFalse(suggestReviewerStub.called);
- assert.isTrue(suggestAccountStub.calledOnce);
- assert.isTrue(suggestAccountStub.calledWith('cansee:42 '));
+ suggestion = provider.makeSuggestionItem(account);
+ assert.deepEqual(suggestion, {
+ name: `${account.name} <${account.email}>`,
+ value: {account, count: 1},
+ });
+
+ suggestion = provider.makeSuggestionItem({account: {}, count: 1});
+ assert.deepEqual(suggestion, {
+ name: 'Name of user not set',
+ value: {account: {}, count: 1},
+ });
+
+ provider = new GrReviewerSuggestionsProvider(
+ getAppContext().restApiService,
+ SUGGESTIONS_PROVIDERS_USERS_TYPES.REVIEWER,
+ {
+ ...createServerInfo(),
+ user: {
+ anonymous_coward_name: 'Anonymous Coward Name',
+ },
+ },
+ true,
+ change._number
+ );
+
+ suggestion = provider.makeSuggestionItem({account: {}, count: 1});
+ assert.deepEqual(suggestion, {
+ name: 'Anonymous Coward Name',
+ value: {account: {}, count: 1},
+ });
+
+ account = {...createAccountDetailWithIdNameAndEmail(3), status: 'OOO'};
+
+ suggestion = provider.makeSuggestionItem({account, count: 1});
+ assert.deepEqual(suggestion, {
+ name: `${account.name} <${account.email}> (OOO)`,
+ value: {account, count: 1},
+ });
+
+ suggestion = provider.makeSuggestionItem(account);
+ assert.deepEqual(suggestion, {
+ name: `${account.name} <${account.email}> (OOO)`,
+ value: {account, count: 1},
+ });
+
+ account3.email = undefined;
+
+ suggestion = provider.makeSuggestionItem(account3);
+ assert.deepEqual(suggestion, {
+ name: account3.name,
+ value: {account: account3, count: 1},
});
});
});
diff --git a/polygerrit-ui/app/utils/async-util.ts b/polygerrit-ui/app/utils/async-util.ts
index 43fd6f5..981bcae 100644
--- a/polygerrit-ui/app/utils/async-util.ts
+++ b/polygerrit-ui/app/utils/async-util.ts
@@ -147,12 +147,18 @@
export const isFalse = (b: boolean) => b === false;
-// An equivalent to Promise.allSettled from ES2020.
-// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/allSettled
-// TODO: Migrate our tooling to ES2020 and remove this method.
export type PromiseResult<T> =
| {status: 'fulfilled'; value: T}
| {status: 'rejected'; reason: string};
+export function isFulfilled<T>(
+ promiseResult?: PromiseResult<T>
+): promiseResult is PromiseResult<T> & {status: 'fulfilled'} {
+ return promiseResult?.status === 'fulfilled';
+}
+
+// An equivalent to Promise.allSettled from ES2020.
+// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/allSettled
+// TODO: Migrate our tooling to ES2020 and remove this method.
export function allSettled<T>(
promises: Promise<T>[]
): Promise<PromiseResult<T>[]> {
diff --git a/polygerrit-ui/app/utils/common-util.ts b/polygerrit-ui/app/utils/common-util.ts
index 9e3bc74..6ccf770 100644
--- a/polygerrit-ui/app/utils/common-util.ts
+++ b/polygerrit-ui/app/utils/common-util.ts
@@ -156,3 +156,22 @@
export function unique<T>(item: T, index: number, array: T[]) {
return array.indexOf(item) === index;
}
+
+/**
+ * Returns the elements that are present in every sub-array. If a compareBy
+ * predicate is passed in, it will be used instead of strict equality.
+ */
+export function intersection<T>(
+ arrays: T[][],
+ compareBy: (t: T, u: T) => boolean = (t, u) => t === u
+): T[] {
+ // Array.prototype.reduce needs either an initialValue or a non-empty array.
+ // Since there is no good initialValue for intersecting (∅ ∩ X = ∅), the
+ // empty array must be checked separately.
+ if (arrays.length === 0) {
+ return [];
+ }
+ return arrays.reduce((result, array) =>
+ result.filter(t => array.find(u => compareBy(t, u)))
+ );
+}
diff --git a/polygerrit-ui/app/utils/common-util_test.ts b/polygerrit-ui/app/utils/common-util_test.ts
index 4156729..8cc523a 100644
--- a/polygerrit-ui/app/utils/common-util_test.ts
+++ b/polygerrit-ui/app/utils/common-util_test.ts
@@ -16,7 +16,12 @@
*/
import '../test/common-test-setup-karma';
-import {hasOwnProperty, areSetsEqual, containsAll} from './common-util';
+import {
+ hasOwnProperty,
+ areSetsEqual,
+ containsAll,
+ intersection,
+} from './common-util';
suite('common-util tests', () => {
suite('hasOwnProperty', () => {
@@ -68,4 +73,28 @@
assert.isFalse(containsAll(new Set([1, 2, 3, 4]), new Set([5])));
assert.isFalse(containsAll(new Set([1, 2, 3, 4]), new Set([1, 2, 3, 5])));
});
+
+ test('intersections', () => {
+ assert.sameDeepMembers(intersection([]), []);
+ assert.sameDeepMembers(intersection([[1, 2, 3]]), [1, 2, 3]);
+ assert.sameDeepMembers(
+ intersection([
+ [1, 2, 3],
+ [2, 3, 4],
+ [5, 3, 2],
+ ]),
+ [2, 3]
+ );
+
+ const foo1 = {value: 5};
+ const foo2 = {value: 5};
+
+ // these foo's will fail strict equality with each other, but a comparator
+ // can make them intersect.
+ assert.sameDeepMembers(intersection([[foo1], [foo2]]), []);
+ assert.sameDeepMembers(
+ intersection([[foo1], [foo2]], (a, b) => a.value === b.value),
+ [foo1]
+ );
+ });
});
