Herald - add support for application emails.

Summary:
Fixes T5039. The trick / possibly lame part here is we only match 1 application email and its undefined which one. e.g. if a user emails us at address x, y, and z only one of those will pick up the mail. Ergo, don't let users define non-sensical herald conditions like "matches all". Also document what I think was non-intuitive about the code with an inline comment; we have to return an array with just a phid from an object and out of context it feels very "what the...???"

Note this needs to be deployed to other applications still, but I think its okay to close T5039 aggressively here since its done from a user story perspective.

Test Plan: set up a herald rule to flag tasks created as blue via app email x. sent an email to x via `bin/mail receive-test` and verified the task had the blue flag

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T5039

Differential Revision: https://secure.phabricator.com/D11564
This commit is contained in:
Bob Trahan
2015-01-29 14:15:38 -08:00
parent ffaed67711
commit ab8f7907de
9 changed files with 72 additions and 13 deletions

View File

@@ -39,6 +39,7 @@ abstract class HeraldAdapter {
const FIELD_AUTHOR_RAW = 'author-raw';
const FIELD_COMMITTER_RAW = 'committer-raw';
const FIELD_IS_NEW_OBJECT = 'new-object';
const FIELD_APPLICATION_EMAIL = 'applicaton-email';
const FIELD_TASK_PRIORITY = 'taskpriority';
const FIELD_TASK_STATUS = 'taskstatus';
const FIELD_ARCANIST_PROJECT = 'arcanist-project';
@@ -98,11 +99,13 @@ abstract class HeraldAdapter {
const VALUE_BUILD_PLAN = 'buildplan';
const VALUE_TASK_PRIORITY = 'taskpriority';
const VALUE_TASK_STATUS = 'taskstatus';
const VALUE_ARCANIST_PROJECT = 'arcanistprojects';
const VALUE_LEGAL_DOCUMENTS = 'legaldocuments';
const VALUE_ARCANIST_PROJECT = 'arcanistprojects';
const VALUE_LEGAL_DOCUMENTS = 'legaldocuments';
const VALUE_APPLICATION_EMAIL = 'applicationemail';
private $contentSource;
private $isNewObject;
private $applicationEmail;
private $customFields = false;
private $customActions = null;
private $queuedTransactions = array();
@@ -156,6 +159,16 @@ abstract class HeraldAdapter {
return $this;
}
public function setApplicationEmail(
PhabricatorMetaMTAApplicationEmail $email) {
$this->applicationEmail = $email;
return $this;
}
public function getApplicationEmail() {
return $this->applicationEmail;
}
abstract public function getPHID();
abstract public function getHeraldName();
@@ -169,6 +182,14 @@ abstract class HeraldAdapter {
return true;
case self::FIELD_IS_NEW_OBJECT:
return $this->getIsNewObject();
case self::FIELD_APPLICATION_EMAIL:
$value = array();
// while there is only one match by implementation, we do set
// comparisons on phids, so return an array with just the phid
if ($this->getApplicationEmail()) {
$value[] = $this->getApplicationEmail()->getPHID();
}
return $value;
default:
if ($this->isHeraldCustomKey($field_name)) {
return $this->getCustomFieldValue($field_name);
@@ -312,6 +333,7 @@ abstract class HeraldAdapter {
self::FIELD_AUTHOR_RAW => pht('Raw author name'),
self::FIELD_COMMITTER_RAW => pht('Raw committer name'),
self::FIELD_IS_NEW_OBJECT => pht('Is newly created?'),
self::FIELD_APPLICATION_EMAIL => pht('Receiving email address'),
self::FIELD_TASK_PRIORITY => pht('Task priority'),
self::FIELD_TASK_STATUS => pht('Task status'),
self::FIELD_ARCANIST_PROJECT => pht('Arcanist Project'),
@@ -401,6 +423,13 @@ abstract class HeraldAdapter {
self::CONDITION_EXISTS,
self::CONDITION_NOT_EXISTS,
);
case self::FIELD_APPLICATION_EMAIL:
return array(
self::CONDITION_INCLUDE_ANY,
self::CONDITION_INCLUDE_NONE,
self::CONDITION_EXISTS,
self::CONDITION_NOT_EXISTS,
);
case self::FIELD_DIFF_FILE:
case self::FIELD_BRANCHES:
return array(
@@ -874,6 +903,8 @@ abstract class HeraldAdapter {
return self::VALUE_PROJECT;
case self::FIELD_REVIEWERS:
return self::VALUE_USER_OR_PROJECT;
case self::FIELD_APPLICATION_EMAIL:
return self::VALUE_APPLICATION_EMAIL;
default:
return self::VALUE_USER;
}

View File

@@ -91,6 +91,7 @@ final class HeraldManiphestTaskAdapter extends HeraldAdapter {
self::FIELD_TASK_PRIORITY,
self::FIELD_TASK_STATUS,
self::FIELD_IS_NEW_OBJECT,
self::FIELD_APPLICATION_EMAIL,
),
parent::getFields());
}

View File

@@ -602,6 +602,7 @@ final class HeraldRuleController extends HeraldController {
'user' => new PhabricatorPeopleDatasource(),
'email' => new PhabricatorMetaMTAMailableDatasource(),
'userorproject' => new PhabricatorProjectOrUserDatasource(),
'applicationemail' => new PhabricatorMetaMTAApplicationEmailDatasource(),
);
foreach ($sources as $key => $source) {

View File

@@ -41,6 +41,7 @@ final class ManiphestCreateMailReceiver extends PhabricatorMailReceiver {
$handler->setActor($sender);
$handler->setExcludeMailRecipientPHIDs(
$mail->loadExcludeMailRecipientPHIDs());
$handler->setApplicationEmail($this->getApplicationEmail());
$handler->processEmail($mail);
$mail->setRelatedPHID($task->getPHID());

View File

@@ -171,6 +171,7 @@ final class ManiphestReplyHandler extends PhabricatorMailReplyHandler {
->setContinueOnNoEffect(true)
->setContinueOnMissingFields(true)
->setContentSource($content_source)
->setApplicationEmail($this->getApplicationEmail())
->applyTransactions($task, $xactions);
$event = new PhabricatorEvent(

View File

@@ -3,6 +3,7 @@
abstract class PhabricatorMailReplyHandler {
private $mailReceiver;
private $applicationEmail;
private $actor;
private $excludePHIDs = array();
@@ -16,6 +17,16 @@ abstract class PhabricatorMailReplyHandler {
return $this->mailReceiver;
}
public function setApplicationEmail(
PhabricatorMetaMTAApplicationEmail $email) {
$this->applicationEmail = $email;
return $this;
}
public function getApplicationEmail() {
return $this->applicationEmail;
}
final public function setActor(PhabricatorUser $actor) {
$this->actor = $actor;
return $this;

View File

@@ -22,6 +22,7 @@ abstract class PhabricatorApplicationTransactionEditor
private $heraldTranscript;
private $subscribers;
private $unmentionablePHIDMap = array();
private $applicationEmail;
private $isPreview;
private $isHeraldEditor;
@@ -185,6 +186,16 @@ abstract class PhabricatorApplicationTransactionEditor
return $this->unmentionablePHIDMap;
}
public function setApplicationEmail(
PhabricatorMetaMTAApplicationEmail $email) {
$this->applicationEmail = $email;
return $this;
}
public function getApplicationEmail() {
return $this->applicationEmail;
}
public function getTransactionTypes() {
$types = array();
@@ -2427,6 +2438,7 @@ abstract class PhabricatorApplicationTransactionEditor
$adapter = $this->buildHeraldAdapter($object, $xactions);
$adapter->setContentSource($this->getContentSource());
$adapter->setIsNewObject($this->getIsNewObject());
$adapter->setApplicationEmail($this->getApplicationEmail());
$xscript = HeraldEngine::loadAndApplyRules($adapter);
$this->setHeraldAdapter($adapter);