Skip to content

Commit d0127f9

Browse files
bobtrahanbobtrahan
committed
Maniphest - add support for !assign command
Summary: also try to centralize some of the command parsing logic. note that differential is still an exception here. it uses a whitelist-style regex. i think long-term we should have this for every app but changing it seemed too big for this diff. Fixes T3937. Test Plan: echo '!assign btrahan' | ./bin/mail receive-test --as xerxes --to T22 ; echo '!claim' | ./bin/mail receive-test --as xerxes --to T22 unit tests passed, though my new one is silly Reviewers: epriestley Reviewed By: epriestley CC: Korvin, epriestley, aran Maniphest Tasks: T3937 Differential Revision: https://secure.phabricator.com/D7307
1 parent 13178ec commit d0127f9

File tree

9 files changed

+120
-56
lines changed

9 files changed

+120
-56
lines changed

‎src/applications/conpherence/mail/ConpherenceReplyHandler.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
6969
->setParentMessageID($mail->getMessageID());
7070

7171
$body = $mail->getCleanTextBody();
72-
$body = trim($body);
7372
$file_phids = $mail->getAttachments();
7473
$body = $this->enhanceBodyWithAttachments(
7574
$body,

‎src/applications/files/mail/FileReplyHandler.php

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
3232
$actor = $this->getActor();
3333
$file = $this->getMailReceiver();
3434

35-
$body = $mail->getCleanTextBody();
36-
$body = trim($body);
35+
$body_data = $mail->parseBody();
36+
$body = $body_data['body'];
3737
$body = $this->enhanceBodyWithAttachments($body, $mail->getAttachments());
3838

3939
$content_source = PhabricatorContentSource::newForSource(
@@ -42,19 +42,8 @@ protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
4242
'id' => $mail->getID(),
4343
));
4444

45-
$lines = explode("\n", trim($body));
46-
$first_line = head($lines);
47-
4845
$xactions = array();
49-
$command = null;
50-
$matches = null;
51-
if (preg_match('/^!(\w+)/', $first_line, $matches)) {
52-
$lines = array_slice($lines, 1);
53-
$body = implode("\n", $lines);
54-
$body = trim($body);
55-
56-
$command = $matches[1];
57-
}
46+
$command = $body_data['body'];
5847

5948
switch ($command) {
6049
case 'unsubscribe':

‎src/applications/legalpad/mail/LegalpadReplyHandler.php

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
3737
$actor = $this->getActor();
3838
$document = $this->getMailReceiver();
3939

40-
$body = $mail->getCleanTextBody();
41-
$body = trim($body);
40+
$body_data = $mail->parseBody();
41+
$body = $body_data['body'];
4242
$body = $this->enhanceBodyWithAttachments($body, $mail->getAttachments());
4343

4444
$content_source = PhabricatorContentSource::newForSource(
@@ -47,19 +47,9 @@ protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
4747
'id' => $mail->getID(),
4848
));
4949

50-
$lines = explode("\n", trim($body));
51-
$first_line = head($lines);
5250

5351
$xactions = array();
54-
$command = null;
55-
$matches = null;
56-
if (preg_match('/^!(\w+)/', $first_line, $matches)) {
57-
$lines = array_slice($lines, 1);
58-
$body = implode("\n", $lines);
59-
$body = trim($body);
60-
61-
$command = $matches[1];
62-
}
52+
$command = $body_data['command'];
6353

