Change attention set logic to take unresolved state into account
Change-Id: Iea08d3460c949c945a790c3316caf95e6fefe6b1
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 0732c0f..a2a75d0 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
@@ -102,12 +102,14 @@
assertNever,
containsAll,
} from '../../../utils/common-util';
-import {CommentThread, isDraft} from '../../diff/gr-comment-api/gr-comment-api';
+import {CommentThread} from '../../../utils/comment-util';
import {GrTextarea} from '../../shared/gr-textarea/gr-textarea';
import {GrAccountChip} from '../../shared/gr-account-chip/gr-account-chip';
import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
import {GrStorage, StorageLocation} from '../../shared/gr-storage/gr-storage';
import {isAttentionSetEnabled} from '../../../utils/attention-set-util';
+import {CODE_REVIEW, getMaxAccounts} from '../../../utils/label-util';
+import {isUnresolved} from '../../../utils/comment-util';
const STORAGE_DEBOUNCE_INTERVAL_MS = 400;
@@ -937,7 +939,8 @@
'_reviewers.*',
'_ccs.*',
'change',
- 'draftCommentThreads'
+ 'draftCommentThreads',
+ '_includeComments'
)
_computeNewAttention(
currentUser?: AccountInfo,
@@ -947,17 +950,22 @@
>,
_?: PolymerDeepPropertyChange<AccountInfoInput[], AccountInfoInput[]>,
change?: ChangeInfo,
- draftCommentThreads?: CommentThread[]
+ draftCommentThreads?: CommentThread[],
+ includeComments?: boolean
) {
if (
currentUser === undefined ||
currentUser._account_id === undefined ||
reviewers === undefined ||
change === undefined ||
- draftCommentThreads === undefined
+ draftCommentThreads === undefined ||
+ includeComments === undefined
) {
return;
}
+ // The draft comments are only relevant for the attention set as long as the
+ // user actually plans to publish their drafts.
+ draftCommentThreads = includeComments ? draftCommentThreads : [];
this._currentAttentionSet = new Set(
Object.keys(change.attention_set || {}).map(
id => parseInt(id) as AccountId
@@ -993,8 +1001,9 @@
}
} else {
// The only reason for adding someone to the attention set for merged or
- // abandoned changes is that someone adds a new comment thread.
- if (change.owner && this._containsNewCommentThread(draftCommentThreads)) {
+ // abandoned changes is that someone makes a comment thread unresolved.
+ const hasUnresolvedDraft = draftCommentThreads.some(isUnresolved);
+ if (change.owner && hasUnresolvedDraft) {
// A change owner must have an _account_id.
newAttention.add(change.owner._account_id!);
}
@@ -1033,25 +1042,23 @@
}
_computeCommentAccounts(threads: CommentThread[]) {
+ const crLabel = this.change?.labels?.[CODE_REVIEW];
+ const maxCrVoteAccountIds = getMaxAccounts(crLabel).map(a => a._account_id);
const accountIds = new Set<AccountId>();
threads.forEach(thread => {
+ const unresolved = isUnresolved(thread);
thread.comments.forEach(comment => {
if (comment.author) {
// A comment author must have an _account_id.
- accountIds.add(comment.author._account_id!);
+ const authorId = comment.author._account_id!;
+ const hasGivenMaxReviewVote = maxCrVoteAccountIds.includes(authorId);
+ if (unresolved || !hasGivenMaxReviewVote) accountIds.add(authorId);
}
});
});
return accountIds;
}
- _containsNewCommentThread(threads: CommentThread[]) {
- return threads.some(
- thread =>
- !!thread.comments && !!thread.comments[0] && isDraft(thread.comments[0])
- );
- }
-
_isNewAttentionEmpty(
config?: ServerInfo,
currentAttentionSet?: Set<AccountId>,
