Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F14356714
D12230.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
20 KB
Referenced Files
None
Subscribers
None
D12230.diff
View Options
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
Details
Attached
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)
Attached To
Mode
D12230: Modernize email command parsing
Attached
Detach File
Event Timeline
Log In to Comment