From 00baa3c1dd9ba8b38a588c7dd1ca53fe5d666550 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 27 Nov 2017 14:08:40 -0800
Subject: [PATCH 01/18] Hide archived projects only on workboards, not
 hovercards

Summary:
See PHI225. Previously, see D15335 / T10413. On workboards, we hide archived project tags since they aren't terribly useful in that context, at least most of the time. Originally, see T10349#159916 and D15297.

However, hovercards reuse this display logic, and it's inconsistent/confusing to hide them there, since the actual "Tags" elements on task pages show them. Narrow the scope of this rule.

Test Plan:
  - Viewed a hovercard for a task with an archived project tagged, saw archived project.
  - Viewed a workboard for the same task, saw only unarchived projects other than the current board tagged (this behavior is unchanged).

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18783
---
 .../PhabricatorBoardRenderingEngine.php       |  4 +++-
 .../project/view/ProjectBoardTaskCard.php     | 20 +++++++++++++++----
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/src/applications/project/engine/PhabricatorBoardRenderingEngine.php b/src/applications/project/engine/PhabricatorBoardRenderingEngine.php
index ca8b0633da..d76497bc21 100644
--- a/src/applications/project/engine/PhabricatorBoardRenderingEngine.php
+++ b/src/applications/project/engine/PhabricatorBoardRenderingEngine.php
@@ -67,7 +67,9 @@ final class PhabricatorBoardRenderingEngine extends Phobject {
     $project_phids = $object->getProjectPHIDs();
     $project_handles = array_select_keys($this->handles, $project_phids);
     if ($project_handles) {
-      $card->setProjectHandles($project_handles);
+      $card
+        ->setHideArchivedProjects(true)
+        ->setProjectHandles($project_handles);
     }
 
     $cover_phid = $object->getCoverImageThumbnailPHID();
diff --git a/src/applications/project/view/ProjectBoardTaskCard.php b/src/applications/project/view/ProjectBoardTaskCard.php
index 1a6807ec45..3a7016ca74 100644
--- a/src/applications/project/view/ProjectBoardTaskCard.php
+++ b/src/applications/project/view/ProjectBoardTaskCard.php
@@ -8,6 +8,7 @@ final class ProjectBoardTaskCard extends Phobject {
   private $owner;
   private $canEdit;
   private $coverImageFile;
+  private $hideArchivedProjects;
 
   public function setViewer(PhabricatorUser $viewer) {
     $this->viewer = $viewer;
@@ -35,6 +36,15 @@ final class ProjectBoardTaskCard extends Phobject {
     return $this->coverImageFile;
   }
 
+  public function setHideArchivedProjects($hide_archived_projects) {
+    $this->hideArchivedProjects = $hide_archived_projects;
+    return $this;
+  }
+
+  public function getHideArchivedProjects() {
+    return $this->hideArchivedProjects;
+  }
+
   public function setTask(ManiphestTask $task) {
     $this->task = $task;
     return $this;
@@ -126,10 +136,12 @@ final class ProjectBoardTaskCard extends Phobject {
     $project_handles = $this->getProjectHandles();
 
     // Remove any archived projects from the list.
-    if ($project_handles) {
-      foreach ($project_handles as $key => $handle) {
-        if ($handle->getStatus() == PhabricatorObjectHandle::STATUS_CLOSED) {
-          unset($project_handles[$key]);
+    if ($this->hideArchivedProjects) {
+      if ($project_handles) {
+        foreach ($project_handles as $key => $handle) {
+          if ($handle->getStatus() == PhabricatorObjectHandle::STATUS_CLOSED) {
+            unset($project_handles[$key]);
+          }
         }
       }
     }

From 3d742eb50baf54433e41090d1ba9d06277fedebc Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 27 Nov 2017 16:10:33 -0800
Subject: [PATCH 02/18] Lightly modernize LegalpadDocumentQuery

Summary: Ref T13024. Updates LegalpadDocumentQuery to use slightly more modern constructions.

Test Plan: Queried for Legalpad documents, got the same results.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13024

Differential Revision: https://secure.phabricator.com/D18785
---
 .../legalpad/query/LegalpadDocumentQuery.php  | 71 +++++++++----------
 1 file changed, 33 insertions(+), 38 deletions(-)

diff --git a/src/applications/legalpad/query/LegalpadDocumentQuery.php b/src/applications/legalpad/query/LegalpadDocumentQuery.php
index 655da955e4..3d79e9f3a1 100644
--- a/src/applications/legalpad/query/LegalpadDocumentQuery.php
+++ b/src/applications/legalpad/query/LegalpadDocumentQuery.php
@@ -77,23 +77,12 @@ final class LegalpadDocumentQuery
     return $this;
   }
 
+  public function newResultObject() {
+    return new LegalpadDocument();
+  }
+
   protected function loadPage() {
-    $table = new LegalpadDocument();
-    $conn_r = $table->establishConnection('r');
-
-    $data = queryfx_all(
-      $conn_r,
-      'SELECT d.* FROM %T d %Q %Q %Q %Q %Q',
-      $table->getTableName(),
-      $this->buildJoinClause($conn_r),
-      $this->buildWhereClause($conn_r),
-      $this->buildGroupClause($conn_r),
-      $this->buildOrderClause($conn_r),
-      $this->buildLimitClause($conn_r));
-
-    $documents = $table->loadAllFromArray($data);
-
-    return $documents;
+    return $this->loadStandardPage($this->newResultObject());
   }
 
   protected function willFilterPage(array $documents) {
@@ -134,12 +123,12 @@ final class LegalpadDocumentQuery
     return $documents;
   }
 
-  protected function buildJoinClause(AphrontDatabaseConnection $conn_r) {
-    $joins = array();
+  protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) {
+    $joins = parent::buildJoinClauseParts($conn);
 
     if ($this->contributorPHIDs !== null) {
       $joins[] = qsprintf(
-        $conn_r,
+        $conn,
         'JOIN edge contributor ON contributor.src = d.phid
           AND contributor.type = %d',
         PhabricatorObjectHasContributorEdgeType::EDGECONST);
@@ -147,79 +136,81 @@ final class LegalpadDocumentQuery
 
     if ($this->signerPHIDs !== null) {
       $joins[] = qsprintf(
-        $conn_r,
+        $conn,
         'JOIN %T signer ON signer.documentPHID = d.phid
           AND signer.signerPHID IN (%Ls)',
         id(new LegalpadDocumentSignature())->getTableName(),
         $this->signerPHIDs);
     }
 
-    return implode(' ', $joins);
+    return $joins;
   }
 
-  protected function buildGroupClause(AphrontDatabaseConnection $conn_r) {
-    if ($this->contributorPHIDs || $this->signerPHIDs) {
-      return 'GROUP BY d.id';
-    } else {
-      return '';
+  protected function shouldGroupQueryResultRows() {
+    if ($this->contributorPHIDs) {
+      return true;
     }
+
+    if ($this->signerPHIDs) {
+      return true;
+    }
+
+    return parent::shouldGroupQueryResultRows();
   }
 
-  protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
-    $where = array();
+  protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
+    $where = parent::buildWhereClauseParts($conn);
 
     if ($this->ids !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'd.id IN (%Ld)',
         $this->ids);
     }
 
     if ($this->phids !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'd.phid IN (%Ls)',
         $this->phids);
     }
 
     if ($this->creatorPHIDs !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'd.creatorPHID IN (%Ls)',
         $this->creatorPHIDs);
     }
 
     if ($this->dateCreatedAfter !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'd.dateCreated >= %d',
         $this->dateCreatedAfter);
     }
 
     if ($this->dateCreatedBefore !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'd.dateCreated <= %d',
         $this->dateCreatedBefore);
     }
 
     if ($this->contributorPHIDs !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'contributor.dst IN (%Ls)',
         $this->contributorPHIDs);
     }
 
     if ($this->signatureRequired !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'd.requireSignature = %d',
         $this->signatureRequired);
     }
 
-    $where[] = $this->buildPagingClause($conn_r);
-
-    return $this->formatWhereClause($where);
+    return $where;
   }
 
   private function loadDocumentBodies(array $documents) {
@@ -275,4 +266,8 @@ final class LegalpadDocumentQuery
     return 'PhabricatorLegalpadApplication';
   }
 
+  protected function getPrimaryTableAlias() {
+    return 'd';
+  }
+
 }

From 71406ca93bf4dbda8a3e0afd0115ed5e28a6c120 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 27 Nov 2017 16:42:40 -0800
Subject: [PATCH 03/18] Lightly modernize LegalpadDocumentSearchEngine

Summary: Depends on D18785. Ref T13024. While I'm in here, update this a bit to use the newer stuff.

Test Plan: Searched for Legalpad documents, saw more modern support (subscribers, order by, viewer()).

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13024

Differential Revision: https://secure.phabricator.com/D18786
---
 .../query/LegalpadDocumentSearchEngine.php    | 138 ++++++------------
 1 file changed, 47 insertions(+), 91 deletions(-)

diff --git a/src/applications/legalpad/query/LegalpadDocumentSearchEngine.php b/src/applications/legalpad/query/LegalpadDocumentSearchEngine.php
index 8b218cf821..1b608b02e3 100644
--- a/src/applications/legalpad/query/LegalpadDocumentSearchEngine.php
+++ b/src/applications/legalpad/query/LegalpadDocumentSearchEngine.php
@@ -11,105 +11,66 @@ final class LegalpadDocumentSearchEngine
     return 'PhabricatorLegalpadApplication';
   }
 
-  public function buildSavedQueryFromRequest(AphrontRequest $request) {
-    $saved = new PhabricatorSavedQuery();
-    $saved->setParameter(
-      'creatorPHIDs',
-      $this->readUsersFromRequest($request, 'creators'));
-
-    $saved->setParameter(
-      'contributorPHIDs',
-      $this->readUsersFromRequest($request, 'contributors'));
-
-    $saved->setParameter(
-      'withViewerSignature',
-      $request->getBool('withViewerSignature'));
-
-    $saved->setParameter('createdStart', $request->getStr('createdStart'));
-    $saved->setParameter('createdEnd', $request->getStr('createdEnd'));
-
-    return $saved;
+  public function newQuery() {
+    return id(new LegalpadDocumentQuery())
+      ->needViewerSignatures(true);
   }
 
-  public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) {
-    $query = id(new LegalpadDocumentQuery())
-      ->needViewerSignatures(true);
+  protected function buildCustomSearchFields() {
+    return array(
+      id(new PhabricatorUsersSearchField())
+        ->setLabel(pht('Signed By'))
+        ->setKey('signerPHIDs')
+        ->setAliases(array('signer', 'signers', 'signerPHID'))
+        ->setDescription(
+          pht('Search for documents signed by given users.')),
+      id(new PhabricatorUsersSearchField())
+        ->setLabel(pht('Creators'))
+        ->setKey('creatorPHIDs')
+        ->setAliases(array('creator', 'creators', 'creatorPHID'))
+        ->setDescription(
+          pht('Search for documents with given creators.')),
+      id(new PhabricatorUsersSearchField())
+        ->setLabel(pht('Contributors'))
+        ->setKey('contributorPHIDs')
+        ->setAliases(array('contributor', 'contributors', 'contributorPHID'))
+        ->setDescription(
+          pht('Search for documents with given contributors.')),
+      id(new PhabricatorSearchDateField())
+        ->setLabel(pht('Created After'))
+        ->setKey('createdStart'),
+      id(new PhabricatorSearchDateField())
+        ->setLabel(pht('Created Before'))
+        ->setKey('createdEnd'),
+    );
+  }
 
-    $creator_phids = $saved->getParameter('creatorPHIDs', array());
-    if ($creator_phids) {
-      $query->withCreatorPHIDs($creator_phids);
+  protected function buildQueryFromParameters(array $map) {
+    $query = $this->newQuery();
+
+    if ($map['signerPHIDs']) {
+      $query->withSignerPHIDs($map['signerPHIDs']);
     }
 
-    $contributor_phids = $saved->getParameter('contributorPHIDs', array());
-    if ($contributor_phids) {
-      $query->withContributorPHIDs($contributor_phids);
+    if ($map['contributorPHIDs']) {
+      $query->withContributorPHIDs($map['creatorPHIDs']);
     }
 
-    if ($saved->getParameter('withViewerSignature')) {
-      $viewer_phid = $this->requireViewer()->getPHID();
-      if ($viewer_phid) {
-        $query->withSignerPHIDs(array($viewer_phid));
-      }
+    if ($map['creatorPHIDs']) {
+      $query->withCreatorPHIDs($map['creatorPHIDs']);
     }
 
-    $start = $this->parseDateTime($saved->getParameter('createdStart'));
-    $end = $this->parseDateTime($saved->getParameter('createdEnd'));
-
-    if ($start) {
-      $query->withDateCreatedAfter($start);
+    if ($map['createdStart']) {
+      $query->withDateCreatedAfter($map['createdStart']);
     }
 
-    if ($end) {
-      $query->withDateCreatedBefore($end);
+    if ($map['createdEnd']) {
+      $query->withDateCreatedAfter($map['createdStart']);
     }
 
     return $query;
   }
 
-  public function buildSearchForm(
-    AphrontFormView $form,
-    PhabricatorSavedQuery $saved_query) {
-
-    $creator_phids = $saved_query->getParameter('creatorPHIDs', array());
-    $contributor_phids = $saved_query->getParameter(
-      'contributorPHIDs', array());
-
-    $viewer_signature = $saved_query->getParameter('withViewerSignature');
-    if (!$this->requireViewer()->getPHID()) {
-      $viewer_signature = false;
-    }
-
-    $form
-      ->appendChild(
-        id(new AphrontFormCheckboxControl())
-          ->addCheckbox(
-            'withViewerSignature',
-            1,
-            pht('Show only documents I have signed.'),
-            $viewer_signature)
-          ->setDisabled(!$this->requireViewer()->getPHID()))
-      ->appendControl(
-        id(new AphrontFormTokenizerControl())
-          ->setDatasource(new PhabricatorPeopleDatasource())
-          ->setName('creators')
-          ->setLabel(pht('Creators'))
-          ->setValue($creator_phids))
-      ->appendControl(
-        id(new AphrontFormTokenizerControl())
-          ->setDatasource(new PhabricatorPeopleDatasource())
-          ->setName('contributors')
-          ->setLabel(pht('Contributors'))
-          ->setValue($contributor_phids));
-
-    $this->buildDateRange(
-      $form,
-      $saved_query,
-      'createdStart',
-      pht('Created After'),
-      'createdEnd',
-      pht('Created Before'));
-  }
-
   protected function getURI($path) {
     return '/legalpad/'.$path;
   }
@@ -130,10 +91,11 @@ final class LegalpadDocumentSearchEngine
     $query = $this->newSavedQuery();
     $query->setQueryKey($query_key);
 
+    $viewer = $this->requireViewer();
+
     switch ($query_key) {
       case 'signed':
-        return $query
-          ->setParameter('withViewerSignature', true);
+        return $query->setParameter('signerPHIDs', array($viewer->getPHID()));
       case 'all':
         return $query;
     }
@@ -141,12 +103,6 @@ final class LegalpadDocumentSearchEngine
     return parent::buildSavedQueryFromBuiltin($query_key);
   }
 
-  protected function getRequiredHandlePHIDsForResultList(
-    array $documents,
-    PhabricatorSavedQuery $query) {
-    return array();
-  }
-
   protected function renderResultList(
     array $documents,
     PhabricatorSavedQuery $query,

From ba4b9f71849f501fc95a204f16de7db871a3ce5a Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 27 Nov 2017 16:10:38 -0800
Subject: [PATCH 04/18] Refactor on-login Legalpad document signature
 requirement

Summary: Depends on D18786. Ref T13024. I'm going to change the order this occurs in, but move it to a separate method and clean it up a little first.

Test Plan: Added a new document as required, reloaded, signed it, got logged into a session.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13024

Differential Revision: https://secure.phabricator.com/D18788
---
 .../base/controller/PhabricatorController.php | 112 ++++++++++++------
 1 file changed, 73 insertions(+), 39 deletions(-)

diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php
index f923eb7b73..961e85effe 100644
--- a/src/applications/base/controller/PhabricatorController.php
+++ b/src/applications/base/controller/PhabricatorController.php
@@ -227,45 +227,9 @@ abstract class PhabricatorController extends AphrontController {
     }
 
 
-    if (!$this->shouldAllowLegallyNonCompliantUsers()) {
-      $legalpad_class = 'PhabricatorLegalpadApplication';
-      $legalpad = id(new PhabricatorApplicationQuery())
-        ->setViewer($user)
-        ->withClasses(array($legalpad_class))
-        ->withInstalled(true)
-        ->execute();
-      $legalpad = head($legalpad);
-
-      $doc_query = id(new LegalpadDocumentQuery())
-        ->setViewer($user)
-        ->withSignatureRequired(1)
-        ->needViewerSignatures(true);
-
-      if ($user->hasSession() &&
-          !$user->getSession()->getIsPartial() &&
-          !$user->getSession()->getSignedLegalpadDocuments() &&
-          $user->isLoggedIn() &&
-          $legalpad) {
-
-        $sign_docs = $doc_query->execute();
-        $must_sign_docs = array();
-        foreach ($sign_docs as $sign_doc) {
-          if (!$sign_doc->getUserSignature($user->getPHID())) {
-            $must_sign_docs[] = $sign_doc;
-          }
-        }
-        if ($must_sign_docs) {
-          $controller = new LegalpadDocumentSignController();
-          $this->getRequest()->setURIMap(array(
-            'id' => head($must_sign_docs)->getID(),
-          ));
-          $this->setCurrentApplication($legalpad);
-          return $this->delegateToController($controller);
-        } else {
-          $engine = id(new PhabricatorAuthSessionEngine())
-            ->signLegalpadDocuments($user, $sign_docs);
-        }
-      }
+    $result = $this->requireLegalpadSignatures();
+    if ($result !== null) {
+      return $result;
     }
 
     // NOTE: We do this last so that users get a login page instead of a 403
@@ -558,6 +522,76 @@ abstract class PhabricatorController extends AphrontController {
     return $this->buildApplicationCrumbs();
   }
 
+  private function requireLegalpadSignatures() {
+    if ($this->shouldAllowLegallyNonCompliantUsers()) {
+      return null;
+    }
+
+    $viewer = $this->getViewer();
+
+    if (!$viewer->hasSession()) {
+      return null;
+    }
+
+    $session = $viewer->getSession();
+    if ($session->getIsPartial()) {
+      // If the user hasn't made it through MFA yet, require they survive
+      // MFA first.
+      return null;
+    }
+
+    if ($session->getSignedLegalpadDocuments()) {
+      return null;
+    }
+
+    if (!$viewer->isLoggedIn()) {
+      return null;
+    }
+
+    $legalpad_class = 'PhabricatorLegalpadApplication';
+    $legalpad_installed = PhabricatorApplication::isClassInstalledForViewer(
+      $legalpad_class,
+      $viewer);
+    if (!$legalpad_installed) {
+      return null;
+    }
+
+    $sign_docs = id(new LegalpadDocumentQuery())
+      ->setViewer($viewer)
+      ->withSignatureRequired(1)
+      ->needViewerSignatures(true)
+      ->execute();
+
+    $must_sign_docs = array();
+    foreach ($sign_docs as $sign_doc) {
+      if (!$sign_doc->getUserSignature($viewer->getPHID())) {
+        $must_sign_docs[] = $sign_doc;
+      }
+    }
+
+    if (!$must_sign_docs) {
+      // If nothing needs to be signed (either because there are no documents
+      // which require a signature, or because the user has already signed
+      // all of them) mark the session as good and continue.
+      $engine = id(new PhabricatorAuthSessionEngine())
+        ->signLegalpadDocuments($viewer, $sign_docs);
+
+      return null;
+    }
+
+    $request = $this->getRequest();
+    $request->setURIMap(
+      array(
+        'id' => head($must_sign_docs)->getID(),
+      ));
+
+    $application = PhabricatorApplication::getByClass($legalpad_class);
+    $this->setCurrentApplication($application);
+
+    $controller = new LegalpadDocumentSignController();
+    return $this->delegateToController($controller);
+  }
+
 
 /* -(  Deprecated  )--------------------------------------------------------- */
 

From e850bc6b957a8ac6c6faa02d57716ce5d60d42f6 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 27 Nov 2017 16:54:26 -0800
Subject: [PATCH 05/18] When requiring login signatures, order documents from
 oldest to newest

Summary: Depends on D18788. Ref T13024. Currently, we prompt users to sign from newest to oldest. This seems less intuitive than oldest to newest.

Test Plan: Dumped document order, saw it swap to oldest-first.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13024

Differential Revision: https://secure.phabricator.com/D18789
---
 src/applications/base/controller/PhabricatorController.php | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php
index 961e85effe..fb9c402083 100644
--- a/src/applications/base/controller/PhabricatorController.php
+++ b/src/applications/base/controller/PhabricatorController.php
@@ -560,6 +560,7 @@ abstract class PhabricatorController extends AphrontController {
       ->setViewer($viewer)
       ->withSignatureRequired(1)
       ->needViewerSignatures(true)
+      ->setOrder('oldest')
       ->execute();
 
     $must_sign_docs = array();

From c7718d3a2126fe77da17467c933aa6bd95f59779 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 27 Nov 2017 17:02:09 -0800
Subject: [PATCH 06/18] Ask users to sign Legalpad documents before requiring
 they enroll in MFA

Summary:
Depends on D18789. Ref T13024. See PHI223. Currently, if `security.require-multi-factor-auth` and Legalpad "Signature Required" documents are //both// set, it's not possible to survive account registration, since MFA is requiried to sign and signatures are required to add MFA.

Instead, check for signatures before requiring MFA enrollment. This makes logical sense, since it's silly to add MFA if you don't agree to a Terms of Service or whatever.

(Note that if you already have MFA, we prompt for that first, before either of these steps, which also makes sense.)

Test Plan: Configured `security.require-multi-factor-auth`. Added a signature-required document. Loaded a page as a new user. Went through signature workflow, then through the MFA enrollment workflow.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13024

Differential Revision: https://secure.phabricator.com/D18790
---
 .../base/controller/PhabricatorController.php | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php
index fb9c402083..09aa24a42e 100644
--- a/src/applications/base/controller/PhabricatorController.php
+++ b/src/applications/base/controller/PhabricatorController.php
@@ -160,6 +160,15 @@ abstract class PhabricatorController extends AphrontController {
       }
     }
 
+    // Require users sign Legalpad documents before we check if they have
+    // MFA. If we don't do this, they can get stuck in a state where they
+    // can't add MFA until they sign, and can't sign until they add MFA.
+    // See T13024 and PHI223.
+    $result = $this->requireLegalpadSignatures();
+    if ($result !== null) {
+      return $result;
+    }
+
     // Check if the user needs to configure MFA.
     $need_mfa = $this->shouldRequireMultiFactorEnrollment();
     $have_mfa = $user->getIsEnrolledInMultiFactor();
@@ -226,12 +235,6 @@ abstract class PhabricatorController extends AphrontController {
       }
     }
 
-
-    $result = $this->requireLegalpadSignatures();
-    if ($result !== null) {
-      return $result;
-    }
-
     // NOTE: We do this last so that users get a login page instead of a 403
     // if they need to login.
     if ($this->shouldRequireAdmin() && !$user->getIsAdmin()) {
@@ -523,6 +526,10 @@ abstract class PhabricatorController extends AphrontController {
   }
 
   private function requireLegalpadSignatures() {
+    if (!$this->shouldRequireLogin()) {
+      return null;
+    }
+
     if ($this->shouldAllowLegallyNonCompliantUsers()) {
       return null;
     }

From ab0f61aa323d3557b6eff806f257a53e2ca5e759 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 27 Nov 2017 17:16:09 -0800
Subject: [PATCH 07/18] Tell users to "Wait Patiently" for admin account
 verification later in the registration process

Summary:
Depends on D18790. Ref T13024. Fixes T8335. Currently, "unapproved" and "disabled" users are bundled together. This prevents users from completing some registration steps (verification, legalpad documents, MFA enrollment) before approval.

Separate approval out and move it to the end so users can do all the required enrollment stuff on their end before we roadblock them.

Test Plan: Required approval, email verification, signatures, and MFA. Registered an account. Verified email, signed documents, enrolled in MFA, and then got prompted to wait for approval.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13024, T8335

Differential Revision: https://secure.phabricator.com/D18791
---
 .../PhabricatorAuthNeedsMultiFactorController.php | 12 ++++++++++++
 .../base/controller/PhabricatorController.php     | 15 +++++++++++----
 .../PhabricatorAccessControlTestCase.php          |  6 +++---
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php b/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php
index f3e2562a1c..d295812d3e 100644
--- a/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php
+++ b/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php
@@ -9,9 +9,21 @@ final class PhabricatorAuthNeedsMultiFactorController
     return false;
   }
 
+  public function shouldRequireEnabledUser() {
+    // Users who haven't been approved yet are allowed to enroll in MFA. We'll
+    // kick disabled users out later.
+    return false;
+  }
+
   public function handleRequest(AphrontRequest $request) {
     $viewer = $this->getViewer();
 
+    if ($viewer->getIsDisabled()) {
+      // We allowed unapproved and disabled users to hit this controller, but
+      // want to kick out disabled users now.
+      return new Aphront400Response();
+    }
+
     $panel = id(new PhabricatorMultiFactorSettingsPanel())
       ->setUser($viewer)
       ->setViewer($viewer)
diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php
index 09aa24a42e..0fee880378 100644
--- a/src/applications/base/controller/PhabricatorController.php
+++ b/src/applications/base/controller/PhabricatorController.php
@@ -137,10 +137,6 @@ abstract class PhabricatorController extends AphrontController {
     }
 
     if ($this->shouldRequireEnabledUser()) {
-      if ($user->isLoggedIn() && !$user->getIsApproved()) {
-        $controller = new PhabricatorAuthNeedsApprovalController();
-        return $this->delegateToController($controller);
-      }
       if ($user->getIsDisabled()) {
         $controller = new PhabricatorDisabledUserController();
         return $this->delegateToController($controller);
@@ -233,6 +229,17 @@ abstract class PhabricatorController extends AphrontController {
           ->withPHIDs(array($application->getPHID()))
           ->executeOne();
       }
+
+      // If users need approval, require they wait here. We do this near the
+      // end so they can take other actions (like verifying email, signing
+      // documents, and enrolling in MFA) while waiting for an admin to take a
+      // look at things. See T13024 for more discussion.
+      if ($this->shouldRequireEnabledUser()) {
+        if ($user->isLoggedIn() && !$user->getIsApproved()) {
+          $controller = new PhabricatorAuthNeedsApprovalController();
+          return $this->delegateToController($controller);
+        }
+      }
     }
 
     // NOTE: We do this last so that users get a login page instead of a 403
diff --git a/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php b/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php
index b6c268d88c..98fa948722 100644
--- a/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php
+++ b/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php
@@ -159,10 +159,10 @@ final class PhabricatorAccessControlTestCase extends PhabricatorTestCase {
         $u_unverified,
         $u_admin,
         $u_public,
+        $u_notapproved,
       ),
       array(
         $u_disabled,
-        $u_notapproved,
       ));
 
 
@@ -224,7 +224,7 @@ final class PhabricatorAccessControlTestCase extends PhabricatorTestCase {
       ));
 
     $this->checkAccess(
-      pht('Application Controller'),
+      pht('Application Controller, No Login Required'),
       id(clone $app_controller)->setConfig('login', false),
       $request,
       array(
@@ -232,10 +232,10 @@ final class PhabricatorAccessControlTestCase extends PhabricatorTestCase {
         $u_unverified,
         $u_admin,
         $u_public,
+        $u_notapproved,
       ),
       array(
         $u_disabled,
-        $u_notapproved,
       ));
   }
 

From dc62d18b473624a2547efb5751f2713ce0047256 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 27 Nov 2017 17:34:43 -0800
Subject: [PATCH 08/18] Allow MFA enrollment before email verification

Summary: Depends on D18791. Ref T13024. This clears up another initialization order issue, where an unverified address could prevent MFA enrollment.

Test Plan: Configured both verification required and MFA required, clicked "Add Factor", got a dialog for the workflow.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13024

Differential Revision: https://secure.phabricator.com/D18792
---
 .../PhabricatorAuthNeedsMultiFactorController.php           | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php b/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php
index d295812d3e..27e03485ca 100644
--- a/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php
+++ b/src/applications/auth/controller/PhabricatorAuthNeedsMultiFactorController.php
@@ -15,6 +15,12 @@ final class PhabricatorAuthNeedsMultiFactorController
     return false;
   }
 
+  public function shouldRequireEmailVerification() {
+    // Users who haven't verified their email addresses yet can still enroll
+    // in MFA.
+    return false;
+  }
+
   public function handleRequest(AphrontRequest $request) {
     $viewer = $this->getViewer();
 

From e919233b3182d55b1d257df3b8ec369d702fff97 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 27 Nov 2017 17:53:58 -0800
Subject: [PATCH 09/18] Don't show personalized menu items until users
 establish a full session

Summary:
Depends on D18792. Fixes T13024. Fixes T89198. Currently, when users are logging in initially (for example, need to enter MFA) we show more menu items than we should.

Notably, we may show some personalized/private account details, like the number of unread notifications (probably not relevant) or a user's saved queries (possibly sensitive). At best these are misleading (they won't work yet) and there's an outside possibility they leak a little bit of private data.

Instead, nuke everything except "Log Out" when users have partial sessions.

Test Plan:
Hit a partial session (MFA required, email verification required) and looked at the menu. Only saw "Log Out".

{F5297713}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13024

Differential Revision: https://secure.phabricator.com/D18793
---
 .../PhabricatorFileDataController.php         |  4 ++
 .../PeopleMainMenuBarExtension.php            | 62 ++++++++++---------
 .../menu/PhabricatorMainMenuBarExtension.php  | 14 +++++
 .../page/menu/PhabricatorMainMenuView.php     | 58 +++++++++++++++--
 4 files changed, 106 insertions(+), 32 deletions(-)

diff --git a/src/applications/files/controller/PhabricatorFileDataController.php b/src/applications/files/controller/PhabricatorFileDataController.php
index da438730cd..98f2cdbd51 100644
--- a/src/applications/files/controller/PhabricatorFileDataController.php
+++ b/src/applications/files/controller/PhabricatorFileDataController.php
@@ -10,6 +10,10 @@ final class PhabricatorFileDataController extends PhabricatorFileController {
     return false;
   }
 
+  public function shouldAllowPartialSessions() {
+    return true;
+  }
+
   public function handleRequest(AphrontRequest $request) {
     $viewer = $request->getViewer();
     $this->phid = $request->getURIData('phid');
diff --git a/src/applications/people/engineextension/PeopleMainMenuBarExtension.php b/src/applications/people/engineextension/PeopleMainMenuBarExtension.php
index cd1cffa7a2..01f1ba7cc1 100644
--- a/src/applications/people/engineextension/PeopleMainMenuBarExtension.php
+++ b/src/applications/people/engineextension/PeopleMainMenuBarExtension.php
@@ -9,6 +9,10 @@ final class PeopleMainMenuBarExtension
     return $viewer->isLoggedIn();
   }
 
+  public function shouldAllowPartialSessions() {
+    return true;
+  }
+
   public function getExtensionOrder() {
     return 1200;
   }
@@ -65,42 +69,44 @@ final class PeopleMainMenuBarExtension
     $view = id(new PhabricatorActionListView())
       ->setViewer($viewer);
 
-    $view->addAction(
-      id(new PhabricatorActionView())
-        ->appendChild($user_view));
+    if ($this->getIsFullSession()) {
+      $view->addAction(
+        id(new PhabricatorActionView())
+          ->appendChild($user_view));
 
-    $view->addAction(
-      id(new PhabricatorActionView())
-        ->setType(PhabricatorActionView::TYPE_DIVIDER));
+      $view->addAction(
+        id(new PhabricatorActionView())
+          ->setType(PhabricatorActionView::TYPE_DIVIDER));
 
-    $view->addAction(
-      id(new PhabricatorActionView())
-        ->setName(pht('Profile'))
-        ->setHref('/p/'.$viewer->getUsername().'/'));
+      $view->addAction(
+        id(new PhabricatorActionView())
+          ->setName(pht('Profile'))
+          ->setHref('/p/'.$viewer->getUsername().'/'));
 
-    $view->addAction(
-      id(new PhabricatorActionView())
-        ->setName(pht('Settings'))
-        ->setHref('/settings/user/'.$viewer->getUsername().'/'));
+      $view->addAction(
+        id(new PhabricatorActionView())
+          ->setName(pht('Settings'))
+          ->setHref('/settings/user/'.$viewer->getUsername().'/'));
 
-    $view->addAction(
-      id(new PhabricatorActionView())
-        ->setName(pht('Manage'))
-        ->setHref('/people/manage/'.$viewer->getID().'/'));
+      $view->addAction(
+        id(new PhabricatorActionView())
+          ->setName(pht('Manage'))
+          ->setHref('/people/manage/'.$viewer->getID().'/'));
 
-    if ($application) {
-      $help_links = $application->getHelpMenuItems($viewer);
-      if ($help_links) {
-        foreach ($help_links as $link) {
-          $view->addAction($link);
+      if ($application) {
+        $help_links = $application->getHelpMenuItems($viewer);
+        if ($help_links) {
+          foreach ($help_links as $link) {
+            $view->addAction($link);
+          }
         }
       }
-    }
 
-    $view->addAction(
-      id(new PhabricatorActionView())
-        ->addSigil('logout-item')
-        ->setType(PhabricatorActionView::TYPE_DIVIDER));
+      $view->addAction(
+        id(new PhabricatorActionView())
+          ->addSigil('logout-item')
+          ->setType(PhabricatorActionView::TYPE_DIVIDER));
+    }
 
     $view->addAction(
       id(new PhabricatorActionView())
diff --git a/src/view/page/menu/PhabricatorMainMenuBarExtension.php b/src/view/page/menu/PhabricatorMainMenuBarExtension.php
index fede2fdb85..d893cccac9 100644
--- a/src/view/page/menu/PhabricatorMainMenuBarExtension.php
+++ b/src/view/page/menu/PhabricatorMainMenuBarExtension.php
@@ -5,6 +5,7 @@ abstract class PhabricatorMainMenuBarExtension extends Phobject {
   private $viewer;
   private $application;
   private $controller;
+  private $isFullSession;
 
   public function setViewer(PhabricatorUser $viewer) {
     $this->viewer = $viewer;
@@ -33,6 +34,15 @@ abstract class PhabricatorMainMenuBarExtension extends Phobject {
     return $this->controller;
   }
 
+  public function setIsFullSession($is_full_session) {
+    $this->isFullSession = $is_full_session;
+    return $this;
+  }
+
+  public function getIsFullSession() {
+    return $this->isFullSession;
+  }
+
   final public function getExtensionKey() {
     return $this->getPhobjectClassConstant('MAINMENUBARKEY');
   }
@@ -41,6 +51,10 @@ abstract class PhabricatorMainMenuBarExtension extends Phobject {
     return true;
   }
 
+  public function shouldAllowPartialSessions() {
+    return false;
+  }
+
   public function isExtensionEnabledForViewer(PhabricatorUser $viewer) {
     if (!$viewer->isLoggedIn()) {
       return false;
diff --git a/src/view/page/menu/PhabricatorMainMenuView.php b/src/view/page/menu/PhabricatorMainMenuView.php
index f9e4032d87..ba4bda41ea 100644
--- a/src/view/page/menu/PhabricatorMainMenuView.php
+++ b/src/view/page/menu/PhabricatorMainMenuView.php
@@ -46,7 +46,9 @@ final class PhabricatorMainMenuView extends AphrontView {
     $app_button = '';
     $aural = null;
 
-    if ($viewer->isLoggedIn() && $viewer->isUserActivated()) {
+    $is_full = $this->isFullSession($viewer);
+
+    if ($is_full) {
       list($menu, $dropdowns, $aural) = $this->renderNotificationMenu();
       if (array_filter($menu)) {
         $alerts[] = $menu;
@@ -54,14 +56,18 @@ final class PhabricatorMainMenuView extends AphrontView {
       $menu_bar = array_merge($menu_bar, $dropdowns);
       $app_button = $this->renderApplicationMenuButton();
       $search_button = $this->renderSearchMenuButton($header_id);
-    } else {
+    } else if (!$viewer->isLoggedIn()) {
       $app_button = $this->renderApplicationMenuButton();
       if (PhabricatorEnv::getEnvConfig('policy.allow-public')) {
         $search_button = $this->renderSearchMenuButton($header_id);
       }
     }
 
-    $search_menu = $this->renderPhabricatorSearchMenu();
+    if ($search_button) {
+      $search_menu = $this->renderPhabricatorSearchMenu();
+    } else {
+      $search_menu = null;
+    }
 
     if ($alerts) {
       $alerts = javelin_tag(
@@ -84,7 +90,9 @@ final class PhabricatorMainMenuView extends AphrontView {
 
     $extensions = PhabricatorMainMenuBarExtension::getAllEnabledExtensions();
     foreach ($extensions as $extension) {
-      $extension->setViewer($viewer);
+      $extension
+        ->setViewer($viewer)
+        ->setIsFullSession($is_full);
 
       $controller = $this->getController();
       if ($controller) {
@@ -96,6 +104,14 @@ final class PhabricatorMainMenuView extends AphrontView {
       }
     }
 
+    if (!$is_full) {
+      foreach ($extensions as $key => $extension) {
+        if (!$extension->shouldAllowPartialSessions()) {
+          unset($extensions[$key]);
+        }
+      }
+    }
+
     foreach ($extensions as $key => $extension) {
       if (!$extension->isExtensionEnabledForViewer($extension->getViewer())) {
         unset($extensions[$key]);
@@ -677,4 +693,38 @@ final class PhabricatorMainMenuView extends AphrontView {
     );
   }
 
+  private function isFullSession(PhabricatorUser $viewer) {
+    if (!$viewer->isLoggedIn()) {
+      return false;
+    }
+
+    if (!$viewer->isUserActivated()) {
+      return false;
+    }
+
+    if (!$viewer->hasSession()) {
+      return false;
+    }
+
+    $session = $viewer->getSession();
+    if ($session->getIsPartial()) {
+      return false;
+    }
+
+    if (!$session->getSignedLegalpadDocuments()) {
+      return false;
+    }
+
+    $mfa_key = 'security.require-multi-factor-auth';
+    $need_mfa = PhabricatorEnv::getEnvConfig($mfa_key);
+    if ($need_mfa) {
+      $have_mfa = $viewer->getIsEnrolledInMultiFactor();
+      if (!$have_mfa) {
+        return false;
+      }
+    }
+
+    return true;
+  }
+
 }

From c7d6fd198cc98cf8b860288031eb4274bdcd94e0 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 27 Nov 2017 15:11:06 -0800
Subject: [PATCH 10/18] Support "Set X to" as an action in Herald for
 tokenizer/datasource custom fields

Summary:
See PHI173. Adds custom field support for Herald actions, and implements actions for "Datasource/Tokenizer" fields.

The only action available for now is "set field to...". Other actions ("Add values", "Remove values") might make sense in the future for these fields, but there's currently no use case. For most other field types (text, select, checkbox, etc) only "Set to" makes sense.

Test Plan:
  - Added a "datasource" custom field to the custom field definition in Config.
  - Added a "if field is empty, set field to default value X" rule to Herald.
  - Created a task with a nonempty field: no Herald trigger.
  - Created a task with an empty field: Herald fired.
  - Reviewed rule and transcripts for text strings.

{F5297615}

{F5297616}

{F5297617}

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18784
---
 src/__phutil_library_map__.php                |   4 +
 .../field/PhabricatorCustomField.php          |  55 +++++++++
 .../PhabricatorCustomFieldHeraldAction.php    | 105 ++++++++++++++++++
 ...habricatorCustomFieldHeraldActionGroup.php |  16 +++
 ...habricatorStandardCustomFieldTokenizer.php |  36 ++++++
 5 files changed, 216 insertions(+)
 create mode 100644 src/infrastructure/customfield/herald/PhabricatorCustomFieldHeraldAction.php
 create mode 100644 src/infrastructure/customfield/herald/PhabricatorCustomFieldHeraldActionGroup.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 424d72dc72..c4890116cf 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -2535,6 +2535,8 @@ phutil_register_library_map(array(
     'PhabricatorCustomFieldEditField' => 'infrastructure/customfield/editor/PhabricatorCustomFieldEditField.php',
     'PhabricatorCustomFieldEditType' => 'infrastructure/customfield/editor/PhabricatorCustomFieldEditType.php',
     'PhabricatorCustomFieldFulltextEngineExtension' => 'infrastructure/customfield/engineextension/PhabricatorCustomFieldFulltextEngineExtension.php',
+    'PhabricatorCustomFieldHeraldAction' => 'infrastructure/customfield/herald/PhabricatorCustomFieldHeraldAction.php',
+    'PhabricatorCustomFieldHeraldActionGroup' => 'infrastructure/customfield/herald/PhabricatorCustomFieldHeraldActionGroup.php',
     'PhabricatorCustomFieldHeraldField' => 'infrastructure/customfield/herald/PhabricatorCustomFieldHeraldField.php',
     'PhabricatorCustomFieldHeraldFieldGroup' => 'infrastructure/customfield/herald/PhabricatorCustomFieldHeraldFieldGroup.php',
     'PhabricatorCustomFieldImplementationIncompleteException' => 'infrastructure/customfield/exception/PhabricatorCustomFieldImplementationIncompleteException.php',
@@ -7868,6 +7870,8 @@ phutil_register_library_map(array(
     'PhabricatorCustomFieldEditField' => 'PhabricatorEditField',
     'PhabricatorCustomFieldEditType' => 'PhabricatorEditType',
     'PhabricatorCustomFieldFulltextEngineExtension' => 'PhabricatorFulltextEngineExtension',
+    'PhabricatorCustomFieldHeraldAction' => 'HeraldAction',
+    'PhabricatorCustomFieldHeraldActionGroup' => 'HeraldActionGroup',
     'PhabricatorCustomFieldHeraldField' => 'HeraldField',
     'PhabricatorCustomFieldHeraldFieldGroup' => 'HeraldFieldGroup',
     'PhabricatorCustomFieldImplementationIncompleteException' => 'Exception',
diff --git a/src/infrastructure/customfield/field/PhabricatorCustomField.php b/src/infrastructure/customfield/field/PhabricatorCustomField.php
index fba299fc8b..f7f4403402 100644
--- a/src/infrastructure/customfield/field/PhabricatorCustomField.php
+++ b/src/infrastructure/customfield/field/PhabricatorCustomField.php
@@ -34,6 +34,7 @@ abstract class PhabricatorCustomField extends Phobject {
   const ROLE_CONDUIT                  = 'conduit';
   const ROLE_HERALD                   = 'herald';
   const ROLE_EDITENGINE = 'EditEngine';
+  const ROLE_HERALDACTION = 'herald.action';
 
 
 /* -(  Building Applications with Custom Fields  )--------------------------- */
@@ -293,6 +294,8 @@ abstract class PhabricatorCustomField extends Phobject {
         return $this->shouldAppearInTransactionMail();
       case self::ROLE_HERALD:
         return $this->shouldAppearInHerald();
+      case self::ROLE_HERALDACTION:
+        return $this->shouldAppearInHeraldActions();
       case self::ROLE_EDITENGINE:
         return $this->shouldAppearInEditView() ||
                $this->shouldAppearInEditEngine();
@@ -1476,4 +1479,56 @@ abstract class PhabricatorCustomField extends Phobject {
   }
 
 
+  public function shouldAppearInHeraldActions() {
+    if ($this->proxy) {
+      return $this->proxy->shouldAppearInHeraldActions();
+    }
+    return false;
+  }
+
+
+  public function getHeraldActionName() {
+    if ($this->proxy) {
+      return $this->proxy->getHeraldActionName();
+    }
+
+    return null;
+  }
+
+
+  public function getHeraldActionStandardType() {
+    if ($this->proxy) {
+      return $this->proxy->getHeraldActionStandardType();
+    }
+
+    return null;
+  }
+
+
+  public function getHeraldActionDescription($value) {
+    if ($this->proxy) {
+      return $this->proxy->getHeraldActionDescription($value);
+    }
+
+    return null;
+  }
+
+
+  public function getHeraldActionEffectDescription($value) {
+    if ($this->proxy) {
+      return $this->proxy->getHeraldActionEffectDescription($value);
+    }
+
+    return null;
+  }
+
+
+  public function getHeraldActionDatasource() {
+    if ($this->proxy) {
+      return $this->proxy->getHeraldActionDatasource();
+    }
+
+    return null;
+  }
+
 }
diff --git a/src/infrastructure/customfield/herald/PhabricatorCustomFieldHeraldAction.php b/src/infrastructure/customfield/herald/PhabricatorCustomFieldHeraldAction.php
new file mode 100644
index 0000000000..1d35ba59a2
--- /dev/null
+++ b/src/infrastructure/customfield/herald/PhabricatorCustomFieldHeraldAction.php
@@ -0,0 +1,105 @@
+<?php
+
+final class PhabricatorCustomFieldHeraldAction extends HeraldAction {
+
+  const ACTIONCONST = 'herald.action.custom';
+
+  const DO_SET_FIELD = 'do.set-custom-field';
+
+  private $customField;
+
+  public function setCustomField(PhabricatorCustomField $custom_field) {
+    $this->customField = $custom_field;
+    return $this;
+  }
+
+  public function getCustomField() {
+    return $this->customField;
+  }
+
+  public function getActionGroupKey() {
+    return PhabricatorCustomFieldHeraldActionGroup::ACTIONGROUPKEY;
+  }
+
+  public function supportsObject($object) {
+    return ($object instanceof PhabricatorCustomFieldInterface);
+  }
+
+  public function supportsRuleType($rule_type) {
+    return true;
+  }
+
+  public function getActionsForObject($object) {
+    $viewer = PhabricatorUser::getOmnipotentUser();
+    $role = PhabricatorCustomField::ROLE_HERALDACTION;
+
+    $field_list = PhabricatorCustomField::getObjectFields($object, $role)
+      ->setViewer($viewer)
+      ->readFieldsFromStorage($object);
+
+    $map = array();
+    foreach ($field_list->getFields() as $field) {
+      $key = $field->getFieldKey();
+      $map[$key] = id(new self())
+        ->setCustomField($field);
+    }
+
+    return $map;
+  }
+
+  public function applyEffect($object, HeraldEffect $effect) {
+    $field = $this->getCustomField();
+    $value = $effect->getTarget();
+    $adapter = $this->getAdapter();
+
+    $old_value = $field->getOldValueForApplicationTransactions();
+    $new_value = id(clone $field)
+      ->setValueFromApplicationTransactions($value)
+      ->getValueForStorage();
+
+    $xaction = $adapter->newTransaction()
+      ->setTransactionType(PhabricatorTransactions::TYPE_CUSTOMFIELD)
+      ->setMetadataValue('customfield:key', $field->getFieldKey())
+      ->setOldValue($old_value)
+      ->setNewValue($new_value);
+
+    $adapter->queueTransaction($xaction);
+
+    $this->logEffect(self::DO_SET_FIELD, $value);
+  }
+
+  public function getHeraldActionName() {
+    return $this->getCustomField()->getHeraldActionName();
+  }
+
+  public function getHeraldActionStandardType() {
+    return $this->getCustomField()->getHeraldActionStandardType();
+  }
+
+  protected function getDatasource() {
+    return $this->getCustomField()->getHeraldActionDatasource();
+  }
+
+  public function renderActionDescription($value) {
+    return $this->getCustomField()->getHeraldActionDescription($value);
+  }
+
+  protected function getActionEffectMap() {
+    return array(
+      self::DO_SET_FIELD => array(
+        'icon' => 'fa-pencil',
+        'color' => 'green',
+        'name' => pht('Set Field Value'),
+      ),
+    );
+  }
+
+  protected function renderActionEffectDescription($type, $data) {
+    switch ($type) {
+      case self::DO_SET_FIELD:
+        return $this->getCustomField()->getHeraldActionEffectDescription($data);
+    }
+  }
+
+
+}
diff --git a/src/infrastructure/customfield/herald/PhabricatorCustomFieldHeraldActionGroup.php b/src/infrastructure/customfield/herald/PhabricatorCustomFieldHeraldActionGroup.php
new file mode 100644
index 0000000000..b8f16ab202
--- /dev/null
+++ b/src/infrastructure/customfield/herald/PhabricatorCustomFieldHeraldActionGroup.php
@@ -0,0 +1,16 @@
+<?php
+
+final class PhabricatorCustomFieldHeraldActionGroup
+  extends HeraldActionGroup {
+
+  const ACTIONGROUPKEY = 'customfield';
+
+  public function getGroupLabel() {
+    return pht('Custom Fields');
+  }
+
+  protected function getGroupOrder() {
+    return 2000;
+  }
+
+}
diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldTokenizer.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldTokenizer.php
index d2b063ffde..9cb19d029d 100644
--- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldTokenizer.php
+++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldTokenizer.php
@@ -65,4 +65,40 @@ abstract class PhabricatorStandardCustomFieldTokenizer
     return new ConduitPHIDListParameterType();
   }
 
+  public function shouldAppearInHeraldActions() {
+    return true;
+  }
+
+  public function getHeraldActionName() {
+    return pht('Set "%s" to', $this->getFieldName());
+  }
+
+  public function getHeraldActionDescription($value) {
+    $list = $this->renderHeraldHandleList($value);
+    return pht('Set "%s" to: %s.', $this->getFieldName(), $list);
+  }
+
+  public function getHeraldActionEffectDescription($value) {
+    return $this->renderHeraldHandleList($value);
+  }
+
+  public function getHeraldActionStandardType() {
+    return HeraldAction::STANDARD_PHID_LIST;
+  }
+
+  public function getHeraldActionDatasource() {
+    return $this->getDatasource();
+  }
+
+  private function renderHeraldHandleList($value) {
+    if (!is_array($value)) {
+      return pht('(Invalid List)');
+    } else {
+      return $this->getViewer()
+        ->renderHandleList($value)
+        ->setAsInline(true)
+        ->render();
+    }
+  }
+
 }

From 54f131dc4baad39f54c164ea57135151c4d38542 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 28 Nov 2017 10:42:16 -0800
Subject: [PATCH 11/18] Make "first broadcast" rules for Differential drafts
 more general

Summary:
See PHI228. Ref T2543. The current logic gets this slightly wrong: prototypes are off, you create a draft with `--draft`, then promote it with "Request Review". This misses both branches.

Instead, test these conditions a little more broadly. We also need to store broadcast state since `getIsNewObject()` isn't good enough with this workflow.

Test Plan:
  - With prototypes on and autopromotion, got a rich email after builds finished.
  - With prototypes off, got a rich email immediately.
  - With prototypes off and `--draft`, got a rich email after "Request Review".

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18801
---
 .../editor/DifferentialTransactionEditor.php  | 36 ++++++++++++++-----
 .../storage/DifferentialRevision.php          | 10 ++++++
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
index 7cea29359f..ae652ba0ec 100644
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -10,6 +10,7 @@ final class DifferentialTransactionEditor
   private $hasReviewTransaction = false;
   private $affectedPaths;
   private $firstBroadcast = false;
+  private $wasDraft = false;
 
   public function getEditorApplicationClass() {
     return 'PhabricatorDifferentialApplication';
@@ -166,6 +167,8 @@ final class DifferentialTransactionEditor
       }
     }
 
+    $this->wasDraft = $object->isDraft();
+
     return parent::expandTransactions($object, $xactions);
   }
 
@@ -1581,10 +1584,6 @@ final class DifferentialTransactionEditor
           $this->setActingAsPHID($author_phid);
         }
 
-        // Mark this as the first broadcast we're sending about the revision
-        // so mail can generate specially.
-        $this->firstBroadcast = true;
-
         $xaction = $object->getApplicationTransactionTemplate()
           ->setAuthorPHID($author_phid)
           ->setTransactionType(
@@ -1612,12 +1611,31 @@ final class DifferentialTransactionEditor
 
         $xactions[] = $xaction;
       }
-    } else {
-      // If this revision is being created into some state other than "Draft",
-      // this is the first broadcast and should include sections like "SUMMARY"
-      // and "TEST PLAN".
-      if ($this->getIsNewObject()) {
+    }
+
+    // If the revision is new or was a draft, and is no longer a draft, we
+    // might be sending the first email about it.
+
+    // This might mean it was created directly into a non-draft state, or
+    // it just automatically undrafted after builds finished, or a user
+    // explicitly promoted it out of the draft state with an action like
+    // "Request Review".
+
+    // If we haven't sent any email about it yet, mark this email as the first
+    // email so the mail gets enriched with "SUMMARY" and "TEST PLAN".
+
+    $is_new = $this->getIsNewObject();
+    $was_draft = $this->wasDraft;
+
+    if (!$object->isDraft() && ($was_draft || $is_new)) {
+      if (!$object->getHasBroadcast()) {
+        // Mark this as the first broadcast we're sending about the revision
+        // so mail can generate specially.
         $this->firstBroadcast = true;
+
+        $object
+          ->setHasBroadcast(true)
+          ->save();
       }
     }
 
diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php
index bf4bec0abc..73f22c91e4 100644
--- a/src/applications/differential/storage/DifferentialRevision.php
+++ b/src/applications/differential/storage/DifferentialRevision.php
@@ -60,6 +60,7 @@ final class DifferentialRevision extends DifferentialDAO
 
   const PROPERTY_CLOSED_FROM_ACCEPTED = 'wasAcceptedBeforeClose';
   const PROPERTY_DRAFT_HOLD = 'draft.hold';
+  const PROPERTY_HAS_BROADCAST = 'draft.broadcast';
 
   public static function initializeNewRevision(PhabricatorUser $actor) {
     $app = id(new PhabricatorApplicationQuery())
@@ -719,6 +720,15 @@ final class DifferentialRevision extends DifferentialDAO
     return $this->setProperty(self::PROPERTY_DRAFT_HOLD, $hold);
   }
 
+  public function getHasBroadcast() {
+    return $this->getProperty(self::PROPERTY_HAS_BROADCAST, false);
+  }
+
+  public function setHasBroadcast($has_broadcast) {
+    return $this->setProperty(self::PROPERTY_HAS_BROADCAST, $has_broadcast);
+  }
+
+
   public function loadActiveBuilds(PhabricatorUser $viewer) {
     $diff = $this->getActiveDiff();
 

From 1b76250f8995744c016c4b8149263e0bc4727b73 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 28 Nov 2017 11:05:15 -0800
Subject: [PATCH 12/18] Disallow "Accept" on drafts, allow "Resign"

Summary:
Depends on D18801. Ref T2543. See PHI229. I missed "Accept" before, but intended to disallow it (like "Reject") since I don't want drafts to be reviewable.

However, "Resign" seems fine to allow? So let's allow that for now.

Test Plan: Was no longer offered "Accept" on draft revisions.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18802
---
 .../xaction/DifferentialRevisionAcceptTransaction.php        | 5 +++++
 .../xaction/DifferentialRevisionResignTransaction.php        | 5 -----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php
index d52e2dbeb7..190a6a4920 100644
--- a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php
@@ -162,6 +162,11 @@ final class DifferentialRevisionAcceptTransaction
           'closed. Only open revisions can be accepted.'));
     }
 
+    if ($object->isDraft()) {
+      throw new Exception(
+        pht('You can not accept a draft revision.'));
+    }
+
     $config_key = 'differential.allow-self-accept';
     if (!PhabricatorEnv::getEnvConfig($config_key)) {
       if ($this->isViewerRevisionAuthor($object, $viewer)) {
diff --git a/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php b/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php
index 40a512202d..53b0b8da01 100644
--- a/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php
@@ -64,11 +64,6 @@ final class DifferentialRevisionResignTransaction
           'been closed. You can only resign from open revisions.'));
     }
 
-    if ($object->isDraft()) {
-      throw new Exception(
-        pht('You can not resign from a draft revision.'));
-    }
-
     $resigned = DifferentialReviewerStatus::STATUS_RESIGNED;
     if ($this->getViewerReviewerStatus($object, $viewer) == $resigned) {
       throw new Exception(

From d8f2630d5c0f9eb5221964944e45b9197753a988 Mon Sep 17 00:00:00 2001
From: Aviv Eyal <aviv@myclaimly.com>
Date: Thu, 30 Nov 2017 15:07:49 +0000
Subject: [PATCH 13/18] Modernize QuickSearch typeahead

Summary:
Use ClassQuery to find datasources for the quick-search.

Mostly, this allows extensions to add quicksearches.

Test Plan:
using `/typeahead/class/`, tested several search terms that make sense.
Removed the tag interface from a datasource, which removed it from results.

Reviewers: epriestley, amckinley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18760
---
 src/__phutil_library_map__.php                 | 14 ++++++++++++++
 .../DiffusionQuickSearchEngineExtension.php    | 12 ++++++++++++
 ...orQuickSearchApplicationEngineExtension.php | 11 +++++++++++
 ...ricatorPeopleQuickSearchEngineExtension.php | 11 +++++++++++
 .../ProjectQuickSearchEngineExtension.php      | 11 +++++++++++
 .../engine/PhabricatorQuickSearchEngine.php    |  8 ++++++++
 .../PhabricatorQuickSearchEngineExtension.php  | 18 ++++++++++++++++++
 .../typeahead/PhabricatorSearchDatasource.php  | 10 ++--------
 ...catorMonogramQuickSearchEngineExtension.php | 11 +++++++++++
 9 files changed, 98 insertions(+), 8 deletions(-)
 create mode 100644 src/applications/diffusion/engineextension/DiffusionQuickSearchEngineExtension.php
 create mode 100644 src/applications/meta/engineextension/PhabricatorQuickSearchApplicationEngineExtension.php
 create mode 100644 src/applications/people/engineextension/PhabricatorPeopleQuickSearchEngineExtension.php
 create mode 100644 src/applications/project/engineextension/ProjectQuickSearchEngineExtension.php
 create mode 100644 src/applications/search/engine/PhabricatorQuickSearchEngine.php
 create mode 100644 src/applications/search/engineextension/PhabricatorQuickSearchEngineExtension.php
 create mode 100644 src/applications/typeahead/engineextension/PhabricatorMonogramQuickSearchEngineExtension.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index c4890116cf..53bbf9a749 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -831,6 +831,7 @@ phutil_register_library_map(array(
     'DiffusionQueryCommitsConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionQueryCommitsConduitAPIMethod.php',
     'DiffusionQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionQueryConduitAPIMethod.php',
     'DiffusionQueryPathsConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php',
+    'DiffusionQuickSearchEngineExtension' => 'applications/diffusion/engineextension/DiffusionQuickSearchEngineExtension.php',
     'DiffusionRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionRawDiffQuery.php',
     'DiffusionRawDiffQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionRawDiffQueryConduitAPIMethod.php',
     'DiffusionReadmeView' => 'applications/diffusion/view/DiffusionReadmeView.php',
@@ -3202,6 +3203,7 @@ phutil_register_library_map(array(
     'PhabricatorMetronomicTriggerClock' => 'infrastructure/daemon/workers/clock/PhabricatorMetronomicTriggerClock.php',
     'PhabricatorModularTransaction' => 'applications/transactions/storage/PhabricatorModularTransaction.php',
     'PhabricatorModularTransactionType' => 'applications/transactions/storage/PhabricatorModularTransactionType.php',
+    'PhabricatorMonogramQuickSearchEngineExtension' => 'applications/typeahead/engineextension/PhabricatorMonogramQuickSearchEngineExtension.php',
     'PhabricatorMonospacedFontSetting' => 'applications/settings/setting/PhabricatorMonospacedFontSetting.php',
     'PhabricatorMonospacedTextareasSetting' => 'applications/settings/setting/PhabricatorMonospacedTextareasSetting.php',
     'PhabricatorMotivatorProfileMenuItem' => 'applications/search/menuitem/PhabricatorMotivatorProfileMenuItem.php',
@@ -3525,6 +3527,7 @@ phutil_register_library_map(array(
     'PhabricatorPeopleProfileTasksController' => 'applications/people/controller/PhabricatorPeopleProfileTasksController.php',
     'PhabricatorPeopleProfileViewController' => 'applications/people/controller/PhabricatorPeopleProfileViewController.php',
     'PhabricatorPeopleQuery' => 'applications/people/query/PhabricatorPeopleQuery.php',
+    'PhabricatorPeopleQuickSearchEngineExtension' => 'applications/people/engineextension/PhabricatorPeopleQuickSearchEngineExtension.php',
     'PhabricatorPeopleRenameController' => 'applications/people/controller/PhabricatorPeopleRenameController.php',
     'PhabricatorPeopleRevisionsProfileMenuItem' => 'applications/people/menuitem/PhabricatorPeopleRevisionsProfileMenuItem.php',
     'PhabricatorPeopleSearchEngine' => 'applications/people/query/PhabricatorPeopleSearchEngine.php',
@@ -3785,6 +3788,9 @@ phutil_register_library_map(array(
     'PhabricatorQueryOrderItem' => 'infrastructure/query/order/PhabricatorQueryOrderItem.php',
     'PhabricatorQueryOrderTestCase' => 'infrastructure/query/order/__tests__/PhabricatorQueryOrderTestCase.php',
     'PhabricatorQueryOrderVector' => 'infrastructure/query/order/PhabricatorQueryOrderVector.php',
+    'PhabricatorQuickSearchApplicationEngineExtension' => 'applications/meta/engineextension/PhabricatorQuickSearchApplicationEngineExtension.php',
+    'PhabricatorQuickSearchEngine' => 'applications/search/engine/PhabricatorQuickSearchEngine.php',
+    'PhabricatorQuickSearchEngineExtension' => 'applications/search/engineextension/PhabricatorQuickSearchEngineExtension.php',
     'PhabricatorRateLimitRequestExceptionHandler' => 'aphront/handler/PhabricatorRateLimitRequestExceptionHandler.php',
     'PhabricatorRecaptchaConfigOptions' => 'applications/config/option/PhabricatorRecaptchaConfigOptions.php',
     'PhabricatorRedirectController' => 'applications/base/controller/PhabricatorRedirectController.php',
@@ -4817,6 +4823,7 @@ phutil_register_library_map(array(
     'ProjectDefaultViewCapability' => 'applications/project/capability/ProjectDefaultViewCapability.php',
     'ProjectEditConduitAPIMethod' => 'applications/project/conduit/ProjectEditConduitAPIMethod.php',
     'ProjectQueryConduitAPIMethod' => 'applications/project/conduit/ProjectQueryConduitAPIMethod.php',
+    'ProjectQuickSearchEngineExtension' => 'applications/project/engineextension/ProjectQuickSearchEngineExtension.php',
     'ProjectRemarkupRule' => 'applications/project/remarkup/ProjectRemarkupRule.php',
     'ProjectRemarkupRuleTestCase' => 'applications/project/remarkup/__tests__/ProjectRemarkupRuleTestCase.php',
     'ProjectReplyHandler' => 'applications/project/mail/ProjectReplyHandler.php',
@@ -5880,6 +5887,7 @@ phutil_register_library_map(array(
     'DiffusionQueryCommitsConduitAPIMethod' => 'DiffusionConduitAPIMethod',
     'DiffusionQueryConduitAPIMethod' => 'DiffusionConduitAPIMethod',
     'DiffusionQueryPathsConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod',
+    'DiffusionQuickSearchEngineExtension' => 'PhabricatorQuickSearchEngineExtension',
     'DiffusionRawDiffQuery' => 'DiffusionFileFutureQuery',
     'DiffusionRawDiffQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod',
     'DiffusionReadmeView' => 'DiffusionView',
@@ -8612,6 +8620,7 @@ phutil_register_library_map(array(
     'PhabricatorMetronomicTriggerClock' => 'PhabricatorTriggerClock',
     'PhabricatorModularTransaction' => 'PhabricatorApplicationTransaction',
     'PhabricatorModularTransactionType' => 'Phobject',
+    'PhabricatorMonogramQuickSearchEngineExtension' => 'PhabricatorQuickSearchEngineExtension',
     'PhabricatorMonospacedFontSetting' => 'PhabricatorStringSetting',
     'PhabricatorMonospacedTextareasSetting' => 'PhabricatorSelectSetting',
     'PhabricatorMotivatorProfileMenuItem' => 'PhabricatorProfileMenuItem',
@@ -9004,6 +9013,7 @@ phutil_register_library_map(array(
     'PhabricatorPeopleProfileTasksController' => 'PhabricatorPeopleProfileController',
     'PhabricatorPeopleProfileViewController' => 'PhabricatorPeopleProfileController',
     'PhabricatorPeopleQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
+    'PhabricatorPeopleQuickSearchEngineExtension' => 'PhabricatorQuickSearchEngineExtension',
     'PhabricatorPeopleRenameController' => 'PhabricatorPeopleController',
     'PhabricatorPeopleRevisionsProfileMenuItem' => 'PhabricatorProfileMenuItem',
     'PhabricatorPeopleSearchEngine' => 'PhabricatorApplicationSearchEngine',
@@ -9319,6 +9329,9 @@ phutil_register_library_map(array(
       'Phobject',
       'Iterator',
     ),
+    'PhabricatorQuickSearchApplicationEngineExtension' => 'PhabricatorQuickSearchEngineExtension',
+    'PhabricatorQuickSearchEngine' => 'Phobject',
+    'PhabricatorQuickSearchEngineExtension' => 'Phobject',
     'PhabricatorRateLimitRequestExceptionHandler' => 'PhabricatorRequestExceptionHandler',
     'PhabricatorRecaptchaConfigOptions' => 'PhabricatorApplicationConfigOptions',
     'PhabricatorRedirectController' => 'PhabricatorController',
@@ -10605,6 +10618,7 @@ phutil_register_library_map(array(
     'ProjectDefaultViewCapability' => 'PhabricatorPolicyCapability',
     'ProjectEditConduitAPIMethod' => 'PhabricatorEditEngineAPIMethod',
     'ProjectQueryConduitAPIMethod' => 'ProjectConduitAPIMethod',
+    'ProjectQuickSearchEngineExtension' => 'PhabricatorQuickSearchEngineExtension',
     'ProjectRemarkupRule' => 'PhabricatorObjectRemarkupRule',
     'ProjectRemarkupRuleTestCase' => 'PhabricatorTestCase',
     'ProjectReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler',
diff --git a/src/applications/diffusion/engineextension/DiffusionQuickSearchEngineExtension.php b/src/applications/diffusion/engineextension/DiffusionQuickSearchEngineExtension.php
new file mode 100644
index 0000000000..80c705b4f8
--- /dev/null
+++ b/src/applications/diffusion/engineextension/DiffusionQuickSearchEngineExtension.php
@@ -0,0 +1,12 @@
+<?php
+
+final class DiffusionQuickSearchEngineExtension
+  extends PhabricatorQuickSearchEngineExtension {
+
+  public function newQuickSearchDatasources() {
+    return array(
+      new DiffusionRepositoryDatasource(),
+      new DiffusionSymbolDatasource(),
+    );
+  }
+}
diff --git a/src/applications/meta/engineextension/PhabricatorQuickSearchApplicationEngineExtension.php b/src/applications/meta/engineextension/PhabricatorQuickSearchApplicationEngineExtension.php
new file mode 100644
index 0000000000..8161c863ba
--- /dev/null
+++ b/src/applications/meta/engineextension/PhabricatorQuickSearchApplicationEngineExtension.php
@@ -0,0 +1,11 @@
+<?php
+
+final class PhabricatorQuickSearchApplicationEngineExtension
+  extends PhabricatorQuickSearchEngineExtension {
+
+  public function newQuickSearchDatasources() {
+    return array(
+      new PhabricatorApplicationDatasource(),
+    );
+  }
+}
diff --git a/src/applications/people/engineextension/PhabricatorPeopleQuickSearchEngineExtension.php b/src/applications/people/engineextension/PhabricatorPeopleQuickSearchEngineExtension.php
new file mode 100644
index 0000000000..565838de72
--- /dev/null
+++ b/src/applications/people/engineextension/PhabricatorPeopleQuickSearchEngineExtension.php
@@ -0,0 +1,11 @@
+<?php
+
+final class PhabricatorPeopleQuickSearchEngineExtension
+  extends PhabricatorQuickSearchEngineExtension {
+
+  public function newQuickSearchDatasources() {
+    return array(
+      new PhabricatorPeopleDatasource(),
+    );
+  }
+}
diff --git a/src/applications/project/engineextension/ProjectQuickSearchEngineExtension.php b/src/applications/project/engineextension/ProjectQuickSearchEngineExtension.php
new file mode 100644
index 0000000000..6bb50e021e
--- /dev/null
+++ b/src/applications/project/engineextension/ProjectQuickSearchEngineExtension.php
@@ -0,0 +1,11 @@
+<?php
+
+final class ProjectQuickSearchEngineExtension
+  extends PhabricatorQuickSearchEngineExtension {
+
+  public function newQuickSearchDatasources() {
+    return array(
+      new PhabricatorProjectDatasource(),
+    );
+  }
+}
diff --git a/src/applications/search/engine/PhabricatorQuickSearchEngine.php b/src/applications/search/engine/PhabricatorQuickSearchEngine.php
new file mode 100644
index 0000000000..4b67dca659
--- /dev/null
+++ b/src/applications/search/engine/PhabricatorQuickSearchEngine.php
@@ -0,0 +1,8 @@
+<?php
+
+final class PhabricatorQuickSearchEngine extends Phobject {
+
+  public function getAllDatasources() {
+    return PhabricatorQuickSearchEngineExtension::getAllDatasources();
+  }
+}
diff --git a/src/applications/search/engineextension/PhabricatorQuickSearchEngineExtension.php b/src/applications/search/engineextension/PhabricatorQuickSearchEngineExtension.php
new file mode 100644
index 0000000000..43f0b4c3ca
--- /dev/null
+++ b/src/applications/search/engineextension/PhabricatorQuickSearchEngineExtension.php
@@ -0,0 +1,18 @@
+<?php
+
+abstract class PhabricatorQuickSearchEngineExtension extends Phobject {
+
+  abstract public function newQuickSearchDatasources();
+
+  final public static function getAllDatasources() {
+    $extensions = id(new PhutilClassMapQuery())
+      ->setAncestorClass(__CLASS__)
+      ->execute();
+
+    $datasources = array();
+    foreach ($extensions as $extension) {
+      $datasources[] = $extension->newQuickSearchDatasources();
+    }
+    return array_mergev($datasources);
+  }
+}
diff --git a/src/applications/search/typeahead/PhabricatorSearchDatasource.php b/src/applications/search/typeahead/PhabricatorSearchDatasource.php
index 2a17ad01eb..ed17a31185 100644
--- a/src/applications/search/typeahead/PhabricatorSearchDatasource.php
+++ b/src/applications/search/typeahead/PhabricatorSearchDatasource.php
@@ -16,14 +16,8 @@ final class PhabricatorSearchDatasource
   }
 
   public function getComponentDatasources() {
-    $sources = array(
-      new PhabricatorPeopleDatasource(),
-      new PhabricatorProjectDatasource(),
-      new PhabricatorApplicationDatasource(),
-      new PhabricatorTypeaheadMonogramDatasource(),
-      new DiffusionRepositoryDatasource(),
-      new DiffusionSymbolDatasource(),
-    );
+    $sources = id(new PhabricatorQuickSearchEngine())
+      ->getAllDatasources();
 
     // These results are always rendered in the full browse display mode, so
     // set the browse flag on all component sources.
diff --git a/src/applications/typeahead/engineextension/PhabricatorMonogramQuickSearchEngineExtension.php b/src/applications/typeahead/engineextension/PhabricatorMonogramQuickSearchEngineExtension.php
new file mode 100644
index 0000000000..078cb44685
--- /dev/null
+++ b/src/applications/typeahead/engineextension/PhabricatorMonogramQuickSearchEngineExtension.php
@@ -0,0 +1,11 @@
+<?php
+
+final class PhabricatorMonogramQuickSearchEngineExtension
+  extends PhabricatorQuickSearchEngineExtension {
+
+  public function newQuickSearchDatasources() {
+    return array(
+      new PhabricatorTypeaheadMonogramDatasource(),
+    );
+  }
+}

From 0807b70ea16743aaa8b103c127cf12056a27de07 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 30 Nov 2017 08:13:10 -0800
Subject: [PATCH 14/18] Add an explicit warning in the Differential transaction
 log when users skip review

Summary:
Ref T10233. See PHI231. When users ignore the `arc land` prompt about bad revision states, make it explicitly clear in the transaction log that they broke the rules.

You can currently figure this out by noticing that there's no "This revision is accepted and ready to land." message, but it's unrealistic to expect non-expert users to look for the //absence// of a message to indicate something, and this state change is often relevant.

Test Plan: {F5302351}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T10233

Differential Revision: https://secure.phabricator.com/D18808
---
 resources/celerity/map.php                    |  6 +--
 src/__phutil_library_map__.php                |  2 +
 .../DifferentialDiffExtractionEngine.php      | 10 +++++
 ...ferentialRevisionWrongStateTransaction.php | 41 +++++++++++++++++++
 webroot/rsrc/css/phui/phui-timeline-view.css  |  4 ++
 5 files changed, 60 insertions(+), 3 deletions(-)
 create mode 100644 src/applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index 78ae8bd461..0ea66bec26 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -9,7 +9,7 @@ return array(
   'names' => array(
     'conpherence.pkg.css' => 'e68cf1fa',
     'conpherence.pkg.js' => '15191c65',
-    'core.pkg.css' => '1a4e0c25',
+    'core.pkg.css' => 'fdb27ef9',
     'core.pkg.js' => '4c79d74f',
     'darkconsole.pkg.js' => '1f9a31bc',
     'differential.pkg.css' => '45951e9e',
@@ -176,7 +176,7 @@ return array(
     'rsrc/css/phui/phui-spacing.css' => '042804d6',
     'rsrc/css/phui/phui-status.css' => 'd5263e49',
     'rsrc/css/phui/phui-tag-view.css' => 'b4719c50',
-    'rsrc/css/phui/phui-timeline-view.css' => 'f21db7ca',
+    'rsrc/css/phui/phui-timeline-view.css' => 'e2ef62b1',
     'rsrc/css/phui/phui-two-column-view.css' => '44ec4951',
     'rsrc/css/phui/workboards/phui-workboard-color.css' => '783cdff5',
     'rsrc/css/phui/workboards/phui-workboard.css' => '3bc85455',
@@ -871,7 +871,7 @@ return array(
     'phui-status-list-view-css' => 'd5263e49',
     'phui-tag-view-css' => 'b4719c50',
     'phui-theme-css' => '9f261c6b',
-    'phui-timeline-view-css' => 'f21db7ca',
+    'phui-timeline-view-css' => 'e2ef62b1',
     'phui-two-column-view-css' => '44ec4951',
     'phui-workboard-color-css' => '783cdff5',
     'phui-workboard-view-css' => '3bc85455',
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 53bbf9a749..11be8a50f0 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -594,6 +594,7 @@ phutil_register_library_map(array(
     'DifferentialRevisionUpdateHistoryView' => 'applications/differential/view/DifferentialRevisionUpdateHistoryView.php',
     'DifferentialRevisionViewController' => 'applications/differential/controller/DifferentialRevisionViewController.php',
     'DifferentialRevisionVoidTransaction' => 'applications/differential/xaction/DifferentialRevisionVoidTransaction.php',
+    'DifferentialRevisionWrongStateTransaction' => 'applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php',
     'DifferentialSchemaSpec' => 'applications/differential/storage/DifferentialSchemaSpec.php',
     'DifferentialSetDiffPropertyConduitAPIMethod' => 'applications/differential/conduit/DifferentialSetDiffPropertyConduitAPIMethod.php',
     'DifferentialStoredCustomField' => 'applications/differential/customfield/DifferentialStoredCustomField.php',
@@ -5647,6 +5648,7 @@ phutil_register_library_map(array(
     'DifferentialRevisionUpdateHistoryView' => 'AphrontView',
     'DifferentialRevisionViewController' => 'DifferentialController',
     'DifferentialRevisionVoidTransaction' => 'DifferentialRevisionTransactionType',
+    'DifferentialRevisionWrongStateTransaction' => 'DifferentialRevisionTransactionType',
     'DifferentialSchemaSpec' => 'PhabricatorConfigSchemaSpec',
     'DifferentialSetDiffPropertyConduitAPIMethod' => 'DifferentialConduitAPIMethod',
     'DifferentialStoredCustomField' => 'DifferentialCustomField',
diff --git a/src/applications/differential/engine/DifferentialDiffExtractionEngine.php b/src/applications/differential/engine/DifferentialDiffExtractionEngine.php
index 952c76c63f..c426c411e7 100644
--- a/src/applications/differential/engine/DifferentialDiffExtractionEngine.php
+++ b/src/applications/differential/engine/DifferentialDiffExtractionEngine.php
@@ -268,6 +268,16 @@ final class DifferentialDiffExtractionEngine extends Phobject {
 
     $xactions = array();
 
+    // If the revision isn't closed or "Accepted", write a warning into the
+    // transaction log. This makes it more clear when users bend the rules.
+    if (!$revision->isClosed() && !$revision->isAccepted()) {
+      $wrong_type = DifferentialRevisionWrongStateTransaction::TRANSACTIONTYPE;
+
+      $xactions[] = id(new DifferentialTransaction())
+        ->setTransactionType($wrong_type)
+        ->setNewValue($revision->getModernRevisionStatus());
+    }
+
     $xactions[] = id(new DifferentialTransaction())
       ->setTransactionType(DifferentialTransaction::TYPE_UPDATE)
       ->setIgnoreOnNoEffect(true)
diff --git a/src/applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php b/src/applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php
new file mode 100644
index 0000000000..e19e54199c
--- /dev/null
+++ b/src/applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php
@@ -0,0 +1,41 @@
+<?php
+
+final class DifferentialRevisionWrongStateTransaction
+  extends DifferentialRevisionTransactionType {
+
+  const TRANSACTIONTYPE = 'differential.revision.wrong';
+
+  public function generateOldValue($object) {
+    return null;
+  }
+
+  public function generateNewValue($object, $value) {
+    return $value;
+  }
+
+  public function getIcon() {
+    return 'fa-exclamation';
+  }
+
+  public function getColor() {
+    return 'pink';
+  }
+
+  public function getActionStrength() {
+    return 4;
+  }
+
+  public function getTitle() {
+    $new_value = $this->getNewValue();
+
+    $status = DifferentialRevisionStatus::newForStatus($new_value);
+
+    return pht(
+      'This revision was not accepted when it landed; it landed in state %s.',
+      $this->renderValue($status->getDisplayName()));
+  }
+
+  public function getTitleForFeed() {
+    return null;
+  }
+}
diff --git a/webroot/rsrc/css/phui/phui-timeline-view.css b/webroot/rsrc/css/phui/phui-timeline-view.css
index 33e0f8ed0c..b0ae9c0044 100644
--- a/webroot/rsrc/css/phui/phui-timeline-view.css
+++ b/webroot/rsrc/css/phui/phui-timeline-view.css
@@ -315,6 +315,10 @@
   background-color: {$violet};
 }
 
+.phui-timeline-icon-fill-pink {
+  background-color: {$pink};
+}
+
 .phui-timeline-icon-fill-grey {
   background-color: #888;
 }

From f786c86a6a428b580ccd5f2ce8c5df3bf7a19fa8 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 30 Nov 2017 13:41:44 -0800
Subject: [PATCH 15/18] Don't require the "gd" extension be installed in order
 to run unit tests

Summary:
Ref T10405. If `gd` is not installed, a number of unit tests currently fail because they generate synthetic users and try to composite profile pictures for them.

Instead, just fall back to a static default if `gd` is not available.

Test Plan: Faked this pathway, created a new user, saw a default profile picture.

Reviewers: amckinley, lpriestley, avivey

Reviewed By: avivey

Maniphest Tasks: T10405

Differential Revision: https://secure.phabricator.com/D18812
---
 .../builtin/PhabricatorFilesComposeAvatarBuiltinFile.php  | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/applications/files/builtin/PhabricatorFilesComposeAvatarBuiltinFile.php b/src/applications/files/builtin/PhabricatorFilesComposeAvatarBuiltinFile.php
index 4e461ec3f5..fcd6f97601 100644
--- a/src/applications/files/builtin/PhabricatorFilesComposeAvatarBuiltinFile.php
+++ b/src/applications/files/builtin/PhabricatorFilesComposeAvatarBuiltinFile.php
@@ -58,6 +58,14 @@ final class PhabricatorFilesComposeAvatarBuiltinFile
   }
 
   private function composeImage($color, $image, $border) {
+    // If we don't have the GD extension installed, just return a static
+    // default profile image rather than trying to compose a dynamic one.
+    if (!function_exists('imagecreatefromstring')) {
+      $root = dirname(phutil_get_library_root('phabricator'));
+      $default_path = $root.'/resources/builtin/profile.png';
+      return Filesystem::readFile($default_path);
+    }
+
     $color_const = hexdec(trim($color, '#'));
     $true_border = self::rgba2gd($border);
     $image_map = self::getImageMap();

From 14cc0abeb37fd411aceed31db22a820537264fe5 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 30 Nov 2017 12:13:07 -0800
Subject: [PATCH 16/18] Fix several safety issues with repository URIs

Summary:
See PHI234. Several issues here:

  - The warning about observing a repository in Read/Write mode checks the raw I/O type, not the effective I/O type. That means we can fail to warn if other URIs are set to "Default", and "Default" is "Read/Write" in practice.
  - There's just an actual typo which prevents the "Observe" version of this error from triggering properly.

Additionally, add more forceful warnings that "Observe" and "Mirror" mean that you want to //replace// a repository with another one, not that we somehow merge branches selectively. It isn't necessarily obvious that "Observe" doesn't mean "merge/union", since the reasons it can't in the general case are somewhat subtle (conflicts between refs with the same names, detecting ref deletion).

Test Plan:
Read documentation. Hit the error locally by trying to "Observe" while in Read/Write mode:

{F5302655}

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18810
---
 .../diffusion/editor/DiffusionURIEditor.php            |  4 ++--
 src/docs/user/userguide/diffusion_uris.diviner         | 10 ++++++++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/applications/diffusion/editor/DiffusionURIEditor.php b/src/applications/diffusion/editor/DiffusionURIEditor.php
index 674efbc158..90ced865f0 100644
--- a/src/applications/diffusion/editor/DiffusionURIEditor.php
+++ b/src/applications/diffusion/editor/DiffusionURIEditor.php
@@ -347,11 +347,11 @@ final class DiffusionURIEditor
                 continue;
               }
 
-              $io_type = $uri->getIoType();
+              $io_type = $uri->getEffectiveIOType();
 
               if ($io_type == PhabricatorRepositoryURI::IO_READWRITE) {
                 if ($no_readwrite) {
-                  $readwite_conflict = $uri;
+                  $readwrite_conflict = $uri;
                   break;
                 }
               }
diff --git a/src/docs/user/userguide/diffusion_uris.diviner b/src/docs/user/userguide/diffusion_uris.diviner
index 140948f55e..a19d38d3a5 100644
--- a/src/docs/user/userguide/diffusion_uris.diviner
+++ b/src/docs/user/userguide/diffusion_uris.diviner
@@ -239,6 +239,11 @@ You can not add a URI in Observe mode if an existing builtin URI is in
 authorities: the observed remote copy and the hosted local copy. Take the
 other URI out of //Read/Write// mode first.
 
+WARNING: If you observe a remote repository, the entire state of the working
+copy that Phabricator maintains will be deleted and replaced with the state of
+the remote. If some changes are present only in Phabricator's working copy,
+they will be unrecoverably destroyed.
+
 **Mirror**: Phabricator will push any changes made to this repository to the
 remote URI, keeping a read-only mirror hosted at that URI up to date.
 
@@ -251,6 +256,11 @@ It is possible to mirror a repository to another repository that is also
 hosted by Phabricator by adding that other repository's URI, although this is
 silly and probably very rarely of any use.
 
+WARNING: If you mirror to a remote repository, the entire state of that remote
+will be replaced with the state of the working copy Phabricator maintains. If
+some changes are present only in the remote, they will be unrecoverably
+destroyed.
+
 **None**: Phabricator will not fetch changes from or push changes to this URI.
 For builtin URIs, it will not let users fetch changes from or push changes to
 this URI.

From 5240cffd9c20c4f759b4ab2327ec4f6fbe2063ab Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 30 Nov 2017 12:33:02 -0800
Subject: [PATCH 17/18] Fix an issue where Diffusion could fatal if the default
 branch was deleted

Summary: See PHI234. In T12931 we improved the behavior of Diffusion when a repository's default branch is set to a branch that does not exist, but in T11823 the way refcursors work changed, and we can now get a cursor (just with no positions) back for a deleted branch. When we did, we didn't handle things gracefully.

Test Plan:
  - Set default branch to a deleted branch, saw nice error instead of fatal.
  - Set default branch to a nonexistent branch which never existed, saw nice error.
  - Set default branch to existing "master", saw repository normally.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18811
---
 .../controller/DiffusionRepositoryController.php   | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/applications/diffusion/controller/DiffusionRepositoryController.php b/src/applications/diffusion/controller/DiffusionRepositoryController.php
index f64b72e1f8..35fce7f03d 100644
--- a/src/applications/diffusion/controller/DiffusionRepositoryController.php
+++ b/src/applications/diffusion/controller/DiffusionRepositoryController.php
@@ -46,8 +46,20 @@ final class DiffusionRepositoryController extends DiffusionController {
         ->withRepositoryPHIDs(array($repository->getPHID()))
         ->withRefTypes(array(PhabricatorRepositoryRefCursor::TYPE_BRANCH))
         ->withRefNames(array($drequest->getBranch()))
+        ->needPositions(true)
         ->execute();
-      if ($ref_cursors) {
+
+      // It's possible that this branch previously existed, but has been
+      // deleted. Make sure we have valid cursor positions, not just cursors.
+      $any_positions = false;
+      foreach ($ref_cursors as $ref_cursor) {
+        if ($ref_cursor->getPositions()) {
+          $any_positions = true;
+          break;
+        }
+      }
+
+      if ($any_positions) {
         // This is a valid branch, so we necessarily have some content.
         $page_has_content = true;
       } else {

From 42034e6739eb2507839dda708963219c430c7e76 Mon Sep 17 00:00:00 2001
From: lkassianik <lyubakassianik@gmail.com>
Date: Fri, 1 Dec 2017 18:16:26 +0000
Subject: [PATCH 18/18] Property list view on Diffusion commits should show
 build status but not Subscriptions, Projects, or Tokens

Summary: Ref T13019, adds build status back to Diffusion commits

Test Plan: Open a Diffusion commit that has a build status, property list view should show the build status, but not Subscriptions, Projects, or Tokens.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Maniphest Tasks: T13019

Differential Revision: https://secure.phabricator.com/D18813
---
 .../diffusion/controller/DiffusionCommitController.php    | 3 ++-
 .../project/events/PhabricatorProjectUIEventListener.php  | 8 ++++++++
 .../events/PhabricatorSubscriptionsUIEventListener.php    | 8 ++++++++
 .../tokens/event/PhabricatorTokenUIEventListener.php      | 8 ++++++++
 4 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php
index d7734856f0..cdce8702ca 100644
--- a/src/applications/diffusion/controller/DiffusionCommitController.php
+++ b/src/applications/diffusion/controller/DiffusionCommitController.php
@@ -438,7 +438,8 @@ final class DiffusionCommitController extends DiffusionController {
     $repository = $drequest->getRepository();
 
     $view = id(new PHUIPropertyListView())
-      ->setUser($this->getRequest()->getUser());
+      ->setUser($this->getRequest()->getUser())
+      ->setObject($commit);
 
     $edge_query = id(new PhabricatorEdgeQuery())
       ->withSourcePHIDs(array($commit_phid))
diff --git a/src/applications/project/events/PhabricatorProjectUIEventListener.php b/src/applications/project/events/PhabricatorProjectUIEventListener.php
index afbc0aab4e..104084bbf7 100644
--- a/src/applications/project/events/PhabricatorProjectUIEventListener.php
+++ b/src/applications/project/events/PhabricatorProjectUIEventListener.php
@@ -8,8 +8,16 @@ final class PhabricatorProjectUIEventListener
   }
 
   public function handleEvent(PhutilEvent $event) {
+    $object = $event->getValue('object');
+
     switch ($event->getType()) {
       case PhabricatorEventType::TYPE_UI_WILLRENDERPROPERTIES:
+        // Hacky solution so that property list view on Diffusion
+        // commits shows build status, but not Projects, Subscriptions,
+        // or Tokens.
+        if ($object instanceof PhabricatorRepositoryCommit) {
+          return;
+        }
         $this->handlePropertyEvent($event);
         break;
     }
diff --git a/src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php b/src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php
index 09b26819c0..5f371d69f3 100644
--- a/src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php
+++ b/src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php
@@ -9,11 +9,19 @@ final class PhabricatorSubscriptionsUIEventListener
   }
 
   public function handleEvent(PhutilEvent $event) {
+    $object = $event->getValue('object');
+
     switch ($event->getType()) {
       case PhabricatorEventType::TYPE_UI_DIDRENDERACTIONS:
         $this->handleActionEvent($event);
         break;
       case PhabricatorEventType::TYPE_UI_WILLRENDERPROPERTIES:
+        // Hacky solution so that property list view on Diffusion
+        // commits shows build status, but not Projects, Subscriptions,
+        // or Tokens.
+        if ($object instanceof PhabricatorRepositoryCommit) {
+          return;
+        }
         $this->handlePropertyEvent($event);
         break;
     }
diff --git a/src/applications/tokens/event/PhabricatorTokenUIEventListener.php b/src/applications/tokens/event/PhabricatorTokenUIEventListener.php
index bbf3438b62..047c5504bf 100644
--- a/src/applications/tokens/event/PhabricatorTokenUIEventListener.php
+++ b/src/applications/tokens/event/PhabricatorTokenUIEventListener.php
@@ -9,11 +9,19 @@ final class PhabricatorTokenUIEventListener
   }
 
   public function handleEvent(PhutilEvent $event) {
+    $object = $event->getValue('object');
+
     switch ($event->getType()) {
       case PhabricatorEventType::TYPE_UI_DIDRENDERACTIONS:
         $this->handleActionEvent($event);
         break;
       case PhabricatorEventType::TYPE_UI_WILLRENDERPROPERTIES:
+        // Hacky solution so that property list view on Diffusion
+        // commits shows build status, but not Projects, Subscriptions,
+        // or Tokens.
+        if ($object instanceof PhabricatorRepositoryCommit) {
+          return;
+        }
         $this->handlePropertyEvent($event);
         break;
     }