Modularize all DifferentialRevision fields

Summary: Ref T8726. Make all the DifferentialRevision stuff modular.

Test Plan:
  - Created a rule with all fields.
  - Ran upgrade.
  - Saw all fields preserved with new modular versions.
  - Used test console to run rule with all fields, verified field values as broadly sensible.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8726

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

resources/sql/autopatches/20150708.herald.1.sql

@@ -0,0 +1,90 @@
+UPDATE {$NAMESPACE}_herald.herald_condition c
+  JOIN {$NAMESPACE}_herald.herald_rule r
+  ON c.ruleID = r.id
+  SET c.fieldName = 'differential.revision.diff.affected'
+  WHERE r.contentType = 'differential'
+  AND c.fieldName = 'diff-file';
+
+UPDATE {$NAMESPACE}_herald.herald_condition c
+  JOIN {$NAMESPACE}_herald.herald_rule r
+  ON c.ruleID = r.id
+  SET c.fieldName = 'differential.revision.author'
+  WHERE r.contentType = 'differential'
+  AND c.fieldName = 'author';
+
+UPDATE {$NAMESPACE}_herald.herald_condition c
+  JOIN {$NAMESPACE}_herald.herald_rule r
+  ON c.ruleID = r.id
+  SET c.fieldName = 'differential.revision.author.projects'
+  WHERE r.contentType = 'differential'
+  AND c.fieldName = 'authorprojects';
+
+UPDATE {$NAMESPACE}_herald.herald_condition c
+  JOIN {$NAMESPACE}_herald.herald_rule r
+  ON c.ruleID = r.id
+  SET c.fieldName = 'differential.revision.diff.new'
+  WHERE r.contentType = 'differential'
+  AND c.fieldName = 'diff-added-content';
+
+UPDATE {$NAMESPACE}_herald.herald_condition c
+  JOIN {$NAMESPACE}_herald.herald_rule r
+  ON c.ruleID = r.id
+  SET c.fieldName = 'differential.revision.diff.content'
+  WHERE r.contentType = 'differential'
+  AND c.fieldName = 'diff-content';
+
+UPDATE {$NAMESPACE}_herald.herald_condition c
+  JOIN {$NAMESPACE}_herald.herald_rule r
+  ON c.ruleID = r.id
+  SET c.fieldName = 'differential.revision.diff.old'
+  WHERE r.contentType = 'differential'
+  AND c.fieldName = 'diff-removed-content';
+
+UPDATE {$NAMESPACE}_herald.herald_condition c
+  JOIN {$NAMESPACE}_herald.herald_rule r
+  ON c.ruleID = r.id
+  SET c.fieldName = 'differential.revision.package'
+  WHERE r.contentType = 'differential'
+  AND c.fieldName = 'affected-package';
+
+UPDATE {$NAMESPACE}_herald.herald_condition c
+  JOIN {$NAMESPACE}_herald.herald_rule r
+  ON c.ruleID = r.id
+  SET c.fieldName = 'differential.revision.repository'
+  WHERE r.contentType = 'differential'
+  AND c.fieldName = 'repository';
+
+UPDATE {$NAMESPACE}_herald.herald_condition c
+  JOIN {$NAMESPACE}_herald.herald_rule r
+  ON c.ruleID = r.id
+  SET c.fieldName = 'differential.revision.repository.projects'
+  WHERE r.contentType = 'differential'
+  AND c.fieldName = 'repository-projects';
+
+UPDATE {$NAMESPACE}_herald.herald_condition c
+  JOIN {$NAMESPACE}_herald.herald_rule r
+  ON c.ruleID = r.id
+  SET c.fieldName = 'differential.revision.reviewers'
+  WHERE r.contentType = 'differential'
+  AND c.fieldName = 'reviewers';
+
+UPDATE {$NAMESPACE}_herald.herald_condition c
+  JOIN {$NAMESPACE}_herald.herald_rule r
+  ON c.ruleID = r.id
+  SET c.fieldName = 'differential.revision.summary'
+  WHERE r.contentType = 'differential'
+  AND c.fieldName = 'body';
+
+UPDATE {$NAMESPACE}_herald.herald_condition c
+  JOIN {$NAMESPACE}_herald.herald_rule r
+  ON c.ruleID = r.id
+  SET c.fieldName = 'differential.revision.title'
+  WHERE r.contentType = 'differential'
+  AND c.fieldName = 'title';
+
+UPDATE {$NAMESPACE}_herald.herald_condition c
+  JOIN {$NAMESPACE}_herald.herald_rule r
+  ON c.ruleID = r.id
+  SET c.fieldName = 'differential.revision.package.owners'
+  WHERE r.contentType = 'differential'
+  AND c.fieldName = 'affected-package-owner';

