Merge changes I5f12df88,Ib2b1b6be
* changes:
Rename gr-messages-list-experimental to gr-messages-list
Remove the cleaner change log experiment
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index 072708e..2cd03f5 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -40,7 +40,6 @@
import '../gr-file-list/gr-file-list.js';
import '../gr-included-in-dialog/gr-included-in-dialog.js';
import '../gr-messages-list/gr-messages-list.js';
-import '../gr-messages-list/gr-messages-list-experimental.js';
import '../gr-related-changes-list/gr-related-changes-list.js';
import '../../diff/gr-apply-fix-dialog/gr-apply-fix-dialog.js';
import '../gr-reply-dialog/gr-reply-dialog.js';
@@ -66,7 +65,6 @@
import {PrimaryTab, SecondaryTab} from '../../../constants/constants.js';
import {NO_ROBOT_COMMENTS_THREADS_MSG} from '../../../constants/messages.js';
import {appContext} from '../../../services/app-context.js';
-import {ExperimentIds} from '../../../services/flags.js';
import {ChangeStatus} from '../../../constants/constants.js';
const CHANGE_ID_ERROR = {
@@ -453,7 +451,6 @@
constructor() {
super();
- this.flagsService = appContext.flagsService;
this.reporting = appContext.reportingService;
}
@@ -544,14 +541,8 @@
}
}
- _isChangeLogExperimentEnabled() {
- return this.flagsService.isEnabled(ExperimentIds.CLEANER_CHANGELOG);
- }
-
get messagesList() {
- const tagName = this._isChangeLogExperimentEnabled()
- ? 'gr-messages-list-experimental' : 'gr-messages-list';
- return this.shadowRoot.querySelector(tagName);
+ return this.shadowRoot.querySelector('gr-messages-list');
}
get threadList() {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js
index c9fdb78..5b89c15 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js
@@ -213,8 +213,7 @@
--paper-tab-ink: var(--link-color);
}
gr-thread-list,
- gr-messages-list,
- gr-messages-list-experimental {
+ gr-messages-list {
display: block;
}
gr-thread-list {
@@ -701,35 +700,19 @@
is="dom-if"
if="[[_isTabActive(_constants.SecondaryTab.CHANGE_LOG, _activeTabs)]]"
>
- <template is="dom-if" if="[[!_isChangeLogExperimentEnabled()]]">
- <gr-messages-list
- class="hideOnMobileOverlay"
- change-num="[[_changeNum]]"
- labels="[[_change.labels]]"
- messages="[[_change.messages]]"
- reviewer-updates="[[_change.reviewer_updates]]"
- change-comments="[[_changeComments]]"
- project-name="[[_change.project]]"
- show-reply-buttons="[[_loggedIn]]"
- on-message-anchor-tap="_handleMessageAnchorTap"
- on-reply="_handleMessageReply"
- ></gr-messages-list>
- </template>
- <template is="dom-if" if="[[_isChangeLogExperimentEnabled()]]">
- <gr-messages-list-experimental
- class="hideOnMobileOverlay"
- change="[[_change]]"
- change-num="[[_changeNum]]"
- labels="[[_change.labels]]"
- messages="[[_change.messages]]"
- reviewer-updates="[[_change.reviewer_updates]]"
- change-comments="[[_changeComments]]"
- project-name="[[_change.project]]"
- show-reply-buttons="[[_loggedIn]]"
- on-message-anchor-tap="_handleMessageAnchorTap"
- on-reply="_handleMessageReply"
- ></gr-messages-list-experimental>
- </template>
+ <gr-messages-list
+ class="hideOnMobileOverlay"
+ change="[[_change]]"
+ change-num="[[_changeNum]]"
+ labels="[[_change.labels]]"
+ messages="[[_change.messages]]"
+ reviewer-updates="[[_change.reviewer_updates]]"
+ change-comments="[[_changeComments]]"
+ project-name="[[_change.project]]"
+ show-reply-buttons="[[_loggedIn]]"
+ on-message-anchor-tap="_handleMessageAnchorTap"
+ on-reply="_handleMessageReply"
+ ></gr-messages-list>
</template>
</section>
</div>
diff --git a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js
deleted file mode 100644
index 3554dff..0000000
--- a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js
+++ /dev/null
@@ -1,108 +0,0 @@
-/**
- * @license
- * Copyright (C) 2015 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-/*
- The custom CSS property `--gr-formatted-text-prose-max-width` controls the max
- width of formatted text blocks that are not code.
-*/
-/*
- FIXME(polymer-modulizer): the above comments were extracted
- from HTML and may be out of place here. Review them and
- then delete this comment!
-*/
-import '../../shared/gr-formatted-text/gr-formatted-text.js';
-import '../../../styles/shared-styles.js';
-import {mixinBehaviors} from '@polymer/polymer/lib/legacy/class.js';
-import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners.js';
-import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
-import {PolymerElement} from '@polymer/polymer/polymer-element.js';
-import {htmlTemplate} from './gr-comment-list_html.js';
-import {BaseUrlBehavior} from '../../../behaviors/base-url-behavior/base-url-behavior.js';
-import {PathListBehavior} from '../../../behaviors/gr-path-list-behavior/gr-path-list-behavior.js';
-import {URLEncodingBehavior} from '../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.js';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
-
-/**
- * @extends PolymerElement
- */
-class GrCommentList extends mixinBehaviors( [
- BaseUrlBehavior,
- PathListBehavior,
- URLEncodingBehavior,
-], GestureEventListeners(
- LegacyElementMixin(
- PolymerElement))) {
- static get template() { return htmlTemplate; }
-
- static get is() { return 'gr-comment-list'; }
-
- static get properties() {
- return {
- changeNum: Number,
- comments: Object,
- patchNum: Number,
- projectName: String,
- /** @type {?} */
- projectConfig: Object,
- };
- }
-
- _computeFilesFromComments(comments) {
- const arr = Object.keys(comments || {});
- return arr.sort(this.specialFilePathCompare);
- }
-
- _isOnParent(comment) {
- return comment.side === 'PARENT';
- }
-
- _computeDiffURL(filePath, changeNum, allComments) {
- if ([filePath, changeNum, allComments].includes(undefined)) {
- return;
- }
- const fileComments = this._computeCommentsForFile(allComments, filePath);
- // This can happen for files that don't exist anymore in the current ps.
- if (fileComments.length === 0) return;
- return GerritNav.getUrlForDiffById(changeNum, this.projectName,
- filePath, fileComments[0].patch_set);
- }
-
- _computeDiffLineURL(filePath, changeNum, patchNum, comment) {
- const basePatchNum = comment.hasOwnProperty('parent') ?
- -comment.parent : null;
- return GerritNav.getUrlForDiffById(changeNum, this.projectName,
- filePath, patchNum, basePatchNum, comment.line,
- this._isOnParent(comment));
- }
-
- _computeCommentsForFile(comments, filePath) {
- // Changes are not picked up by the dom-repeat due to the array instance
- // identity not changing even when it has elements added/removed from it.
- return (comments[filePath] || []).slice();
- }
-
- _computePatchDisplayName(comment) {
- if (this._isOnParent(comment)) {
- return 'Base, ';
- }
- if (comment.patch_set != this.patchNum) {
- return `PS${comment.patch_set}, `;
- }
- return '';
- }
-}
-
-customElements.define(GrCommentList.is, GrCommentList);
diff --git a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_html.js b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_html.js
deleted file mode 100644
index 2811828..0000000
--- a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_html.js
+++ /dev/null
@@ -1,103 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-import {html} from '@polymer/polymer/lib/utils/html-tag.js';
-
-export const htmlTemplate = html`
- <style include="shared-styles">
- :host {
- display: block;
- word-wrap: break-word;
- }
- .file {
- padding: var(--spacing-s) 0;
- }
- .container {
- display: flex;
- padding: var(--spacing-s) 0;
- }
- .lineNum {
- margin-right: var(--spacing-s);
- min-width: 135px;
- text-align: right;
- }
- .patchset-level-comment-text {
- margin-right: var(--spacing-m);
- }
- .message {
- flex: 1;
- --gr-formatted-text-prose-max-width: 80ch;
- }
- @media screen and (max-width: 50em) {
- .container {
- flex-direction: column;
- }
- .lineNum {
- margin-right: 0;
- min-width: initial;
- text-align: left;
- }
- }
- </style>
- <template
- is="dom-repeat"
- items="[[_computeFilesFromComments(comments)]]"
- as="file"
- >
- <div class="file">
- <template is="dom-if" if="[[!shouldHideFile(file)]]">
- <a
- class="fileLink"
- href="[[_computeDiffURL(file, changeNum, comments)]]"
- >[[computeDisplayPath(file)]]
- </a>
- </template>
- </div>
- <template
- is="dom-repeat"
- items="[[_computeCommentsForFile(comments, file)]]"
- as="comment"
- >
- <div class="container">
- <template is="dom-if" if="[[shouldHideFile(file)]]">
- <span class="patchset-level-comment-text">
- Patchset Comment:
- </span>
- </template>
- <template is="dom-if" if="[[!shouldHideFile(file)]]">
- <a
- class="lineNum"
- href$="[[_computeDiffLineURL(file, changeNum, comment.patch_set, comment)]]"
- >
- <span hidden$="[[!comment.line]]">
- <span>[[_computePatchDisplayName(comment)]]</span>
- Line <span>[[comment.line]]</span>
- </span>
- <span hidden$="[[comment.line]]">
- File comment:
- </span>
- </a>
- </template>
- <gr-formatted-text
- class="message"
- no-trailing-margin=""
- content="[[comment.message]]"
- config="[[projectConfig.commentlinks]]"
- ></gr-formatted-text>
- </div>
- </template>
- </template>
-`;
diff --git a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_test.js b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_test.js
deleted file mode 100644
index 704d83f..0000000
--- a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_test.js
+++ /dev/null
@@ -1,114 +0,0 @@
-/**
- * @license
- * Copyright (C) 2016 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import '../../../test/common-test-setup-karma.js';
-import './gr-comment-list.js';
-import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
-
-const basicFixture = fixtureFromElement('gr-comment-list');
-
-suite('gr-comment-list tests', () => {
- let element;
-
- setup(() => {
- element = basicFixture.instantiate();
-
- sinon.stub(GerritNav, 'mapCommentlinks').callsFake( x => x);
- });
-
- test('_computeFilesFromComments w/ special file path sorting', () => {
- const comments = {
- 'file_b.html': [],
- 'file_c.css': [],
- 'file_a.js': [],
- 'test.cc': [],
- 'test.h': [],
- };
- const expected = [
- 'file_a.js',
- 'file_b.html',
- 'file_c.css',
- 'test.h',
- 'test.cc',
- ];
- const actual = element._computeFilesFromComments(comments);
- assert.deepEqual(actual, expected);
-
- assert.deepEqual(element._computeFilesFromComments(null), []);
- });
-
- test('_computePatchDisplayName', () => {
- const comment = {line: 123, side: 'REVISION', patch_set: 10};
-
- element.patchNum = 10;
- assert.equal(element._computePatchDisplayName(comment), '');
-
- element.patchNum = 9;
- assert.equal(element._computePatchDisplayName(comment), 'PS10, ');
-
- comment.side = 'PARENT';
- assert.equal(element._computePatchDisplayName(comment), 'Base, ');
- });
-
- test('config commentlinks propagate to formatted text', () => {
- element.comments = {
- 'test.h': [{
- author: {name: 'foo'},
- patch_set: 4,
- line: 10,
- updated: '2017-10-30 20:48:40.000000000',
- message: 'Ideadbeefdeadbeef',
- unresolved: true,
- }],
- };
- element.projectConfig = {
- commentlinks: {foo: {link: '#/q/$1', match: '(I[0-9a-f]{8,40})'}},
- };
- flushAsynchronousOperations();
- const formattedText = dom(element.root).querySelector(
- 'gr-formatted-text.message');
- assert.isOk(formattedText.config);
- assert.deepEqual(formattedText.config,
- element.projectConfig.commentlinks);
- });
-
- test('_computeDiffLineURL', () => {
- const getUrlStub = sinon.stub(GerritNav, 'getUrlForDiffById');
- element.projectName = 'proj';
- element.changeNum = 123;
-
- const comment = {line: 456};
- element._computeDiffLineURL('foo.cc', 123, 4, comment);
- assert.isTrue(getUrlStub.calledOnce);
- assert.deepEqual(getUrlStub.lastCall.args,
- [123, 'proj', 'foo.cc', 4, null, 456, false]);
-
- comment.side = 'PARENT';
- element._computeDiffLineURL('foo.cc', 123, 4, comment);
- assert.isTrue(getUrlStub.calledTwice);
- assert.deepEqual(getUrlStub.lastCall.args,
- [123, 'proj', 'foo.cc', 4, null, 456, true]);
-
- comment.parent = 12;
- element._computeDiffLineURL('foo.cc', 123, 4, comment);
- assert.isTrue(getUrlStub.calledThrice);
- assert.deepEqual(getUrlStub.lastCall.args,
- [123, 'proj', 'foo.cc', 4, -12, 456, true]);
- });
-});
-
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.js b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
index 7163952..0da687f 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.js
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
@@ -24,9 +24,6 @@
import '../../shared/gr-rest-api-interface/gr-rest-api-interface.js';
import '../../../styles/shared-styles.js';
import '../../../styles/gr-voting-styles.js';
-import '../gr-comment-list/gr-comment-list.js';
-import {appContext} from '../../../services/app-context.js';
-import {ExperimentIds} from '../../../services/flags.js';
import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners.js';
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
import {PolymerElement} from '@polymer/polymer/polymer-element.js';
@@ -136,8 +133,7 @@
},
_commentCountText: {
type: Number,
- computed: '_computeCommentCountText(comments,'
- + ' message.commentThreads.length, _isCleanerLogExperimentEnabled)',
+ computed: '_computeCommentCountText(message.commentThreads.length)',
},
_loggedIn: {
type: Boolean,
@@ -151,7 +147,6 @@
type: Boolean,
value: false,
},
- _isCleanerLogExperimentEnabled: Boolean,
};
}
@@ -163,7 +158,6 @@
constructor() {
super();
- this.flagsService = appContext.flagsService;
}
/** @override */
@@ -176,8 +170,6 @@
/** @override */
ready() {
super.ready();
- this._isCleanerLogExperimentEnabled = this.flagsService
- .isEnabled(ExperimentIds.CLEANER_CHANGELOG);
this.$.restAPI.getConfig().then(config => {
this.config = config;
});
@@ -197,33 +189,13 @@
}
}
- _computeCommentCountText(
- comments, threadsLength, isCleanerLogExperimentEnabled) {
- // TODO(taoalpha): clean up after cleaner-changelog experiment launched
- if (isCleanerLogExperimentEnabled) {
- if (threadsLength === 0) {
- return undefined;
- } else if (threadsLength === 1) {
- return '1 comment';
- } else {
- return `${threadsLength} comments`;
- }
+ _computeCommentCountText(threadsLength) {
+ if (threadsLength === 0) {
+ return undefined;
+ } else if (threadsLength === 1) {
+ return '1 comment';
} else {
- if (!comments) return undefined;
- let count = 0;
- for (const file in comments) {
- if (comments.hasOwnProperty(file)) {
- const commentArray = comments[file] || [];
- count += commentArray.length;
- }
- }
- if (count === 0) {
- return undefined;
- } else if (count === 1) {
- return '1 comment';
- } else {
- return `${count} comments`;
- }
+ return `${threadsLength} comments`;
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.js b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.js
index de4a72a..af57782 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.js
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.js
@@ -65,7 +65,6 @@
gr-button {
margin: 0 -4px;
}
- .collapsed gr-comment-list,
.collapsed gr-thread-list,
.collapsed .replyBtn,
.collapsed .deleteBtn,
@@ -246,27 +245,16 @@
</gr-button>
</div>
</template>
- <template is="dom-if" if="[[!_isCleanerLogExperimentEnabled]]">
- <gr-comment-list
- comments="[[comments]]"
- change-num="[[changeNum]]"
- patch-num="[[message._revision_number]]"
- project-name="[[projectName]]"
- project-config="[[_projectConfig]]"
- ></gr-comment-list>
- </template>
- <template is="dom-if" if="[[_isCleanerLogExperimentEnabled]]">
- <gr-thread-list
- change="[[change]]"
- hidden$="[[!message.commentThreads.length]]"
- threads="[[message.commentThreads]]"
- change-num="[[changeNum]]"
- logged-in="[[_loggedIn]]"
- hide-toggle-buttons
- on-thread-list-modified="_onThreadListModified"
- >
- </gr-thread-list>
- </template>
+ <gr-thread-list
+ change="[[change]]"
+ hidden$="[[!message.commentThreads.length]]"
+ threads="[[message.commentThreads]]"
+ change-num="[[changeNum]]"
+ logged-in="[[_loggedIn]]"
+ hide-toggle-buttons
+ on-thread-list-modified="_onThreadListModified"
+ >
+ </gr-thread-list>
</template>
</div>
</template>
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental.js
deleted file mode 100644
index ee5f0b9..0000000
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental.js
+++ /dev/null
@@ -1,433 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-import '@polymer/paper-toggle-button/paper-toggle-button.js';
-import '../../shared/gr-button/gr-button.js';
-import '../../shared/gr-icons/gr-icons.js';
-import '../gr-message/gr-message.js';
-import '../../../styles/shared-styles.js';
-import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
-import {mixinBehaviors} from '@polymer/polymer/lib/legacy/class.js';
-import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners.js';
-import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
-import {PolymerElement} from '@polymer/polymer/polymer-element.js';
-import {htmlTemplate} from './gr-messages-list-experimental_html.js';
-import {KeyboardShortcutBehavior} from '../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.js';
-import {parseDate} from '../../../utils/date-util.js';
-import {MessageTag} from '../../../constants/constants.js';
-import {appContext} from '../../../services/app-context.js';
-
-/**
- * The content of the enum is also used in the UI for the button text.
- *
- * @enum {string}
- */
-const ExpandAllState = {
- EXPAND_ALL: 'Expand All',
- COLLAPSE_ALL: 'Collapse All',
-};
-
-/**
- * Computes message author's comments for this change message. The backend
- * sets comment.change_message_id for matching, so this computation is fairly
- * straightforward.
- */
-function computeThreads(message, allMessages, changeComments) {
- if ([message, allMessages, changeComments].includes(undefined)) {
- return [];
- }
- if (message._index === undefined) {
- return [];
- }
-
- return changeComments.getAllThreadsForChange().filter(
- thread => thread.comments.map(comment => {
- // collapse all by default
- comment.collapsed = true;
- return comment;
- }).some(comment => {
- const condition = comment.change_message_id === message.id;
- // Since getAllThreadsForChange() always returns a new copy of
- // all comments we can modify them here without worrying about
- // polluting other threads.
- comment.collapsed = !condition;
- return condition;
- })
- );
-}
-
-/**
- * If messages have the same tag, then that influences grouping and whether
- * a message is initally hidden or not, see isImportant(). So we are applying
- * some "magic" rules here in order to hide exactly the right messages.
- *
- * 1. If a message does not have a tag, but is associated with robot comments,
- * then it gets a tag.
- *
- * 2. Use the same tag for some of Gerrit's standard events, if they should be
- * considered one group, e.g. normal and wip patchset uploads.
- *
- * 3. Everything beyond the ~ character is cut off from the tag. That gives
- * tools control over which messages will be hidden.
- */
-function computeTag(message) {
- if (!message.tag) {
- const threads = message.commentThreads || [];
- const comments = threads.map(
- t => t.comments.find(c => c.change_message_id === message.id));
- const isRobot = comments.some(c => c && !!c.robot_id);
- return isRobot ? 'autogenerated:has-robot-comments' : undefined;
- }
-
- if (message.tag === MessageTag.TAG_NEW_WIP_PATCHSET) {
- return MessageTag.TAG_NEW_PATCHSET;
- }
- if (message.tag === MessageTag.TAG_UNSET_ASSIGNEE) {
- return MessageTag.TAG_SET_ASSIGNEE;
- }
- if (message.tag === MessageTag.TAG_UNSET_PRIVATE) {
- return MessageTag.TAG_SET_PRIVATE;
- }
- if (message.tag === MessageTag.TAG_SET_WIP) {
- return MessageTag.TAG_SET_READY;
- }
-
- return message.tag.replace(/~.*/, '');
-}
-
-/**
- * Try to set a revision number that makes sense, if none is set. Just copy
- * over the revision number of the next older message. This is mostly relevant
- * for reviewer updates. Other messages should typically have the revision
- * number already set.
- */
-function computeRevision(message, allMessages) {
- if (message._revision_number > 0) return message._revision_number;
- let revision = 0;
- for (const m of allMessages) {
- if (m.date > message.date) break;
- if (m._revision_number > revision) revision = m._revision_number;
- }
- return revision > 0 ? revision : undefined;
-}
-
-/**
- * Unimportant messages are initially hidden.
- *
- * Human messages are always important. They have an undefined tag.
- *
- * Autogenerated messages are unimportant, if there is a message with the same
- * tag and a higher revision number.
- */
-function computeIsImportant(message, allMessages) {
- if (!message.tag) return true;
-
- const hasSameTag = m => m.tag === message.tag;
- const revNumber = message._revision_number || 0;
- const hasHigherRevisionNumber = m => m._revision_number > revNumber;
- return !allMessages.filter(hasSameTag).some(hasHigherRevisionNumber);
-}
-
-export const TEST_ONLY = {
- computeThreads,
- computeTag,
- computeRevision,
- computeIsImportant,
-};
-
-/**
- * @extends PolymerElement
- */
-class GrMessagesListExperimental extends mixinBehaviors( [
- KeyboardShortcutBehavior,
-], GestureEventListeners(
- LegacyElementMixin(
- PolymerElement))) {
- static get template() { return htmlTemplate; }
-
- static get is() { return 'gr-messages-list-experimental'; }
-
- static get properties() {
- return {
- /** @type {?} */
- change: Object,
- changeNum: Number,
- /**
- * These are just the change messages. They are combined with reviewer
- * updates below. So _combinedMessages is the more important property.
- */
- messages: {
- type: Array,
- value() { return []; },
- },
- /**
- * These are just the reviewer updates. They are combined with change
- * messages above. So _combinedMessages is the more important property.
- */
- reviewerUpdates: {
- type: Array,
- value() { return []; },
- },
- changeComments: Object,
- projectName: String,
- showReplyButtons: {
- type: Boolean,
- value: false,
- },
- labels: Object,
-
- /**
- * Keeps track of the state of the "Expand All" toggle button. Note that
- * you can individually expand/collapse some messages without affecting
- * the toggle button's state.
- *
- * @type {ExpandAllState}
- */
- _expandAllState: {
- type: String,
- value: ExpandAllState.EXPAND_ALL,
- },
- _expandAllTitle: {
- type: String,
- computed: '_computeExpandAllTitle(_expandAllState)',
- },
-
- _showAllActivity: {
- type: Boolean,
- value: false,
- observer: '_observeShowAllActivity',
- },
- /**
- * The merged array of change messages and reviewer updates.
- */
- _combinedMessages: {
- type: Array,
- computed: '_computeCombinedMessages(messages, reviewerUpdates, '
- + 'changeComments)',
- observer: '_combinedMessagesChanged',
- },
-
- _labelExtremes: {
- type: Object,
- computed: '_computeLabelExtremes(labels.*)',
- },
- };
- }
-
- constructor() {
- super();
- this.reporting = appContext.reportingService;
- }
-
- scrollToMessage(messageID) {
- const selector = `[data-message-id="${messageID}"]`;
- const el = this.shadowRoot.querySelector(selector);
-
- if (!el && this._showAllActivity) {
- console.warn(`Failed to scroll to message: ${messageID}`);
- return;
- }
- if (!el) {
- this._showAllActivity = true;
- setTimeout(() => this.scrollToMessage(messageID));
- return;
- }
-
- el.set('message.expanded', true);
- let top = el.offsetTop;
- for (let offsetParent = el.offsetParent;
- offsetParent;
- offsetParent = offsetParent.offsetParent) {
- top += offsetParent.offsetTop;
- }
- window.scrollTo(0, top);
- this._highlightEl(el);
- }
-
- _observeShowAllActivity(showAllActivity) {
- // We have to call render() such that the dom-repeat filter picks up the
- // change.
- this.$.messageRepeat.render();
- }
-
- /**
- * Filter for the dom-repeat of combinedMessages.
- */
- _isMessageVisible(message) {
- return this._showAllActivity || message.isImportant;
- }
-
- /**
- * Merges change messages and reviewer updates into one array. Also processes
- * all messages and updates, aligns or massages some of the properties.
- */
- _computeCombinedMessages(messages, reviewerUpdates, changeComments) {
- const params = [messages, reviewerUpdates, changeComments];
- if (params.some(o => o === undefined)) return [];
-
- let mi = 0;
- let ri = 0;
- let combinedMessages = [];
- let mDate;
- let rDate;
- for (let i = 0; i < messages.length; i++) {
- messages[i]._index = i;
- }
-
- while (mi < messages.length || ri < reviewerUpdates.length) {
- if (mi >= messages.length) {
- combinedMessages = combinedMessages.concat(reviewerUpdates.slice(ri));
- break;
- }
- if (ri >= reviewerUpdates.length) {
- combinedMessages = combinedMessages.concat(messages.slice(mi));
- break;
- }
- mDate = mDate || parseDate(messages[mi].date);
- rDate = rDate || parseDate(reviewerUpdates[ri].date);
- if (rDate < mDate) {
- combinedMessages.push(reviewerUpdates[ri++]);
- rDate = null;
- } else {
- combinedMessages.push(messages[mi++]);
- mDate = null;
- }
- }
- combinedMessages.forEach(m => {
- if (m.expanded === undefined) {
- m.expanded = false;
- }
- m.commentThreads = computeThreads(m, combinedMessages, changeComments);
- m._revision_number = computeRevision(m, combinedMessages);
- m.tag = computeTag(m);
- });
- // computeIsImportant() depends on tags and revision numbers already being
- // updated for all messages, so we have to compute this in its own forEach
- // loop.
- combinedMessages.forEach(m => {
- m.isImportant = computeIsImportant(m, combinedMessages);
- });
- return combinedMessages;
- }
-
- _updateExpandedStateOfAllMessages(exp) {
- if (this._combinedMessages) {
- for (let i = 0; i < this._combinedMessages.length; i++) {
- this._combinedMessages[i].expanded = exp;
- this.notifyPath(`_combinedMessages.${i}.expanded`);
- }
- }
- }
-
- _computeExpandAllTitle(_expandAllState) {
- if (_expandAllState === ExpandAllState.COLLAPSED_ALL) {
- return this.createTitle(
- this.Shortcut.COLLAPSE_ALL_MESSAGES, this.ShortcutSection.ACTIONS);
- }
- if (_expandAllState === ExpandAllState.EXPAND_ALL) {
- return this.createTitle(
- this.Shortcut.EXPAND_ALL_MESSAGES, this.ShortcutSection.ACTIONS);
- }
- return '';
- }
-
- _highlightEl(el) {
- const highlightedEls =
- dom(this.root).querySelectorAll('.highlighted');
- for (const highlightedEl of highlightedEls) {
- highlightedEl.classList.remove('highlighted');
- }
- function handleAnimationEnd() {
- el.removeEventListener('animationend', handleAnimationEnd);
- el.classList.remove('highlighted');
- }
- el.addEventListener('animationend', handleAnimationEnd);
- el.classList.add('highlighted');
- }
-
- /**
- * @param {boolean} expand
- */
- handleExpandCollapse(expand) {
- this._expandAllState = expand ? ExpandAllState.COLLAPSE_ALL
- : ExpandAllState.EXPAND_ALL;
- this._updateExpandedStateOfAllMessages(expand);
- }
-
- _handleExpandCollapseTap(e) {
- e.preventDefault();
- this.handleExpandCollapse(
- this._expandAllState === ExpandAllState.EXPAND_ALL);
- }
-
- _handleAnchorClick(e) {
- this.scrollToMessage(e.detail.id);
- }
-
- _isVisibleShowAllActivityToggle(messages = []) {
- return messages.some(m => !m.isImportant);
- }
-
- _computeHiddenEntriesCount(messages = []) {
- return messages.filter(m => !m.isImportant).length;
- }
-
- /**
- * This method is for reporting stats only.
- */
- _combinedMessagesChanged(combinedMessages) {
- if (combinedMessages) {
- if (combinedMessages.length === 0) return;
- const tags = combinedMessages.map(
- message => message.tag || message.type ||
- (message.comments ? 'comments' : 'none'));
- const tagsCounted = tags.reduce((acc, val) => {
- acc[val] = (acc[val] || 0) + 1;
- return acc;
- }, {all: combinedMessages.length});
- this.reporting.reportInteraction('messages-count', tagsCounted);
- }
- }
-
- /**
- * Compute a mapping from label name to objects representing the minimum and
- * maximum possible values for that label.
- */
- _computeLabelExtremes(labelRecord) {
- const extremes = {};
- const labels = labelRecord.base;
- if (!labels) { return extremes; }
- for (const key of Object.keys(labels)) {
- if (!labels[key] || !labels[key].values) { continue; }
- const values = Object.keys(labels[key].values)
- .map(v => parseInt(v, 10));
- values.sort((a, b) => a - b);
- if (!values.length) { continue; }
- extremes[key] = {min: values[0], max: values[values.length - 1]};
- }
- return extremes;
- }
-
- /**
- * Work around a issue on iOS when clicking turns into double tap
- */
- _onTapShowAllActivityToggle(e) {
- e.preventDefault();
- }
-}
-
-customElements.define(GrMessagesListExperimental.is,
- GrMessagesListExperimental);
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_html.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_html.js
deleted file mode 100644
index 212de59..0000000
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_html.js
+++ /dev/null
@@ -1,115 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-import {html} from '@polymer/polymer/lib/utils/html-tag.js';
-
-export const htmlTemplate = html`
- <style include="shared-styles">
- :host {
- display: flex;
- justify-content: space-between;
- }
- .header {
- align-items: center;
- border-top: 1px solid var(--border-color);
- border-bottom: 1px solid var(--border-color);
- display: flex;
- justify-content: space-between;
- padding: var(--spacing-s) var(--spacing-l);
- }
- .highlighted {
- animation: 3s fadeOut;
- }
- @keyframes fadeOut {
- 0% {
- background-color: var(--emphasis-color);
- }
- 100% {
- background-color: var(--view-background-color);
- }
- }
- .container {
- align-items: center;
- display: flex;
- }
- .hiddenEntries {
- color: var(--deemphasized-text-color);
- }
- gr-message:not(:last-of-type) {
- border-bottom: 1px solid var(--border-color);
- }
- gr-message {
- background-color: var(--background-color-secondary);
- }
- .experimentMessage {
- padding: var(--spacing-s) var(--spacing-m);
- background-color: var(--emphasis-color);
- border-radius: var(--border-radius);
- }
- .experimentMessage iron-icon {
- vertical-align: top;
- }
- </style>
- <div class="header">
- <div id="showAllActivityToggleContainer" class="container">
- <template
- is="dom-if"
- if="[[_isVisibleShowAllActivityToggle(_combinedMessages)]]"
- >
- <paper-toggle-button
- class="showAllActivityToggle"
- checked="{{_showAllActivity}}"
- aria-labelledby="showAllEntriesLabel"
- role="switch"
- on-tap="_onTapShowAllActivityToggle"
- ></paper-toggle-button>
- <div id="showAllEntriesLabel">
- <span>Show all entries</span>
- <span class="hiddenEntries" hidden$="[[_showAllActivity]]">
- ([[_computeHiddenEntriesCount(_combinedMessages)]] hidden)
- </span>
- </div>
- <span class="transparent separator"></span>
- </template>
- </div>
- <gr-button
- id="collapse-messages"
- link=""
- title="[[_expandAllTitle]]"
- on-click="_handleExpandCollapseTap"
- >
- [[_expandAllState]]
- </gr-button>
- </div>
- <template
- id="messageRepeat"
- is="dom-repeat"
- items="[[_combinedMessages]]"
- as="message"
- filter="_isMessageVisible"
- >
- <gr-message
- change="[[change]]"
- change-num="[[changeNum]]"
- message="[[message]]"
- project-name="[[projectName]]"
- show-reply-button="[[showReplyButtons]]"
- on-message-anchor-tap="_handleAnchorClick"
- label-extremes="[[_labelExtremes]]"
- data-message-id$="[[message.id]]"
- ></gr-message>
- </template>
-`;
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_test.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_test.js
deleted file mode 100644
index 1a0969f..0000000
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_test.js
+++ /dev/null
@@ -1,544 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import '../../../test/common-test-setup-karma.js';
-import '../../diff/gr-comment-api/gr-comment-api.js';
-import './gr-messages-list-experimental.js';
-import {createCommentApiMockWithTemplateElement} from '../../../test/mocks/comment-api.js';
-import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
-import {TEST_ONLY} from './gr-messages-list-experimental.js';
-import {MessageTag} from '../../../constants/constants.js';
-import {html} from '@polymer/polymer/lib/utils/html-tag.js';
-
-createCommentApiMockWithTemplateElement(
- 'gr-messages-list-experimental-comment-mock-api', html`
- <gr-messages-list-experimental
- id="messagesList"
- change-comments="[[_changeComments]]"></gr-messages-list-experimental>
- <gr-comment-api id="commentAPI"></gr-comment-api>
-`);
-
-const basicFixture = fixtureFromTemplate(html`
-<gr-messages-list-experimental-comment-mock-api>
- <gr-messages-list-experimental></gr-messages-list-experimental>
-</gr-messages-list-experimental-comment-mock-api>
-`);
-
-const randomMessage = function(opt_params) {
- const params = opt_params || {};
- const author1 = {
- _account_id: 1115495,
- name: 'Andrew Bonventre',
- email: 'andybons@chromium.org',
- };
- return {
- id: params.id || Math.random().toString(),
- date: params.date || '2016-01-12 20:28:33.038000',
- message: params.message || Math.random().toString(),
- _revision_number: params._revision_number || 1,
- author: params.author || author1,
- tag: params.tag,
- };
-};
-
-function generateRandomMessages(count) {
- return new Array(count).fill()
- .map(() => randomMessage());
-}
-
-suite('gr-messages-list-experimental tests', () => {
- let element;
- let messages;
-
- let commentApiWrapper;
-
- const getMessages = function() {
- return dom(element.root).querySelectorAll('gr-message');
- };
-
- const MESSAGE_ID_0 = '1234ccc949c6d482b061be6a28e10782abf0e7af';
- const MESSAGE_ID_1 = '8c19ccc949c6d482b061be6a28e10782abf0e7af';
- const MESSAGE_ID_2 = 'e7bfdbc842f6b6d8064bc68e0f52b673f40c0ca5';
-
- const author = {
- _account_id: 42,
- name: 'Marvin the Paranoid Android',
- email: 'marvin@sirius.org',
- };
-
- const createComment = function() {
- return {
- id: '1a2b3c4d',
- message: 'some random test text',
- change_message_id: '8a7b6c5d',
- updated: '2016-01-01 01:02:03.000000000',
- line: 1,
- patch_set: 1,
- author,
- };
- };
-
- const comments = {
- file1: [
- {
- ...createComment(),
- change_message_id: MESSAGE_ID_0,
- in_reply_to: '6505d749_f0bec0aa',
- author: {
- email: 'some@email.com',
- _account_id: 123,
- },
- },
- {
- ...createComment(),
- id: '2b3c4d5e',
- change_message_id: MESSAGE_ID_1,
- in_reply_to: 'c5912363_6b820105',
- },
- {
- ...createComment(),
- id: '2b3c4d5e',
- change_message_id: MESSAGE_ID_1,
- in_reply_to: '6505d749_f0bec0aa',
- },
- {
- ...createComment(),
- id: '34ed05d749_10ed44b2',
- change_message_id: MESSAGE_ID_2,
- },
- ],
- file2: [
- {
- ...createComment(),
- change_message_id: MESSAGE_ID_1,
- in_reply_to: 'c5912363_4b7d450a',
- id: '450a935e_4f260d25',
- },
- ],
- };
-
- suite('basic tests', () => {
- setup(() => {
- stub('gr-rest-api-interface', {
- getConfig() { return Promise.resolve({}); },
- getLoggedIn() { return Promise.resolve(false); },
- getDiffComments() { return Promise.resolve(comments); },
- getDiffRobotComments() { return Promise.resolve({}); },
- getDiffDrafts() { return Promise.resolve({}); },
- });
-
- messages = generateRandomMessages(3);
- // Element must be wrapped in an element with direct access to the
- // comment API.
- commentApiWrapper = basicFixture.instantiate();
- element = commentApiWrapper.$.messagesList;
- element.messages = messages;
-
- // Stub methods on the changeComments object after changeComments has
- // been initialized.
- return commentApiWrapper.loadComments();
- });
-
- test('expand/collapse all', () => {
- let allMessageEls = getMessages();
- for (const message of allMessageEls) {
- message._expanded = false;
- }
- MockInteractions.tap(allMessageEls[1]);
- assert.isTrue(allMessageEls[1]._expanded);
-
- MockInteractions.tap(element.shadowRoot
- .querySelector('#collapse-messages'));
- allMessageEls = getMessages();
- for (const message of allMessageEls) {
- assert.isTrue(message._expanded);
- }
-
- MockInteractions.tap(element.shadowRoot
- .querySelector('#collapse-messages'));
- allMessageEls = getMessages();
- for (const message of allMessageEls) {
- assert.isFalse(message._expanded);
- }
- });
-
- test('expand/collapse from external keypress', () => {
- // Start with one expanded message. -> not all collapsed
- element.scrollToMessage(messages[1].id);
- assert.isFalse([...getMessages()].filter(m => m._expanded).length == 0);
-
- // Press 'z' -> all collapsed
- element.handleExpandCollapse(false);
- assert.isTrue([...getMessages()].filter(m => m._expanded).length == 0);
-
- // Press 'x' -> all expanded
- element.handleExpandCollapse(true);
- assert.isTrue([...getMessages()].filter(m => !m._expanded).length == 0);
-
- // Press 'z' -> all collapsed
- element.handleExpandCollapse(false);
- assert.isTrue([...getMessages()].filter(m => m._expanded).length == 0);
- });
-
- test('showAllActivity does not appear when all msgs are important', () => {
- assert.isOk(element.shadowRoot
- .querySelector('#showAllActivityToggleContainer'));
- assert.isNotOk(element.shadowRoot
- .querySelector('.showAllActivityToggle'));
- });
-
- test('scroll to message', () => {
- const allMessageEls = getMessages();
- for (const message of allMessageEls) {
- message.set('message.expanded', false);
- }
-
- const scrollToStub = sinon.stub(window, 'scrollTo');
- const highlightStub = sinon.stub(element, '_highlightEl');
-
- element.scrollToMessage('invalid');
-
- for (const message of allMessageEls) {
- assert.isFalse(message._expanded,
- 'expected gr-message to not be expanded');
- }
-
- const messageID = messages[1].id;
- element.scrollToMessage(messageID);
- assert.isTrue(
- element.shadowRoot
- .querySelector('[data-message-id="' + messageID + '"]')
- ._expanded);
-
- assert.isTrue(scrollToStub.calledOnce);
- assert.isTrue(highlightStub.calledOnce);
- });
-
- test('scroll to message offscreen', () => {
- const scrollToStub = sinon.stub(window, 'scrollTo');
- const highlightStub = sinon.stub(element, '_highlightEl');
- element.messages = generateRandomMessages(25);
- flushAsynchronousOperations();
- assert.isFalse(scrollToStub.called);
- assert.isFalse(highlightStub.called);
-
- const messageID = element.messages[1].id;
- element.scrollToMessage(messageID);
- assert.isTrue(scrollToStub.calledOnce);
- assert.isTrue(highlightStub.calledOnce);
- assert.isTrue(
- element.shadowRoot
- .querySelector('[data-message-id="' + messageID + '"]')
- ._expanded);
- });
-
- test('associating messages with comments', () => {
- const messages = [].concat(
- randomMessage(),
- {
- _index: 5,
- _revision_number: 4,
- message: 'Uploaded patch set 4.',
- date: '2016-09-28 13:36:33.000000000',
- author,
- id: '8c19ccc949c6d482b061be6a28e10782abf0e7af',
- },
- {
- _index: 6,
- _revision_number: 4,
- message: 'Patch Set 4:\n\n(6 comments)',
- date: '2016-09-28 13:36:33.000000000',
- author,
- id: 'e7bfdbc842f6b6d8064bc68e0f52b673f40c0ca5',
- }
- );
- element.messages = messages;
- flushAsynchronousOperations();
- const messageElements = getMessages();
- assert.equal(messageElements.length, messages.length);
- assert.deepEqual(messageElements[1].message, messages[1]);
- assert.deepEqual(messageElements[2].message, messages[2]);
- });
-
- test('threads', () => {
- const messages = [
- {
- _index: 5,
- _revision_number: 4,
- message: 'Uploaded patch set 4.',
- date: '2016-09-28 13:36:33.000000000',
- author,
- id: '8c19ccc949c6d482b061be6a28e10782abf0e7af',
- },
- ];
- element.messages = messages;
- flushAsynchronousOperations();
- const messageElements = getMessages();
- // threads
- assert.equal(
- messageElements[0].message.commentThreads.length,
- 3);
- // first thread contains 1 comment
- assert.equal(
- messageElements[0].message.commentThreads[0].comments.length,
- 1);
- });
-
- test('updateTag human message', () => {
- const m = randomMessage();
- assert.equal(TEST_ONLY.computeTag(m), undefined);
- });
-
- test('updateTag nothing to change', () => {
- const m = randomMessage();
- const tag = 'something-normal';
- m.tag = tag;
- assert.equal(TEST_ONLY.computeTag(m), tag);
- });
-
- test('updateTag TAG_NEW_WIP_PATCHSET', () => {
- const m = randomMessage();
- m.tag = MessageTag.TAG_NEW_WIP_PATCHSET;
- assert.equal(TEST_ONLY.computeTag(m), MessageTag.TAG_NEW_PATCHSET);
- });
-
- test('updateTag remove postfix', () => {
- const m = randomMessage();
- m.tag = 'something~withpostfix';
- assert.equal(TEST_ONLY.computeTag(m), 'something');
- });
-
- test('updateTag with robot comments', () => {
- const m = randomMessage();
- m.commentThreads = [{
- comments: [{
- robot_id: 'id314',
- change_message_id: m.id,
- }],
- }];
- assert.notEqual(TEST_ONLY.computeTag(m), undefined);
- });
-
- test('setRevisionNumber nothing to change', () => {
- const m1 = randomMessage();
- const m2 = randomMessage();
- assert.equal(TEST_ONLY.computeRevision(m1, [m1, m2]), 1);
- assert.equal(TEST_ONLY.computeRevision(m2, [m1, m2]), 1);
- });
-
- test('setRevisionNumber reviewer updates', () => {
- const m1 = randomMessage(
- {
- tag: MessageTag.TAG_REVIEWER_UPDATE,
- date: '2020-01-01 10:00:00.000000000',
- });
- m1._revision_number = undefined;
- const m2 = randomMessage(
- {
- date: '2020-01-02 10:00:00.000000000',
- });
- m2._revision_number = 1;
- const m3 = randomMessage(
- {
- tag: MessageTag.TAG_REVIEWER_UPDATE,
- date: '2020-01-03 10:00:00.000000000',
- });
- m3._revision_number = undefined;
- const m4 = randomMessage(
- {
- date: '2020-01-04 10:00:00.000000000',
- });
- m4._revision_number = 2;
- const m5 = randomMessage(
- {
- tag: MessageTag.TAG_REVIEWER_UPDATE,
- date: '2020-01-05 10:00:00.000000000',
- });
- m5._revision_number = undefined;
- const allMessages = [m1, m2, m3, m4, m5];
- assert.equal(TEST_ONLY.computeRevision(m1, allMessages), undefined);
- assert.equal(TEST_ONLY.computeRevision(m2, allMessages), 1);
- assert.equal(TEST_ONLY.computeRevision(m3, allMessages), 1);
- assert.equal(TEST_ONLY.computeRevision(m4, allMessages), 2);
- assert.equal(TEST_ONLY.computeRevision(m5, allMessages), 2);
- });
-
- test('isImportant human message', () => {
- const m = randomMessage();
- assert.isTrue(TEST_ONLY.computeIsImportant(m, []));
- assert.isTrue(TEST_ONLY.computeIsImportant(m, [m]));
- });
-
- test('isImportant even with a tag', () => {
- const m1 = randomMessage();
- const m2 = randomMessage({tag: 'autogenerated:gerrit1'});
- const m3 = randomMessage({tag: 'autogenerated:gerrit2'});
- assert.isTrue(TEST_ONLY.computeIsImportant(m2, []));
- assert.isTrue(TEST_ONLY.computeIsImportant(m1, [m1, m2, m3]));
- assert.isTrue(TEST_ONLY.computeIsImportant(m2, [m1, m2, m3]));
- assert.isTrue(TEST_ONLY.computeIsImportant(m3, [m1, m2, m3]));
- });
-
- test('isImportant filters same tag and older revision', () => {
- const m1 = randomMessage({tag: 'auto', _revision_number: 2});
- const m2 = randomMessage({tag: 'auto', _revision_number: 1});
- const m3 = randomMessage({tag: 'auto'});
- assert.isTrue(TEST_ONLY.computeIsImportant(m1, [m1]));
- assert.isTrue(TEST_ONLY.computeIsImportant(m2, [m2]));
- assert.isTrue(TEST_ONLY.computeIsImportant(m1, [m1, m2]));
- assert.isFalse(TEST_ONLY.computeIsImportant(m2, [m1, m2]));
- assert.isTrue(TEST_ONLY.computeIsImportant(m1, [m1, m3]));
- assert.isFalse(TEST_ONLY.computeIsImportant(m3, [m1, m3]));
- assert.isTrue(TEST_ONLY.computeIsImportant(m1, [m1, m2, m3]));
- assert.isFalse(TEST_ONLY.computeIsImportant(m2, [m1, m2, m3]));
- assert.isFalse(TEST_ONLY.computeIsImportant(m3, [m1, m2, m3]));
- });
-
- test('isImportant is evaluated after tag update', () => {
- const m1 = randomMessage(
- {tag: MessageTag.TAG_NEW_PATCHSET, _revision_number: 1});
- const m2 = randomMessage(
- {tag: MessageTag.TAG_NEW_WIP_PATCHSET, _revision_number: 2});
- element.messages = [m1, m2];
- flushAsynchronousOperations();
- assert.isFalse(m1.isImportant);
- assert.isTrue(m2.isImportant);
- });
-
- test('messages without author do not throw', () => {
- const messages = [{
- _index: 5,
- _revision_number: 4,
- message: 'Uploaded patch set 4.',
- date: '2016-09-28 13:36:33.000000000',
- id: '8c19ccc949c6d482b061be6a28e10782abf0e7af',
- }];
- element.messages = messages;
- flushAsynchronousOperations();
- const messageEls = getMessages();
- assert.equal(messageEls.length, 1);
- assert.equal(messageEls[0].message.message, messages[0].message);
- });
- });
-
- suite('gr-messages-list-experimental automate tests', () => {
- let element;
- let messages;
-
- let commentApiWrapper;
-
- setup(() => {
- stub('gr-rest-api-interface', {
- getConfig() { return Promise.resolve({}); },
- getLoggedIn() { return Promise.resolve(false); },
- getDiffComments() { return Promise.resolve({}); },
- getDiffRobotComments() { return Promise.resolve({}); },
- getDiffDrafts() { return Promise.resolve({}); },
- });
-
- messages = [
- randomMessage(),
- randomMessage({tag: 'auto', _revision_number: 2}),
- randomMessage({tag: 'auto', _revision_number: 3}),
- ];
-
- // Element must be wrapped in an element with direct access to the
- // comment API.
- commentApiWrapper = basicFixture.instantiate();
- element = commentApiWrapper.$.messagesList;
- sinon.spy(commentApiWrapper.$.commentAPI, 'loadAll');
- element.messages = messages;
-
- // Stub methods on the changeComments object after changeComments has
- // been initialized.
- return commentApiWrapper.loadComments();
- });
-
- test('hide autogenerated button is not hidden', () => {
- const toggle = dom(element.root).querySelector('.showAllActivityToggle');
- assert.isOk(toggle);
- });
-
- test('one unimportant message is hidden initially', () => {
- const displayedMsgs = dom(element.root).querySelectorAll('gr-message');
- assert.equal(displayedMsgs.length, 2);
- });
-
- test('unimportant messages hidden after toggle', () => {
- element._showAllActivity = true;
- const toggle = dom(element.root).querySelector('.showAllActivityToggle');
- assert.isOk(toggle);
- MockInteractions.tap(toggle);
- flushAsynchronousOperations();
- const displayedMsgs = dom(element.root).querySelectorAll('gr-message');
- assert.equal(displayedMsgs.length, 2);
- });
-
- test('unimportant messages shown after toggle', () => {
- element._showAllActivity = false;
- const toggle = dom(element.root).querySelector('.showAllActivityToggle');
- assert.isOk(toggle);
- MockInteractions.tap(toggle);
- flushAsynchronousOperations();
- const displayedMsgs = dom(element.root).querySelectorAll('gr-message');
- assert.equal(displayedMsgs.length, 3);
- });
-
- test('_computeLabelExtremes', () => {
- const computeSpy = sinon.spy(element, '_computeLabelExtremes');
-
- element.labels = null;
- assert.isTrue(computeSpy.calledOnce);
- assert.deepEqual(computeSpy.lastCall.returnValue, {});
-
- element.labels = {};
- assert.isTrue(computeSpy.calledTwice);
- assert.deepEqual(computeSpy.lastCall.returnValue, {});
-
- element.labels = {'my-label': {}};
- assert.isTrue(computeSpy.calledThrice);
- assert.deepEqual(computeSpy.lastCall.returnValue, {});
-
- element.labels = {'my-label': {values: {}}};
- assert.equal(computeSpy.callCount, 4);
- assert.deepEqual(computeSpy.lastCall.returnValue, {});
-
- element.labels = {'my-label': {values: {'-12': {}}}};
- assert.equal(computeSpy.callCount, 5);
- assert.deepEqual(computeSpy.lastCall.returnValue,
- {'my-label': {min: -12, max: -12}});
-
- element.labels = {
- 'my-label': {values: {'-2': {}, '-1': {}, '0': {}, '+1': {}, '+2': {}}},
- };
- assert.equal(computeSpy.callCount, 6);
- assert.deepEqual(computeSpy.lastCall.returnValue,
- {'my-label': {min: -2, max: 2}});
-
- element.labels = {
- 'my-label': {values: {'-12': {}}},
- 'other-label': {values: {'-1': {}, ' 0': {}, '+1': {}}},
- };
- assert.equal(computeSpy.callCount, 7);
- assert.deepEqual(computeSpy.lastCall.returnValue, {
- 'my-label': {min: -12, max: -12},
- 'other-label': {min: -1, max: 1},
- });
- });
- });
-});
-
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
index 30482e2..cd61396 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
@@ -1,6 +1,6 @@
/**
* @license
- * Copyright (C) 2015 The Android Open Source Project
+ * Copyright (C) 2020 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -16,9 +16,10 @@
*/
import '@polymer/paper-toggle-button/paper-toggle-button.js';
import '../../shared/gr-button/gr-button.js';
+import '../../shared/gr-icons/gr-icons.js';
import '../gr-message/gr-message.js';
import '../../../styles/shared-styles.js';
-import {flush, dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
+import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
import {mixinBehaviors} from '@polymer/polymer/lib/legacy/class.js';
import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners.js';
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
@@ -26,16 +27,9 @@
import {htmlTemplate} from './gr-messages-list_html.js';
import {KeyboardShortcutBehavior} from '../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.js';
import {parseDate} from '../../../utils/date-util.js';
+import {MessageTag} from '../../../constants/constants.js';
import {appContext} from '../../../services/app-context.js';
-const MAX_INITIAL_SHOWN_MESSAGES = 20;
-const MESSAGES_INCREMENT = 5;
-
-const ReportingEvent = {
- SHOW_ALL: 'show-all-messages',
- SHOW_MORE: 'show-more-messages',
-};
-
/**
* The content of the enum is also used in the UI for the button text.
*
@@ -47,6 +41,114 @@
};
/**
+ * Computes message author's comments for this change message. The backend
+ * sets comment.change_message_id for matching, so this computation is fairly
+ * straightforward.
+ */
+function computeThreads(message, allMessages, changeComments) {
+ if ([message, allMessages, changeComments].includes(undefined)) {
+ return [];
+ }
+ if (message._index === undefined) {
+ return [];
+ }
+
+ return changeComments.getAllThreadsForChange().filter(
+ thread => thread.comments.map(comment => {
+ // collapse all by default
+ comment.collapsed = true;
+ return comment;
+ }).some(comment => {
+ const condition = comment.change_message_id === message.id;
+ // Since getAllThreadsForChange() always returns a new copy of
+ // all comments we can modify them here without worrying about
+ // polluting other threads.
+ comment.collapsed = !condition;
+ return condition;
+ })
+ );
+}
+
+/**
+ * If messages have the same tag, then that influences grouping and whether
+ * a message is initally hidden or not, see isImportant(). So we are applying
+ * some "magic" rules here in order to hide exactly the right messages.
+ *
+ * 1. If a message does not have a tag, but is associated with robot comments,
+ * then it gets a tag.
+ *
+ * 2. Use the same tag for some of Gerrit's standard events, if they should be
+ * considered one group, e.g. normal and wip patchset uploads.
+ *
+ * 3. Everything beyond the ~ character is cut off from the tag. That gives
+ * tools control over which messages will be hidden.
+ */
+function computeTag(message) {
+ if (!message.tag) {
+ const threads = message.commentThreads || [];
+ const comments = threads.map(
+ t => t.comments.find(c => c.change_message_id === message.id));
+ const isRobot = comments.some(c => c && !!c.robot_id);
+ return isRobot ? 'autogenerated:has-robot-comments' : undefined;
+ }
+
+ if (message.tag === MessageTag.TAG_NEW_WIP_PATCHSET) {
+ return MessageTag.TAG_NEW_PATCHSET;
+ }
+ if (message.tag === MessageTag.TAG_UNSET_ASSIGNEE) {
+ return MessageTag.TAG_SET_ASSIGNEE;
+ }
+ if (message.tag === MessageTag.TAG_UNSET_PRIVATE) {
+ return MessageTag.TAG_SET_PRIVATE;
+ }
+ if (message.tag === MessageTag.TAG_SET_WIP) {
+ return MessageTag.TAG_SET_READY;
+ }
+
+ return message.tag.replace(/~.*/, '');
+}
+
+/**
+ * Try to set a revision number that makes sense, if none is set. Just copy
+ * over the revision number of the next older message. This is mostly relevant
+ * for reviewer updates. Other messages should typically have the revision
+ * number already set.
+ */
+function computeRevision(message, allMessages) {
+ if (message._revision_number > 0) return message._revision_number;
+ let revision = 0;
+ for (const m of allMessages) {
+ if (m.date > message.date) break;
+ if (m._revision_number > revision) revision = m._revision_number;
+ }
+ return revision > 0 ? revision : undefined;
+}
+
+/**
+ * Unimportant messages are initially hidden.
+ *
+ * Human messages are always important. They have an undefined tag.
+ *
+ * Autogenerated messages are unimportant, if there is a message with the same
+ * tag and a higher revision number.
+ */
+function computeIsImportant(message, allMessages) {
+ if (!message.tag) return true;
+
+ const hasSameTag = m => m.tag === message.tag;
+ const revNumber = message._revision_number || 0;
+ const hasHigherRevisionNumber = m => m._revision_number > revNumber;
+ return !allMessages.filter(hasSameTag).some(hasHigherRevisionNumber);
+}
+
+export const TEST_ONLY = {
+ computeThreads,
+ computeTag,
+ computeRevision,
+ computeIsImportant,
+};
+
+/**
* @extends PolymerElement
*/
class GrMessagesList extends mixinBehaviors( [
@@ -60,11 +162,21 @@
static get properties() {
return {
+ /** @type {?} */
+ change: Object,
changeNum: Number,
+ /**
+ * These are just the change messages. They are combined with reviewer
+ * updates below. So _combinedMessages is the more important property.
+ */
messages: {
type: Array,
value() { return []; },
},
+ /**
+ * These are just the reviewer updates. They are combined with change
+ * messages above. So _combinedMessages is the more important property.
+ */
reviewerUpdates: {
type: Array,
value() { return []; },
@@ -93,24 +205,19 @@
computed: '_computeExpandAllTitle(_expandAllState)',
},
- _hideAutomated: {
+ _showAllActivity: {
type: Boolean,
value: false,
+ observer: '_observeShowAllActivity',
},
/**
- * The messages after processing and including merged reviewer updates.
+ * The merged array of change messages and reviewer updates.
*/
- _processedMessages: {
+ _combinedMessages: {
type: Array,
- computed: '_computeItems(messages, reviewerUpdates)',
- observer: '_processedMessagesChanged',
- },
- /**
- * The subset of _processedMessages that is visible to the user.
- */
- _visibleMessages: {
- type: Array,
- value() { return []; },
+ computed: '_computeCombinedMessages(messages, reviewerUpdates, '
+ + 'changeComments)',
+ observer: '_combinedMessagesChanged',
},
_labelExtremes: {
@@ -126,27 +233,17 @@
}
scrollToMessage(messageID) {
- let el = this.shadowRoot
- .querySelector('[data-message-id="' + messageID + '"]');
- // If the message is hidden, expand the hidden messages back to that
- // point.
- if (!el) {
- let index;
- for (index = 0; index < this._processedMessages.length; index++) {
- if (this._processedMessages[index].id === messageID) {
- break;
- }
- }
- if (index === this._processedMessages.length) { return; }
+ const selector = `[data-message-id="${messageID}"]`;
+ const el = this.shadowRoot.querySelector(selector);
- const newMessages = this._processedMessages.slice(index,
- -this._visibleMessages.length);
- // Add newMessages to the beginning of _visibleMessages.
- this.splice(...['_visibleMessages', 0, 0].concat(newMessages));
- // Allow the dom-repeat to stamp.
- flush();
- el = this.shadowRoot
- .querySelector('[data-message-id="' + messageID + '"]');
+ if (!el && this._showAllActivity) {
+ console.warn(`Failed to scroll to message: ${messageID}`);
+ return;
+ }
+ if (!el) {
+ this._showAllActivity = true;
+ setTimeout(() => this.scrollToMessage(messageID));
+ return;
}
el.set('message.expanded', true);
@@ -160,22 +257,30 @@
this._highlightEl(el);
}
- _isAutomated(message) {
- return !!(message.reviewer ||
- (message.tag && message.tag.startsWith('autogenerated')));
+ _observeShowAllActivity(showAllActivity) {
+ // We have to call render() such that the dom-repeat filter picks up the
+ // change.
+ this.$.messageRepeat.render();
}
- _computeItems(messages, reviewerUpdates) {
- // Polymer 2: check for undefined
- if ([messages, reviewerUpdates].includes(undefined)) {
- return [];
- }
+ /**
+ * Filter for the dom-repeat of combinedMessages.
+ */
+ _isMessageVisible(message) {
+ return this._showAllActivity || message.isImportant;
+ }
- messages = messages || [];
- reviewerUpdates = reviewerUpdates || [];
+ /**
+ * Merges change messages and reviewer updates into one array. Also processes
+ * all messages and updates, aligns or massages some of the properties.
+ */
+ _computeCombinedMessages(messages, reviewerUpdates, changeComments) {
+ const params = [messages, reviewerUpdates, changeComments];
+ if (params.some(o => o === undefined)) return [];
+
let mi = 0;
let ri = 0;
- let result = [];
+ let combinedMessages = [];
let mDate;
let rDate;
for (let i = 0; i < messages.length; i++) {
@@ -184,44 +289,45 @@
while (mi < messages.length || ri < reviewerUpdates.length) {
if (mi >= messages.length) {
- result = result.concat(reviewerUpdates.slice(ri));
+ combinedMessages = combinedMessages.concat(reviewerUpdates.slice(ri));
break;
}
if (ri >= reviewerUpdates.length) {
- result = result.concat(messages.slice(mi));
+ combinedMessages = combinedMessages.concat(messages.slice(mi));
break;
}
mDate = mDate || parseDate(messages[mi].date);
rDate = rDate || parseDate(reviewerUpdates[ri].date);
if (rDate < mDate) {
- result.push(reviewerUpdates[ri++]);
+ combinedMessages.push(reviewerUpdates[ri++]);
rDate = null;
} else {
- result.push(messages[mi++]);
+ combinedMessages.push(messages[mi++]);
mDate = null;
}
}
- result.forEach(m => {
+ combinedMessages.forEach(m => {
if (m.expanded === undefined) {
m.expanded = false;
}
+ m.commentThreads = computeThreads(m, combinedMessages, changeComments);
+ m._revision_number = computeRevision(m, combinedMessages);
+ m.tag = computeTag(m);
});
- return result;
+ // computeIsImportant() depends on tags and revision numbers already being
+ // updated for all messages, so we have to compute this in its own forEach
+ // loop.
+ combinedMessages.forEach(m => {
+ m.isImportant = computeIsImportant(m, combinedMessages);
+ });
+ return combinedMessages;
}
- _updateExpandedStateOfAllMessages(expanded) {
- if (this._processedMessages) {
- for (let i = 0; i < this._processedMessages.length; i++) {
- this._processedMessages[i].expanded = expanded;
- }
- }
- // _visibleMessages is a subarray of _processedMessages
- // _processedMessages contains all items from _visibleMessages
- // At this point all _visibleMessages.expanded values are set,
- // and notifyPath must be used to notify Polymer about changes.
- if (this._visibleMessages) {
- for (let i = 0; i < this._visibleMessages.length; i++) {
- this.notifyPath(`_visibleMessages.${i}.expanded`);
+ _updateExpandedStateOfAllMessages(exp) {
+ if (this._combinedMessages) {
+ for (let i = 0; i < this._combinedMessages.length; i++) {
+ this._combinedMessages[i].expanded = exp;
+ this.notifyPath(`_combinedMessages.${i}.expanded`);
}
}
}
@@ -271,181 +377,31 @@
this.scrollToMessage(e.detail.id);
}
- _hasAutomatedMessages(messages) {
- if (!messages) { return false; }
- for (const message of messages) {
- if (this._isAutomated(message)) {
- return true;
- }
- }
- return false;
+ _isVisibleShowAllActivityToggle(messages = []) {
+ return messages.some(m => !m.isImportant);
+ }
+
+ _computeHiddenEntriesCount(messages = []) {
+ return messages.filter(m => !m.isImportant).length;
}
/**
- * Computes message author's file comments for change's message.
- * Method uses this.messages to find next message and relies on messages
- * to be sorted by date field descending.
- *
- * @param {!Object} changeComments changeComment object, which includes
- * a method to get all published comments (including robot comments),
- * which returns a Hash of arrays of comments, filename as key.
- * @param {!Object} message
- * @return {!Object} Hash of arrays of comments, filename as key.
+ * This method is for reporting stats only.
*/
- _computeCommentsForMessage(changeComments, message) {
- if ([changeComments, message].includes(undefined)) {
- return {};
- }
- const comments = changeComments.getAllPublishedComments();
- if (message._index === undefined || !comments || !this.messages) {
- return {};
- }
- const messages = this.messages || [];
- const index = message._index;
- const authorId = message.author && message.author._account_id;
- const mDate = parseDate(message.date).getTime();
- // NB: Messages array has oldest messages first.
- let nextMDate;
- if (index > 0) {
- for (let i = index - 1; i >= 0; i--) {
- if (messages[i] && messages[i].author &&
- messages[i].author._account_id === authorId) {
- nextMDate = parseDate(messages[i].date).getTime();
- break;
- }
- }
- }
- const msgComments = {};
- for (const file in comments) {
- if (!comments.hasOwnProperty(file)) { continue; }
- const fileComments = comments[file];
- for (let i = 0; i < fileComments.length; i++) {
- if (fileComments[i].author &&
- fileComments[i].author._account_id !== authorId) {
- continue;
- }
- const cDate = parseDate(fileComments[i].updated).getTime();
- if (cDate <= mDate) {
- if (nextMDate && cDate <= nextMDate) {
- continue;
- }
- msgComments[file] = msgComments[file] || [];
- msgComments[file].push(fileComments[i]);
- }
- }
- }
- return msgComments;
- }
-
- /**
- * Returns the number of messages to splice to the beginning of
- * _visibleMessages. This is the minimum of the total number of messages
- * remaining in the list and the number of messages needed to display five
- * more visible messages in the list.
- */
- _getDelta(visibleMessages, messages, hideAutomated) {
- if ([visibleMessages, messages].includes(undefined)) {
- return 0;
- }
-
- let delta = MESSAGES_INCREMENT;
- const msgsRemaining = messages.length - visibleMessages.length;
-
- if (hideAutomated) {
- let counter = 0;
- let i;
- for (i = msgsRemaining; i > 0 && counter < MESSAGES_INCREMENT; i--) {
- if (!this._isAutomated(messages[i - 1])) { counter++; }
- }
- delta = msgsRemaining - i;
- }
- return Math.min(msgsRemaining, delta);
- }
-
- /**
- * Gets the number of messages that would be visible, but do not currently
- * exist in _visibleMessages.
- */
- _numRemaining(visibleMessages, messages, hideAutomated) {
- if ([visibleMessages, messages].includes(undefined)) {
- return 0;
- }
-
- if (hideAutomated) {
- return this._getHumanMessages(messages).length -
- this._getHumanMessages(visibleMessages).length;
- }
- return messages.length - visibleMessages.length;
- }
-
- _computeIncrementText(visibleMessages, messages, hideAutomated) {
- let delta = this._getDelta(visibleMessages, messages, hideAutomated);
- delta = Math.min(
- this._numRemaining(visibleMessages, messages, hideAutomated), delta);
- return 'Show ' + Math.min(MESSAGES_INCREMENT, delta) + ' more';
- }
-
- _getHumanMessages(messages) {
- return messages.filter(msg => !this._isAutomated(msg));
- }
-
- _computeShowHideTextHidden(visibleMessages, messages,
- hideAutomated) {
- if ([visibleMessages, messages].includes(undefined)) {
- return 0;
- }
-
- if (hideAutomated) {
- messages = this._getHumanMessages(messages);
- visibleMessages = this._getHumanMessages(visibleMessages);
- }
- return visibleMessages.length >= messages.length;
- }
-
- _handleShowAllTap() {
- this._visibleMessages = this._processedMessages;
- this.reporting.reportInteraction(ReportingEvent.SHOW_ALL);
- }
-
- _handleIncrementShownMessages() {
- const delta = this._getDelta(this._visibleMessages,
- this._processedMessages, this._hideAutomated);
- const len = this._visibleMessages.length;
- const newMessages = this._processedMessages.slice(-(len + delta), -len);
- // Add newMessages to the beginning of _visibleMessages
- this.splice(...['_visibleMessages', 0, 0].concat(newMessages));
- this.reporting.reportInteraction(ReportingEvent.SHOW_MORE);
- }
-
- _processedMessagesChanged(messages) {
- if (messages) {
- this._visibleMessages = messages.slice(-MAX_INITIAL_SHOWN_MESSAGES);
-
- if (messages.length === 0) return;
- const tags = messages.map(message => message.tag || message.type ||
- (message.comments ? 'comments' : 'none'));
+ _combinedMessagesChanged(combinedMessages) {
+ if (combinedMessages) {
+ if (combinedMessages.length === 0) return;
+ const tags = combinedMessages.map(
+ message => message.tag || message.type ||
+ (message.comments ? 'comments' : 'none'));
const tagsCounted = tags.reduce((acc, val) => {
acc[val] = (acc[val] || 0) + 1;
return acc;
- }, {all: messages.length});
+ }, {all: combinedMessages.length});
this.reporting.reportInteraction('messages-count', tagsCounted);
}
}
- _computeNumMessagesText(visibleMessages, messages,
- hideAutomated) {
- const total =
- this._numRemaining(visibleMessages, messages, hideAutomated);
- return total === 1 ? 'Show 1 message' : 'Show all ' + total + ' messages';
- }
-
- _computeIncrementHidden(visibleMessages, messages,
- hideAutomated) {
- const total =
- this._numRemaining(visibleMessages, messages, hideAutomated);
- return total <= this._getDelta(visibleMessages, messages, hideAutomated);
- }
-
/**
* Compute a mapping from label name to objects representing the minimum and
* maximum possible values for that label.
@@ -468,9 +424,10 @@
/**
* Work around a issue on iOS when clicking turns into double tap
*/
- _onTapHideAutomated(e) {
+ _onTapShowAllActivityToggle(e) {
e.preventDefault();
}
}
-customElements.define(GrMessagesList.is, GrMessagesList);
+customElements.define(GrMessagesList.is,
+ GrMessagesList);
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_html.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_html.js
index 2636a54..c696c8c 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_html.js
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_html.js
@@ -18,8 +18,7 @@
export const htmlTemplate = html`
<style include="shared-styles">
- :host,
- .messageListControls {
+ :host {
display: flex;
justify-content: space-between;
}
@@ -31,9 +30,6 @@
justify-content: space-between;
padding: var(--spacing-s) var(--spacing-l);
}
- #messageControlsContainer {
- padding: 0 var(--spacing-l);
- }
.highlighted {
animation: 3s fadeOut;
}
@@ -45,47 +41,42 @@
background-color: var(--view-background-color);
}
}
- #messageControlsContainer {
- align-items: center;
- background-color: var(--background-color-secondary);
- border-bottom: 1px solid var(--border-color);
- display: flex;
- height: 2.25em;
- justify-content: center;
- }
- #messageControlsContainer gr-button {
- padding: var(--spacing-s) 0;
- }
.container {
align-items: center;
display: flex;
}
+ .hiddenEntries {
+ color: var(--deemphasized-text-color);
+ }
gr-message:not(:last-of-type) {
border-bottom: 1px solid var(--border-color);
}
- gr-message:nth-child(2n) {
+ gr-message {
background-color: var(--background-color-secondary);
}
- gr-message:nth-child(2n + 1) {
- background-color: var(--background-color-tertiary);
- }
</style>
<div class="header">
- <span
- id="automatedMessageToggleContainer"
- class="container"
- hidden$="[[!_hasAutomatedMessages(messages)]]"
- >
- <paper-toggle-button
- id="automatedMessageToggle"
- checked="{{_hideAutomated}}"
- aria-labelledby="onlyCommentsLabel"
- role="switch"
- on-tap="_onTapHideAutomated"
- ></paper-toggle-button>
- <span id="onlyCommentsLabel">Only comments</span>
- <span class="transparent separator"></span>
- </span>
+ <div id="showAllActivityToggleContainer" class="container">
+ <template
+ is="dom-if"
+ if="[[_isVisibleShowAllActivityToggle(_combinedMessages)]]"
+ >
+ <paper-toggle-button
+ class="showAllActivityToggle"
+ checked="{{_showAllActivity}}"
+ aria-labelledby="showAllEntriesLabel"
+ role="switch"
+ on-tap="_onTapShowAllActivityToggle"
+ ></paper-toggle-button>
+ <div id="showAllEntriesLabel">
+ <span>Show all entries</span>
+ <span class="hiddenEntries" hidden$="[[_showAllActivity]]">
+ ([[_computeHiddenEntriesCount(_combinedMessages)]] hidden)
+ </span>
+ </div>
+ <span class="transparent separator"></span>
+ </template>
+ </div>
<gr-button
id="collapse-messages"
link=""
@@ -95,35 +86,17 @@
[[_expandAllState]]
</gr-button>
</div>
- <span
- id="messageControlsContainer"
- hidden$="[[_computeShowHideTextHidden(_visibleMessages, _processedMessages, _hideAutomated, _visibleMessages.length)]]"
+ <template
+ id="messageRepeat"
+ is="dom-repeat"
+ items="[[_combinedMessages]]"
+ as="message"
+ filter="_isMessageVisible"
>
- <gr-button id="oldMessagesBtn" link="" on-click="_handleShowAllTap">
- [[_computeNumMessagesText(_visibleMessages, _processedMessages,
- _hideAutomated, _visibleMessages.length)]]
- </gr-button>
- <span
- class="container"
- hidden$="[[_computeIncrementHidden(_visibleMessages, _processedMessages, _hideAutomated, _visibleMessages.length)]]"
- >
- <span class="transparent separator"></span>
- <gr-button
- id="incrementMessagesBtn"
- link=""
- on-click="_handleIncrementShownMessages"
- >
- [[_computeIncrementText(_visibleMessages, _processedMessages,
- _hideAutomated, _visibleMessages.length)]]
- </gr-button>
- </span>
- </span>
- <template is="dom-repeat" items="[[_visibleMessages]]" as="message">
<gr-message
+ change="[[change]]"
change-num="[[changeNum]]"
message="[[message]]"
- comments="[[_computeCommentsForMessage(changeComments, message)]]"
- hide-automated="[[_hideAutomated]]"
project-name="[[projectName]]"
show-reply-button="[[showReplyButtons]]"
on-message-anchor-tap="_handleAnchorClick"
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.js
index 4f05dfc..c3245fe 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.js
@@ -1,7 +1,6 @@
-import {createCommentApiMockWithTemplateElement} from '../../../test/mocks/comment-api';
/**
* @license
- * Copyright (C) 2016 The Android Open Source Project
+ * Copyright (C) 2020 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -19,8 +18,10 @@
import '../../../test/common-test-setup-karma.js';
import '../../diff/gr-comment-api/gr-comment-api.js';
import './gr-messages-list.js';
-import '../../../test/mocks/comment-api.js';
+import {createCommentApiMockWithTemplateElement} from '../../../test/mocks/comment-api.js';
import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
+import {TEST_ONLY} from './gr-messages-list.js';
+import {MessageTag} from '../../../constants/constants.js';
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
createCommentApiMockWithTemplateElement(
@@ -50,40 +51,15 @@
message: params.message || Math.random().toString(),
_revision_number: params._revision_number || 1,
author: params.author || author1,
+ tag: params.tag,
};
};
-const randomAutomated = function(opt_params) {
- return Object.assign({tag: 'autogenerated:gerrit:replace'},
- randomMessage(opt_params));
-};
-
function generateRandomMessages(count) {
return new Array(count).fill()
.map(() => randomMessage());
}
-function generateRandomAutomatedMessages(count) {
- return new Array(count).fill()
- .map(() => randomAutomated());
-}
-
-// Returns a shuffled copy of array
-export function shuffle(arr) {
- const result = [];
- for (const item of arr) {
- // Random number in the interval [0..array.length]
- const j = Math.floor(Math.random() * (arr.length + 1));
- if (j < result.length) {
- result.push(result[j]);
- result[j] = item;
- } else {
- result.push(item);
- }
- }
- return result;
-}
-
suite('gr-messages-list tests', () => {
let element;
let messages;
@@ -94,54 +70,63 @@
return dom(element.root).querySelectorAll('gr-message');
};
+ const MESSAGE_ID_0 = '1234ccc949c6d482b061be6a28e10782abf0e7af';
+ const MESSAGE_ID_1 = '8c19ccc949c6d482b061be6a28e10782abf0e7af';
+ const MESSAGE_ID_2 = 'e7bfdbc842f6b6d8064bc68e0f52b673f40c0ca5';
+
const author = {
_account_id: 42,
name: 'Marvin the Paranoid Android',
email: 'marvin@sirius.org',
};
+ const createComment = function() {
+ return {
+ id: '1a2b3c4d',
+ message: 'some random test text',
+ change_message_id: '8a7b6c5d',
+ updated: '2016-01-01 01:02:03.000000000',
+ line: 1,
+ patch_set: 1,
+ author,
+ };
+ };
+
const comments = {
file1: [
{
- message: 'message text',
- updated: '2016-09-27 00:18:03.000000000',
+ ...createComment(),
+ change_message_id: MESSAGE_ID_0,
in_reply_to: '6505d749_f0bec0aa',
- line: 62,
- id: '6505d749_10ed44b2',
- patch_set: 2,
author: {
email: 'some@email.com',
_account_id: 123,
},
},
{
- message: 'message text',
- updated: '2016-09-27 00:18:03.000000000',
+ ...createComment(),
+ id: '2b3c4d5e',
+ change_message_id: MESSAGE_ID_1,
in_reply_to: 'c5912363_6b820105',
- line: 42,
- id: '450a935e_0f1c05db',
- patch_set: 2,
- author,
},
{
- message: 'message text',
- updated: '2016-09-27 00:18:03.000000000',
+ ...createComment(),
+ id: '2b3c4d5e',
+ change_message_id: MESSAGE_ID_1,
in_reply_to: '6505d749_f0bec0aa',
- line: 62,
- id: '6505d749_10ed44b2',
- patch_set: 2,
- author,
+ },
+ {
+ ...createComment(),
+ id: '34ed05d749_10ed44b2',
+ change_message_id: MESSAGE_ID_2,
},
],
file2: [
{
- message: 'message text',
- updated: '2016-09-27 00:18:03.000000000',
+ ...createComment(),
+ change_message_id: MESSAGE_ID_1,
in_reply_to: 'c5912363_4b7d450a',
- line: 132,
id: '450a935e_4f260d25',
- patch_set: 2,
- author,
},
],
};
@@ -168,126 +153,6 @@
return commentApiWrapper.loadComments();
});
- test('show some old messages', () => {
- assert.isTrue(element.$.messageControlsContainer.hasAttribute('hidden'));
- element.messages = generateRandomMessages(26);
- flushAsynchronousOperations();
-
- assert.isFalse(element.$.messageControlsContainer.hasAttribute('hidden'));
- assert.equal(getMessages().length, 20);
- assert.equal(element.$.incrementMessagesBtn.innerText.toUpperCase()
- .trim(), 'SHOW 5 MORE');
- MockInteractions.tap(element.$.incrementMessagesBtn);
- flushAsynchronousOperations();
-
- assert.equal(getMessages().length, 25);
- assert.equal(element.$.incrementMessagesBtn.innerText.toUpperCase()
- .trim(), 'SHOW 1 MORE');
- MockInteractions.tap(element.$.incrementMessagesBtn);
- flushAsynchronousOperations();
-
- assert.isTrue(element.$.messageControlsContainer.hasAttribute('hidden'));
- assert.equal(getMessages().length, 26);
- });
-
- test('show all old messages', () => {
- assert.isTrue(element.$.messageControlsContainer.hasAttribute('hidden'));
- element.messages = generateRandomMessages(26);
- flushAsynchronousOperations();
-
- assert.isFalse(element.$.messageControlsContainer.hasAttribute('hidden'));
- assert.equal(getMessages().length, 20);
- assert.equal(element.$.oldMessagesBtn.innerText.toUpperCase(),
- 'SHOW ALL 6 MESSAGES');
- MockInteractions.tap(element.$.oldMessagesBtn);
- flushAsynchronousOperations();
-
- assert.equal(getMessages().length, 26);
- assert.isTrue(element.$.messageControlsContainer.hasAttribute('hidden'));
- });
-
- test('message count respects automated', () => {
- element.messages = generateRandomAutomatedMessages(10)
- .concat(generateRandomMessages(11));
- flushAsynchronousOperations();
-
- assert.equal(element.$.oldMessagesBtn.innerText.toUpperCase(),
- 'SHOW 1 MESSAGE');
- assert.isFalse(element.$.messageControlsContainer.hasAttribute('hidden'));
- MockInteractions.tap(element.$.automatedMessageToggle);
- flushAsynchronousOperations();
-
- assert.isTrue(element.$.messageControlsContainer.hasAttribute('hidden'));
- });
-
- test('message count still respects non-automated on toggle', () => {
- element.messages = generateRandomMessages(10)
- .concat(generateRandomAutomatedMessages(11));
- flushAsynchronousOperations();
-
- assert.equal(element.$.oldMessagesBtn.innerText.toUpperCase(),
- 'SHOW 1 MESSAGE');
- assert.isFalse(element.$.messageControlsContainer.hasAttribute('hidden'));
- MockInteractions.tap(element.$.automatedMessageToggle);
- flushAsynchronousOperations();
-
- assert.equal(element.$.oldMessagesBtn.innerText.toUpperCase(),
- 'SHOW 1 MESSAGE');
- assert.isFalse(element.$.messageControlsContainer.hasAttribute('hidden'));
- });
-
- test('show all messages respects expand', () => {
- element.messages = generateRandomAutomatedMessages(10)
- .concat(generateRandomMessages(11));
- flushAsynchronousOperations();
-
- MockInteractions.tap(element.shadowRoot
- .querySelector('#collapse-messages')); // Expand all.
- flushAsynchronousOperations();
-
- let messages = getMessages();
- assert.equal(messages.length, 20);
- for (const message of messages) {
- assert.isTrue(message._expanded);
- }
-
- MockInteractions.tap(element.$.oldMessagesBtn);
- flushAsynchronousOperations();
-
- messages = getMessages();
- assert.equal(messages.length, 21);
- for (const message of messages) {
- assert.isTrue(message._expanded);
- }
- });
-
- test('show all messages respects collapse', () => {
- element.messages = generateRandomAutomatedMessages(10)
- .concat(generateRandomMessages(11));
- flushAsynchronousOperations();
-
- MockInteractions.tap(element.shadowRoot
- .querySelector('#collapse-messages')); // Expand all.
- MockInteractions.tap(element.shadowRoot
- .querySelector('#collapse-messages')); // Collapse all.
- flushAsynchronousOperations();
-
- let messages = getMessages();
- assert.equal(messages.length, 20);
- for (const message of messages) {
- assert.isFalse(message._expanded);
- }
-
- MockInteractions.tap(element.$.oldMessagesBtn);
- flushAsynchronousOperations();
-
- messages = getMessages();
- assert.equal(messages.length, 21);
- for (const message of messages) {
- assert.isFalse(message._expanded);
- }
- });
-
test('expand/collapse all', () => {
let allMessageEls = getMessages();
for (const message of allMessageEls) {
@@ -329,9 +194,11 @@
assert.isTrue([...getMessages()].filter(m => m._expanded).length == 0);
});
- test('hide messages does not appear when no automated messages', () => {
+ test('showAllActivity does not appear when all msgs are important', () => {
assert.isOk(element.shadowRoot
- .querySelector('#automatedMessageToggleContainer[hidden]'));
+ .querySelector('#showAllActivityToggleContainer'));
+ assert.isNotOk(element.shadowRoot
+ .querySelector('.showAllActivityToggle'));
});
test('scroll to message', () => {
@@ -373,14 +240,13 @@
element.scrollToMessage(messageID);
assert.isTrue(scrollToStub.calledOnce);
assert.isTrue(highlightStub.calledOnce);
- assert.equal(element._visibleMessages.length, 24);
assert.isTrue(
element.shadowRoot
.querySelector('[data-message-id="' + messageID + '"]')
._expanded);
});
- test('messages', () => {
+ test('associating messages with comments', () => {
const messages = [].concat(
randomMessage(),
{
@@ -401,26 +267,156 @@
}
);
element.messages = messages;
- const isAuthor = function(author, message) {
- return message.author._account_id === author._account_id;
- };
- const isMarvin = isAuthor.bind(null, author);
flushAsynchronousOperations();
const messageElements = getMessages();
assert.equal(messageElements.length, messages.length);
assert.deepEqual(messageElements[1].message, messages[1]);
assert.deepEqual(messageElements[2].message, messages[2]);
- assert.deepEqual(messageElements[1].comments.file1,
- comments.file1.filter(isMarvin).map(c => {
- return {...c,
- path: 'file1'};
- }));
- assert.deepEqual(messageElements[1].comments.file2,
- comments.file2.filter(isMarvin).map(c => {
- return {...c,
- path: 'file2'};
- }));
- assert.deepEqual(messageElements[2].comments, {});
+ });
+
+ test('threads', () => {
+ const messages = [
+ {
+ _index: 5,
+ _revision_number: 4,
+ message: 'Uploaded patch set 4.',
+ date: '2016-09-28 13:36:33.000000000',
+ author,
+ id: '8c19ccc949c6d482b061be6a28e10782abf0e7af',
+ },
+ ];
+ element.messages = messages;
+ flushAsynchronousOperations();
+ const messageElements = getMessages();
+ // threads
+ assert.equal(
+ messageElements[0].message.commentThreads.length,
+ 3);
+ // first thread contains 1 comment
+ assert.equal(
+ messageElements[0].message.commentThreads[0].comments.length,
+ 1);
+ });
+
+ test('updateTag human message', () => {
+ const m = randomMessage();
+ assert.equal(TEST_ONLY.computeTag(m), undefined);
+ });
+
+ test('updateTag nothing to change', () => {
+ const m = randomMessage();
+ const tag = 'something-normal';
+ m.tag = tag;
+ assert.equal(TEST_ONLY.computeTag(m), tag);
+ });
+
+ test('updateTag TAG_NEW_WIP_PATCHSET', () => {
+ const m = randomMessage();
+ m.tag = MessageTag.TAG_NEW_WIP_PATCHSET;
+ assert.equal(TEST_ONLY.computeTag(m), MessageTag.TAG_NEW_PATCHSET);
+ });
+
+ test('updateTag remove postfix', () => {
+ const m = randomMessage();
+ m.tag = 'something~withpostfix';
+ assert.equal(TEST_ONLY.computeTag(m), 'something');
+ });
+
+ test('updateTag with robot comments', () => {
+ const m = randomMessage();
+ m.commentThreads = [{
+ comments: [{
+ robot_id: 'id314',
+ change_message_id: m.id,
+ }],
+ }];
+ assert.notEqual(TEST_ONLY.computeTag(m), undefined);
+ });
+
+ test('setRevisionNumber nothing to change', () => {
+ const m1 = randomMessage();
+ const m2 = randomMessage();
+ assert.equal(TEST_ONLY.computeRevision(m1, [m1, m2]), 1);
+ assert.equal(TEST_ONLY.computeRevision(m2, [m1, m2]), 1);
+ });
+
+ test('setRevisionNumber reviewer updates', () => {
+ const m1 = randomMessage(
+ {
+ tag: MessageTag.TAG_REVIEWER_UPDATE,
+ date: '2020-01-01 10:00:00.000000000',
+ });
+ m1._revision_number = undefined;
+ const m2 = randomMessage(
+ {
+ date: '2020-01-02 10:00:00.000000000',
+ });
+ m2._revision_number = 1;
+ const m3 = randomMessage(
+ {
+ tag: MessageTag.TAG_REVIEWER_UPDATE,
+ date: '2020-01-03 10:00:00.000000000',
+ });
+ m3._revision_number = undefined;
+ const m4 = randomMessage(
+ {
+ date: '2020-01-04 10:00:00.000000000',
+ });
+ m4._revision_number = 2;
+ const m5 = randomMessage(
+ {
+ tag: MessageTag.TAG_REVIEWER_UPDATE,
+ date: '2020-01-05 10:00:00.000000000',
+ });
+ m5._revision_number = undefined;
+ const allMessages = [m1, m2, m3, m4, m5];
+ assert.equal(TEST_ONLY.computeRevision(m1, allMessages), undefined);
+ assert.equal(TEST_ONLY.computeRevision(m2, allMessages), 1);
+ assert.equal(TEST_ONLY.computeRevision(m3, allMessages), 1);
+ assert.equal(TEST_ONLY.computeRevision(m4, allMessages), 2);
+ assert.equal(TEST_ONLY.computeRevision(m5, allMessages), 2);
+ });
+
+ test('isImportant human message', () => {
+ const m = randomMessage();
+ assert.isTrue(TEST_ONLY.computeIsImportant(m, []));
+ assert.isTrue(TEST_ONLY.computeIsImportant(m, [m]));
+ });
+
+ test('isImportant even with a tag', () => {
+ const m1 = randomMessage();
+ const m2 = randomMessage({tag: 'autogenerated:gerrit1'});
+ const m3 = randomMessage({tag: 'autogenerated:gerrit2'});
+ assert.isTrue(TEST_ONLY.computeIsImportant(m2, []));
+ assert.isTrue(TEST_ONLY.computeIsImportant(m1, [m1, m2, m3]));
+ assert.isTrue(TEST_ONLY.computeIsImportant(m2, [m1, m2, m3]));
+ assert.isTrue(TEST_ONLY.computeIsImportant(m3, [m1, m2, m3]));
+ });
+
+ test('isImportant filters same tag and older revision', () => {
+ const m1 = randomMessage({tag: 'auto', _revision_number: 2});
+ const m2 = randomMessage({tag: 'auto', _revision_number: 1});
+ const m3 = randomMessage({tag: 'auto'});
+ assert.isTrue(TEST_ONLY.computeIsImportant(m1, [m1]));
+ assert.isTrue(TEST_ONLY.computeIsImportant(m2, [m2]));
+ assert.isTrue(TEST_ONLY.computeIsImportant(m1, [m1, m2]));
+ assert.isFalse(TEST_ONLY.computeIsImportant(m2, [m1, m2]));
+ assert.isTrue(TEST_ONLY.computeIsImportant(m1, [m1, m3]));
+ assert.isFalse(TEST_ONLY.computeIsImportant(m3, [m1, m3]));
+ assert.isTrue(TEST_ONLY.computeIsImportant(m1, [m1, m2, m3]));
+ assert.isFalse(TEST_ONLY.computeIsImportant(m2, [m1, m2, m3]));
+ assert.isFalse(TEST_ONLY.computeIsImportant(m3, [m1, m2, m3]));
+ });
+
+ test('isImportant is evaluated after tag update', () => {
+ const m1 = randomMessage(
+ {tag: MessageTag.TAG_NEW_PATCHSET, _revision_number: 1});
+ const m2 = randomMessage(
+ {tag: MessageTag.TAG_NEW_WIP_PATCHSET, _revision_number: 2});
+ element.messages = [m1, m2];
+ flushAsynchronousOperations();
+ assert.isFalse(m1.isImportant);
+ assert.isTrue(m2.isImportant);
});
test('messages without author do not throw', () => {
@@ -437,18 +433,6 @@
assert.equal(messageEls.length, 1);
assert.equal(messageEls[0].message.message, messages[0].message);
});
-
- test('hide increment text if increment >= total remaining', () => {
- // Test with stubbed return values, as _numRemaining and _getDelta have
- // their own tests.
- sinon.stub(element, '_getDelta').returns(5);
- const remainingStub = sinon.stub(element, '_numRemaining').returns(6);
- assert.isFalse(element._computeIncrementHidden(null, null, null));
- remainingStub.restore();
-
- sinon.stub(element, '_numRemaining').returns(4);
- assert.isTrue(element._computeIncrementHidden(null, null, null));
- });
});
suite('gr-messages-list automate tests', () => {
@@ -457,18 +441,6 @@
let commentApiWrapper;
- const getMessages = function() {
- return dom(element.root).querySelectorAll('gr-message');
- };
- const getHiddenMessages = function() {
- return dom(element.root).querySelectorAll('gr-message[hidden]');
- };
-
- const randomMessageReviewer = {
- reviewer: {},
- date: '2016-01-13 20:30:33.038000',
- };
-
setup(() => {
stub('gr-rest-api-interface', {
getConfig() { return Promise.resolve({}); },
@@ -478,8 +450,11 @@
getDiffDrafts() { return Promise.resolve({}); },
});
- messages = generateRandomAutomatedMessages(2);
- messages.push(randomMessageReviewer);
+ messages = [
+ randomMessage(),
+ randomMessage({tag: 'auto', _revision_number: 2}),
+ randomMessage({tag: 'auto', _revision_number: 3}),
+ ];
// Element must be wrapped in an element with direct access to the
// comment API.
@@ -494,89 +469,33 @@
});
test('hide autogenerated button is not hidden', () => {
- assert.isNotOk(element.shadowRoot
- .querySelector('#automatedMessageToggle[hidden]'));
+ const toggle = dom(element.root).querySelector('.showAllActivityToggle');
+ assert.isOk(toggle);
});
- test('autogenerated messages are not hidden initially', () => {
- const allHiddenMessageEls = getHiddenMessages();
-
- // There are no hidden messages.
- assert.isFalse(!!allHiddenMessageEls.length);
+ test('one unimportant message is hidden initially', () => {
+ const displayedMsgs = dom(element.root).querySelectorAll('gr-message');
+ assert.equal(displayedMsgs.length, 2);
});
- test('autogenerated messages hidden after comments only toggle', () => {
- let allHiddenMessageEls = getHiddenMessages();
-
- element._hideAutomated = false;
- MockInteractions.tap(element.$.automatedMessageToggle);
+ test('unimportant messages hidden after toggle', () => {
+ element._showAllActivity = true;
+ const toggle = dom(element.root).querySelector('.showAllActivityToggle');
+ assert.isOk(toggle);
+ MockInteractions.tap(toggle);
flushAsynchronousOperations();
- const allMessageEls = getMessages();
- allHiddenMessageEls = getHiddenMessages();
-
- // Autogenerated messages are now hidden.
- assert.equal(allHiddenMessageEls.length, allMessageEls.length);
+ const displayedMsgs = dom(element.root).querySelectorAll('gr-message');
+ assert.equal(displayedMsgs.length, 2);
});
- test('autogenerated messages not hidden after comments only toggle',
- () => {
- let allHiddenMessageEls = getHiddenMessages();
-
- element._hideAutomated = true;
- MockInteractions.tap(element.$.automatedMessageToggle);
- allHiddenMessageEls = getHiddenMessages();
-
- // Autogenerated messages are now hidden.
- assert.isFalse(!!allHiddenMessageEls.length);
- });
-
- test('_getDelta', () => {
- let messages = [randomMessage()];
- assert.equal(element._getDelta([], messages, false), 1);
- assert.equal(element._getDelta([], messages, true), 1);
-
- messages = generateRandomMessages(7);
- assert.equal(element._getDelta([], messages, false), 5);
- assert.equal(element._getDelta([], messages, true), 5);
-
- messages = generateRandomMessages(4)
- .concat(generateRandomAutomatedMessages(2))
- .concat(generateRandomMessages(3));
-
- const dummyArr = generateRandomMessages(2);
- assert.equal(element._getDelta(dummyArr, messages, false), 5);
- assert.equal(element._getDelta(dummyArr, messages, true), 7);
- });
-
- test('_getHumanMessages', () => {
- assert.equal(
- element._getHumanMessages(
- generateRandomAutomatedMessages(5)).length, 0);
- assert.equal(
- element._getHumanMessages(generateRandomMessages(5)).length, 5);
-
- let messages = shuffle(generateRandomMessages(5)
- .concat(generateRandomAutomatedMessages(5)));
- messages = element._getHumanMessages(messages);
- assert.equal(messages.length, 5);
- assert.isFalse(element._hasAutomatedMessages(messages));
- });
-
- test('initially show only 20 messages', () => {
- sinon.stub(element.reporting, 'reportInteraction').callsFake(
- (eventName, details) => {
- assert.equal(typeof(eventName), 'string');
- if (details) {
- assert.equal(typeof(details), 'object');
- }
- });
- const messages = Array.from(Array(23).keys())
- .map(() => {
- return {};
- });
- element._processedMessagesChanged(messages);
-
- assert.equal(element._visibleMessages.length, 20);
+ test('unimportant messages shown after toggle', () => {
+ element._showAllActivity = false;
+ const toggle = dom(element.root).querySelector('.showAllActivityToggle');
+ assert.isOk(toggle);
+ MockInteractions.tap(toggle);
+ flushAsynchronousOperations();
+ const displayedMsgs = dom(element.root).querySelectorAll('gr-message');
+ assert.equal(displayedMsgs.length, 3);
});
test('_computeLabelExtremes', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
index 24bc142..b5d40d2 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
@@ -28,7 +28,6 @@
import '../gr-label-scores/gr-label-scores.js';
import '../gr-thread-list/gr-thread-list.js';
import '../../../styles/shared-styles.js';
-import '../gr-comment-list/gr-comment-list.js';
import {mixinBehaviors} from '@polymer/polymer/lib/legacy/class.js';
import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners.js';
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
diff --git a/polygerrit-ui/app/services/flags.js b/polygerrit-ui/app/services/flags.js
index 64f0115..6313255 100644
--- a/polygerrit-ui/app/services/flags.js
+++ b/polygerrit-ui/app/services/flags.js
@@ -20,7 +20,6 @@
* @desc Experiment ids used in Gerrit.
*/
export const ExperimentIds = {
- CLEANER_CHANGELOG: 'UiFeature__cleaner_changelog',
PATCHSET_COMMENTS: 'UiFeature__patchset_comments',
};
