Show mentioned by in the attention set reason
In case the user is in the attention set because someone replied
on the change and they are mentioned in a comment, then update
the reason to specify that they were mentioned in a comment.
Google-bug-id: b/245517947
Release-Notes: skip
Screenshot: https://imgur.com/a/uFz25bi
Change-Id: I2bc8669f8dfa42e95169f3a482cfd30240b0098c
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 37db6d9..9911a49 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
@@ -33,7 +33,11 @@
} from '../../../utils/account-util';
import {IronA11yAnnouncer} from '@polymer/iron-a11y-announcer/iron-a11y-announcer';
import {TargetElement} from '../../../api/plugin';
-import {FixIronA11yAnnouncer, ParsedChangeInfo} from '../../../types/types';
+import {
+ FixIronA11yAnnouncer,
+ notUndefined,
+ ParsedChangeInfo,
+} from '../../../types/types';
import {
AccountInfoInput,
AccountInput,
@@ -102,7 +106,10 @@
import {debounce, DelayedTask} from '../../../utils/async-util';
import {StorageLocation} from '../../../services/storage/gr-storage';
import {Interaction, Timing} from '../../../constants/reporting';
-import {getReplyByReason} from '../../../utils/attention-set-util';
+import {
+ getMentionedReason,
+ getReplyByReason,
+} from '../../../utils/attention-set-util';
import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api';
import {resolve} from '../../../models/dependency';
import {changeModelToken} from '../../../models/change/change-model';
@@ -396,6 +403,8 @@
private readonly getConfigModel = resolve(this, configModelToken);
+ private readonly accountsModel = getAppContext().accountsModel;
+
private mentionedUsersInUnresolvedDrafts: AccountInfo[] = [];
private latestPatchNum?: PatchSetNumber;
@@ -1523,10 +1532,35 @@
reviewInput.ignore_automatic_attention_set_rules = true;
reviewInput.add_to_attention_set = [];
- for (const user of this.newAttentionSet) {
- if (!this.currentAttentionSet.has(user)) {
- reviewInput.add_to_attention_set.push({user, reason});
+ const allAccounts = this.allAccounts();
+
+ const newAttentionSetAdditions: AccountInfo[] = Array.from(
+ this.newAttentionSet
+ )
+ .filter(user => !this.currentAttentionSet.has(user))
+ .map(user => allAccounts.find(a => getUserId(a) === user))
+ .filter(notUndefined);
+
+ const newAttentionSetUsers = (
+ await Promise.all(
+ newAttentionSetAdditions.map(a => this.accountsModel.fillDetails(a))
+ )
+ ).filter(notUndefined);
+
+ for (const user of newAttentionSetUsers) {
+ let reason;
+ if (this.flagsService.isEnabled(KnownExperimentId.MENTION_USERS)) {
+ reason =
+ getMentionedReason(
+ this.draftCommentThreads,
+ this.account,
+ user,
+ this.serverConfig
+ ) ?? '';
+ } else {
+ reason = getReplyByReason(this.account, this.serverConfig);
}
+ reviewInput.add_to_attention_set.push({user: getUserId(user), reason});
}
reviewInput.remove_from_attention_set = [];
for (const user of this.currentAttentionSet) {
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 b38dc8c..b04288b 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
@@ -397,6 +397,7 @@
element.account = {_account_id: 123 as AccountId};
element.newAttentionSet = new Set([314 as AccountId]);
+ element.uploader = createAccountWithId(314);
const saveReviewPromise = interceptSaveReview();
queryAndAssert<GrButton>(element, '.send').click();
@@ -426,6 +427,7 @@
await element.updateComplete;
element.account = {};
+ element.uploader = createAccountWithId(314);
element.newAttentionSet = new Set([314 as AccountId]);
const saveReviewPromise = interceptSaveReview();
@@ -1146,10 +1148,7 @@
const review = await saveReviewPromise;
await element.updateComplete;
- assert.isFalse(
- element.disabled,
- 'Element should be enabled when done sending reply.'
- );
+ await waitUntil(() => element.disabled === false);
assert.equal(element.patchsetLevelDraftMessage.length, 0);
assert.deepEqual(review, {
drafts: 'PUBLISH_ALL_REVISIONS',
diff --git a/polygerrit-ui/app/utils/attention-set-util.ts b/polygerrit-ui/app/utils/attention-set-util.ts
index 6d84118..77834bd 100644
--- a/polygerrit-ui/app/utils/attention-set-util.ts
+++ b/polygerrit-ui/app/utils/attention-set-util.ts
@@ -11,6 +11,7 @@
isServiceUser,
replaceTemplates,
} from './account-util';
+import {CommentThread, isMentionedThread, isUnresolved} from './comment-util';
import {hasOwnProperty} from './common-util';
export function canHaveAttention(account?: AccountInfo): boolean {
@@ -48,6 +49,21 @@
);
}
+export function getMentionedReason(
+ threads: CommentThread[],
+ account?: AccountInfo,
+ mentionedAccount?: AccountInfo,
+ config?: ServerInfo
+) {
+ const mentionedThreads = threads
+ .filter(isUnresolved)
+ .filter(t => isMentionedThread(t, mentionedAccount));
+ if (mentionedThreads.length > 0) {
+ return `${getAccountTemplate(account, config)} mentioned you in a comment`;
+ }
+ return getReplyByReason(account, config);
+}
+
export function getAddedByReason(account?: AccountInfo, config?: ServerInfo) {
return `Added by ${getAccountTemplate(
account,
diff --git a/polygerrit-ui/app/utils/attention-set-util_test.ts b/polygerrit-ui/app/utils/attention-set-util_test.ts
index 2b302c7..1634b23 100644
--- a/polygerrit-ui/app/utils/attention-set-util_test.ts
+++ b/polygerrit-ui/app/utils/attention-set-util_test.ts
@@ -4,7 +4,13 @@
* SPDX-License-Identifier: Apache-2.0
*/
import '../test/common-test-setup-karma';
-import {createChange, createServerInfo} from '../test/test-data-generators';
+import {
+ createAccountDetailWithIdNameAndEmail,
+ createChange,
+ createComment,
+ createCommentThread,
+ createServerInfo,
+} from '../test/test-data-generators';
import {
AccountId,
AccountInfo,
@@ -12,7 +18,11 @@
EmailAddress,
ServerInfo,
} from '../types/common';
-import {getReason, hasAttention} from './attention-set-util';
+import {
+ getMentionedReason,
+ getReason,
+ hasAttention,
+} from './attention-set-util';
import {DefaultDisplayNameConfig} from '../api/rest-api';
import {AccountsVisibility} from '../constants/constants';
import {assert} from '@open-wc/testing';
@@ -31,6 +41,20 @@
_account_id: 31415926536 as AccountId,
};
+const MENTION_ACCOUNT: AccountInfo = {
+ email: 'mention@gmail.com' as EmailAddress,
+ username: 'mention',
+ name: 'Mention User',
+ _account_id: 31415926537 as AccountId,
+};
+
+const MENTION_ACCOUNT_2: AccountInfo = {
+ email: 'mention2@gmail.com' as EmailAddress,
+ username: 'mention2',
+ name: 'Mention2 User',
+ _account_id: 31415926538 as AccountId,
+};
+
const change: ChangeInfo = {
...createChange(),
attention_set: {
@@ -43,6 +67,16 @@
reason: 'Added by <GERRIT_ACCOUNT_31415926535>',
reason_account: KERMIT,
},
+ '31415926537': {
+ account: MENTION_ACCOUNT,
+ reason: '<GERRIT_ACCOUNT_31415926535> replied on the change',
+ reason_account: KERMIT,
+ },
+ '31415926538': {
+ account: MENTION_ACCOUNT_2,
+ reason: 'Bot voted negatively on the change',
+ reason_account: KERMIT,
+ },
},
};
@@ -66,4 +100,54 @@
assert.equal(getReason(config, KERMIT, change), 'a good reason');
assert.equal(getReason(config, OTHER_ACCOUNT, change), 'Added by kermit');
});
+
+ test('getMentionReason', () => {
+ let comment = {
+ ...createComment(),
+ message: `hey @${MENTION_ACCOUNT.email} take a look at this`,
+ unresolved: true,
+ author: {
+ ...createAccountDetailWithIdNameAndEmail(1),
+ },
+ };
+
+ assert.equal(
+ getMentionedReason(
+ [createCommentThread([comment])],
+ KERMIT,
+ KERMIT,
+ config
+ ),
+ '<GERRIT_ACCOUNT_31415926535> replied on the change'
+ );
+
+ assert.equal(
+ getMentionedReason(
+ [createCommentThread([comment])],
+ KERMIT,
+ MENTION_ACCOUNT,
+ config
+ ),
+ '<GERRIT_ACCOUNT_31415926535> mentioned you in a comment'
+ );
+
+ // resolved mention hence does not change reason
+ comment = {
+ ...createComment(),
+ message: `hey @${MENTION_ACCOUNT.email} take a look at this`,
+ unresolved: false,
+ author: {
+ ...createAccountDetailWithIdNameAndEmail(1),
+ },
+ };
+ assert.equal(
+ getMentionedReason(
+ [createCommentThread([comment])],
+ KERMIT,
+ MENTION_ACCOUNT,
+ config
+ ),
+ '<GERRIT_ACCOUNT_31415926535> replied on the change'
+ );
+ });
});