src/__phutil_library_map__.php

@@ -436,7 +436,13 @@ phutil_register_library_map(array(
     'DifferentialReviewersField' => 'applications/differential/customfield/DifferentialReviewersField.php',
     'DifferentialReviewersView' => 'applications/differential/view/DifferentialReviewersView.php',
     'DifferentialRevision' => 'applications/differential/storage/DifferentialRevision.php',
+    'DifferentialRevisionAffectedFilesHeraldField' => 'applications/differential/herald/DifferentialRevisionAffectedFilesHeraldField.php',
+    'DifferentialRevisionAuthorHeraldField' => 'applications/differential/herald/DifferentialRevisionAuthorHeraldField.php',
+    'DifferentialRevisionAuthorProjectsHeraldField' => 'applications/differential/herald/DifferentialRevisionAuthorProjectsHeraldField.php',
     'DifferentialRevisionCloseDetailsController' => 'applications/differential/controller/DifferentialRevisionCloseDetailsController.php',
+    'DifferentialRevisionContentAddedHeraldField' => 'applications/differential/herald/DifferentialRevisionContentAddedHeraldField.php',
+    'DifferentialRevisionContentHeraldField' => 'applications/differential/herald/DifferentialRevisionContentHeraldField.php',
+    'DifferentialRevisionContentRemovedHeraldField' => 'applications/differential/herald/DifferentialRevisionContentRemovedHeraldField.php',
     'DifferentialRevisionControlSystem' => 'applications/differential/constants/DifferentialRevisionControlSystem.php',
     'DifferentialRevisionDependedOnByRevisionEdgeType' => 'applications/differential/edge/DifferentialRevisionDependedOnByRevisionEdgeType.php',
     'DifferentialRevisionDependsOnRevisionEdgeType' => 'applications/differential/edge/DifferentialRevisionDependsOnRevisionEdgeType.php',
@@ -445,15 +451,23 @@ phutil_register_library_map(array(
     'DifferentialRevisionHasCommitEdgeType' => 'applications/differential/edge/DifferentialRevisionHasCommitEdgeType.php',
     'DifferentialRevisionHasReviewerEdgeType' => 'applications/differential/edge/DifferentialRevisionHasReviewerEdgeType.php',
     'DifferentialRevisionHasTaskEdgeType' => 'applications/differential/edge/DifferentialRevisionHasTaskEdgeType.php',
+    'DifferentialRevisionHeraldField' => 'applications/differential/herald/DifferentialRevisionHeraldField.php',
     'DifferentialRevisionIDField' => 'applications/differential/customfield/DifferentialRevisionIDField.php',
     'DifferentialRevisionLandController' => 'applications/differential/controller/DifferentialRevisionLandController.php',
     'DifferentialRevisionListController' => 'applications/differential/controller/DifferentialRevisionListController.php',
     'DifferentialRevisionListView' => 'applications/differential/view/DifferentialRevisionListView.php',
     'DifferentialRevisionMailReceiver' => 'applications/differential/mail/DifferentialRevisionMailReceiver.php',
     'DifferentialRevisionPHIDType' => 'applications/differential/phid/DifferentialRevisionPHIDType.php',
+    'DifferentialRevisionPackageHeraldField' => 'applications/differential/herald/DifferentialRevisionPackageHeraldField.php',
+    'DifferentialRevisionPackageOwnerHeraldField' => 'applications/differential/herald/DifferentialRevisionPackageOwnerHeraldField.php',
     'DifferentialRevisionQuery' => 'applications/differential/query/DifferentialRevisionQuery.php',
+    'DifferentialRevisionRepositoryHeraldField' => 'applications/differential/herald/DifferentialRevisionRepositoryHeraldField.php',
+    'DifferentialRevisionRepositoryProjectsHeraldField' => 'applications/differential/herald/DifferentialRevisionRepositoryProjectsHeraldField.php',
+    'DifferentialRevisionReviewersHeraldField' => 'applications/differential/herald/DifferentialRevisionReviewersHeraldField.php',
     'DifferentialRevisionSearchEngine' => 'applications/differential/query/DifferentialRevisionSearchEngine.php',
     'DifferentialRevisionStatus' => 'applications/differential/constants/DifferentialRevisionStatus.php',
+    'DifferentialRevisionSummaryHeraldField' => 'applications/differential/herald/DifferentialRevisionSummaryHeraldField.php',
+    'DifferentialRevisionTitleHeraldField' => 'applications/differential/herald/DifferentialRevisionTitleHeraldField.php',
     'DifferentialRevisionUpdateHistoryView' => 'applications/differential/view/DifferentialRevisionUpdateHistoryView.php',
     'DifferentialRevisionViewController' => 'applications/differential/controller/DifferentialRevisionViewController.php',
     'DifferentialSchemaSpec' => 'applications/differential/storage/DifferentialSchemaSpec.php',
@@ -971,7 +985,7 @@ phutil_register_library_map(array(
     'HeraldDAO' => 'applications/herald/storage/HeraldDAO.php',
     'HeraldDifferentialAdapter' => 'applications/differential/herald/HeraldDifferentialAdapter.php',
     'HeraldDifferentialDiffAdapter' => 'applications/differential/herald/HeraldDifferentialDiffAdapter.php',
-    'HeraldDifferentialRevisionAdapter' => 'applications/herald/adapter/HeraldDifferentialRevisionAdapter.php',
+    'HeraldDifferentialRevisionAdapter' => 'applications/differential/herald/HeraldDifferentialRevisionAdapter.php',
     'HeraldDisableController' => 'applications/herald/controller/HeraldDisableController.php',
     'HeraldEffect' => 'applications/herald/engine/HeraldEffect.php',
     'HeraldEngine' => 'applications/herald/engine/HeraldEngine.php',
@@ -3905,7 +3919,13 @@ phutil_register_library_map(array(
       'PhabricatorDestructibleInterface',
       'PhabricatorProjectInterface',
     ),
+    'DifferentialRevisionAffectedFilesHeraldField' => 'DifferentialRevisionHeraldField',
+    'DifferentialRevisionAuthorHeraldField' => 'DifferentialRevisionHeraldField',
+    'DifferentialRevisionAuthorProjectsHeraldField' => 'DifferentialRevisionHeraldField',
     'DifferentialRevisionCloseDetailsController' => 'DifferentialController',
+    'DifferentialRevisionContentAddedHeraldField' => 'DifferentialRevisionHeraldField',
+    'DifferentialRevisionContentHeraldField' => 'DifferentialRevisionHeraldField',
+    'DifferentialRevisionContentRemovedHeraldField' => 'DifferentialRevisionHeraldField',
     'DifferentialRevisionControlSystem' => 'Phobject',
     'DifferentialRevisionDependedOnByRevisionEdgeType' => 'PhabricatorEdgeType',
     'DifferentialRevisionDependsOnRevisionEdgeType' => 'PhabricatorEdgeType',
@@ -3914,15 +3934,23 @@ phutil_register_library_map(array(
     'DifferentialRevisionHasCommitEdgeType' => 'PhabricatorEdgeType',
     'DifferentialRevisionHasReviewerEdgeType' => 'PhabricatorEdgeType',
     'DifferentialRevisionHasTaskEdgeType' => 'PhabricatorEdgeType',
+    'DifferentialRevisionHeraldField' => 'HeraldField',
     'DifferentialRevisionIDField' => 'DifferentialCustomField',
     'DifferentialRevisionLandController' => 'DifferentialController',
     'DifferentialRevisionListController' => 'DifferentialController',
     'DifferentialRevisionListView' => 'AphrontView',
     'DifferentialRevisionMailReceiver' => 'PhabricatorObjectMailReceiver',
     'DifferentialRevisionPHIDType' => 'PhabricatorPHIDType',
+    'DifferentialRevisionPackageHeraldField' => 'DifferentialRevisionHeraldField',
+    'DifferentialRevisionPackageOwnerHeraldField' => 'DifferentialRevisionHeraldField',
     'DifferentialRevisionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
+    'DifferentialRevisionRepositoryHeraldField' => 'DifferentialRevisionHeraldField',
+    'DifferentialRevisionRepositoryProjectsHeraldField' => 'DifferentialRevisionHeraldField',
+    'DifferentialRevisionReviewersHeraldField' => 'DifferentialRevisionHeraldField',
     'DifferentialRevisionSearchEngine' => 'PhabricatorApplicationSearchEngine',
     'DifferentialRevisionStatus' => 'Phobject',
+    'DifferentialRevisionSummaryHeraldField' => 'DifferentialRevisionHeraldField',
+    'DifferentialRevisionTitleHeraldField' => 'DifferentialRevisionHeraldField',
     'DifferentialRevisionUpdateHistoryView' => 'AphrontView',
     'DifferentialRevisionViewController' => 'DifferentialController',
     'DifferentialSchemaSpec' => 'PhabricatorConfigSchemaSpec',

src/applications/differential/herald/DifferentialRevisionAffectedFilesHeraldField.php

@@ -0,0 +1,30 @@
+<?php
+
+final class DifferentialRevisionAffectedFilesHeraldField
+  extends DifferentialRevisionHeraldField {
+
+  const FIELDCONST = 'differential.revision.diff.affected';
+
+  public function getHeraldFieldName() {
+    return pht('Affected files');
+  }
+
+  public function getHeraldFieldValue($object) {
+    return $this->getAdapter()->loadAffectedPaths();
+  }
+
+  protected function getHeraldFieldStandardConditions() {
+    return self::STANDARD_TEXT_LIST;
+  }
+
+  public function getHeraldFieldValueType($condition) {
+    switch ($condition) {
+      case HeraldAdapter::CONDITION_EXISTS:
+      case HeraldAdapter::CONDITION_NOT_EXISTS:
+        return HeraldAdapter::VALUE_NONE;
+      default:
+        return HeraldAdapter::VALUE_TEXT;
+    }
+  }
+
+}

src/applications/differential/herald/DifferentialRevisionAuthorHeraldField.php

@@ -0,0 +1,24 @@
+<?php
+
+final class DifferentialRevisionAuthorHeraldField
+  extends DifferentialRevisionHeraldField {
+
+  const FIELDCONST = 'differential.revision.author';
+
+  public function getHeraldFieldName() {
+    return pht('Author');
+  }
+
+  public function getHeraldFieldValue($object) {
+    return $object->getAuthorPHID();
+  }
+
+  protected function getHeraldFieldStandardConditions() {
+    return self::STANDARD_PHID;
+  }
+
+  public function getHeraldFieldValueType($condition) {
+    return HeraldAdapter::VALUE_USER;
+  }
+
+}

src/applications/differential/herald/DifferentialRevisionAuthorProjectsHeraldField.php

@@ -0,0 +1,32 @@
+<?php
+
+final class DifferentialRevisionAuthorProjectsHeraldField
+  extends DifferentialRevisionHeraldField {
+
+  const FIELDCONST = 'differential.revision.author.projects';
+
+  public function getHeraldFieldName() {
+    return pht("Author's projects");
+  }
+
+  public function getHeraldFieldValue($object) {
+    return PhabricatorEdgeQuery::loadDestinationPHIDs(
+      $object->getAuthorPHID(),
+      PhabricatorProjectMemberOfProjectEdgeType::EDGECONST);
+  }
+
+  protected function getHeraldFieldStandardConditions() {
+    return self::STANDARD_LIST;
+  }
+
+  public function getHeraldFieldValueType($condition) {
+    switch ($condition) {
+      case HeraldAdapter::CONDITION_EXISTS:
+      case HeraldAdapter::CONDITION_NOT_EXISTS:
+        return HeraldAdapter::VALUE_NONE;
+      default:
+        return HeraldAdapter::VALUE_PROJECT;
+    }
+  }
+
+}

src/applications/differential/herald/DifferentialRevisionContentAddedHeraldField.php

@@ -0,0 +1,24 @@
+<?php
+
+final class DifferentialRevisionContentAddedHeraldField
+  extends DifferentialRevisionHeraldField {
+
+  const FIELDCONST = 'differential.revision.diff.new';
+
+  public function getHeraldFieldName() {
+    return pht('Added file content');
+  }
+
+  public function getHeraldFieldValue($object) {
+    return $this->getAdapter()->loadAddedContentDictionary();
+  }
+
+  protected function getHeraldFieldStandardConditions() {
+    return self::STANDARD_TEXT_MAP;
+  }
+
+  public function getHeraldFieldValueType($condition) {
+    return HeraldAdapter::VALUE_TEXT;
+  }
+
+}

src/applications/differential/herald/DifferentialRevisionContentHeraldField.php

@@ -0,0 +1,24 @@
+<?php
+
+final class DifferentialRevisionContentHeraldField
+  extends DifferentialRevisionHeraldField {
+
+  const FIELDCONST = 'differential.revision.diff.content';
+
+  public function getHeraldFieldName() {
+    return pht('Changed file content');
+  }
+
+  public function getHeraldFieldValue($object) {
+    return $this->getAdapter()->loadContentDictionary();
+  }
+
+  protected function getHeraldFieldStandardConditions() {
+    return self::STANDARD_TEXT_MAP;
+  }
+
+  public function getHeraldFieldValueType($condition) {
+    return HeraldAdapter::VALUE_TEXT;
+  }
+
+}

src/applications/differential/herald/DifferentialRevisionContentRemovedHeraldField.php

@@ -0,0 +1,24 @@
+<?php
+
+final class DifferentialRevisionContentRemovedHeraldField
+  extends DifferentialRevisionHeraldField {
+
+  const FIELDCONST = 'differential.revision.diff.old';
+
+  public function getHeraldFieldName() {
+    return pht('Removed file content');
+  }
+
+  public function getHeraldFieldValue($object) {
+    return $this->getAdapter()->loadRemovedContentDictionary();
+  }
+
+  protected function getHeraldFieldStandardConditions() {
+    return self::STANDARD_TEXT_MAP;
+  }
+
+  public function getHeraldFieldValueType($condition) {
+    return HeraldAdapter::VALUE_TEXT;
+  }
+
+}

src/applications/differential/herald/DifferentialRevisionHeraldField.php

@@ -0,0 +1,9 @@
+<?php
+
+abstract class DifferentialRevisionHeraldField extends HeraldField {
+
+  public function supportsObject($object) {
+    return ($object instanceof DifferentialRevision);
+  }
+
+}

src/applications/differential/herald/DifferentialRevisionPackageHeraldField.php

@@ -0,0 +1,31 @@
+<?php
+
+final class DifferentialRevisionPackageHeraldField
+  extends DifferentialRevisionHeraldField {
+
+  const FIELDCONST = 'differential.revision.package';
+
+  public function getHeraldFieldName() {
+    return pht('Affected packages');
+  }
+
+  public function getHeraldFieldValue($object) {
+    $packages = $this->getAdapter()->loadAffectedPackages();
+    return mpull($packages, 'getPHID');
+  }
+
+  protected function getHeraldFieldStandardConditions() {
+    return self::STANDARD_LIST;
+  }
+
+  public function getHeraldFieldValueType($condition) {
+    switch ($condition) {
+      case HeraldAdapter::CONDITION_EXISTS:
+      case HeraldAdapter::CONDITION_NOT_EXISTS:
+        return HeraldAdapter::VALUE_NONE;
+      default:
+        return HeraldAdapter::VALUE_OWNERS_PACKAGE;
+    }
+  }
+
+}

src/applications/differential/herald/DifferentialRevisionPackageOwnerHeraldField.php

@@ -0,0 +1,36 @@
+<?php
+
+final class DifferentialRevisionPackageOwnerHeraldField
+  extends DifferentialRevisionHeraldField {
+
+  const FIELDCONST = 'differential.revision.package.owners';
+
+  public function getHeraldFieldName() {
+    return pht('Affected package owners');
+  }
+
+  public function getHeraldFieldValue($object) {
+    $packages = $this->getAdapter()->loadAffectedPackages();
+    if (!$packages) {
+      return array();
+    }
+
+    $owners = PhabricatorOwnersOwner::loadAllForPackages($packages);
+    return mpull($owners, 'getUserPHID');
+  }
+
+  protected function getHeraldFieldStandardConditions() {
+    return self::STANDARD_LIST;
+  }
+
+  public function getHeraldFieldValueType($condition) {
+    switch ($condition) {
+      case HeraldAdapter::CONDITION_EXISTS:
+      case HeraldAdapter::CONDITION_NOT_EXISTS:
+        return HeraldAdapter::VALUE_NONE;
+      default:
+        return HeraldAdapter::VALUE_USER_OR_PROJECT;
+    }
+  }
+
+}

src/applications/differential/herald/DifferentialRevisionRepositoryHeraldField.php

@@ -0,0 +1,36 @@
+<?php
+
+final class DifferentialRevisionRepositoryHeraldField
+  extends DifferentialRevisionHeraldField {
+
+  const FIELDCONST = 'differential.revision.repository';
+
+  public function getHeraldFieldName() {
+    return pht('Repository');
+  }
+
+  public function getHeraldFieldValue($object) {
+    $repository = $this->getAdapter()->loadRepository();
+
+    if (!$repository) {
+      return null;
+    }
+
+    return $repository->getPHID();
+  }
+
+  protected function getHeraldFieldStandardConditions() {
+    return self::STANDARD_PHID_NULLABLE;
+  }
+
+  public function getHeraldFieldValueType($condition) {
+    switch ($condition) {
+      case HeraldAdapter::CONDITION_EXISTS:
+      case HeraldAdapter::CONDITION_NOT_EXISTS:
+        return HeraldAdapter::VALUE_NONE;
+      default:
+        return HeraldAdapter::VALUE_REPOSITORY;
+    }
+  }
+
+}

src/applications/differential/herald/DifferentialRevisionRepositoryProjectsHeraldField.php

@@ -0,0 +1,37 @@
+<?php
+
+final class DifferentialRevisionRepositoryProjectsHeraldField
+  extends DifferentialRevisionHeraldField {
+
+  const FIELDCONST = 'differential.revision.repository.projects';
+
+  public function getHeraldFieldName() {
+    return pht('Repository projects');
+  }
+
+  public function getHeraldFieldValue($object) {
+    $repository = $this->getAdapter()->loadRepository();
+    if (!$repository) {
+      return array();
+    }
+
+    return PhabricatorEdgeQuery::loadDestinationPHIDs(
+      $repository->getPHID(),
+      PhabricatorProjectObjectHasProjectEdgeType::EDGECONST);
+  }
+
+  protected function getHeraldFieldStandardConditions() {
+    return self::STANDARD_LIST;
+  }
+
+  public function getHeraldFieldValueType($condition) {
+    switch ($condition) {
+      case HeraldAdapter::CONDITION_EXISTS:
+      case HeraldAdapter::CONDITION_NOT_EXISTS:
+        return HeraldAdapter::VALUE_NONE;
+      default:
+        return HeraldAdapter::VALUE_PROJECT;
+    }
+  }
+
+}

src/applications/differential/herald/DifferentialRevisionReviewersHeraldField.php

@@ -0,0 +1,30 @@
+<?php
+
+final class DifferentialRevisionReviewersHeraldField
+  extends DifferentialRevisionHeraldField {
+
+  const FIELDCONST = 'differential.revision.reviewers';
+
+  public function getHeraldFieldName() {
+    return pht('Reviewers');
+  }
+
+  public function getHeraldFieldValue($object) {
+    return $this->getAdapter()->loadReviewers();
+  }
+
+  protected function getHeraldFieldStandardConditions() {
+    return self::STANDARD_LIST;
+  }
+
+  public function getHeraldFieldValueType($condition) {
+    switch ($condition) {
+      case HeraldAdapter::CONDITION_EXISTS:
+      case HeraldAdapter::CONDITION_NOT_EXISTS:
+        return HeraldAdapter::VALUE_NONE;
+      default:
+        return HeraldAdapter::VALUE_USER_OR_PROJECT;
+    }
+  }
+
+}

src/applications/differential/herald/DifferentialRevisionSummaryHeraldField.php

@@ -0,0 +1,27 @@
+<?php
+
+final class DifferentialRevisionSummaryHeraldField
+  extends DifferentialRevisionHeraldField {
+
+  const FIELDCONST = 'differential.revision.summary';
+
+  public function getHeraldFieldName() {
+    return pht('Revision summary');
+  }
+
+  public function getHeraldFieldValue($object) {
+    // NOTE: For historical reasons, this field includes the test plan. We
+    // could maybe try to fix this some day, but it probably aligns reasonably
+    // well with user expectation without harming anything.
+    return $object->getSummary()."\n\n".$object->getTestPlan();
+  }
+
+  protected function getHeraldFieldStandardConditions() {
+    return self::STANDARD_TEXT;
+  }
+
+  public function getHeraldFieldValueType($condition) {
+    return HeraldAdapter::VALUE_TEXT;
+  }
+
+}

src/applications/differential/herald/DifferentialRevisionTitleHeraldField.php

@@ -0,0 +1,24 @@
+<?php
+
+final class DifferentialRevisionTitleHeraldField
+  extends DifferentialRevisionHeraldField {
+
+  const FIELDCONST = 'differential.revision.title';
+
+  public function getHeraldFieldName() {
+    return pht('Revision title');
+  }
+
+  public function getHeraldFieldValue($object) {
+    return $object->getTitle();
+  }
+
+  protected function getHeraldFieldStandardConditions() {
+    return self::STANDARD_TEXT;
+  }
+
+  public function getHeraldFieldValueType($condition) {
+    return HeraldAdapter::VALUE_TEXT;
+  }
+
+}

src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php

@@ -3,7 +3,6 @@
 final class HeraldDifferentialRevisionAdapter
   extends HeraldDifferentialAdapter {
 
-  protected $diff;
   protected $revision;
 
   protected $explicitReviewers;
@@ -32,10 +31,6 @@ final class HeraldDifferentialRevisionAdapter
     return $this->revision;
   }
 
-  public function getDiff() {
-    return $this->diff;
-  }
-
   public function getAdapterContentType() {
     return 'differential';
   }
@@ -62,26 +57,6 @@ final class HeraldDifferentialRevisionAdapter
     }
   }
 
-  public function getFields() {
-    return array_merge(
-      array(
-        self::FIELD_TITLE,
-        self::FIELD_BODY,
-        self::FIELD_AUTHOR,
-        self::FIELD_AUTHOR_PROJECTS,
-        self::FIELD_REVIEWERS,
-        self::FIELD_REPOSITORY,
-        self::FIELD_REPOSITORY_PROJECTS,
-        self::FIELD_DIFF_FILE,
-        self::FIELD_DIFF_CONTENT,
-        self::FIELD_DIFF_ADDED_CONTENT,
-        self::FIELD_DIFF_REMOVED_CONTENT,
-        self::FIELD_AFFECTED_PACKAGE,
-        self::FIELD_AFFECTED_PACKAGE_OWNER,
-      ),
-      parent::getFields());
-  }
-
   public function getRepetitionOptions() {
     return array(
       HeraldRepetitionPolicyConfig::EVERY,
@@ -103,7 +78,7 @@ final class HeraldDifferentialRevisionAdapter
       ->executeOne();
 
     $object->revision = $revision;
-    $object->diff = $diff;
+    $object->setDiff($diff);
 
     return $object;
   }
@@ -129,17 +104,13 @@ final class HeraldDifferentialRevisionAdapter
     return $this->buildPlans;
   }
 
-  public function getPHID() {
-    return $this->revision->getPHID();
-  }
-
   public function getHeraldName() {
     return $this->revision->getTitle();
   }
 
   protected function loadChangesets() {
     if ($this->changesets === null) {
-      $this->changesets = $this->diff->loadChangesets();
+      $this->changesets = $this->getDiff()->loadChangesets();
     }
     return $this->changesets;
   }
@@ -175,66 +146,15 @@ final class HeraldDifferentialRevisionAdapter
     return $this->affectedPackages;
   }
 
-  public function getHeraldField($field) {
-    switch ($field) {
-      case self::FIELD_TITLE:
-        return $this->revision->getTitle();
-        break;
-      case self::FIELD_BODY:
-        return $this->revision->getSummary()."\n".
-               $this->revision->getTestPlan();
-        break;
-      case self::FIELD_AUTHOR:
-        return $this->revision->getAuthorPHID();
-        break;
-      case self::FIELD_AUTHOR_PROJECTS:
-        $author_phid = $this->revision->getAuthorPHID();
-        if (!$author_phid) {
-          return array();
-        }
-
-        $projects = id(new PhabricatorProjectQuery())
-          ->setViewer(PhabricatorUser::getOmnipotentUser())
-          ->withMemberPHIDs(array($author_phid))
-          ->execute();
-
-        return mpull($projects, 'getPHID');
-      case self::FIELD_DIFF_FILE:
-        return $this->loadAffectedPaths();
-      case self::FIELD_REVIEWERS:
+  public function loadReviewers() {
+    // TODO: This can probably go away as I believe it's just a performance
+    // optimization, just retaining it while modularizing fields to limit the
+    // scope of that change.
     if (isset($this->explicitReviewers)) {
       return array_keys($this->explicitReviewers);
     } else {
       return $this->revision->getReviewers();
     }
-      case self::FIELD_REPOSITORY:
-        $repository = $this->loadRepository();
-        if (!$repository) {
-          return null;
-        }
-        return $repository->getPHID();
-      case self::FIELD_REPOSITORY_PROJECTS:
-        $repository = $this->loadRepository();
-        if (!$repository) {
-          return array();
-        }
-        return $repository->getProjectPHIDs();
-      case self::FIELD_DIFF_CONTENT:
-        return $this->loadContentDictionary();
-      case self::FIELD_DIFF_ADDED_CONTENT:
-        return $this->loadAddedContentDictionary();
-      case self::FIELD_DIFF_REMOVED_CONTENT:
-        return $this->loadRemovedContentDictionary();
-      case self::FIELD_AFFECTED_PACKAGE:
-        $packages = $this->loadAffectedPackages();
-        return mpull($packages, 'getPHID');
-      case self::FIELD_AFFECTED_PACKAGE_OWNER:
-        $packages = $this->loadAffectedPackages();
-        return PhabricatorOwnersOwner::loadAffiliatedUserPHIDs(
-          mpull($packages, 'getID'));
-    }
-
-    return parent::getHeraldField($field);
   }
 
   public function getActions($rule_type) {

src/applications/diffusion/herald/DiffusionCommitPackageOwnerHeraldField.php

@@ -30,7 +30,7 @@ final class DiffusionCommitPackageOwnerHeraldField
       case HeraldAdapter::CONDITION_NOT_EXISTS:
         return HeraldAdapter::VALUE_NONE;
       default:
-        return HeraldAdapter::VALUE_USER;
+        return HeraldAdapter::VALUE_USER_OR_PROJECT;
     }
   }
 

src/applications/herald/adapter/HeraldAdapter.php

@@ -6,7 +6,6 @@ abstract class HeraldAdapter extends Phobject {
   const FIELD_BODY                   = 'body';
   const FIELD_AUTHOR                 = 'author';
   const FIELD_REVIEWER               = 'reviewer';
-  const FIELD_REVIEWERS              = 'reviewers';
   const FIELD_COMMITTER              = 'committer';
   const FIELD_DIFF_FILE              = 'diff-file';
   const FIELD_DIFF_CONTENT           = 'diff-content';
@@ -367,7 +366,6 @@ abstract class HeraldAdapter extends Phobject {
       self::FIELD_AUTHOR => pht('Author'),
       self::FIELD_COMMITTER => pht('Committer'),
       self::FIELD_REVIEWER => pht('Reviewer'),
-      self::FIELD_REVIEWERS => pht('Reviewers'),
       self::FIELD_DIFF_FILE => pht('Any changed filename'),
       self::FIELD_DIFF_CONTENT => pht('Any changed file content'),
       self::FIELD_DIFF_ADDED_CONTENT => pht('Any added file content'),
@@ -459,7 +457,6 @@ abstract class HeraldAdapter extends Phobject {
           self::CONDITION_EXISTS,
           self::CONDITION_NOT_EXISTS,
         );
-      case self::FIELD_REVIEWERS:
       case self::FIELD_AUTHOR_PROJECTS:
       case self::FIELD_AFFECTED_PACKAGE:
       case self::FIELD_AFFECTED_PACKAGE_OWNER:
@@ -922,8 +919,6 @@ abstract class HeraldAdapter extends Phobject {
           case self::FIELD_PUSHER_PROJECTS:
           case self::FIELD_REPOSITORY_PROJECTS:
             return self::VALUE_PROJECT;
-          case self::FIELD_REVIEWERS:
-            return self::VALUE_USER_OR_PROJECT;
           default:
             return self::VALUE_USER;
         }