mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-29 10:12:41 +01:00
robustify Differential and Maniphest mailhandlers wrt attachments
Summary: a few things - make the parent mailhandler class not send "blank body" error if you have attachments - make both differential and maniphest append a list of attachments to the body if any exist - BONUS - made the cc stuff work in Maniphest Test Plan: I haven't actually tested this yet. :( i need to figure out how to send a mail with an attachment from the command-line and figured I'd serve this up first. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T2012 Differential Revision: https://secure.phabricator.com/D3868
This commit is contained in:
parent
e0b9a63388
commit
afae26ad94
4 changed files with 48 additions and 20 deletions
|
@ -107,10 +107,10 @@ class DifferentialReplyHandler extends PhabricatorMailReplyHandler {
|
||||||
|
|
||||||
protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
|
protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
|
||||||
$this->receivedMail = $mail;
|
$this->receivedMail = $mail;
|
||||||
$this->handleAction($mail->getCleanTextBody());
|
$this->handleAction($mail->getCleanTextBody(), $mail->getAttachments());
|
||||||
}
|
}
|
||||||
|
|
||||||
public function handleAction($body) {
|
public function handleAction($body, array $attachments) {
|
||||||
// all commands start with a bang and separated from the body by a newline
|
// all commands start with a bang and separated from the body by a newline
|
||||||
// to make sure that actual feedback text couldn't trigger an action.
|
// to make sure that actual feedback text couldn't trigger an action.
|
||||||
// unrecognized commands will be parsed as part of the comment.
|
// unrecognized commands will be parsed as part of the comment.
|
||||||
|
@ -135,6 +135,8 @@ class DifferentialReplyHandler extends PhabricatorMailReplyHandler {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
$body = $this->enhanceBodyWithAttachments($body, $attachments);
|
||||||
|
|
||||||
try {
|
try {
|
||||||
$editor = new DifferentialCommentEditor(
|
$editor = new DifferentialCommentEditor(
|
||||||
$this->getMailReceiver(),
|
$this->getMailReceiver(),
|
||||||
|
|
|
@ -63,9 +63,9 @@ final class ManiphestReplyHandler extends PhabricatorMailReplyHandler {
|
||||||
|
|
||||||
$body = $mail->getCleanTextBody();
|
$body = $mail->getCleanTextBody();
|
||||||
$body = trim($body);
|
$body = trim($body);
|
||||||
|
$body = $this->enhanceBodyWithAttachments($body, $mail->getAttachments());
|
||||||
|
|
||||||
$xactions = array();
|
$xactions = array();
|
||||||
|
|
||||||
$content_source = PhabricatorContentSource::newForSource(
|
$content_source = PhabricatorContentSource::newForSource(
|
||||||
PhabricatorContentSource::SOURCE_EMAIL,
|
PhabricatorContentSource::SOURCE_EMAIL,
|
||||||
array(
|
array(
|
||||||
|
@ -76,6 +76,7 @@ final class ManiphestReplyHandler extends PhabricatorMailReplyHandler {
|
||||||
$template->setContentSource($content_source);
|
$template->setContentSource($content_source);
|
||||||
$template->setAuthorPHID($user->getPHID());
|
$template->setAuthorPHID($user->getPHID());
|
||||||
|
|
||||||
|
|
||||||
if ($is_new_task) {
|
if ($is_new_task) {
|
||||||
// If this is a new task, create a "User created this task." transaction
|
// If this is a new task, create a "User created this task." transaction
|
||||||
// and then set the title and description.
|
// and then set the title and description.
|
||||||
|
@ -134,21 +135,14 @@ final class ManiphestReplyHandler extends PhabricatorMailReplyHandler {
|
||||||
$xactions[] = $xaction;
|
$xactions[] = $xaction;
|
||||||
}
|
}
|
||||||
|
|
||||||
// TODO: We should look at CCs on the mail and add them as CCs.
|
$ccs = $mail->loadCCPHIDs();
|
||||||
|
if ($ccs) {
|
||||||
$files = $mail->getAttachments();
|
$old_ccs = $task->getCCPHIDs();
|
||||||
if ($files) {
|
$new_ccs = array_unique(array_merge($old_ccs, $ccs));
|
||||||
$file_xaction = clone $template;
|
$cc_xaction = clone $template;
|
||||||
$file_xaction->setTransactionType(ManiphestTransactionType::TYPE_ATTACH);
|
$cc_xaction->setTransactionType(ManiphestTransactionType::TYPE_CCS);
|
||||||
|
$cc_xaction->setNewValue($new_ccs);
|
||||||
$phid_type = PhabricatorPHIDConstants::PHID_TYPE_FILE;
|
$xactions[] = $cc_xaction;
|
||||||
$new = $task->getAttached();
|
|
||||||
foreach ($files as $file_phid) {
|
|
||||||
$new[$phid_type][$file_phid] = array();
|
|
||||||
}
|
|
||||||
|
|
||||||
$file_xaction->setNewValue($new);
|
|
||||||
$xactions[] = $file_xaction;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
$event = new PhabricatorEvent(
|
$event = new PhabricatorEvent(
|
||||||
|
|
|
@ -73,7 +73,9 @@ abstract class PhabricatorMailReplyHandler {
|
||||||
|
|
||||||
private function sanityCheckEmail(PhabricatorMetaMTAReceivedMail $mail) {
|
private function sanityCheckEmail(PhabricatorMetaMTAReceivedMail $mail) {
|
||||||
$body = $mail->getCleanTextBody();
|
$body = $mail->getCleanTextBody();
|
||||||
if (empty($body)) {
|
$attachments = $mail->getAttachments();
|
||||||
|
|
||||||
|
if (empty($body) && empty($attachments)) {
|
||||||
return 'Empty email body. Email should begin with an !action and / or '.
|
return 'Empty email body. Email should begin with an !action and / or '.
|
||||||
'text to comment. Inline replies and signatures are ignored.';
|
'text to comment. Inline replies and signatures are ignored.';
|
||||||
}
|
}
|
||||||
|
@ -305,4 +307,26 @@ EOBODY;
|
||||||
return $this->getSingleReplyHandlerPrefix($address);
|
return $this->getSingleReplyHandlerPrefix($address);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
final protected function enhanceBodyWithAttachments($body,
|
||||||
|
array $attachments) {
|
||||||
|
if (!$attachments) {
|
||||||
|
return $body;
|
||||||
|
}
|
||||||
|
|
||||||
|
$files = id(new PhabricatorFile())
|
||||||
|
->loadAllWhere('phid in (%Ls)', $attachments);
|
||||||
|
|
||||||
|
// if we have some text then double return before adding our file list
|
||||||
|
if ($body) {
|
||||||
|
$body .= "\n\n";
|
||||||
|
}
|
||||||
|
|
||||||
|
foreach ($files as $file) {
|
||||||
|
$file_str = sprintf('- {F%d, layout=link}', $file->getID());
|
||||||
|
$body .= $file_str."\n";
|
||||||
|
}
|
||||||
|
|
||||||
|
return rtrim($body);
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -68,6 +68,14 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO {
|
||||||
$this->getCCAddresses()
|
$this->getCCAddresses()
|
||||||
);
|
);
|
||||||
|
|
||||||
|
return $this->loadPHIDsFromAddresses($addresses);
|
||||||
|
}
|
||||||
|
|
||||||
|
final public function loadCCPHIDs() {
|
||||||
|
return $this->loadPHIDsFromAddresses($this->getCCAddresses());
|
||||||
|
}
|
||||||
|
|
||||||
|
private function loadPHIDsFromAddresses(array $addresses) {
|
||||||
$users = id(new PhabricatorUserEmail())
|
$users = id(new PhabricatorUserEmail())
|
||||||
->loadAllWhere('address IN (%Ls)', $addresses);
|
->loadAllWhere('address IN (%Ls)', $addresses);
|
||||||
$user_phids = mpull($users, 'getUserPHID');
|
$user_phids = mpull($users, 'getUserPHID');
|
||||||
|
|
Loading…
Reference in a new issue