Refine logic for when to show "outdated votes" messages
In change 344058 the logic was changed such that *all* patchset uploads
with outdated votes would be shown by default. This has led to an
unexpected amount of clutter in the Change Log.
Do not show such messages for WIP patchset uploads.
Only show the latest of such patchset uploads with outdated votes.
Change 344058 is completely reverted, because it seemed more appropriate
to implement the logic in `computeTag()` than in `computeIsImportant()`.
Release-Notes: skip
Google-Bug-Id: b/241597611
Change-Id: Id6eeecf1d0e50e6baf39438233740f1480fa611c
diff --git a/polygerrit-ui/app/constants/constants.ts b/polygerrit-ui/app/constants/constants.ts
index c8ce09e..503c35a 100644
--- a/polygerrit-ui/app/constants/constants.ts
+++ b/polygerrit-ui/app/constants/constants.ts
@@ -68,6 +68,7 @@
export enum MessageTag {
TAG_DELETE_REVIEWER = 'autogenerated:gerrit:deleteReviewer',
TAG_NEW_PATCHSET = 'autogenerated:gerrit:newPatchSet',
+ TAG_NEW_PATCHSET_OUTDATED_VOTES = 'autogenerated:gerrit:newPatchSetOutdatedVotes',
TAG_NEW_WIP_PATCHSET = 'autogenerated:gerrit:newWipPatchSet',
TAG_REVIEWER_UPDATE = 'autogenerated:gerrit:reviewerUpdate',
TAG_SET_PRIVATE = 'autogenerated:gerrit:setPrivate',
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 93fc6e7..da775f6 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
@@ -108,6 +108,9 @@
*
* 3. Everything beyond the ~ character is cut off from the tag. That gives
* tools control over which messages will be hidden.
+ *
+ * 4. (Non-WIP) patchset uploads get a separate tag when they invalidate any
+ * votes.
*/
function computeTag(message: CombinedMessage) {
if (!message.tag) {
@@ -120,6 +123,15 @@
return hasRobotComments ? 'autogenerated:has-robot-comments' : undefined;
}
+ if (message.tag === MessageTag.TAG_NEW_PATCHSET) {
+ const hasOutdatedVotes =
+ isChangeMessageInfo(message) &&
+ message.message.indexOf('\nOutdated Votes:\n') !== -1;
+
+ return hasOutdatedVotes
+ ? MessageTag.TAG_NEW_PATCHSET_OUTDATED_VOTES
+ : MessageTag.TAG_NEW_PATCHSET;
+ }
if (message.tag === MessageTag.TAG_NEW_WIP_PATCHSET) {
return MessageTag.TAG_NEW_PATCHSET;
}
@@ -218,9 +230,6 @@
*
* Autogenerated messages are unimportant, if there is a message with the same
* tag and a higher revision number.
- *
- * New Patchset messages that have outdated Votes are an exception to the above
- * and are important.
*/
function computeIsImportant(
message: CombinedMessage,
@@ -228,14 +237,6 @@
) {
if (!message.tag) return true;
- if (
- message.tag === MessageTag.TAG_NEW_PATCHSET &&
- isChangeMessageInfo(message) &&
- message.message.indexOf('\nOutdated Votes:\n') !== -1
- ) {
- return true;
- }
-
const hasSameTag = (m: CombinedMessage) => m.tag === message.tag;
const revNumber = message._revision_number || 0;
const hasHigherRevisionNumber = (m: CombinedMessage) =>
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 118381a..a022db6 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
@@ -343,6 +343,19 @@
assert.equal(TEST_ONLY.computeTag(m), MessageTag.TAG_NEW_PATCHSET);
});
+ test('updateTag for outdated votes', () => {
+ const m = randomMessage();
+ m.tag = MessageTag.TAG_NEW_PATCHSET as ReviewInputTag;
+ m.message = '\nUploaded patch set 35.\n\nOutdated Votes:\n';
+ assert.equal(
+ TEST_ONLY.computeTag(m),
+ MessageTag.TAG_NEW_PATCHSET_OUTDATED_VOTES
+ );
+
+ m.tag = MessageTag.TAG_NEW_WIP_PATCHSET as ReviewInputTag;
+ assert.equal(TEST_ONLY.computeTag(m), MessageTag.TAG_NEW_PATCHSET);
+ });
+
test('updateTag remove postfix', () => {
const m = randomMessage();
m.tag = 'something~withpostfix' as ReviewInputTag;
@@ -503,35 +516,6 @@
});
});
- suite('gr-messages-list marks outdated votes as important', () => {
- setup(async () => {
- stubRestApi('getLoggedIn').returns(Promise.resolve(false));
- stubRestApi('getDiffComments').returns(Promise.resolve({}));
- stubRestApi('getDiffRobotComments').returns(Promise.resolve({}));
- stubRestApi('getDiffDrafts').returns(Promise.resolve({}));
-
- messages = [
- randomMessage({
- ...randomMessage(),
- tag: MessageTag.TAG_NEW_PATCHSET as ReviewInputTag,
- message:
- '\nUploaded patch set 35.\n\nInitial upload\n\nOutdated Votes:\n',
- }),
- ];
-
- element = await fixture<GrMessagesList>(
- html`<gr-messages-list></gr-messages-list>`
- );
- element.messages = messages;
- await element.updateComplete;
- });
-
- test('outdated votes message is shown.', () => {
- const displayedMsgs = queryAll<GrMessage>(element, 'gr-message');
- assert.equal(displayedMsgs.length, 1);
- });
- });
-
suite('gr-messages-list automate tests', () => {
let element: GrMessagesList;
let messages: ChangeMessageInfo[];