6454
switch ($command) {
6555
case 'unsubscribe':

‎src/applications/maniphest/mail/ManiphestReplyHandler.php

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
4545

4646
$user = $this->getActor();
4747

48-
$body = $mail->getCleanTextBody();
49-
$body = trim($body);
48+
$body_data = $mail->parseBody();
49+
$body = $body_data['body'];
5050
$body = $this->enhanceBodyWithAttachments($body, $mail->getAttachments());
5151

5252
$xactions = array();
@@ -73,18 +73,9 @@ protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
7373
$task->setPriority(ManiphestTaskPriority::getDefaultPriority());
7474

7575
} else {
76-
$lines = explode("\n", trim($body));
77-
$first_line = head($lines);
7876

79-
$command = null;
80-
$matches = null;
81-
if (preg_match('/^!(\w+)/', $first_line, $matches)) {
82-
$lines = array_slice($lines, 1);
83-
$body = implode("\n", $lines);
84-
$body = trim($body);
85-
86-
$command = $matches[1];
87-
}
77+
$command = $body_data['command'];
78+
$command_value = $body_data['command_value'];
8879

8980
$ttype = PhabricatorTransactions::TYPE_COMMENT;
9081
$new_value = null;
@@ -97,6 +88,23 @@ protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
9788
$ttype = ManiphestTransaction::TYPE_OWNER;
9889
$new_value = $user->getPHID();
9990
break;
91+
case 'assign':
92+
$ttype = ManiphestTransaction::TYPE_OWNER;
93+
if ($command_value) {
94+
$assign_users = id(new PhabricatorPeopleQuery())
95+
->setViewer($user)
96+
->withUsernames(array($command_value))
97+
->execute();
98+
if ($assign_users) {
99+
$assign_user = head($assign_users);
100+
$new_value = $assign_user->getPHID();
101+
}
102+
}
103+
// assign to the user by default
104+
if (!$new_value) {
105+
$new_value = $user->getPHID();
106+
}
107+
break;
100108
case 'unsubscribe':
101109
$is_unsub = true;
102110
$ttype = ManiphestTransaction::TYPE_CCS;

‎src/applications/metamta/parser/PhabricatorMetaMTAEmailBodyParser.php

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,51 @@
22

33
final class PhabricatorMetaMTAEmailBodyParser {
44

5+
/**
6+
* Mails can have bodies such as
7+
*
8+
* !claim
9+
*
10+
* taking this task
11+
*
12+
* Or
13+
*
14+
* !assign epriestley
15+
*
16+
* please, take this task I took; its hard
17+
*
18+
* This function parses such an email body and returns a dictionary
19+
* containing a clean body text (e.g. "taking this task"), a $command
20+
* (e.g. !claim, !assign) and a $command_value (e.g. "epriestley" in the
21+
* !assign example.)
22+
*
23+
* @return dict
24+
*/
25+
public function parseBody($body) {
26+
$body = $this->stripTextBody($body);
27+
$lines = explode("\n", trim($body));
28+
$first_line = head($lines);
29+
30+
$command = null;
31+
$command_value = null;
32+
$matches = null;
33+
if (preg_match('/^!(\w+)\s*(\S+)?/', $first_line, $matches)) {
34+
$lines = array_slice($lines, 1);
35+
$body = implode("\n", $lines);
36+
$body = trim($body);
37+
38+
$command = $matches[1];
39+
$command_value = idx($matches, 2);
40+
}
41+
42+
return array(
43+
'body' => $body,
44+
'command' => $command,
45+
'command_value' => $command_value);
46+
}
47+
548
public function stripTextBody($body) {
6-
return $this->stripSignature($this->stripQuotedText($body));
49+
return trim($this->stripSignature($this->stripQuotedText($body)));
750
}
851

952
private function stripQuotedText($body) {

‎src/applications/metamta/parser/__tests__/PhabricatorMetaMTAEmailBodyParserTestCase.php

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,44 @@ public function testQuotedTextStripping() {
1212
}
1313
}
1414

15+
public function testEmailBodyCommandParsing() {
16+
$bodies = $this->getEmailBodiesWithFullCommands();
17+
foreach ($bodies as $body) {
18+
$parser = new PhabricatorMetaMTAEmailBodyParser();
19+
$body_data = $parser->parseBody($body);
20+
$this->assertEqual('OKAY', $body_data['body']);
21+
$this->assertEqual('whatevs', $body_data['command']);
22+
$this->assertEqual('dude', $body_data['command_value']);
23+
}
24+
$bodies = $this->getEmailBodiesWithPartialCommands();
25+
foreach ($bodies as $body) {
26+
$parser = new PhabricatorMetaMTAEmailBodyParser();
27+
$body_data = $parser->parseBody($body);
28+
$this->assertEqual('OKAY', $body_data['body']);
29+
$this->assertEqual('whatevs', $body_data['command']);
30+
$this->assertEqual(null, $body_data['command_value']);
31+
}
32+
}
33+
34+
private function getEmailBodiesWithFullCommands() {
35+
$bodies = $this->getEmailBodies();
36+
$with_commands = array();
37+
foreach ($bodies as $body) {
38+
$with_commands[] = "!whatevs dude\n" . $body;
39+
}
40+
return $with_commands;
41+
}
42+
43+
private function getEmailBodiesWithPartialCommands() {
44+
$bodies = $this->getEmailBodies();
45+
$with_commands = array();
46+
foreach ($bodies as $body) {
47+
$with_commands[] = "!whatevs\n" . $body;
48+
}
49+
return $with_commands;
50+
}
51+
52+
1553
private function getEmailBodies() {
1654
$trailing_space = ' ';
1755

‎src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ protected function getDefaultPrivateReplyHandlerEmailAddress(
307307
return $this->getSingleReplyHandlerPrefix($address);
308308
}
309309

310-
final protected function enhanceBodyWithAttachments(
310+
final protected function enhanceBodyWithAttachments(
311311
$body,
312312
array $attachments,
313313
$format = '- {F%d, layout=link}') {

‎src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,12 +120,17 @@ public function processReceivedMail() {
120120
}
121121

122122
public function getCleanTextBody() {
123-
$body = idx($this->bodies, 'text');
124-
123+
$body = $this->getRawTextBody();
125124
$parser = new PhabricatorMetaMTAEmailBodyParser();
126125
return $parser->stripTextBody($body);
127126
}
128127

128+
public function parseBody() {
129+
$body = $this->getRawTextBody();
130+
$parser = new PhabricatorMetaMTAEmailBodyParser();
131+
return $parser->parseBody($body);
132+
}
133+
129134
public function getRawTextBody() {
130135
return idx($this->bodies, 'text');
131136
}

‎src/applications/paste/mail/PasteReplyHandler.php

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
3232
$actor = $this->getActor();
3333
$paste = $this->getMailReceiver();
3434

35-
$body = $mail->getCleanTextBody();
36-
$body = trim($body);
35+
$body_data = $mail->parseBody();
36+
$body = $body_data['body'];
3737
$body = $this->enhanceBodyWithAttachments($body, $mail->getAttachments());
3838

3939
$content_source = PhabricatorContentSource::newForSource(
@@ -46,15 +46,7 @@ protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
4646
$first_line = head($lines);
4747

4848
$xactions = array();
49-
$command = null;
50-
$matches = null;
51-
if (preg_match('/^!(\w+)/', $first_line, $matches)) {
52-
$lines = array_slice($lines, 1);
53-
$body = implode("\n", $lines);
54-
$body = trim($body);
55-
56-
$command = $matches[1];
57-
}
49+
$command = $body_data['command'];
5850

5951
switch ($command) {
6052
case 'unsubscribe':

0 commit comments

Comments
 (0)