Page MenuHomePhabricator

D12230.diff
No OneTemporary

D12230.diff

diff --git a/src/applications/differential/mail/DifferentialReplyHandler.php b/src/applications/differential/mail/DifferentialReplyHandler.php
--- a/src/applications/differential/mail/DifferentialReplyHandler.php
+++ b/src/applications/differential/mail/DifferentialReplyHandler.php
@@ -1,13 +1,6 @@
<?php
-/**
- * NOTE: Do not extend this!
- *
- * @concrete-extensible
- */
-class DifferentialReplyHandler extends PhabricatorMailReplyHandler {
-
- private $receivedMail;
+final class DifferentialReplyHandler extends PhabricatorMailReplyHandler {
public function validateMailReceiver($mail_receiver) {
if (!($mail_receiver instanceof DifferentialRevision)) {
@@ -48,82 +41,72 @@
}
protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
- $this->receivedMail = $mail;
- $this->handleAction($mail->getCleanTextBody(), $mail->getAttachments());
- }
-
- public function handleAction($body, array $attachments) {
- // 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.
- // unrecognized commands will be parsed as part of the comment.
- $command = DifferentialAction::ACTION_COMMENT;
- $supported_commands = $this->getSupportedCommands();
- $regex = "/\A\n*!(".implode('|', $supported_commands).")\n*/";
- $matches = array();
- if (preg_match($regex, $body, $matches)) {
- $command = $matches[1];
- $body = trim(str_replace('!'.$command, '', $body));
- }
-
- $actor = $this->getActor();
- if (!$actor) {
- throw new Exception('No actor is set for the reply action.');
- }
+ $revision = $this->getMailReceiver();
+ $viewer = $this->getActor();
- switch ($command) {
- case 'unsubscribe':
- id(new PhabricatorSubscriptionsEditor())
- ->setActor($actor)
- ->setObject($this->getMailReceiver())
- ->unsubscribe(array($actor->getPHID()))
- ->save();
- // TODO: Send the user a confirmation email?
- return null;
- }
-
- $body = $this->enhanceBodyWithAttachments($body, $attachments);
+ $body_data = $mail->parseBody();
+ $body = $body_data['body'];
+ $body = $this->enhanceBodyWithAttachments($body, $mail->getAttachments());
$xactions = array();
-
- if ($command && ($command != DifferentialAction::ACTION_COMMENT)) {
- $xactions[] = id(new DifferentialTransaction())
- ->setTransactionType(DifferentialTransaction::TYPE_ACTION)
- ->setNewValue($command);
+ $content_source = PhabricatorContentSource::newForSource(
+ PhabricatorContentSource::SOURCE_EMAIL,
+ array(
+ 'id' => $mail->getID(),
+ ));
+
+ $template = id(new DifferentialTransaction());
+
+ $commands = $body_data['commands'];
+ foreach ($commands as $command_argv) {
+ $command = head($command_argv);
+ switch ($command) {
+ case 'unsubscribe':
+ $xactions[] = id(clone $template)
+ ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
+ ->setNewValue(
+ array(
+ '-' => array($viewer->getPHID()),
+ ));
+ break;
+ case DifferentialAction::ACTION_ACCEPT:
+ $accept_key = 'differential.enable-email-accept';
+ $can_accept = PhabricatorEnv::getEnvConfig($accept_key);
+ if (!$can_accept) {
+ throw new Exception(
+ pht(
+ 'You can not !accept revisions over email because '.
+ 'Differential is configured to disallow this.'));
+ }
+ // Fall through...
+ case DifferentialAction::ACTION_REJECT:
+ case DifferentialAction::ACTION_ABANDON:
+ case DifferentialAction::ACTION_RECLAIM:
+ case DifferentialAction::ACTION_RESIGN:
+ case DifferentialAction::ACTION_RETHINK:
+ case DifferentialAction::ACTION_CLAIM:
+ case DifferentialAction::ACTION_REOPEN:
+ $xactions[] = id(clone $template)
+ ->setTransactionType(DifferentialTransaction::TYPE_ACTION)
+ ->setNewValue($command);
+ break;
+ }
}
- if (strlen($body)) {
- $xactions[] = id(new DifferentialTransaction())
- ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
- ->attachComment(
- id(new DifferentialTransactionComment())
- ->setContent($body));
- }
+ $xactions[] = id(clone $template)
+ ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
+ ->attachComment(
+ id(new DifferentialTransactionComment())
+ ->setContent($body));
$editor = id(new DifferentialTransactionEditor())
- ->setActor($actor)
+ ->setActor($viewer)
+ ->setContentSource($content_source)
->setExcludeMailRecipientPHIDs($this->getExcludeMailRecipientPHIDs())
->setContinueOnMissingFields(true)
->setContinueOnNoEffect(true);
- // NOTE: We have to be careful about this because Facebook's
- // implementation jumps straight into handleAction() and will not have
- // a PhabricatorMetaMTAReceivedMail object.
- if ($this->receivedMail) {
- $content_source = PhabricatorContentSource::newForSource(
- PhabricatorContentSource::SOURCE_EMAIL,
- array(
- 'id' => $this->receivedMail->getID(),
- ));
- $editor->setContentSource($content_source);
- $editor->setParentMessageID($this->receivedMail->getMessageID());
- } else {
- $content_source = PhabricatorContentSource::newForSource(
- PhabricatorContentSource::SOURCE_LEGACY,
- array());
- $editor->setContentSource($content_source);
- }
-
- $editor->applyTransactions($this->getMailReceiver(), $xactions);
+ $editor->applyTransactions($revision, $xactions);
}
}
diff --git a/src/applications/files/mail/FileReplyHandler.php b/src/applications/files/mail/FileReplyHandler.php
--- a/src/applications/files/mail/FileReplyHandler.php
+++ b/src/applications/files/mail/FileReplyHandler.php
@@ -32,22 +32,23 @@
));
$xactions = array();
- $command = $body_data['body'];
-
- switch ($command) {
- case 'unsubscribe':
- $xaction = id(new PhabricatorFileTransaction())
- ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
- ->setNewValue(array('-' => array($actor->getPHID())));
- $xactions[] = $xaction;
- break;
+ $commands = $body_data['commands'];
+ foreach ($commands as $command) {
+ switch (head($command)) {
+ case 'unsubscribe':
+ $xaction = id(new PhabricatorFileTransaction())
+ ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
+ ->setNewValue(array('-' => array($actor->getPHID())));
+ $xactions[] = $xaction;
+ break;
+ }
}
$xactions[] = id(new PhabricatorFileTransaction())
->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
->attachComment(
- id(new PhabricatorFileTransactionComment())
- ->setContent($body));
+ id(new PhabricatorFileTransactionComment())
+ ->setContent($body));
$editor = id(new PhabricatorFileEditor())
->setActor($actor)
@@ -55,15 +56,7 @@
->setContinueOnNoEffect(true)
->setIsPreview(false);
- try {
- $xactions = $editor->applyTransactions($file, $xactions);
- } catch (PhabricatorApplicationTransactionNoEffectException $ex) {
- // just do nothing, though unclear why you're sending a blank email
- return true;
- }
-
- $head_xaction = head($xactions);
- return $head_xaction->getID();
+ $editor->applyTransactions($file, $xactions);
}
}
diff --git a/src/applications/legalpad/mail/LegalpadReplyHandler.php b/src/applications/legalpad/mail/LegalpadReplyHandler.php
--- a/src/applications/legalpad/mail/LegalpadReplyHandler.php
+++ b/src/applications/legalpad/mail/LegalpadReplyHandler.php
@@ -31,27 +31,28 @@
'id' => $mail->getID(),
));
-
$xactions = array();
- $command = $body_data['command'];
- switch ($command) {
- case 'unsubscribe':
- $xaction = id(new LegalpadTransaction())
- ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
- ->setNewValue(array('-' => array($actor->getPHID())));
- $xactions[] = $xaction;
- break;
+ $commands = $body_data['commands'];
+ foreach ($commands as $command) {
+ switch (head($command)) {
+ case 'unsubscribe':
+ $xaction = id(new LegalpadTransaction())
+ ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
+ ->setNewValue(array('-' => array($actor->getPHID())));
+ $xactions[] = $xaction;
+ break;
+ }
}
$xactions[] = id(new LegalpadTransaction())
->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
->attachComment(
id(new LegalpadTransactionComment())
- ->setDocumentID($document->getID())
- ->setLineNumber(0)
- ->setLineLength(0)
- ->setContent($body));
+ ->setDocumentID($document->getID())
+ ->setLineNumber(0)
+ ->setLineLength(0)
+ ->setContent($body));
$editor = id(new LegalpadDocumentEditor())
->setActor($actor)
@@ -59,15 +60,7 @@
->setContinueOnNoEffect(true)
->setIsPreview(false);
- try {
- $xactions = $editor->applyTransactions($document, $xactions);
- } catch (PhabricatorApplicationTransactionNoEffectException $ex) {
- // just do nothing, though unclear why you're sending a blank email
- return true;
- }
-
- $head_xaction = head($xactions);
- return $head_xaction->getID();
+ $editor->applyTransactions($document, $xactions);
}
}
diff --git a/src/applications/maniphest/mail/ManiphestReplyHandler.php b/src/applications/maniphest/mail/ManiphestReplyHandler.php
--- a/src/applications/maniphest/mail/ManiphestReplyHandler.php
+++ b/src/applications/maniphest/mail/ManiphestReplyHandler.php
@@ -63,8 +63,16 @@
} else {
- $command = $body_data['command'];
- $command_value = $body_data['command_value'];
+ $commands = $body_data['commands'];
+
+ // TODO: Support multiple commands.
+ if ($commands) {
+ $command_argv = head($commands);
+ } else {
+ $command_argv = array();
+ }
+ $command = idx($command_argv, 0);
+ $command_value = idx($command_argv, 1);
$ttype = PhabricatorTransactions::TYPE_COMMENT;
$new_value = null;
diff --git a/src/applications/metamta/parser/PhabricatorMetaMTAEmailBodyParser.php b/src/applications/metamta/parser/PhabricatorMetaMTAEmailBodyParser.php
--- a/src/applications/metamta/parser/PhabricatorMetaMTAEmailBodyParser.php
+++ b/src/applications/metamta/parser/PhabricatorMetaMTAEmailBodyParser.php
@@ -16,36 +16,66 @@
* please, take this task I took; its hard
*
* This function parses such an email body and returns a dictionary
- * containing a clean body text (e.g. "taking this task"), a $command
- * (e.g. !claim, !assign) and a $command_value (e.g. "epriestley" in the
- * !assign example.)
+ * containing a clean body text (e.g. "taking this task"), and a list of
+ * commands. For example, this body above might parse as:
*
- * @return dict
+ * array(
+ * 'body' => 'please, take this task I took; its hard',
+ * 'commands' => array(
+ * array('assign', 'epriestley'),
+ * ),
+ * )
+ *
+ * @param string Raw mail text body.
+ * @return dict Parsed body.
*/
public function parseBody($body) {
$body = $this->stripTextBody($body);
- $lines = explode("\n", trim($body));
- $first_line = head($lines);
-
- $command = null;
- $command_value = null;
- $matches = null;
- if (preg_match('/^!(\w+)\s*(\S+)?/', $first_line, $matches)) {
- $lines = array_slice($lines, 1);
- $body = implode("\n", $lines);
- $body = trim($body);
-
- $command = $matches[1];
- $command_value = idx($matches, 2);
- }
+
+ $commands = array();
+
+ $lines = phutil_split_lines($body, $retain_endings = true);
+
+ // We'll match commands at the beginning and end of the mail, but not
+ // in the middle of the mail body.
+ list($top_commands, $lines) = $this->stripCommands($lines);
+ list($end_commands, $lines) = $this->stripCommands(array_reverse($lines));
+ $lines = array_reverse($lines);
+ $commands = array_merge($top_commands, array_reverse($end_commands));
+
+ $lines = rtrim(implode('', $lines));
return array(
- 'body' => $body,
- 'command' => $command,
- 'command_value' => $command_value,
+ 'body' => $lines,
+ 'commands' => $commands,
);
}
+ private function stripCommands(array $lines) {
+ $saw_command = false;
+ $commands = array();
+ foreach ($lines as $key => $line) {
+ if (!strlen(trim($line)) && $saw_command) {
+ unset($lines[$key]);
+ continue;
+ }
+
+ $matches = null;
+ if (!preg_match('/^\s*!(\w+.*$)/', $line, $matches)) {
+ break;
+ }
+
+ $arg_str = $matches[1];
+ $argv = preg_split('/[,\s]+/', trim($arg_str));
+ $commands[] = $argv;
+ unset($lines[$key]);
+
+ $saw_command = true;
+ }
+
+ return array($commands, $lines);
+ }
+
public function stripTextBody($body) {
return trim($this->stripSignature($this->stripQuotedText($body)));
}
diff --git a/src/applications/metamta/parser/__tests__/PhabricatorMetaMTAEmailBodyParserTestCase.php b/src/applications/metamta/parser/__tests__/PhabricatorMetaMTAEmailBodyParserTestCase.php
--- a/src/applications/metamta/parser/__tests__/PhabricatorMetaMTAEmailBodyParserTestCase.php
+++ b/src/applications/metamta/parser/__tests__/PhabricatorMetaMTAEmailBodyParserTestCase.php
@@ -18,16 +18,56 @@
$parser = new PhabricatorMetaMTAEmailBodyParser();
$body_data = $parser->parseBody($body);
$this->assertEqual('OKAY', $body_data['body']);
- $this->assertEqual('whatevs', $body_data['command']);
- $this->assertEqual('dude', $body_data['command_value']);
+ $this->assertEqual(
+ array(
+ array('whatevs', 'dude'),
+ ),
+ $body_data['commands']);
}
+
$bodies = $this->getEmailBodiesWithPartialCommands();
foreach ($bodies as $body) {
$parser = new PhabricatorMetaMTAEmailBodyParser();
$body_data = $parser->parseBody($body);
$this->assertEqual('OKAY', $body_data['body']);
- $this->assertEqual('whatevs', $body_data['command']);
- $this->assertEqual(null, $body_data['command_value']);
+ $this->assertEqual(
+ array(
+ array('whatevs'),
+ ),
+ $body_data['commands']);
+ }
+
+ $bodies = $this->getEmailBodiesWithMultipleCommands();
+ foreach ($bodies as $body) {
+ $parser = new PhabricatorMetaMTAEmailBodyParser();
+ $body_data = $parser->parseBody($body);
+ $this->assertEqual("preface\n\nOKAY", $body_data['body']);
+ $this->assertEqual(
+ array(
+ array('top1'),
+ array('top2'),
+ ),
+ $body_data['commands']);
+ }
+
+ $bodies = $this->getEmailBodiesWithSplitCommands();
+ foreach ($bodies as $body) {
+ $parser = new PhabricatorMetaMTAEmailBodyParser();
+ $body_data = $parser->parseBody($body);
+ $this->assertEqual('OKAY', $body_data['body']);
+ $this->assertEqual(
+ array(
+ array('cmd1'),
+ array('cmd2'),
+ ),
+ $body_data['commands']);
+ }
+
+ $bodies = $this->getEmailBodiesWithMiddleCommands();
+ foreach ($bodies as $body) {
+ $parser = new PhabricatorMetaMTAEmailBodyParser();
+ $body_data = $parser->parseBody($body);
+ $this->assertEqual("HEAD\n!cmd2\nTAIL", $body_data['body']);
}
}
@@ -63,6 +103,30 @@
return $with_commands;
}
+ private function getEmailBodiesWithMultipleCommands() {
+ $bodies = $this->getEmailBodies();
+ $with_commands = array();
+ foreach ($bodies as $body) {
+ $with_commands[] = "!top1\n\n!top2\n\npreface\n\n".$body;
+ }
+ return $with_commands;
+ }
+
+ private function getEmailBodiesWithSplitCommands() {
+ $with_split = array();
+ $with_split[] = "!cmd1\n!cmd2\nOKAY";
+ $with_split[] = "!cmd1\nOKAY\n!cmd2";
+ $with_split[] = "OKAY\n!cmd1\n!cmd2";
+ return $with_split;
+ }
+
+ private function getEmailBodiesWithMiddleCommands() {
+ $with_middle = array();
+ $with_middle[] = "!cmd1\nHEAD\n!cmd2\nTAIL\n!cmd3";
+ $with_middle[] = "!cmd1\nHEAD\n!cmd2\nTAIL";
+ $with_middle[] = "HEAD\n!cmd2\nTAIL\n!cmd3";
+ return $with_middle;
+ }
private function getEmailBodies() {
$trailing_space = ' ';
diff --git a/src/applications/paste/mail/PasteReplyHandler.php b/src/applications/paste/mail/PasteReplyHandler.php
--- a/src/applications/paste/mail/PasteReplyHandler.php
+++ b/src/applications/paste/mail/PasteReplyHandler.php
@@ -35,15 +35,17 @@
$first_line = head($lines);
$xactions = array();
- $command = $body_data['command'];
- switch ($command) {
- case 'unsubscribe':
- $xaction = id(new PhabricatorPasteTransaction())
- ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
- ->setNewValue(array('-' => array($actor->getPHID())));
- $xactions[] = $xaction;
- break;
+ $commands = $body_data['commands'];
+ foreach ($commands as $command) {
+ switch (head($command)) {
+ case 'unsubscribe':
+ $xaction = id(new PhabricatorPasteTransaction())
+ ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
+ ->setNewValue(array('-' => array($actor->getPHID())));
+ $xactions[] = $xaction;
+ break;
+ }
}
$xactions[] = id(new PhabricatorPasteTransaction())
@@ -58,15 +60,7 @@
->setContinueOnNoEffect(true)
->setIsPreview(false);
- try {
- $xactions = $editor->applyTransactions($paste, $xactions);
- } catch (PhabricatorApplicationTransactionNoEffectException $ex) {
- // just do nothing, though unclear why you're sending a blank email
- return true;
- }
-
- $head_xaction = head($xactions);
- return $head_xaction->getID();
+ $editor->applyTransactions($paste, $xactions);
}
}
diff --git a/src/applications/releeph/mail/ReleephRequestMailReceiver.php b/src/applications/releeph/mail/ReleephRequestMailReceiver.php
--- a/src/applications/releeph/mail/ReleephRequestMailReceiver.php
+++ b/src/applications/releeph/mail/ReleephRequestMailReceiver.php
@@ -8,11 +8,11 @@
}
protected function getObjectPattern() {
- return 'RQ[1-9]\d*';
+ return 'Y[1-9]\d*';
}
protected function loadObject($pattern, PhabricatorUser $viewer) {
- $id = (int)substr($pattern, 2);
+ $id = (int)substr($pattern, 1);
return id(new ReleephRequestQuery())
->setViewer($viewer)
diff --git a/src/applications/releeph/mail/ReleephRequestReplyHandler.php b/src/applications/releeph/mail/ReleephRequestReplyHandler.php
--- a/src/applications/releeph/mail/ReleephRequestReplyHandler.php
+++ b/src/applications/releeph/mail/ReleephRequestReplyHandler.php
@@ -10,11 +10,11 @@
public function getPrivateReplyHandlerEmailAddress(
PhabricatorObjectHandle $handle) {
- return $this->getDefaultPrivateReplyHandlerEmailAddress($handle, 'RERQ');
+ return $this->getDefaultPrivateReplyHandlerEmailAddress($handle, 'Y');
}
public function getPublicReplyHandlerEmailAddress() {
- return $this->getDefaultPublicReplyHandlerEmailAddress('RERQ');
+ return $this->getDefaultPublicReplyHandlerEmailAddress('Y');
}
protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
@@ -27,11 +27,6 @@
'id' => $mail->getID(),
));
- $editor = id(new ReleephRequestTransactionalEditor())
- ->setActor($user)
- ->setContentSource($content_source)
- ->setParentMessageID($mail->getMessageID());
-
$body = $mail->getCleanTextBody();
$xactions = array();
@@ -39,9 +34,13 @@
->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
->attachComment($body);
- $editor->applyTransactions($rq, $xactions);
+ $editor = id(new ReleephRequestTransactionalEditor())
+ ->setActor($user)
+ ->setContentSource($content_source)
+ ->setContinueOnNoEffect(true)
+ ->setParentMessageID($mail->getMessageID());
- return $rq;
+ $editor->applyTransactions($rq, $xactions);
}
}

File Metadata

Mime Type
text/plain
Expires
Fri, Dec 20, 11:36 PM (17 h, 37 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6912243
Default Alt Text
D12230.diff (20 KB)

Event Timeline