diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -2969,34 +2969,62 @@ $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; - } - $header = $xaction->getTitleForMail(); - if ($header !== null) { - $headers[] = $header; - } + // 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. - $header_html = $xaction->getTitleForHTMLMail(); - if ($header_html !== null) { - $headers_html[] = $header_html; - } + // 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) { - $comments[] = $comment; + $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; + } + + $header_html = $xaction->getTitleForHTMLMail(); + if ($header_html !== null) { + $headers_html[] = $header_html; + } } if ($xaction->hasChangeDetailsForMail()) { $details[] = $xaction; } + + if ($is_comment) { + $seen_comment = true; + } } $headers_text = implode("\n", $headers); @@ -3029,8 +3057,7 @@ $object_label); } - $xactions_style = array( - ); + $xactions_style = array(); $header_action = phutil_tag( 'td', @@ -3057,7 +3084,25 @@ $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); }