Hide service account reviewer updates in change log In the change log, reviewer updates from service accounts (e.g. bot accounts adding reviewers) are currently treated as important by default. This causes automated reviewer additions to appear in the main change log rather than being hidden behind the "Show all entries" toggle. Update `computeIsImportant` in `gr-messages-list` so that reviewer updates from service users are marked as unimportant by default. Release-Notes: skip Change-Id: I37fa7dd2e12dcd7a78e66fd323763013a8eea5ee
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 03c79a8..65fc242 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
@@ -22,6 +22,7 @@ VotingRangeInfo, } from '../../../types/common'; import {GrMessage, MessageAnchorTapDetail} from '../gr-message/gr-message'; +import {isServiceUser} from '../../../utils/account-util'; import {getVotingRange} from '../../../utils/label-util'; import { FormattedReviewerUpdateInfo, @@ -225,10 +226,28 @@ * Autogenerated messages are unimportant, if there is a message with the same * tag and a higher revision number. */ +function isReviewerUpdateMessage(message: CombinedMessage): boolean { + return ( + (message as FormattedReviewerUpdateInfo).type === 'REVIEWER_UPDATE' || + message.tag === MessageTag.TAG_REVIEWER_UPDATE + ); +} + function computeIsImportant( message: CombinedMessage, allMessages: CombinedMessage[] ) { + const author = message.author; + const realAuthor = + (message as ChangeMessageInfo).real_author ?? + (message as FormattedReviewerUpdateInfo).realAuthor; + if ( + (isServiceUser(author) || isServiceUser(realAuthor)) && + isReviewerUpdateMessage(message) + ) { + return false; + } + if (!message.tag) return true; const hasSameTag = (m: CombinedMessage) => m.tag === message.tag;
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 66a1951..8ec1bde 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
@@ -7,7 +7,7 @@ import '../../../test/common-test-setup'; import './gr-messages-list'; import {CombinedMessage, GrMessagesList, TEST_ONLY} from './gr-messages-list'; -import {MessageTag} from '../../../constants/constants'; +import {AccountTag, MessageTag} from '../../../constants/constants'; import { query, queryAll, @@ -59,7 +59,7 @@ }; }; -const randomMessage = function (params?: ChangeMessageInfo) { +const randomMessage = function (params?: Partial<ChangeMessageInfo>) { params = params || ({} as ChangeMessageInfo); const author1 = { _account_id: 1115495 as AccountId, @@ -488,6 +488,68 @@ assert.isFalse(TEST_ONLY.computeIsImportant(m3, [m1, m2, m3])); }); + test('isImportant service user reviewer update vs other messages', () => { + const reviewerUpdateFromBot = { + ...randomMessage(), + author: { + _account_id: 123 as AccountId, + tags: [AccountTag.SERVICE_USER], + }, + type: 'REVIEWER_UPDATE' as const, + tag: MessageTag.TAG_REVIEWER_UPDATE as ReviewInputTag, + }; + const reviewerUpdateWithRealAuthorBot = { + ...randomMessage(), + author: { + _account_id: 456 as AccountId, + }, + real_author: { + _account_id: 123 as AccountId, + tags: [AccountTag.SERVICE_USER], + }, + type: 'REVIEWER_UPDATE' as const, + tag: MessageTag.TAG_REVIEWER_UPDATE as ReviewInputTag, + }; + const formattedReviewerUpdateWithRealAuthorBot = { + author: { + _account_id: 456 as AccountId, + }, + realAuthor: { + _account_id: 123 as AccountId, + tags: [AccountTag.SERVICE_USER], + }, + date: '2020-01-01 00:00:00.000000000' as Timestamp, + type: 'REVIEWER_UPDATE' as const, + tag: MessageTag.TAG_REVIEWER_UPDATE as const, + updates: [], + }; + const commentFromBot = randomMessage({ + author: { + _account_id: 123 as AccountId, + tags: [AccountTag.SERVICE_USER], + }, + message: 'Build succeeded: 10 tests passed', + }); + assert.isFalse( + TEST_ONLY.computeIsImportant(reviewerUpdateFromBot, [ + reviewerUpdateFromBot, + ]) + ); + assert.isFalse( + TEST_ONLY.computeIsImportant(reviewerUpdateWithRealAuthorBot, [ + reviewerUpdateWithRealAuthorBot, + ]) + ); + assert.isFalse( + TEST_ONLY.computeIsImportant(formattedReviewerUpdateWithRealAuthorBot, [ + formattedReviewerUpdateWithRealAuthorBot, + ]) + ); + assert.isTrue( + TEST_ONLY.computeIsImportant(commentFromBot, [commentFromBot]) + ); + }); + test('isImportant is evaluated after tag update', async () => { const m1 = randomMessage({ ...randomMessage(),