1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-09-20 09:18:48 +02:00

When mail (like "!history" mail) has multiple comments, label them separately

Summary:
Depends on D19372. Ref T13124. See PHI505. Currently, if you `!history` a task with a lot of comments, you get output like this:

> alice added a comment.
> bailey added a comment.
> alice added a comment.
> alice added a comment.
>
> AAAA
>
> BBBB
>
> AAAA
>
> AAAA

This is impossible to read. Put the "alice added a comment." headers above the actual comments for comments after the first.

These types of mail messages are unusual, but occur in several cases:

  - The new `!history` command.
  - Multiple comments on a draft revision before it promotes out of draft.
  - (Probably?) Conduit API updates which submit multiple comment transactions for some reason.

Test Plan: Used `bin/mail receive-test` to send a `!history` command to a task, saw a much more readable rendering of the transaction log in the resulting email.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13124

Differential Revision: https://secure.phabricator.com/D19373
This commit is contained in:
epriestley 2018-04-16 07:49:47 -07:00
parent 25965260c4
commit f9b3673fbb

View file

@ -2969,16 +2969,44 @@ abstract class PhabricatorApplicationTransactionEditor
$object_label = null,
$object_href = null) {
// First, remove transactions which shouldn't be rendered in mail.
foreach ($xactions as $key => $xaction) {
if ($xaction->shouldHideForMail($xactions)) {
unset($xactions[$key]);
}
}
$headers = array();
$headers_html = array();
$comments = array();
$details = array();
$seen_comment = false;
foreach ($xactions as $xaction) {
if ($xaction->shouldHideForMail($xactions)) {
continue;
// Most mail has zero or one comments. In these cases, we render the
// "alice added a comment." transaction in the header, like a normal
// transaction.
// Some mail, like Differential undraft mail or "!history" mail, may
// have two or more comments. In these cases, we'll put the first
// "alice added a comment." transaction in the header normally, but
// move the other transactions down so they provide context above the
// actual comment.
$comment = $xaction->getBodyForMail();
if ($comment !== null) {
$is_comment = true;
$comments[] = array(
'xaction' => $xaction,
'comment' => $comment,
'initial' => !$seen_comment,
);
} else {
$is_comment = false;
}
if (!$is_comment || !$seen_comment) {
$header = $xaction->getTitleForMail();
if ($header !== null) {
$headers[] = $header;
@ -2988,15 +3016,15 @@ abstract class PhabricatorApplicationTransactionEditor
if ($header_html !== null) {
$headers_html[] = $header_html;
}
$comment = $xaction->getBodyForMail();
if ($comment !== null) {
$comments[] = $comment;
}
if ($xaction->hasChangeDetailsForMail()) {
$details[] = $xaction;
}
if ($is_comment) {
$seen_comment = true;
}
}
$headers_text = implode("\n", $headers);
@ -3029,8 +3057,7 @@ abstract class PhabricatorApplicationTransactionEditor
$object_label);
}
$xactions_style = array(
);
$xactions_style = array();
$header_action = phutil_tag(
'td',
@ -3057,7 +3084,25 @@ abstract class PhabricatorApplicationTransactionEditor
$body->addRawHTMLSection($headers_html);
foreach ($comments as $comment) {
foreach ($comments as $spec) {
$xaction = $spec['xaction'];
$comment = $spec['comment'];
$is_initial = $spec['initial'];
// If this is not the first comment in the mail, add the header showing
// who wrote the comment immediately above the comment.
if (!$is_initial) {
$header = $xaction->getTitleForMail();
if ($header !== null) {
$body->addRawPlaintextSection($header);
}
$header_html = $xaction->getTitleForHTMLMail();
if ($header_html !== null) {
$body->addRawHTMLSection($header_html);
}
}
$body->addRemarkupSection(null, $comment);
}