mirror of
https://we.phorge.it/source/phorge.git
synced 2025-01-18 18:51:12 +01:00
Use standard infrastructure for Audit email generation
Summary: Ref T4896. Replace custom stuff with standard stuff. Test Plan: - Sent a bunch of email and it all looked sensible/correct. - Made sure to test inlines, specifically, as they're a bit tricky. Reviewers: joshuaspence, btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T4896 Differential Revision: https://secure.phabricator.com/D10112
This commit is contained in:
epriestley
parent
b787d3ef0d
commit
64736264a6
3 changed files with 173 additions and 222 deletions
|
|
@ -217,6 +217,8 @@ final class PhabricatorAuditCommentEditor extends PhabricatorEditor {
|
|||
$commit->updateAuditStatus($requests);
|
||||
$commit->save();
|
||||
|
||||
$commit->attachAudits($requests);
|
||||
|
||||
// Convert old comments into real transactions and apply them with a
|
||||
// normal editor.
|
||||
|
||||
|
|
@ -240,6 +242,8 @@ final class PhabricatorAuditCommentEditor extends PhabricatorEditor {
|
|||
->setContinueOnNoEffect(true)
|
||||
->setContinueOnMissingFields(true)
|
||||
->setContentSource($content_source)
|
||||
->setExcludeMailRecipientPHIDs($this->getExcludeMailRecipientPHIDs())
|
||||
->setDisableEmail($this->noEmail)
|
||||
->applyTransactions($commit, $xactions);
|
||||
|
||||
$feed_dont_publish_phids = array();
|
||||
|
|
@ -262,14 +266,6 @@ final class PhabricatorAuditCommentEditor extends PhabricatorEditor {
|
|||
foreach ($comments as $comment) {
|
||||
$this->publishFeedStory($comment, $feed_phids);
|
||||
}
|
||||
|
||||
if (!$this->noEmail) {
|
||||
$this->sendMail(
|
||||
$comments,
|
||||
$other_comments,
|
||||
$inline_comments,
|
||||
$requests);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -337,217 +333,4 @@ final class PhabricatorAuditCommentEditor extends PhabricatorEditor {
|
|||
->publish();
|
||||
}
|
||||
|
||||
private function sendMail(
|
||||
array $comments,
|
||||
array $other_comments,
|
||||
array $inline_comments,
|
||||
array $requests) {
|
||||
|
||||
assert_instances_of($comments, 'PhabricatorAuditComment');
|
||||
assert_instances_of($other_comments, 'PhabricatorAuditComment');
|
||||
assert_instances_of($inline_comments, 'PhabricatorInlineCommentInterface');
|
||||
|
||||
$commit = $this->commit;
|
||||
|
||||
$data = $commit->loadCommitData();
|
||||
$summary = $data->getSummary();
|
||||
|
||||
$commit_phid = $commit->getPHID();
|
||||
$phids = array($commit_phid);
|
||||
$handles = id(new PhabricatorHandleQuery())
|
||||
->setViewer($this->getActor())
|
||||
->withPHIDs($phids)
|
||||
->execute();
|
||||
$handle = $handles[$commit_phid];
|
||||
|
||||
$name = $handle->getName();
|
||||
|
||||
$map = array(
|
||||
PhabricatorAuditActionConstants::CONCERN => 'Raised Concern',
|
||||
PhabricatorAuditActionConstants::ACCEPT => 'Accepted',
|
||||
PhabricatorAuditActionConstants::RESIGN => 'Resigned',
|
||||
PhabricatorAuditActionConstants::CLOSE => 'Closed',
|
||||
PhabricatorAuditActionConstants::ADD_CCS => 'Added CCs',
|
||||
PhabricatorAuditActionConstants::ADD_AUDITORS => 'Added Auditors',
|
||||
);
|
||||
if ($comments) {
|
||||
$any_action = head($comments)->getAction();
|
||||
} else {
|
||||
$any_action = null;
|
||||
}
|
||||
$verb = idx($map, $any_action, 'Commented On');
|
||||
|
||||
$reply_handler = self::newReplyHandlerForCommit($commit);
|
||||
|
||||
$prefix = PhabricatorEnv::getEnvConfig('metamta.diffusion.subject-prefix');
|
||||
|
||||
$repository = id(new PhabricatorRepositoryQuery())
|
||||
->setViewer($this->getActor())
|
||||
->withIDs(array($commit->getRepositoryID()))
|
||||
->executeOne();
|
||||
$threading = self::getMailThreading($repository, $commit);
|
||||
list($thread_id, $thread_topic) = $threading;
|
||||
|
||||
$body = $this->renderMailBody(
|
||||
$comments,
|
||||
"{$name}: {$summary}",
|
||||
$handle,
|
||||
$reply_handler,
|
||||
$inline_comments);
|
||||
|
||||
$email_to = array();
|
||||
$email_cc = array();
|
||||
|
||||
$email_to[$this->getActor()->getPHID()] = true;
|
||||
|
||||
$author_phid = $data->getCommitDetail('authorPHID');
|
||||
if ($author_phid) {
|
||||
$email_to[$author_phid] = true;
|
||||
}
|
||||
|
||||
foreach ($other_comments as $other_comment) {
|
||||
$email_cc[$other_comment->getActorPHID()] = true;
|
||||
}
|
||||
|
||||
$subscribers = PhabricatorSubscribersQuery::loadSubscribersForPHID(
|
||||
$commit->getPHID());
|
||||
foreach ($subscribers as $subscriber) {
|
||||
$email_cc[$subscriber] = true;
|
||||
}
|
||||
|
||||
foreach ($requests as $request) {
|
||||
switch ($request->getAuditStatus()) {
|
||||
case PhabricatorAuditStatusConstants::AUDIT_REQUIRED:
|
||||
$email_cc[$request->getAuditorPHID()] = true;
|
||||
break;
|
||||
case PhabricatorAuditStatusConstants::RESIGNED:
|
||||
unset($email_cc[$request->getAuditorPHID()]);
|
||||
break;
|
||||
case PhabricatorAuditStatusConstants::CONCERNED:
|
||||
case PhabricatorAuditStatusConstants::AUDIT_REQUESTED:
|
||||
$email_to[$request->getAuditorPHID()] = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
$email_to = array_keys($email_to);
|
||||
$email_cc = array_keys($email_cc);
|
||||
|
||||
$phids = array_merge($email_to, $email_cc);
|
||||
$handles = id(new PhabricatorHandleQuery())
|
||||
->setViewer($this->getActor())
|
||||
->withPHIDs($phids)
|
||||
->execute();
|
||||
|
||||
// NOTE: Always set $is_new to false, because the "first" mail in the
|
||||
// thread is the Herald notification of the commit.
|
||||
$is_new = false;
|
||||
|
||||
$template = id(new PhabricatorMetaMTAMail())
|
||||
->setSubject("{$name}: {$summary}")
|
||||
->setSubjectPrefix($prefix)
|
||||
->setVarySubjectPrefix("[{$verb}]")
|
||||
->setFrom($this->getActor()->getPHID())
|
||||
->setThreadID($thread_id, $is_new)
|
||||
->addHeader('Thread-Topic', $thread_topic)
|
||||
->setRelatedPHID($commit->getPHID())
|
||||
->setExcludeMailRecipientPHIDs($this->getExcludeMailRecipientPHIDs())
|
||||
->setIsBulk(true)
|
||||
->setBody($body);
|
||||
|
||||
$mails = $reply_handler->multiplexMail(
|
||||
$template,
|
||||
array_select_keys($handles, $email_to),
|
||||
array_select_keys($handles, $email_cc));
|
||||
|
||||
foreach ($mails as $mail) {
|
||||
$mail->saveAndSend();
|
||||
}
|
||||
}
|
||||
|
||||
public static function getMailThreading(
|
||||
PhabricatorRepository $repository,
|
||||
PhabricatorRepositoryCommit $commit) {
|
||||
|
||||
return array(
|
||||
'diffusion-audit-'.$commit->getPHID(),
|
||||
'Commit r'.$repository->getCallsign().$commit->getCommitIdentifier(),
|
||||
);
|
||||
}
|
||||
|
||||
public static function newReplyHandlerForCommit($commit) {
|
||||
$reply_handler = PhabricatorEnv::newObjectFromConfig(
|
||||
'metamta.diffusion.reply-handler');
|
||||
$reply_handler->setMailReceiver($commit);
|
||||
return $reply_handler;
|
||||
}
|
||||
|
||||
private function renderMailBody(
|
||||
array $comments,
|
||||
$cname,
|
||||
PhabricatorObjectHandle $handle,
|
||||
PhabricatorMailReplyHandler $reply_handler,
|
||||
array $inline_comments) {
|
||||
|
||||
assert_instances_of($comments, 'PhabricatorAuditComment');
|
||||
assert_instances_of($inline_comments, 'PhabricatorInlineCommentInterface');
|
||||
|
||||
$commit = $this->commit;
|
||||
$actor = $this->getActor();
|
||||
$name = $actor->getUsername();
|
||||
|
||||
$body = new PhabricatorMetaMTAMailBody();
|
||||
foreach ($comments as $comment) {
|
||||
if ($comment->getAction() == PhabricatorAuditActionConstants::INLINE) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$verb = PhabricatorAuditActionConstants::getActionPastTenseVerb(
|
||||
$comment->getAction());
|
||||
|
||||
$body->addRawSection("{$name} {$verb} commit {$cname}.");
|
||||
|
||||
$content = $comment->getContent();
|
||||
if (strlen($content)) {
|
||||
$body->addRawSection($comment->getContent());
|
||||
}
|
||||
}
|
||||
|
||||
if ($inline_comments) {
|
||||
$block = array();
|
||||
|
||||
$path_map = id(new DiffusionPathQuery())
|
||||
->withPathIDs(mpull($inline_comments, 'getPathID'))
|
||||
->execute();
|
||||
$path_map = ipull($path_map, 'path', 'id');
|
||||
|
||||
foreach ($inline_comments as $inline) {
|
||||
$path = idx($path_map, $inline->getPathID());
|
||||
if ($path === null) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$start = $inline->getLineNumber();
|
||||
$len = $inline->getLineLength();
|
||||
if ($len) {
|
||||
$range = $start.'-'.($start + $len);
|
||||
} else {
|
||||
$range = $start;
|
||||
}
|
||||
|
||||
$content = $inline->getContent();
|
||||
$block[] = "{$path}:{$range} {$content}";
|
||||
}
|
||||
|
||||
$body->addTextSection(pht('INLINE COMMENTS'), implode("\n", $block));
|
||||
}
|
||||
|
||||
$body->addTextSection(
|
||||
pht('COMMIT'),
|
||||
PhabricatorEnv::getProductionURI($handle->getURI()));
|
||||
$body->addReplySection($reply_handler->getReplyHandlerInstructions());
|
||||
|
||||
return $body->render();
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -115,4 +115,123 @@ final class PhabricatorAuditEditor
|
|||
return true;
|
||||
}
|
||||
|
||||
protected function shouldSendMail(
|
||||
PhabricatorLiskDAO $object,
|
||||
array $xactions) {
|
||||
return true;
|
||||
}
|
||||
|
||||
protected function buildReplyHandler(PhabricatorLiskDAO $object) {
|
||||
$reply_handler = PhabricatorEnv::newObjectFromConfig(
|
||||
'metamta.diffusion.reply-handler');
|
||||
$reply_handler->setMailReceiver($object);
|
||||
return $reply_handler;
|
||||
}
|
||||
|
||||
protected function getMailSubjectPrefix() {
|
||||
return PhabricatorEnv::getEnvConfig('metamta.diffusion.subject-prefix');
|
||||
}
|
||||
|
||||
protected function getMailThreadID(PhabricatorLiskDAO $object) {
|
||||
// For backward compatibility, use this legacy thread ID.
|
||||
return 'diffusion-audit-'.$object->getPHID();
|
||||
}
|
||||
|
||||
protected function buildMailTemplate(PhabricatorLiskDAO $object) {
|
||||
$identifier = $object->getCommitIdentifier();
|
||||
$repository = $object->getRepository();
|
||||
$monogram = $repository->getMonogram();
|
||||
|
||||
$summary = $object->getSummary();
|
||||
$name = $repository->formatCommitName($identifier);
|
||||
|
||||
$subject = "{$name}: {$summary}";
|
||||
$thread_topic = "Commit {$monogram}{$identifier}";
|
||||
|
||||
return id(new PhabricatorMetaMTAMail())
|
||||
->setSubject($subject)
|
||||
->addHeader('Thread-Topic', $thread_topic);
|
||||
}
|
||||
|
||||
protected function getMailTo(PhabricatorLiskDAO $object) {
|
||||
$phids = array();
|
||||
if ($object->getAuthorPHID()) {
|
||||
$phids[] = $object->getAuthorPHID();
|
||||
}
|
||||
|
||||
$status_resigned = PhabricatorAuditStatusConstants::RESIGNED;
|
||||
foreach ($object->getAudits() as $audit) {
|
||||
if ($audit->getAuditStatus() != $status_resigned) {
|
||||
$phids[] = $audit->getAuditorPHID();
|
||||
}
|
||||
}
|
||||
|
||||
return $phids;
|
||||
}
|
||||
|
||||
protected function buildMailBody(
|
||||
PhabricatorLiskDAO $object,
|
||||
array $xactions) {
|
||||
|
||||
$body = parent::buildMailBody($object, $xactions);
|
||||
|
||||
$type_inline = PhabricatorAuditActionConstants::INLINE;
|
||||
|
||||
$inlines = array();
|
||||
foreach ($xactions as $xaction) {
|
||||
if ($xaction->getTransactionType() == $type_inline) {
|
||||
$inlines[] = $xaction;
|
||||
}
|
||||
}
|
||||
|
||||
if ($inlines) {
|
||||
$body->addTextSection(
|
||||
pht('INLINE COMMENTS'),
|
||||
$this->renderInlineCommentsForMail($object, $inlines));
|
||||
}
|
||||
|
||||
$monogram = $object->getRepository()->formatCommitName(
|
||||
$object->getCommitIdentifier());
|
||||
|
||||
$body->addTextSection(
|
||||
pht('COMMIT'),
|
||||
PhabricatorEnv::getProductionURI('/'.$monogram));
|
||||
|
||||
return $body;
|
||||
}
|
||||
|
||||
private function renderInlineCommentsForMail(
|
||||
PhabricatorLiskDAO $object,
|
||||
array $inline_xactions) {
|
||||
|
||||
$inlines = mpull($inline_xactions, 'getComment');
|
||||
|
||||
$block = array();
|
||||
|
||||
$path_map = id(new DiffusionPathQuery())
|
||||
->withPathIDs(mpull($inlines, 'getPathID'))
|
||||
->execute();
|
||||
$path_map = ipull($path_map, 'path', 'id');
|
||||
|
||||
foreach ($inlines as $inline) {
|
||||
$path = idx($path_map, $inline->getPathID());
|
||||
if ($path === null) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$start = $inline->getLineNumber();
|
||||
$len = $inline->getLineLength();
|
||||
if ($len) {
|
||||
$range = $start.'-'.($start + $len);
|
||||
} else {
|
||||
$range = $start;
|
||||
}
|
||||
|
||||
$content = $inline->getContent();
|
||||
$block[] = "{$path}:{$range} {$content}";
|
||||
}
|
||||
|
||||
return implode("\n", $block);
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -42,6 +42,28 @@ final class PhabricatorAuditTransaction
|
|||
return $phids;
|
||||
}
|
||||
|
||||
public function getActionName() {
|
||||
|
||||
switch ($this->getTransactionType()) {
|
||||
case PhabricatorAuditActionConstants::ACTION:
|
||||
switch ($this->getNewValue()) {
|
||||
case PhabricatorAuditActionConstants::CONCERN:
|
||||
return pht('Raised Concern');
|
||||
case PhabricatorAuditActionConstants::ACCEPT:
|
||||
return pht('Accepted');
|
||||
case PhabricatorAuditActionConstants::RESIGN:
|
||||
return pht('Resigned');
|
||||
case PhabricatorAuditActionConstants::CLOSE:
|
||||
return pht('Clsoed');
|
||||
}
|
||||
break;
|
||||
case PhabricatorAuditActionConstants::ADD_AUDITORS:
|
||||
return pht('Added Auditors');
|
||||
}
|
||||
|
||||
return parent::getActionName();
|
||||
}
|
||||
|
||||
public function getColor() {
|
||||
|
||||
$type = $this->getTransactionType();
|
||||
|
|
@ -63,7 +85,7 @@ final class PhabricatorAuditTransaction
|
|||
$old = $this->getOldValue();
|
||||
$new = $this->getNewValue();
|
||||
|
||||
$author_handle = $this->getHandle($this->getAuthorPHID())->renderLink();
|
||||
$author_handle = $this->renderHandleLink($this->getAuthorPHID());
|
||||
|
||||
$type = $this->getTransactionType();
|
||||
|
||||
|
|
@ -157,4 +179,31 @@ final class PhabricatorAuditTransaction
|
|||
return parent::getTitle();
|
||||
}
|
||||
|
||||
// TODO: These two mail methods can likely be abstracted by introducing a
|
||||
// formal concept of "inline comment" transactions.
|
||||
|
||||
public function shouldHideForMail(array $xactions) {
|
||||
$type_inline = PhabricatorAuditActionConstants::INLINE;
|
||||
switch ($this->getTransactionType()) {
|
||||
case $type_inline:
|
||||
foreach ($xactions as $xaction) {
|
||||
if ($xaction->getTransactionType() != $type_inline) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return ($this !== head($xactions));
|
||||
}
|
||||
|
||||
return parent::shouldHideForMail($xactions);
|
||||
}
|
||||
|
||||
public function getBodyForMail() {
|
||||
switch ($this->getTransactionType()) {
|
||||
case PhabricatorAuditActionConstants::INLINE:
|
||||
return null;
|
||||
}
|
||||
|
||||
return parent::getBodyForMail();
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue