Revision substate CLOSED_FROM_ACCEPTED

Summary:
Ref T9838.

Add a Properties field to Revision, and update a `wasAcceptedBeforeClose` when closing a revision.

Test Plan:
A quick run through the obvious steps (Close with commit/manually,  with or w/o accept) and calling `differential.query` shows the `wasAcceptedBeforeClose` property was setup correctly.

Pushing closed + accepted passes the relevant herald, which was my immediate issue; Pushing un-accepted is blocked.
Test the "commit" rule (Different from "pre-commit") by hacking the DB and running the "has accepted revision" rule in a test-console.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T9838

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

resources/sql/autopatches/20160201.revision.properties.1.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_differential.differential_revision
+ADD properties LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT};

resources/sql/autopatches/20160201.revision.properties.2.sql

@@ -0,0 +1,2 @@
+UPDATE {$NAMESPACE}_differential.differential_revision
+SET properties = '{}' WHERE properties = '';

src/applications/differential/conduit/DifferentialQueryConduitAPIMethod.php

@@ -206,6 +206,7 @@ final class DifferentialQueryConduitAPIMethod
         'statusName'          =>
           ArcanistDifferentialRevisionStatus::getNameForRevisionStatus(
             $revision->getStatus()),
+        'properties' => $revision->getProperties(),
         'branch'              => $diff->getBranch(),
         'summary'             => $revision->getSummary(),
         'testPlan'            => $revision->getTestPlan(),

src/applications/differential/editor/DifferentialTransactionEditor.php

@@ -182,6 +182,7 @@ final class DifferentialTransactionEditor
     $status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION;
     $status_plan = ArcanistDifferentialRevisionStatus::CHANGES_PLANNED;
     $status_abandoned = ArcanistDifferentialRevisionStatus::ABANDONED;
+    $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED;
 
     switch ($xaction->getTransactionType()) {
       case DifferentialTransaction::TYPE_INLINE:
@@ -233,7 +234,12 @@ final class DifferentialTransactionEditor
             $object->setStatus($status_review);
             return;
           case DifferentialAction::ACTION_CLOSE:
+            $old_status = $object->getStatus();
             $object->setStatus(ArcanistDifferentialRevisionStatus::CLOSED);
+            $was_accepted = ($old_status == $status_accepted);
+            $object->setProperty(
+              DifferentialRevision::PROPERTY_CLOSED_FROM_ACCEPTED,
+              $was_accepted);
             return;
           case DifferentialAction::ACTION_CLAIM:
             $object->setAuthorPHID($this->getActingAsPHID());

src/applications/differential/storage/DifferentialRevision.php

@@ -35,6 +35,7 @@ final class DifferentialRevision extends DifferentialDAO
   protected $repositoryPHID;
   protected $viewPolicy = PhabricatorPolicies::POLICY_USER;
   protected $editPolicy = PhabricatorPolicies::POLICY_USER;
+  protected $properties = array();
 
   private $relationships = self::ATTACHABLE;
   private $commits = self::ATTACHABLE;
@@ -53,6 +54,8 @@ final class DifferentialRevision extends DifferentialDAO
   const RELATION_REVIEWER     = 'revw';
   const RELATION_SUBSCRIBED   = 'subd';
 
+  const PROPERTY_CLOSED_FROM_ACCEPTED = 'wasAcceptedBeforeClose';
+
   public static function initializeNewRevision(PhabricatorUser $actor) {
     $app = id(new PhabricatorApplicationQuery())
       ->setViewer($actor)
@@ -76,6 +79,7 @@ final class DifferentialRevision extends DifferentialDAO
       self::CONFIG_SERIALIZATION => array(
         'attached'      => self::SERIALIZATION_JSON,
         'unsubscribed'  => self::SERIALIZATION_JSON,
+        'properties' => self::SERIALIZATION_JSON,
       ),
       self::CONFIG_COLUMN_SCHEMA => array(
         'title' => 'text255',
@@ -114,6 +118,19 @@ final class DifferentialRevision extends DifferentialDAO
     ) + parent::getConfiguration();
   }
 
+  public function setProperty($key, $value) {
+    $this->properties[$key] = $value;
+    return $this;
+  }
+
+  public function getProperty($key, $default = null) {
+    return idx($this->properties, $key, $default);
+  }
+
+  public function hasRevisionProperty($key) {
+    return array_key_exists($key, $this->properties);
+  }
+
   public function getMonogram() {
     $id = $this->getID();
     return "D{$id}";

src/applications/diffusion/herald/DiffusionCommitRevisionAcceptedHeraldField.php

@@ -19,10 +19,31 @@ final class DiffusionCommitRevisionAcceptedHeraldField
       return null;
     }
 
+    $status = $revision->getStatus();
+
+    switch ($status) {
+      case ArcanistDifferentialRevisionStatus::ACCEPTED:
+        return $revision->getPHID();
+      case ArcanistDifferentialRevisionStatus::CLOSED:
+        if ($revision->hasRevisionProperty(
+            DifferentialRevision::PROPERTY_CLOSED_FROM_ACCEPTED)) {
+
+          if ($revision->getProperty(
+              DifferentialRevision::PROPERTY_CLOSED_FROM_ACCEPTED)) {
+            return $revision->getPHID();
+          } else {
+            return null;
+          }
+        } else {
+          // continue on to old-style precommitRevisionStatus
+          break;
+        }
+      default:
+        return null;
+    }
+
     $data = $object->getCommitData();
-    $status = $data->getCommitDetail(
-      'precommitRevisionStatus',
-      $revision->getStatus());
+    $status = $data->getCommitDetail('precommitRevisionStatus');
 
     switch ($status) {
       case ArcanistDifferentialRevisionStatus::ACCEPTED:

src/applications/diffusion/herald/DiffusionPreCommitContentRevisionAcceptedHeraldField.php

@@ -20,13 +20,20 @@ final class DiffusionPreCommitContentRevisionAcceptedHeraldField
       return null;
     }
 
-    $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED;
-    if ($revision->getStatus() != $status_accepted) {
-      return null;
-    }
+    switch ($revision->getStatus()) {
+      case ArcanistDifferentialRevisionStatus::ACCEPTED:
+        return $revision->getPHID();
+      case ArcanistDifferentialRevisionStatus::CLOSED:
+        if ($revision->getProperty(
+          DifferentialRevision::PROPERTY_CLOSED_FROM_ACCEPTED)) {
 
           return $revision->getPHID();
         }
+        break;
+    }
+
+    return null;
+  }
 
   protected function getHeraldFieldStandardType() {
     return self::STANDARD_PHID_BOOL;