Drive Differential e-mail message by field specification

Summary: Refactoring before the actual change.

Test Plan: None yet.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

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

src/applications/differential/field/selector/DifferentialDefaultFieldSelector.php

@@ -88,5 +88,26 @@ final class DifferentialDefaultFieldSelector
     return array_values($map);
   }
 
+  public function sortFieldsForMail(array $fields) {
+    assert_instances_of($fields, 'DifferentialFieldSpecification');
+
+    $map = array();
+    foreach ($fields as $field) {
+      $map[get_class($field)] = $field;
+    }
+
+    $map = array_select_keys(
+      $map,
+      array(
+        'DifferentialSummaryFieldSpecification',
+        'DifferentialTestPlanFieldSpecification',
+        'DifferentialRevisionIDFieldSpecification',
+        'DifferentialBranchFieldSpecification',
+        'DifferentialCommitsFieldSpecification',
+      )) + $map;
+
+    return array_values($map);
+  }
+
 }
 

src/applications/differential/field/selector/DifferentialFieldSelector.php

@@ -33,4 +33,9 @@ abstract class DifferentialFieldSelector {
     return $fields;
   }
 
+  public function sortFieldsForMail(array $fields) {
+    assert_instances_of($fields, 'DifferentialFieldSpecification');
+    return $fields;
+  }
+
 }

src/applications/differential/field/specification/DifferentialBranchFieldSpecification.php

@@ -38,4 +38,21 @@ final class DifferentialBranchFieldSpecification
     return phutil_escape_html($branch);
   }
 
+  public function renderValueForMail() {
+    $status = $this->getRevision()->getStatus();
+
+    if ($status != ArcanistDifferentialRevisionStatus::NEEDS_REVISION &&
+        $status != ArcanistDifferentialRevisionStatus::ACCEPTED) {
+      return null;
+    }
+
+    $diff = $this->getRevision()->loadActiveDiff();
+    if ($diff) {
+      $branch = $diff->getBranch();
+      if ($branch) {
+        return "BRANCH\n  $branch";
+      }
+    }
+  }
+
 }

src/applications/differential/field/specification/DifferentialCommitsFieldSpecification.php

@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -50,4 +50,34 @@ final class DifferentialCommitsFieldSpecification
     return $revision->getCommitPHIDs();
   }
 
+  public function renderValueForMail() {
+    $revision = $this->getRevision();
+
+    if ($revision->getStatus() != ArcanistDifferentialRevisionStatus::CLOSED) {
+      return null;
+    }
+
+    $phids = $revision->loadCommitPHIDs();
+    if (!$phids) {
+      return null;
+    }
+
+    $body = array();
+    $handles = id(new PhabricatorObjectHandleData($phids))->loadHandles();
+    if (count($handles) == 1) {
+      $body[] = "COMMIT";
+    } else {
+      // This is unlikely to ever happen since we'll send this mail the
+      // first time we discover a commit, but it's not impossible if data
+      // was migrated, etc.
+      $body[] = "COMMITS";
+    }
+
+    foreach ($handles as $handle) {
+      $body[] = '  '.PhabricatorEnv::getProductionURI($handle->getURI());
+    }
+
+    return implode("\n", $body);
+  }
+
 }

src/applications/differential/field/specification/DifferentialFieldSpecification.php

@@ -28,6 +28,7 @@
  * @task edit Extending the Revision Edit Interface
  * @task view Extending the Revision View Interface
  * @task list Extending the Revision List Interface
+ * @task mail Extending the E-mail Interface
  * @task conduit Extending the Conduit View Interface
  * @task commit Extending Commit Messages
  * @task load Loading Additional Data
@@ -367,6 +368,22 @@ abstract class DifferentialFieldSpecification {
   }
 
 
+/* -(  Extending the E-mail Interface  )------------------------------------- */
+
+
+  /**
+   * Return plain text to render in e-mail messages. The text may span
+   * multiple lines.
+   *
+   * @return string|null Plain text, or null for no message.
+   *
+   * @task mail
+   */
+  public function renderValueForMail() {
+    return null;
+  }
+
+
 /* -(  Extending the Conduit Interface  )------------------------------------ */
 
 

src/applications/differential/field/specification/DifferentialRevisionIDFieldSpecification.php

@@ -104,4 +104,9 @@ final class DifferentialRevisionIDFieldSpecification
     return 'D'.$revision->getID();
   }
 
+  public function renderValueForMail() {
+    $uri = PhabricatorEnv::getProductionURI('/D'.$this->id);
+    return "REVISION DETAIL\n  {$uri}";
+  }
+
 }

src/applications/differential/mail/DifferentialCommentMail.php

@@ -161,40 +161,15 @@ final class DifferentialCommentMail extends DifferentialMail {
       $body[] = null;
     }
 
-    $body[] = $this->renderRevisionDetailLink();
+    $selector = DifferentialFieldSelector::newSelector();
-    $body[] = null;
+    $aux_fields = $selector->sortFieldsForMail(
+      $selector->getFieldSpecifications());
 
-    $revision = $this->getRevision();
+    foreach ($aux_fields as $field) {
-    $status = $revision->getStatus();
+      $field->setRevision($this->getRevision());
-
+      $text = $field->renderValueForMail();
-    if ($status == ArcanistDifferentialRevisionStatus::NEEDS_REVISION ||
+      if ($text !== null) {
-        $status == ArcanistDifferentialRevisionStatus::ACCEPTED) {
+        $body[] = $text;
-      $diff = $revision->loadActiveDiff();
-      if ($diff) {
-        $branch = $diff->getBranch();
-        if ($branch) {
-          $body[] = "BRANCH\n  $branch";
-          $body[] = null;
-        }
-      }
-    }
-
-    if ($status == ArcanistDifferentialRevisionStatus::CLOSED) {
-      $phids = $revision->loadCommitPHIDs();
-      if ($phids) {
-        $handles = id(new PhabricatorObjectHandleData($phids))->loadHandles();
-        if (count($handles) == 1) {
-          $body[] = "COMMIT";
-        } else {
-          // This is unlikely to ever happen since we'll send this mail the
-          // first time we discover a commit, but it's not impossible if data
-          // was migrated, etc.
-          $body[] = "COMMITS";
-        }
-
-        foreach ($handles as $handle) {
-          $body[] = '  '.PhabricatorEnv::getProductionURI($handle->getURI());
-        }
         $body[] = null;
       }
     }