Revert "Make messages more conversational"
This reverts commit 512f4048219d664a31762c40e4a646187ca76309.
Reason for revert: internal issue
Change-Id: I6532dffb85a892321c8d8ead2a194708396de92d
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.html b/polygerrit-ui/app/elements/change/gr-message/gr-message.html
index 5608dec..00157ab 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.html
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.html
@@ -178,34 +178,20 @@
<gr-account-label
account="[[author]]"
hide-avatar></gr-account-label>
- <template is="dom-if" if="[[_successfulParse]]">
- <template is="dom-if" if="[[_parsedVotes.length]]">voted</template>
- <template is="dom-repeat" items="[[_parsedVotes]]" as="score">
- <span class$="score [[_computeScoreClass(score, labelExtremes)]]">
- [[score.label]] [[score.value]]
- </span>
- </template>
- [[_computeConversationalString(_parsedVotes, _parsedPatchNum, _parsedCommentCount)]]
+ <template is="dom-repeat" items="[[_getScores(message)]]" as="score">
+ <span class$="score [[_computeScoreClass(score, labelExtremes)]]">
+ [[score.label]] [[score.value]]
+ </span>
</template>
</div>
<template is="dom-if" if="[[message.message]]">
<div class="content">
- <template is="dom-if" if="[[_successfulParse]]">
- <div class="message hideOnOpen">[[_parsedChangeMessage]]</div>
- <gr-formatted-text
- no-trailing-margin
- class="message hideOnCollapsed"
- content="[[_parsedChangeMessage]]"
- config="[[_projectConfig.commentlinks]]"></gr-formatted-text>
- </template>
- <template is="dom-if" if="[[!_successfulParse]]">
- <div class="message hideOnOpen">[[message.message]]</div>
- <gr-formatted-text
- no-trailing-margin
- class="message hideOnCollapsed"
- content="[[message.message]]"
- config="[[_projectConfig.commentlinks]]"></gr-formatted-text>
- </template>
+ <div class="message hideOnOpen">[[message.message]]</div>
+ <gr-formatted-text
+ no-trailing-margin
+ class="message hideOnCollapsed"
+ content="[[message.message]]"
+ config="[[_projectConfig.commentlinks]]"></gr-formatted-text>
<div class="replyContainer" hidden$="[[!showReplyButton]]" hidden>
<gr-button link small on-tap="_handleReplyTap">Reply</gr-button>
</div>
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 addd660..0590c73 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.js
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
@@ -17,8 +17,7 @@
(function() {
'use strict';
- const PATCH_SET_PREFIX_PATTERN = /^Patch Set (\d)+:[ ]?/;
- const COMMENTS_COUNT_PATTERN = /^\((\d+)( inline)? comments?\)$/;
+ const PATCH_SET_PREFIX_PATTERN = /^Patch Set \d+: /;
const LABEL_TITLE_SCORE_PATTERN = /^([A-Za-z0-9-]+)([+-]\d+)$/;
Polymer({
@@ -102,17 +101,10 @@
type: Boolean,
value: false,
},
-
- _parsedPatchNum: String,
- _parsedCommentCount: String,
- _parsedVotes: Array,
- _parsedChangeMessage: String,
- _successfulParse: Boolean,
},
observers: [
'_updateExpandedClass(message.expanded)',
- '_consumeMessage(message.message)',
],
ready() {
@@ -192,6 +184,19 @@
return event.type === 'REVIEWER_UPDATE';
},
+ _getScores(message) {
+ if (!message.message) { return []; }
+ const line = message.message.split('\n', 1)[0];
+ const patchSetPrefix = PATCH_SET_PREFIX_PATTERN;
+ if (!line.match(patchSetPrefix)) { return []; }
+ const scoresRaw = line.split(patchSetPrefix)[1];
+ if (!scoresRaw) { return []; }
+ return scoresRaw.split(' ')
+ .map(s => s.match(LABEL_TITLE_SCORE_PATTERN))
+ .filter(ms => ms && ms.length === 3)
+ .map(ms => ({label: ms[1], value: ms[2]}));
+ },
+
_computeScoreClass(score, labelExtremes) {
const classes = [];
if (score.value > 0) {
@@ -255,73 +260,5 @@
e.stopPropagation();
this.set('message.expanded', !this.message.expanded);
},
-
- /**
- * Attempts to consume a change message to create a shorter and more legible
- * format. If the function encounters unexpected characters at any point, it
- * sets the _successfulParse flag to false and terminates, causing the UI to
- * fall back to displaying the entirety of the change message.
- *
- * A successful parse results in a one-liner that reads:
- * `${AVATAR} voted ${VOTES} and left ${NUM} comment(s) on ${PATCHSET}`
- *
- * @param {string} text
- */
- _consumeMessage(text) {
- this._parsedPatchNum = '';
- this._parsedCommentCount = '';
- this._parsedChangeMessage = '';
- this._parsedVotes = [];
- if (!text) {
- // No message body means nothing to parse.
- this._successfulParse = false;
- return;
- }
- const lines = text.split('\n');
- const messageLines = lines.shift().split(PATCH_SET_PREFIX_PATTERN);
- if (!messageLines[1]) {
- // Message is in an unexpected format.
- this._successfulParse = false;
- return;
- }
- this._parsedPatchNum = messageLines[1];
- if (messageLines[2]) {
- // Content after the colon is always vote information. If it is in the
- // most up to date schema, parse it. Otherwise, cancel the parsing
- // completely.
- let match;
- for (const score of messageLines[2].split(' ')) {
- match = score.match(LABEL_TITLE_SCORE_PATTERN);
- if (!match || match.length !== 3) {
- this._successfulParse = false;
- return;
- }
- this._parsedVotes.push({label: match[1], value: match[2]});
- }
- }
- // Remove empty line.
- lines.shift();
- if (lines.length) {
- const commentMatch = lines[0].match(COMMENTS_COUNT_PATTERN);
- if (commentMatch) {
- this._parsedCommentCount = commentMatch[1];
- // Remove comment line and the following empty line.
- lines.splice(0, 2);
- }
- this._parsedChangeMessage = lines.join('\n');
- }
- this._successfulParse = true;
- },
-
- _computeConversationalString(votes, patchNum, commentCount) {
- let clause = ' on Patch Set ' + patchNum;
- if (commentCount) {
- let commentStr = ' comment';
- if (parseInt(commentCount, 10) > 1) { commentStr += 's'; }
- clause = ' left ' + commentCount + commentStr + clause;
- if (votes.length) { clause = ' and' + clause; }
- }
- return clause;
- },
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
index 6bb4618..870f366 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
@@ -169,9 +169,7 @@
author: {},
expanded: false,
message: 'Patch Set 1: Verified+1 Code-Review-2 Trybot-Ready+1',
- _revision_number: 1,
};
- element.comments = {};
element.labelExtremes = {
'Verified': {max: 1, min: -1},
'Code-Review': {max: 2, min: -2},
@@ -201,145 +199,5 @@
const scoreChips = Polymer.dom(element.root).querySelectorAll('.score');
assert.equal(scoreChips.length, 0);
});
-
- suite('_consumeMessage', () => {
- const assertConsumeFailed = str => {
- element._consumeMessage(str);
- assert.isFalse(element._successfulParse);
- };
-
- test('no message body', () => {
- assertConsumeFailed('');
- });
-
- test('known old schema', () => {
- const str = 'Patch Set 1: Looks good to me, approved; Verified';
- assertConsumeFailed(str);
- });
-
- test('known old schema 2', () => {
- const str = [
- 'Patch Set 2: Looks good to me',
- '',
- 'Patch set 2 compiles, and runs as expected.',
- ].join('\n');
- assertConsumeFailed(str);
- });
-
- test('known old schema 3', () => {
- const str = 'Patch Set 2: (1 inline comment)';
- assertConsumeFailed(str);
- });
-
- test('known old schema 4', () => {
- const str = [
- 'Patch Set 2: Looks good to me',
- '',
- '(1 inline comment)',
- ].join('\n');
- assertConsumeFailed(str);
- });
-
- test('just change message', () => {
- const str = [
- 'Patch Set 2:',
- '',
- 'I think you should reconsider this approach.',
- 'It really makes no sense.',
- ].join('\n');
-
- element._consumeMessage(str);
-
- assert.isTrue(element._successfulParse);
- assert.equal(element._parsedPatchNum, '2');
- assert.equal(element._parsedCommentCount, '');
- assert.deepEqual(element._parsedVotes, []);
- assert.equal(element._parsedChangeMessage, [
- 'I think you should reconsider this approach.',
- 'It really makes no sense.',
- ].join('\n'));
- });
-
- test('just votes', () => {
- element._consumeMessage('Patch Set 2: Code-Review-Label+1 Verified+1');
-
- assert.isTrue(element._successfulParse);
- assert.equal(element._parsedPatchNum, '2');
- assert.equal(element._parsedCommentCount, '');
- assert.deepEqual(element._parsedVotes, [
- {label: 'Code-Review-Label', value: '+1'},
- {label: 'Verified', value: '+1'},
- ]);
- assert.equal(element._parsedChangeMessage, '');
- });
-
- test('just comments', () => {
- const str = [
- 'Patch Set 2:',
- '',
- '(8 comments)',
- ].join('\n');
-
- element._consumeMessage(str);
-
- assert.isTrue(element._successfulParse);
- assert.equal(element._parsedPatchNum, '2');
- assert.equal(element._parsedCommentCount, '8');
- assert.deepEqual(element._parsedVotes, []);
- assert.equal(element._parsedChangeMessage, '');
- });
-
- test('vote with comments and change message', () => {
- const str = [
- 'Patch Set 2: Code-Review-Label+1 Verified+1',
- '',
- '(1 comment)',
- '',
- 'LGTM',
- '',
- 'Nice work, just one nit.',
- ].join('\n');
-
- element._consumeMessage(str);
-
- assert.isTrue(element._successfulParse);
- assert.equal(element._parsedPatchNum, '2');
- assert.equal(element._parsedCommentCount, '1');
- assert.deepEqual(element._parsedVotes, [
- {label: 'Code-Review-Label', value: '+1'},
- {label: 'Verified', value: '+1'},
- ]);
- assert.equal(element._parsedChangeMessage, [
- 'LGTM',
- '',
- 'Nice work, just one nit.',
- ].join('\n'));
- });
-
- test('vote with change message', () => {
- const str = [
- 'Patch Set 2: Code-Review-Label+2 Verified+1',
- '',
- 'LGTM',
- '',
- 'Nice work.',
- ].join('\n');
-
- element._consumeMessage(str);
-
- assert.isTrue(element._successfulParse);
- assert.equal(element._parsedPatchNum, '2');
- assert.equal(element._parsedCommentCount, '');
- assert.deepEqual(element._parsedVotes, [
- {label: 'Code-Review-Label', value: '+2'},
- {label: 'Verified', value: '+1'},
- ]);
- assert.equal(element._parsedChangeMessage, [
- 'LGTM',
- '',
- 'Nice work.',
- ].join('\n'));
- });
- });
});
</script>
