Consolidate outbound mail status in a new class

Summary: Ref T5791. This collects outbound mail status in one place and makes the list view a little spiffier.

Test Plan: Looked at list and detail views. Grepped for changed classes/constants.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T5791

Differential Revision: https://secure.phabricator.com/D13884

src/__phutil_library_map__.php

@@ -1248,7 +1248,6 @@ phutil_register_library_map(array(
     'ManiphestTransactionSaveController' => 'applications/maniphest/controller/ManiphestTransactionSaveController.php',
     'ManiphestUpdateConduitAPIMethod' => 'applications/maniphest/conduit/ManiphestUpdateConduitAPIMethod.php',
     'ManiphestView' => 'applications/maniphest/view/ManiphestView.php',
-    'MetaMTAConstants' => 'applications/metamta/constants/MetaMTAConstants.php',
     'MetaMTAEmailTransactionCommand' => 'applications/metamta/command/MetaMTAEmailTransactionCommand.php',
     'MetaMTAEmailTransactionCommandTestCase' => 'applications/metamta/command/__tests__/MetaMTAEmailTransactionCommandTestCase.php',
     'MetaMTAMailReceivedGarbageCollector' => 'applications/metamta/garbagecollector/MetaMTAMailReceivedGarbageCollector.php',
@@ -2258,6 +2257,7 @@ phutil_register_library_map(array(
     'PhabricatorMailManagementShowOutboundWorkflow' => 'applications/metamta/management/PhabricatorMailManagementShowOutboundWorkflow.php',
     'PhabricatorMailManagementVolumeWorkflow' => 'applications/metamta/management/PhabricatorMailManagementVolumeWorkflow.php',
     'PhabricatorMailManagementWorkflow' => 'applications/metamta/management/PhabricatorMailManagementWorkflow.php',
+    'PhabricatorMailOutboundStatus' => 'applications/metamta/constants/PhabricatorMailOutboundStatus.php',
     'PhabricatorMailReceiver' => 'applications/metamta/receiver/PhabricatorMailReceiver.php',
     'PhabricatorMailReceiverTestCase' => 'applications/metamta/receiver/__tests__/PhabricatorMailReceiverTestCase.php',
     'PhabricatorMailReplyHandler' => 'applications/metamta/replyhandler/PhabricatorMailReplyHandler.php',
@@ -5012,12 +5012,11 @@ phutil_register_library_map(array(
     'ManiphestTransactionSaveController' => 'ManiphestController',
     'ManiphestUpdateConduitAPIMethod' => 'ManiphestConduitAPIMethod',
     'ManiphestView' => 'AphrontView',
-    'MetaMTAConstants' => 'Phobject',
     'MetaMTAEmailTransactionCommand' => 'Phobject',
     'MetaMTAEmailTransactionCommandTestCase' => 'PhabricatorTestCase',
     'MetaMTAMailReceivedGarbageCollector' => 'PhabricatorGarbageCollector',
     'MetaMTAMailSentGarbageCollector' => 'PhabricatorGarbageCollector',
-    'MetaMTAReceivedMailStatus' => 'MetaMTAConstants',
+    'MetaMTAReceivedMailStatus' => 'Phobject',
     'MultimeterContext' => 'MultimeterDimension',
     'MultimeterControl' => 'Phobject',
     'MultimeterController' => 'PhabricatorController',
@@ -6182,6 +6181,7 @@ phutil_register_library_map(array(
     'PhabricatorMailManagementShowOutboundWorkflow' => 'PhabricatorMailManagementWorkflow',
     'PhabricatorMailManagementVolumeWorkflow' => 'PhabricatorMailManagementWorkflow',
     'PhabricatorMailManagementWorkflow' => 'PhabricatorManagementWorkflow',
+    'PhabricatorMailOutboundStatus' => 'Phobject',
     'PhabricatorMailReceiver' => 'Phobject',
     'PhabricatorMailReceiverTestCase' => 'PhabricatorTestCase',
     'PhabricatorMailReplyHandler' => 'Phobject',

src/applications/metamta/PhabricatorMetaMTAWorker.php

@@ -18,7 +18,7 @@ final class PhabricatorMetaMTAWorker
         pht('Unable to load message!'));
     }
 
-    if ($message->getStatus() != PhabricatorMetaMTAMail::STATUS_QUEUE) {
+    if ($message->getStatus() != PhabricatorMailOutboundStatus::STATUS_QUEUE) {
       return;
     }
 

src/applications/metamta/constants/MetaMTAConstants.php

@@ -1,3 +0,0 @@
-<?php
-
-abstract class MetaMTAConstants extends Phobject {}

src/applications/metamta/constants/MetaMTAReceivedMailStatus.php

@@ -1,7 +1,7 @@
 <?php
 
 final class MetaMTAReceivedMailStatus
-  extends MetaMTAConstants {
+  extends Phobject {
 
   const STATUS_DUPLICATE            = 'err:duplicate';
   const STATUS_FROM_PHABRICATOR     = 'err:self';

src/applications/metamta/constants/PhabricatorMailOutboundStatus.php

@@ -0,0 +1,44 @@
+<?php
+
+final class PhabricatorMailOutboundStatus
+  extends Phobject {
+
+  const STATUS_QUEUE = 'queued';
+  const STATUS_SENT  = 'sent';
+  const STATUS_FAIL  = 'fail';
+  const STATUS_VOID  = 'void';
+
+
+  public static function getStatusName($status_code) {
+    $names = array(
+      self::STATUS_QUEUE => pht('Queued'),
+      self::STATUS_FAIL  => pht('Delivery Failed'),
+      self::STATUS_SENT  => pht('Sent'),
+      self::STATUS_VOID  => pht('Voided'),
+    );
+    $status_code = coalesce($status_code, '?');
+    return idx($names, $status_code, $status_code);
+  }
+
+  public static function getStatusIcon($status_code) {
+    $icons = array(
+      self::STATUS_QUEUE => 'fa-clock-o',
+      self::STATUS_FAIL  => 'fa-warning',
+      self::STATUS_SENT  => 'fa-envelope',
+      self::STATUS_VOID  => 'fa-trash',
+    );
+    return idx($icons, $status_code, 'fa-question-circle');
+  }
+
+  public static function getStatusColor($status_code) {
+    $colors = array(
+      self::STATUS_QUEUE => 'blue',
+      self::STATUS_FAIL  => 'red',
+      self::STATUS_SENT  => 'green',
+      self::STATUS_VOID  => 'black',
+    );
+
+    return idx($colors, $status_code, 'yellow');
+  }
+
+}

src/applications/metamta/controller/PhabricatorMetaMTAMailViewController.php

@@ -25,34 +25,10 @@ final class PhabricatorMetaMTAMailViewController
       ->setUser($viewer)
       ->setPolicyObject($mail);
 
-    switch ($mail->getStatus()) {
+    $status = $mail->getStatus();
-      case PhabricatorMetaMTAMail::STATUS_QUEUE:
+    $name = PhabricatorMailOutboundStatus::getStatusName($status);
-        $icon = 'fa-clock-o';
+    $icon = PhabricatorMailOutboundStatus::getStatusIcon($status);
-        $color = 'blue';
+    $color = PhabricatorMailOutboundStatus::getStatusColor($status);
-        $name = pht('Queued');
-        break;
-      case PhabricatorMetaMTAMail::STATUS_SENT:
-        $icon = 'fa-envelope';
-        $color = 'green';
-        $name = pht('Sent');
-        break;
-      case PhabricatorMetaMTAMail::STATUS_FAIL:
-        $icon = 'fa-envelope';
-        $color = 'red';
-        $name = pht('Delivery Failed');
-        break;
-      case PhabricatorMetaMTAMail::STATUS_VOID:
-        $icon = 'fa-envelope';
-        $color = 'black';
-        $name = pht('Voided');
-        break;
-      default:
-        $icon = 'fa-question-circle';
-        $color = 'yellow';
-        $name = pht('Unknown');
-        break;
-    }
-
     $header->setStatus($icon, $color, $name);
 
     $crumbs = $this->buildApplicationCrumbs()
@@ -249,6 +225,8 @@ final class PhabricatorMetaMTAMailViewController
     $properties = id(new PHUIPropertyListView())
       ->setUser($viewer);
 
+    $properties->addProperty(pht('Message PHID'), $mail->getPHID());
+
     $details = $mail->getMessage();
     if (!strlen($details)) {
       $details = phutil_tag('em', array(), pht('None'));

src/applications/metamta/management/PhabricatorMailManagementListOutboundWorkflow.php

@@ -45,7 +45,7 @@ final class PhabricatorMailManagementListOutboundWorkflow
 
       $table->addRow(array(
         'id'      => $mail->getID(),
-        'status'  => PhabricatorMetaMTAMail::getReadableStatus($status),
+        'status'  => PhabricatorMailOutboundStatus::getStatusName($status),
         'subject' => $mail->getSubject(),
       ));
     }

src/applications/metamta/management/PhabricatorMailManagementResendWorkflow.php

@@ -47,7 +47,7 @@ final class PhabricatorMailManagementResendWorkflow
     }
 
     foreach ($messages as $message) {
-      $message->setStatus(PhabricatorMetaMTAMail::STATUS_QUEUE);
+      $message->setStatus(PhabricatorMailOutboundStatus::STATUS_QUEUE);
       $message->save();
 
       $mailer_task = PhabricatorWorker::scheduleTask(

src/applications/metamta/query/PhabricatorMetaMTAMailSearchEngine.php

@@ -101,7 +101,7 @@ final class PhabricatorMetaMTAMailSearchEngine
 
     foreach ($mails as $mail) {
       if ($mail->hasSensitiveContent()) {
-        $header = pht('< content redacted >');
+        $header = phutil_tag('em', array(), pht('Content Redacted'));
       } else {
         $header = $mail->getSubject();
       }
@@ -110,9 +110,16 @@ final class PhabricatorMetaMTAMailSearchEngine
         ->setUser($viewer)
         ->setObject($mail)
         ->setEpoch($mail->getDateCreated())
-        ->setObjectName('Mail '.$mail->getID())
+        ->setObjectName(pht('Mail %d', $mail->getID()))
         ->setHeader($header)
-        ->setHref($this->getURI('detail/'.$mail->getID()));
+        ->setHref($this->getURI('detail/'.$mail->getID().'/'));
+
+      $status = $mail->getStatus();
+      $status_name = PhabricatorMailOutboundStatus::getStatusName($status);
+      $status_icon = PhabricatorMailOutboundStatus::getStatusIcon($status);
+      $status_color = PhabricatorMailOutboundStatus::getStatusColor($status);
+      $item->setStatusIcon($status_icon.' '.$status_color, $status_name);
+
       $list->addItem($item);
     }
 

src/applications/metamta/storage/PhabricatorMetaMTAMail.php

@@ -7,11 +7,6 @@ final class PhabricatorMetaMTAMail
   extends PhabricatorMetaMTADAO
   implements PhabricatorPolicyInterface {
 
-  const STATUS_QUEUE = 'queued';
-  const STATUS_SENT  = 'sent';
-  const STATUS_FAIL  = 'fail';
-  const STATUS_VOID  = 'void';
-
   const RETRY_DELAY   = 5;
 
   protected $actorPHID;
@@ -24,7 +19,7 @@ final class PhabricatorMetaMTAMail
 
   public function __construct() {
 
-    $this->status     = self::STATUS_QUEUE;
+    $this->status     = PhabricatorMailOutboundStatus::STATUS_QUEUE;
     $this->parameters = array('sensitive' => true);
 
     parent::__construct();
@@ -430,7 +425,7 @@ final class PhabricatorMetaMTAMail
     }
 
     if (!$force_send) {
-      if ($this->getStatus() != self::STATUS_QUEUE) {
+      if ($this->getStatus() != PhabricatorMailOutboundStatus::STATUS_QUEUE) {
         throw new Exception(pht('Trying to send an already-sent mail!'));
       }
     }
@@ -662,7 +657,7 @@ final class PhabricatorMetaMTAMail
       $this->setParam('actors.sent', $actor_list);
 
       if (!$add_to && !$add_cc) {
-        $this->setStatus(self::STATUS_VOID);
+        $this->setStatus(PhabricatorMailOutboundStatus::STATUS_VOID);
         $this->setMessage(
           pht(
             'Message has no valid recipients: all To/Cc are disabled, '.
@@ -673,7 +668,7 @@ final class PhabricatorMetaMTAMail
       if ($this->getIsErrorEmail()) {
         $all_recipients = array_merge($add_to, $add_cc);
         if ($this->shouldRateLimitMail($all_recipients)) {
-          $this->setStatus(self::STATUS_VOID);
+          $this->setStatus(PhabricatorMailOutboundStatus::STATUS_VOID);
           $this->setMessage(
             pht(
               'This is an error email, but one or more recipients have '.
@@ -684,7 +679,7 @@ final class PhabricatorMetaMTAMail
       }
 
       if (PhabricatorEnv::getEnvConfig('phabricator.silent')) {
-        $this->setStatus(self::STATUS_VOID);
+        $this->setStatus(PhabricatorMailOutboundStatus::STATUS_VOID);
         $this->setMessage(
           pht(
             'Phabricator is running in silent mode. See `%s` '.
@@ -716,7 +711,7 @@ final class PhabricatorMetaMTAMail
       }
     } catch (Exception $ex) {
       $this
-        ->setStatus(self::STATUS_FAIL)
+        ->setStatus(PhabricatorMailOutboundStatus::STATUS_FAIL)
         ->setMessage($ex->getMessage())
         ->save();
 
@@ -732,13 +727,13 @@ final class PhabricatorMetaMTAMail
           pht('Mail adapter encountered an unexpected, unspecified failure.'));
       }
 
-      $this->setStatus(self::STATUS_SENT);
+      $this->setStatus(PhabricatorMailOutboundStatus::STATUS_SENT);
       $this->save();
 
       return $this;
     } catch (PhabricatorMetaMTAPermanentFailureException $ex) {
       $this
-        ->setStatus(self::STATUS_FAIL)
+        ->setStatus(PhabricatorMailOutboundStatus::STATUS_FAIL)
         ->setMessage($ex->getMessage())
         ->save();
 
@@ -752,17 +747,6 @@ final class PhabricatorMetaMTAMail
     }
   }
 
-  public static function getReadableStatus($status_code) {
-    $readable = array(
-      self::STATUS_QUEUE => pht('Queued for Delivery'),
-      self::STATUS_FAIL  => pht('Delivery Failed'),
-      self::STATUS_SENT  => pht('Sent'),
-      self::STATUS_VOID  => pht('Void'),
-    );
-    $status_code = coalesce($status_code, '?');
-    return idx($readable, $status_code, $status_code);
-  }
-
   private function generateThreadIndex($seed, $is_first_mail) {
     // When threading, Outlook ignores the 'References' and 'In-Reply-To'
     // headers that most clients use. Instead, it uses a custom 'Thread-Index'

src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php

@@ -20,7 +20,7 @@ final class PhabricatorMetaMTAMailTestCase extends PhabricatorTestCase {
     $mailer = new PhabricatorMailImplementationTestAdapter();
     $mail->sendNow($force = true, $mailer);
     $this->assertEqual(
-      PhabricatorMetaMTAMail::STATUS_SENT,
+      PhabricatorMailOutboundStatus::STATUS_SENT,
       $mail->getStatus());
 
 
@@ -36,7 +36,7 @@ final class PhabricatorMetaMTAMailTestCase extends PhabricatorTestCase {
       // Ignore.
     }
     $this->assertEqual(
-      PhabricatorMetaMTAMail::STATUS_QUEUE,
+      PhabricatorMailOutboundStatus::STATUS_QUEUE,
       $mail->getStatus());
 
 
@@ -52,7 +52,7 @@ final class PhabricatorMetaMTAMailTestCase extends PhabricatorTestCase {
       // Ignore.
     }
     $this->assertEqual(
-      PhabricatorMetaMTAMail::STATUS_FAIL,
+      PhabricatorMailOutboundStatus::STATUS_FAIL,
       $mail->getStatus());
   }