From f16365ed0778e2513f28166b9e4a84c78780ccab Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sun, 8 Sep 2019 12:56:48 -0700
Subject: [PATCH 01/23] Weaken the guidance recommending that installs enable
 "STRICT_ALL_TABLES"

Summary: Ref T13404. Enabling "STRICT_ALL_TABLES" is good, but if you don't want to bother it doesn't matter too much. All upstream development has been on "STRICT_ALL_TABLES" for a long time.

Test Plan: {F6847839}

Maniphest Tasks: T13404

Differential Revision: https://secure.phabricator.com/D20790
---
 .../check/PhabricatorMySQLSetupCheck.php      | 57 ++++++++++++-------
 1 file changed, 37 insertions(+), 20 deletions(-)

diff --git a/src/applications/config/check/PhabricatorMySQLSetupCheck.php b/src/applications/config/check/PhabricatorMySQLSetupCheck.php
index a4048cbc33..5a0d50e425 100644
--- a/src/applications/config/check/PhabricatorMySQLSetupCheck.php
+++ b/src/applications/config/check/PhabricatorMySQLSetupCheck.php
@@ -50,30 +50,47 @@ final class PhabricatorMySQLSetupCheck extends PhabricatorSetupCheck {
     if (!in_array('STRICT_ALL_TABLES', $modes)) {
       $summary = pht(
         'MySQL is not in strict mode (on host "%s"), but using strict mode '.
-        'is strongly encouraged.',
+        'is recommended.',
         $host_name);
 
       $message = pht(
-        "On database host \"%s\", the global %s is not set to %s. ".
-        "It is strongly encouraged that you enable this mode when running ".
-        "Phabricator.\n\n".
-        "By default MySQL will silently ignore some types of errors, which ".
-        "can cause data loss and raise security concerns. Enabling strict ".
-        "mode makes MySQL raise an explicit error instead, and prevents this ".
-        "entire class of problems from doing any damage.\n\n".
-        "You can find more information about this mode (and how to configure ".
-        "it) in the MySQL manual. Usually, it is sufficient to add this to ".
-        "your %s file (in the %s section) and then restart %s:\n\n".
-        "%s\n".
-        "(Note that if you run other applications against the same database, ".
-        "they may not work in strict mode. Be careful about enabling it in ".
-        "these cases.)",
+        'On database host "%s", the global "sql_mode" setting does not '.
+        'include the "STRICT_ALL_TABLES" mode. Enabling this mode is '.
+        'recommended to generally improve how MySQL handles certain errors.'.
+        "\n\n".
+        'Without this mode enabled, MySQL will silently ignore some error '.
+        'conditions, including inserts which attempt to store more data in '.
+        'a column than actually fits. This behavior is usually undesirable '.
+        'and can lead to data corruption (by truncating multibyte characters '.
+        'in the middle), data loss (by discarding the data which does not '.
+        'fit into the column), or security concerns (for example, by '.
+        'truncating keys or credentials).'.
+        "\n\n".
+        'Phabricator is developed and tested in "STRICT_ALL_TABLES" mode so '.
+        'you should normally never encounter these situations, but may run '.
+        'into them if you interact with the database directly, run '.
+        'third-party code, develop extensions, or just encounter a bug in '.
+        'the software.'.
+        "\n\n".
+        'Enabling "STRICT_ALL_TABLES" makes MySQL raise an explicit error '.
+        'if one of these unusual situations does occur. This is a safer '.
+        'behavior and prevents these situations from causing secret, subtle, '.
+        'and potentially serious issues later on.'.
+        "\n\n".
+        'You can find more information about this mode (and how to configure '.
+        'it) in the MySQL manual. Usually, it is sufficient to add this to '.
+        'your "my.cnf" file (in the "[mysqld]" section) and then '.
+        'restart "mysqld":'.
+        "\n\n".
+        '%s'.
+        "\n".
+        'Note that if you run other applications against the same database, '.
+        'they may not work in strict mode.'.
+        "\n\n".
+        'If you can not or do not want to enable "STRICT_ALL_TABLES", you '.
+        'can safely ignore this warning. Phabricator will work correctly '.
+        'with this mode enabled or disabled.',
         $host_name,
-        phutil_tag('tt', array(), 'sql_mode'),
-        phutil_tag('tt', array(), 'STRICT_ALL_TABLES'),
-        phutil_tag('tt', array(), 'my.cnf'),
-        phutil_tag('tt', array(), '[mysqld]'),
-        phutil_tag('tt', array(), 'mysqld'),
         phutil_tag('pre', array(), 'sql_mode=STRICT_ALL_TABLES'));
 
       $this->newIssue('mysql.mode')

From 535c8e6bdc1e9db145c599e6c47a9c2370b5ad67 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sun, 8 Sep 2019 12:59:59 -0700
Subject: [PATCH 02/23] Remove the "ONLY_FULL_GROUP_BY" SQL mode setup warning
 and change the setup key for "STRICT_ALL_TABLES"

Summary:
Ref T13404. Except for one known issue in Multimeter, Phabricator appears to function properly in this mode. It is broadly desirable that we run in this mode; it's good on its own, and enabled by default in at least some recent MySQL.

Additionally, "ONLY_FULL_GROUP_BY" and "STRICT_ALL_TABLES" shared a setup key, so ignoring one would ignore both. Change the key so that existing ignores on "ONLY_FULL_GROUP_BY" do not mask "STRICT_ALL_TABLES" warnings.

Test Plan: Grepped for `ONLY_FULL_GROUP_BY`.

Maniphest Tasks: T13404

Differential Revision: https://secure.phabricator.com/D20791
---
 .../check/PhabricatorMySQLSetupCheck.php      | 45 +------------------
 1 file changed, 1 insertion(+), 44 deletions(-)

diff --git a/src/applications/config/check/PhabricatorMySQLSetupCheck.php b/src/applications/config/check/PhabricatorMySQLSetupCheck.php
index 5a0d50e425..de9a9e8b54 100644
--- a/src/applications/config/check/PhabricatorMySQLSetupCheck.php
+++ b/src/applications/config/check/PhabricatorMySQLSetupCheck.php
@@ -93,7 +93,7 @@ final class PhabricatorMySQLSetupCheck extends PhabricatorSetupCheck {
         $host_name,
         phutil_tag('pre', array(), 'sql_mode=STRICT_ALL_TABLES'));
 
-      $this->newIssue('mysql.mode')
+      $this->newIssue('sql_mode.strict')
         ->setName(pht('MySQL %s Mode Not Set', 'STRICT_ALL_TABLES'))
         ->setSummary($summary)
         ->setMessage($message)
@@ -101,49 +101,6 @@ final class PhabricatorMySQLSetupCheck extends PhabricatorSetupCheck {
         ->addMySQLConfig('sql_mode');
     }
 
-    if (in_array('ONLY_FULL_GROUP_BY', $modes)) {
-      $summary = pht(
-        'MySQL is in ONLY_FULL_GROUP_BY mode (on host "%s"), but using this '.
-        'mode is strongly discouraged.',
-        $host_name);
-
-      $message = pht(
-        "On database host \"%s\", the global %s is set to %s. ".
-        "It is strongly encouraged that you disable this mode when running ".
-        "Phabricator.\n\n".
-        "With %s enabled, MySQL rejects queries for which the select list ".
-        "or (as of MySQL 5.0.23) %s list refer to nonaggregated columns ".
-        "that are not named in the %s clause. More importantly, Phabricator ".
-        "does not work properly with this mode enabled.\n\n".
-        "You can find more information about this mode (and how to configure ".
-        "it) in the MySQL manual. Usually, it is sufficient to change the %s ".
-        "in your %s file (in the %s section) and then restart %s:\n\n".
-        "%s\n".
-        "(Note that if you run other applications against the same database, ".
-        "they may not work with %s. Be careful about enabling ".
-        "it in these cases and consider migrating Phabricator to a different ".
-        "database.)",
-        $host_name,
-        phutil_tag('tt', array(), 'sql_mode'),
-        phutil_tag('tt', array(), 'ONLY_FULL_GROUP_BY'),
-        phutil_tag('tt', array(), 'ONLY_FULL_GROUP_BY'),
-        phutil_tag('tt', array(), 'HAVING'),
-        phutil_tag('tt', array(), 'GROUP BY'),
-        phutil_tag('tt', array(), 'sql_mode'),
-        phutil_tag('tt', array(), 'my.cnf'),
-        phutil_tag('tt', array(), '[mysqld]'),
-        phutil_tag('tt', array(), 'mysqld'),
-        phutil_tag('pre', array(), 'sql_mode=STRICT_ALL_TABLES'),
-        phutil_tag('tt', array(), 'ONLY_FULL_GROUP_BY'));
-
-      $this->newIssue('mysql.mode')
-        ->setName(pht('MySQL %s Mode Set', 'ONLY_FULL_GROUP_BY'))
-        ->setSummary($summary)
-        ->setMessage($message)
-        ->setDatabaseRef($ref)
-        ->addMySQLConfig('sql_mode');
-    }
-
     $is_innodb_fulltext = false;
     $is_myisam_fulltext = false;
     if ($this->shouldUseMySQLSearchEngine()) {

From 63c7302af1309eb7f3874166042953491a092e30 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 9 Sep 2019 12:10:21 -0700
Subject: [PATCH 03/23] Fix global search scope fatal on 404 page (or other
 pages with no Application)

Summary: Ref T13405. Some pages don't have a contextual application.

Test Plan: Viewed 404 page, no more fatal.

Maniphest Tasks: T13405

Differential Revision: https://secure.phabricator.com/D20792
---
 src/view/page/menu/PhabricatorMainMenuSearchView.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/view/page/menu/PhabricatorMainMenuSearchView.php b/src/view/page/menu/PhabricatorMainMenuSearchView.php
index aaa7c5a160..60e7233e96 100644
--- a/src/view/page/menu/PhabricatorMainMenuSearchView.php
+++ b/src/view/page/menu/PhabricatorMainMenuSearchView.php
@@ -118,7 +118,7 @@ final class PhabricatorMainMenuSearchView extends AphrontView {
 
   public static function getGlobalSearchScopeItems(
     PhabricatorUser $viewer,
-    PhabricatorApplication $application) {
+    PhabricatorApplication $application = null) {
 
     $items = array();
     $items[] = array(

From 278092974f2f4115cc97172e099ec7fe3941eeb1 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 9 Sep 2019 12:15:00 -0700
Subject: [PATCH 04/23] Don't offer personal saved queries in global "Search
 Scope" settings dropdown

Summary: Fixes T13405. We currently offer non-global custom saved queries here, but this doesn't make sense as a global default setting.

Test Plan: Saved a global search query, edited global search settings, no longer saw the non-global query as an option.

Maniphest Tasks: T13405

Differential Revision: https://secure.phabricator.com/D20793
---
 .../setting/PhabricatorSearchScopeSetting.php   |  3 ++-
 .../page/menu/PhabricatorMainMenuSearchView.php | 17 ++++++++++++++---
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/src/applications/settings/setting/PhabricatorSearchScopeSetting.php b/src/applications/settings/setting/PhabricatorSearchScopeSetting.php
index 54fa12a84c..c4b1941540 100644
--- a/src/applications/settings/setting/PhabricatorSearchScopeSetting.php
+++ b/src/applications/settings/setting/PhabricatorSearchScopeSetting.php
@@ -25,7 +25,8 @@ final class PhabricatorSearchScopeSetting
   protected function getSelectOptions() {
     $scopes = PhabricatorMainMenuSearchView::getGlobalSearchScopeItems(
       $this->getViewer(),
-      new PhabricatorSettingsApplication());
+      new PhabricatorSettingsApplication(),
+      $only_global = true);
 
     $scope_map = array();
     foreach ($scopes as $scope) {
diff --git a/src/view/page/menu/PhabricatorMainMenuSearchView.php b/src/view/page/menu/PhabricatorMainMenuSearchView.php
index 60e7233e96..e038eec6c8 100644
--- a/src/view/page/menu/PhabricatorMainMenuSearchView.php
+++ b/src/view/page/menu/PhabricatorMainMenuSearchView.php
@@ -118,7 +118,8 @@ final class PhabricatorMainMenuSearchView extends AphrontView {
 
   public static function getGlobalSearchScopeItems(
     PhabricatorUser $viewer,
-    PhabricatorApplication $application = null) {
+    PhabricatorApplication $application = null,
+    $global_only = false) {
 
     $items = array();
     $items[] = array(
@@ -154,14 +155,24 @@ final class PhabricatorMainMenuSearchView extends AphrontView {
     $engine = id(new PhabricatorSearchApplicationSearchEngine())
       ->setViewer($viewer);
     $engine_queries = $engine->loadEnabledNamedQueries();
-    $query_map = mpull($engine_queries, 'getQueryName', 'getQueryKey');
-    foreach ($query_map as $query_key => $query_name) {
+    foreach ($engine_queries as $query) {
+      $query_key = $query->getQueryKey();
       if ($query_key == 'all') {
         // Skip the builtin "All" query since it's redundant with the default
         // setting.
         continue;
       }
 
+      // In the global "Settings" panel, we don't want to offer personal
+      // queries the viewer may have saved.
+      if ($global_only) {
+        if (!$query->isGlobal()) {
+          continue;
+        }
+      }
+
+      $query_name = $query->getQueryName();
+
       $items[] = array(
         'icon' => 'fa-certificate',
         'name' => $query_name,

From aaaea5759133c1690733e4cecbe0ef9351b47e4e Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 9 Sep 2019 12:28:17 -0700
Subject: [PATCH 05/23] Fix fatal during redirection safety check for searching
 for Phabricator base-uri with no trailing slash

Summary: Fixes T13412. If you search for "https://phabricator.example.com" with no trailing slash, we currently redirect you to "", which is fouled by a safety check in the redirection flow.

Test Plan:
  - Searched for "https://local.phacility.com"; before: fatal in redirection; after: clean redirect.
  - Searched for other self-URIs, got normal redirects.

Maniphest Tasks: T13412

Differential Revision: https://secure.phabricator.com/D20794
---
 .../PhabricatorDatasourceURIEngineExtension.php      | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/applications/meta/engineextension/PhabricatorDatasourceURIEngineExtension.php b/src/applications/meta/engineextension/PhabricatorDatasourceURIEngineExtension.php
index 147aa9635f..16d8dd9315 100644
--- a/src/applications/meta/engineextension/PhabricatorDatasourceURIEngineExtension.php
+++ b/src/applications/meta/engineextension/PhabricatorDatasourceURIEngineExtension.php
@@ -26,7 +26,17 @@ final class PhabricatorDatasourceURIEngineExtension
         ->setProtocol(null)
         ->setPort(null);
 
-      return phutil_string_cast($uri);
+      $uri = phutil_string_cast($uri);
+
+      // See T13412. If the URI was in the form "http://dev.example.com" with
+      // no trailing slash, there may be no path. Redirecting to the empty
+      // string is considered an error by safety checks during redirection,
+      // so treat this like the user entered the URI with a trailing slash.
+      if (!strlen($uri)) {
+        $uri = '/';
+      }
+
+      return $uri;
     }
 
     return null;

From d965d9a669b58ed8968cda51d1467d3aeb570fc9 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 9 Sep 2019 12:38:36 -0700
Subject: [PATCH 06/23] Index Herald fields, not just actions, when identifying
 objects related to a particular Herald rule

Summary:
Fixes T13408. Currently, when a package (or other object) appears in a field (rather than an action), it is not indexed.

Instead: index fields too, not just actions.

Test Plan:
  - Wrote a rule like "[ Affected packages include ] ...".
  - Updated the search index.
  - Saw rule appear on "Affected By Herald Rules" on the package detail page.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13408

Differential Revision: https://secure.phabricator.com/D20795
---
 .../20190909.herald.01.rebuild.php            |  3 +++
 .../HeraldRuleIndexEngineExtension.php        | 14 ++++++++++++
 src/applications/herald/field/HeraldField.php | 22 +++++++++++++++++++
 3 files changed, 39 insertions(+)
 create mode 100644 resources/sql/autopatches/20190909.herald.01.rebuild.php

diff --git a/resources/sql/autopatches/20190909.herald.01.rebuild.php b/resources/sql/autopatches/20190909.herald.01.rebuild.php
new file mode 100644
index 0000000000..a29b7d2f45
--- /dev/null
+++ b/resources/sql/autopatches/20190909.herald.01.rebuild.php
@@ -0,0 +1,3 @@
+<?php
+
+PhabricatorRebuildIndexesWorker::rebuildObjectsWithQuery('HeraldRuleQuery');
diff --git a/src/applications/herald/engineextension/HeraldRuleIndexEngineExtension.php b/src/applications/herald/engineextension/HeraldRuleIndexEngineExtension.php
index 5df229ccd6..ca28116d4c 100644
--- a/src/applications/herald/engineextension/HeraldRuleIndexEngineExtension.php
+++ b/src/applications/herald/engineextension/HeraldRuleIndexEngineExtension.php
@@ -37,6 +37,20 @@ final class HeraldRuleIndexEngineExtension
 
     $phids = array();
 
+    $fields = HeraldField::getAllFields();
+    foreach ($rule->getConditions() as $condition_record) {
+      $field = idx($fields, $condition_record->getFieldName());
+
+      if (!$field) {
+        continue;
+      }
+
+      $affected_phids = $field->getPHIDsAffectedByCondition($condition_record);
+      foreach ($affected_phids as $phid) {
+        $phids[] = $phid;
+      }
+    }
+
     $actions = HeraldAction::getAllActions();
     foreach ($rule->getActions() as $action_record) {
       $action = idx($actions, $action_record->getAction());
diff --git a/src/applications/herald/field/HeraldField.php b/src/applications/herald/field/HeraldField.php
index 611ea2cdb3..3df242712e 100644
--- a/src/applications/herald/field/HeraldField.php
+++ b/src/applications/herald/field/HeraldField.php
@@ -176,6 +176,28 @@ abstract class HeraldField extends Phobject {
     return $value_type->renderEditorValue($value);
   }
 
+  public function getPHIDsAffectedByCondition(HeraldCondition $condition) {
+    $phids = array();
+
+    $standard_type = $this->getHeraldFieldStandardType();
+    switch ($standard_type) {
+      case self::STANDARD_PHID:
+      case self::STANDARD_PHID_NULLABLE:
+        $phid = $condition->getValue();
+        if ($phid) {
+          $phids[] = $phid;
+        }
+        break;
+      case self::STANDARD_PHID_LIST:
+        foreach ($condition->getValue() as $phid) {
+          $phids[] = $phid;
+        }
+        break;
+    }
+
+    return $phids;
+  }
+
   final public function setAdapter(HeraldAdapter $adapter) {
     $this->adapter = $adapter;
     return $this;

From 454771446306f213a3ee68f3d89c8ad1e5e6bbf6 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 9 Sep 2019 13:00:38 -0700
Subject: [PATCH 07/23] Add a "Remove flag" action to Herald

Summary: Fixes T13409. This is a companion to the existing "Mark with flag" rule.

Test Plan: Used a "remove flag" rule on an object with no flag (not removed), the right type of flag (removed), and a different type of flag (not removed).

Maniphest Tasks: T13409

Differential Revision: https://secure.phabricator.com/D20796
---
 src/__phutil_library_map__.php                |  6 +-
 .../PhabricatorFlagAddFlagHeraldAction.php    | 15 +---
 .../herald/PhabricatorFlagHeraldAction.php    | 18 +++++
 .../PhabricatorFlagRemoveFlagHeraldAction.php | 79 +++++++++++++++++++
 4 files changed, 104 insertions(+), 14 deletions(-)
 create mode 100644 src/applications/flag/herald/PhabricatorFlagHeraldAction.php
 create mode 100644 src/applications/flag/herald/PhabricatorFlagRemoveFlagHeraldAction.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index e6e9063453..fad919f4dd 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -3438,8 +3438,10 @@ phutil_register_library_map(array(
     'PhabricatorFlagDeleteController' => 'applications/flag/controller/PhabricatorFlagDeleteController.php',
     'PhabricatorFlagDestructionEngineExtension' => 'applications/flag/engineextension/PhabricatorFlagDestructionEngineExtension.php',
     'PhabricatorFlagEditController' => 'applications/flag/controller/PhabricatorFlagEditController.php',
+    'PhabricatorFlagHeraldAction' => 'applications/flag/herald/PhabricatorFlagHeraldAction.php',
     'PhabricatorFlagListController' => 'applications/flag/controller/PhabricatorFlagListController.php',
     'PhabricatorFlagQuery' => 'applications/flag/query/PhabricatorFlagQuery.php',
+    'PhabricatorFlagRemoveFlagHeraldAction' => 'applications/flag/herald/PhabricatorFlagRemoveFlagHeraldAction.php',
     'PhabricatorFlagSearchEngine' => 'applications/flag/query/PhabricatorFlagSearchEngine.php',
     'PhabricatorFlagSelectControl' => 'applications/flag/view/PhabricatorFlagSelectControl.php',
     'PhabricatorFlaggableInterface' => 'applications/flag/interface/PhabricatorFlaggableInterface.php',
@@ -9799,7 +9801,7 @@ phutil_register_library_map(array(
       'PhabricatorFlagDAO',
       'PhabricatorPolicyInterface',
     ),
-    'PhabricatorFlagAddFlagHeraldAction' => 'HeraldAction',
+    'PhabricatorFlagAddFlagHeraldAction' => 'PhabricatorFlagHeraldAction',
     'PhabricatorFlagColor' => 'PhabricatorFlagConstants',
     'PhabricatorFlagConstants' => 'Phobject',
     'PhabricatorFlagController' => 'PhabricatorController',
@@ -9807,8 +9809,10 @@ phutil_register_library_map(array(
     'PhabricatorFlagDeleteController' => 'PhabricatorFlagController',
     'PhabricatorFlagDestructionEngineExtension' => 'PhabricatorDestructionEngineExtension',
     'PhabricatorFlagEditController' => 'PhabricatorFlagController',
+    'PhabricatorFlagHeraldAction' => 'HeraldAction',
     'PhabricatorFlagListController' => 'PhabricatorFlagController',
     'PhabricatorFlagQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
+    'PhabricatorFlagRemoveFlagHeraldAction' => 'PhabricatorFlagHeraldAction',
     'PhabricatorFlagSearchEngine' => 'PhabricatorApplicationSearchEngine',
     'PhabricatorFlagSelectControl' => 'AphrontFormControl',
     'PhabricatorFlaggableInterface' => 'PhabricatorPHIDInterface',
diff --git a/src/applications/flag/herald/PhabricatorFlagAddFlagHeraldAction.php b/src/applications/flag/herald/PhabricatorFlagAddFlagHeraldAction.php
index e4ae03915f..57772ecb5f 100644
--- a/src/applications/flag/herald/PhabricatorFlagAddFlagHeraldAction.php
+++ b/src/applications/flag/herald/PhabricatorFlagAddFlagHeraldAction.php
@@ -1,6 +1,7 @@
 <?php
 
-final class PhabricatorFlagAddFlagHeraldAction extends HeraldAction {
+final class PhabricatorFlagAddFlagHeraldAction
+  extends PhabricatorFlagHeraldAction {
 
   const ACTIONCONST = 'flag';
 
@@ -11,18 +12,6 @@ final class PhabricatorFlagAddFlagHeraldAction extends HeraldAction {
     return pht('Mark with flag');
   }
 
-  public function getActionGroupKey() {
-    return HeraldSupportActionGroup::ACTIONGROUPKEY;
-  }
-
-  public function supportsObject($object) {
-    return ($object instanceof PhabricatorFlaggableInterface);
-  }
-
-  public function supportsRuleType($rule_type) {
-    return ($rule_type == HeraldRuleTypeConfig::RULE_TYPE_PERSONAL);
-  }
-
   public function applyEffect($object, HeraldEffect $effect) {
     $phid = $this->getAdapter()->getPHID();
     $rule = $effect->getRule();
diff --git a/src/applications/flag/herald/PhabricatorFlagHeraldAction.php b/src/applications/flag/herald/PhabricatorFlagHeraldAction.php
new file mode 100644
index 0000000000..6f87a4ee6c
--- /dev/null
+++ b/src/applications/flag/herald/PhabricatorFlagHeraldAction.php
@@ -0,0 +1,18 @@
+<?php
+
+abstract class PhabricatorFlagHeraldAction
+  extends HeraldAction {
+
+  public function getActionGroupKey() {
+    return HeraldSupportActionGroup::ACTIONGROUPKEY;
+  }
+
+  public function supportsObject($object) {
+    return ($object instanceof PhabricatorFlaggableInterface);
+  }
+
+  public function supportsRuleType($rule_type) {
+    return ($rule_type === HeraldRuleTypeConfig::RULE_TYPE_PERSONAL);
+  }
+
+}
diff --git a/src/applications/flag/herald/PhabricatorFlagRemoveFlagHeraldAction.php b/src/applications/flag/herald/PhabricatorFlagRemoveFlagHeraldAction.php
new file mode 100644
index 0000000000..93d5b02e2f
--- /dev/null
+++ b/src/applications/flag/herald/PhabricatorFlagRemoveFlagHeraldAction.php
@@ -0,0 +1,79 @@
+<?php
+
+final class PhabricatorFlagRemoveFlagHeraldAction
+  extends PhabricatorFlagHeraldAction {
+
+  const ACTIONCONST = 'unflag';
+
+  const DO_UNFLAG = 'do.unflag';
+  const DO_IGNORE_UNFLAG = 'do.ignore-unflag';
+
+  public function getHeraldActionName() {
+    return pht('Remove flag');
+  }
+
+  public function applyEffect($object, HeraldEffect $effect) {
+    $phid = $this->getAdapter()->getPHID();
+    $rule = $effect->getRule();
+    $author = $rule->getAuthor();
+
+    $flag = PhabricatorFlagQuery::loadUserFlag($author, $phid);
+    if (!$flag) {
+      $this->logEffect(self::DO_IGNORE_UNFLAG, null);
+      return;
+    }
+
+    if ($flag->getColor() !== $effect->getTarget()) {
+      $this->logEffect(self::DO_IGNORE_UNFLAG, $flag->getColor());
+      return;
+    }
+
+    $flag->delete();
+
+    $this->logEffect(self::DO_UNFLAG, $flag->getColor());
+  }
+
+  public function getHeraldActionValueType() {
+    return id(new HeraldSelectFieldValue())
+      ->setKey('flag.color')
+      ->setOptions(PhabricatorFlagColor::getColorNameMap())
+      ->setDefault(PhabricatorFlagColor::COLOR_BLUE);
+  }
+
+  protected function getActionEffectMap() {
+    return array(
+      self::DO_IGNORE_UNFLAG => array(
+        'icon' => 'fa-times',
+        'color' => 'grey',
+        'name' => pht('Did Not Remove Flag'),
+      ),
+      self::DO_UNFLAG => array(
+        'icon' => 'fa-flag',
+        'name' => pht('Removed Flag'),
+      ),
+    );
+  }
+
+  public function renderActionDescription($value) {
+    $color = PhabricatorFlagColor::getColorName($value);
+    return pht('Remove %s flag.', $color);
+  }
+
+  protected function renderActionEffectDescription($type, $data) {
+    switch ($type) {
+      case self::DO_IGNORE_UNFLAG:
+        if (!$data) {
+          return pht('Not marked with any flag.');
+        } else {
+          return pht(
+            'Marked with flag of the wrong color ("%s").',
+            PhabricatorFlagColor::getColorName($data));
+        }
+      case self::DO_UNFLAG:
+        return pht(
+          'Removed "%s" flag.',
+          PhabricatorFlagColor::getColorName($data));
+    }
+  }
+
+}

From 7593a265d5931877d38756282ddd66daf6ec99b8 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 9 Sep 2019 13:09:46 -0700
Subject: [PATCH 08/23] When Herald changes object subscribers, always hide the
 feed story

Summary:
Fixes T8952. These feed stories are not interesting and tend to be generated as collateral damage when a non-story update is made to an old task and someone has a "subscribe me" Herald rule.

Also clean up some of the Herald field/condition indexing behavior slightly.

Test Plan: Wrote a "Subscribe X" herald rule, made a trivial update to a task. Before: low-value feed story; after: no feed story.

Maniphest Tasks: T8952

Differential Revision: https://secure.phabricator.com/D20797
---
 src/applications/herald/field/HeraldField.php | 23 ++++++++++---------
 .../PhabricatorApplicationTransaction.php     | 13 ++++++-----
 2 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/src/applications/herald/field/HeraldField.php b/src/applications/herald/field/HeraldField.php
index 3df242712e..6f14091aff 100644
--- a/src/applications/herald/field/HeraldField.php
+++ b/src/applications/herald/field/HeraldField.php
@@ -177,25 +177,26 @@ abstract class HeraldField extends Phobject {
   }
 
   public function getPHIDsAffectedByCondition(HeraldCondition $condition) {
-    $phids = array();
+    try {
+      $standard_type = $this->getHeraldFieldStandardType();
+    } catch (PhutilMethodNotImplementedException $ex) {
+      $standard_type = null;
+    }
 
-    $standard_type = $this->getHeraldFieldStandardType();
     switch ($standard_type) {
       case self::STANDARD_PHID:
       case self::STANDARD_PHID_NULLABLE:
-        $phid = $condition->getValue();
-        if ($phid) {
-          $phids[] = $phid;
-        }
-        break;
       case self::STANDARD_PHID_LIST:
-        foreach ($condition->getValue() as $phid) {
-          $phids[] = $phid;
+        $phids = $condition->getValue();
+
+        if (!is_array($phids)) {
+          $phids = array();
         }
-        break;
+
+        return $phids;
     }
 
-    return $phids;
+    return array();
   }
 
   final public function setAdapter(HeraldAdapter $adapter) {
diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
index 7c327393f8..5fa4562164 100644
--- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
+++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
@@ -775,6 +775,13 @@ abstract class PhabricatorApplicationTransaction
       case PhabricatorTransactions::TYPE_TOKEN:
       case PhabricatorTransactions::TYPE_MFA:
         return true;
+      case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+        // See T8952. When an application (usually Herald) modifies
+        // subscribers, this tends to be very uninteresting.
+        if ($this->isApplicationAuthor()) {
+          return true;
+        }
+        break;
       case PhabricatorTransactions::TYPE_EDGE:
         $edge_type = $this->getMetadataValue('edge:type');
         switch ($edge_type) {
@@ -1387,12 +1394,6 @@ abstract class PhabricatorApplicationTransaction
           return 25;
         }
 
-        if ($this->isApplicationAuthor()) {
-          // When applications (most often: Herald) change subscriptions it
-          // is very uninteresting.
-          return 1;
-        }
-
         // In other cases, subscriptions are more interesting than comments
         // (which are shown anyway) but less interesting than any other type of
         // transaction.

From 1d1a60fdda88b6b2ebea09e26fa788b89a9a0c18 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 9 Sep 2019 13:24:27 -0700
Subject: [PATCH 09/23] Improve rendering of Herald rules in "Another Herald
 rule..." field

Summary:
Fixes T9136.

  - Fix a bug where the name is rendered improperly.
  - Put disabled rules at the bottom.
  - Always show the rule monogram so you can distingiush between rules with the same name.

Test Plan: {F6849915}

Maniphest Tasks: T9136

Differential Revision: https://secure.phabricator.com/D20798
---
 .../herald/controller/HeraldRuleController.php   | 13 ++-----------
 src/applications/herald/storage/HeraldRule.php   | 16 ++++++++++++++++
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/src/applications/herald/controller/HeraldRuleController.php b/src/applications/herald/controller/HeraldRuleController.php
index d05ed2d525..73c1e8c39c 100644
--- a/src/applications/herald/controller/HeraldRuleController.php
+++ b/src/applications/herald/controller/HeraldRuleController.php
@@ -404,8 +404,8 @@ final class HeraldRuleController extends HeraldController {
     HeraldAdapter $adapter) {
 
     $all_rules = $this->loadRulesThisRuleMayDependUpon($rule);
-    $all_rules = mpull($all_rules, 'getName', 'getPHID');
-    asort($all_rules);
+    $all_rules = msortv($all_rules, 'getEditorSortVector');
+    $all_rules = mpull($all_rules, 'getEditorDisplayName', 'getPHID');
 
     $all_fields = $adapter->getFieldNameMap();
     $all_conditions = $adapter->getConditionNameMap();
@@ -674,15 +674,6 @@ final class HeraldRuleController extends HeraldController {
         ->execute();
     }
 
-    // mark disabled rules as disabled since they are not useful as such;
-    // don't filter though to keep edit cases sane / expected
-    foreach ($all_rules as $current_rule) {
-      if ($current_rule->getIsDisabled()) {
-        $current_rule->makeEphemeral();
-        $current_rule->setName($rule->getName().' '.pht('(Disabled)'));
-      }
-    }
-
     // A rule can not depend upon itself.
     unset($all_rules[$rule->getID()]);
 
diff --git a/src/applications/herald/storage/HeraldRule.php b/src/applications/herald/storage/HeraldRule.php
index a9c131e717..07be991bda 100644
--- a/src/applications/herald/storage/HeraldRule.php
+++ b/src/applications/herald/storage/HeraldRule.php
@@ -259,6 +259,22 @@ final class HeraldRule extends HeraldDAO
     return '/'.$this->getMonogram();
   }
 
+  public function getEditorSortVector() {
+    return id(new PhutilSortVector())
+      ->addInt($this->getIsDisabled() ? 1 : 0)
+      ->addString($this->getName());
+  }
+
+  public function getEditorDisplayName() {
+    $name = pht('%s %s', $this->getMonogram(), $this->getName());
+
+    if ($this->getIsDisabled()) {
+      $name = pht('%s (Disabled)', $name);
+    }
+
+    return $name;
+  }
+
 
 /* -(  Repetition Policies  )------------------------------------------------ */
 

From d2e1c4163a3b4695d67d91b292c3e1ac7ec5e4d3 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 9 Sep 2019 13:33:53 -0700
Subject: [PATCH 10/23] When a project has a custom icon, use that icon to
 label the project policy in the policy dropown

Summary:
Fixes T8808. Currently, all project use the default ("Briefcase") project icon when they appear in a policy dropdown.

Since project policies are separated out into a "Members of Projects" section of the dropdown anyway, there is no reason not to use the actual project icon, which is often more clear.

Test Plan: {F6849927}

Maniphest Tasks: T8808

Differential Revision: https://secure.phabricator.com/D20799
---
 src/applications/policy/storage/PhabricatorPolicy.php | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/applications/policy/storage/PhabricatorPolicy.php b/src/applications/policy/storage/PhabricatorPolicy.php
index 2df8fdf6a0..3d6b4ca609 100644
--- a/src/applications/policy/storage/PhabricatorPolicy.php
+++ b/src/applications/policy/storage/PhabricatorPolicy.php
@@ -85,8 +85,10 @@ final class PhabricatorPolicy
     $phid_type = phid_get_type($policy_identifier);
     switch ($phid_type) {
       case PhabricatorProjectProjectPHIDType::TYPECONST:
-        $policy->setType(PhabricatorPolicyType::TYPE_PROJECT);
-        $policy->setName($handle->getName());
+        $policy
+          ->setType(PhabricatorPolicyType::TYPE_PROJECT)
+          ->setName($handle->getName())
+          ->setIcon($handle->getIcon());
         break;
       case PhabricatorPeopleUserPHIDType::TYPECONST:
         $policy->setType(PhabricatorPolicyType::TYPE_USER);

From a0ade503e1663d2cb60ce433833843952305ef61 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 11 Sep 2019 21:01:56 -0700
Subject: [PATCH 11/23] Remove "Moved Document from ..." notice in Phriction

Summary:
Ref T13410. See PHI1431. Currently, when you move a document in Phriction, the target shows a "This document was moved from ..." banner until it is edited.

This banner isn't particularly useful, and it's distracting and it isn't obvious how to dismiss it, and making a trivial edit to dismiss it is awkward.

This information is also already available in the transaction log.

Just remove this banner since it doesn't really serve any clear purpose.

Test Plan:
  - Moved a page in Phriction, then loaded the destination page. Before change: header banner. After change: nothing.
  - Viewed a normal (non-moved) page, saw normal behavior.
  - Reviewed transactions, saw "Moved from ..." in the timeline.

Maniphest Tasks: T13410

Differential Revision: https://secure.phabricator.com/D20800
---
 .../PhrictionDocumentController.php           | 31 -------------------
 .../PhrictionDocumentMoveToTransaction.php    |  2 +-
 2 files changed, 1 insertion(+), 32 deletions(-)

diff --git a/src/applications/phriction/controller/PhrictionDocumentController.php b/src/applications/phriction/controller/PhrictionDocumentController.php
index bf7282b35a..5d30b5a90b 100644
--- a/src/applications/phriction/controller/PhrictionDocumentController.php
+++ b/src/applications/phriction/controller/PhrictionDocumentController.php
@@ -293,37 +293,6 @@ final class PhrictionDocumentController
       } else {
         throw new Exception(pht("Unknown document status '%s'!", $doc_status));
       }
-
-      $move_notice = null;
-      if ($current_status == PhrictionChangeType::CHANGE_MOVE_HERE) {
-        $from_doc_id = $content->getChangeRef();
-
-        $slug_uri = null;
-
-        // If the old document exists and is visible, provide a link to it.
-        $from_docs = id(new PhrictionDocumentQuery())
-          ->setViewer($viewer)
-          ->withIDs(array($from_doc_id))
-          ->execute();
-        if ($from_docs) {
-          $from_doc = head($from_docs);
-          $slug_uri = PhrictionDocument::getSlugURI($from_doc->getSlug());
-        }
-
-        $move_notice = id(new PHUIInfoView())
-          ->setSeverity(PHUIInfoView::SEVERITY_NOTICE);
-
-        if ($slug_uri) {
-          $move_notice->appendChild(
-            pht(
-              'This document was moved from %s.',
-              phutil_tag('a', array('href' => $slug_uri), $slug_uri)));
-        } else {
-          // Render this for consistency, even though it's a bit silly.
-          $move_notice->appendChild(
-            pht('This document was moved from elsewhere.'));
-        }
-      }
     }
 
     $children = $this->renderDocumentChildren($slug);
diff --git a/src/applications/phriction/xaction/PhrictionDocumentMoveToTransaction.php b/src/applications/phriction/xaction/PhrictionDocumentMoveToTransaction.php
index d18c436cd0..078e9a0130 100644
--- a/src/applications/phriction/xaction/PhrictionDocumentMoveToTransaction.php
+++ b/src/applications/phriction/xaction/PhrictionDocumentMoveToTransaction.php
@@ -49,7 +49,7 @@ final class PhrictionDocumentMoveToTransaction
     $new = $this->getNewValue();
 
     return pht(
-      '%s moved this document from %s',
+      '%s moved this document from %s.',
       $this->renderAuthor(),
       $this->renderHandle($new['phid']));
   }

From a35d7c3c218a4f053ad35849a972eee513161dc0 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 11 Sep 2019 21:50:25 -0700
Subject: [PATCH 12/23] Update rendering of policy edit transactions in
 Applications

Summary:
Ref T13411. Since circa D19829, transactions have rendered policy changes in a modern way, notably making "Custom Policy" clickable to show the policy rules.

Edit transactions in Applications still use a separate, older approach to render policies. This produces policy renderings which don't use modern quoting rules and don't link in a modern way.

Make Applications use the same rendering code that other transactions (like normal edit/view edits) use.

Test Plan: Edited policies in Applications, saw more useful transactions in the log. Clicked "Custom Policy" in the transaction log and got a useful explanation of the policy.

Maniphest Tasks: T13411

Differential Revision: https://secure.phabricator.com/D20801
---
 ...atorApplicationPolicyChangeTransaction.php | 46 ++++---------------
 ...rApplicationTransactionValueController.php |  1 +
 2 files changed, 9 insertions(+), 38 deletions(-)

diff --git a/src/applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php b/src/applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php
index 5364a3d4fa..3c58b42ab5 100644
--- a/src/applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php
+++ b/src/applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php
@@ -52,15 +52,12 @@ final class PhabricatorApplicationPolicyChangeTransaction
   }
 
   public function getTitle() {
-    $old = $this->renderApplicationPolicy($this->getOldValue());
-    $new = $this->renderApplicationPolicy($this->getNewValue());
-
     return pht(
-      '%s changed the "%s" policy from "%s" to "%s".',
+      '%s changed the %s policy from %s to %s.',
       $this->renderAuthor(),
       $this->renderCapability(),
-      $old,
-      $new);
+      $this->renderOldPolicy(),
+      $this->renderNewPolicy());
   }
 
   public function getTitleForFeed() {
@@ -68,12 +65,12 @@ final class PhabricatorApplicationPolicyChangeTransaction
     $new = $this->renderApplicationPolicy($this->getNewValue());
 
     return pht(
-      '%s changed the "%s" policy for application %s from "%s" to "%s".',
+      '%s changed the %s policy for application %s from %s to %s.',
       $this->renderAuthor(),
       $this->renderCapability(),
       $this->renderObject(),
-      $old,
-      $new);
+      $this->renderOldPolicy(),
+      $this->renderNewPolicy());
   }
 
   public function validateTransactions($object, array $xactions) {
@@ -165,38 +162,11 @@ final class PhabricatorApplicationPolicyChangeTransaction
     return $errors;
   }
 
-  private function renderApplicationPolicy($name) {
-    $policies = $this->getAllPolicies();
-    if (empty($policies[$name])) {
-      // Not a standard policy, check for a custom policy.
-      $policy = id(new PhabricatorPolicyQuery())
-        ->setViewer($this->getViewer())
-        ->withPHIDs(array($name))
-        ->executeOne();
-      $policies[$name] = $policy;
-    }
-
-    $policy = idx($policies, $name);
-    return $this->renderValue($policy->getFullName());
-  }
-
-  private function getAllPolicies() {
-    if (!$this->policies) {
-      $viewer = $this->getViewer();
-      $application = $this->getObject();
-      $this->policies = id(new PhabricatorPolicyQuery())
-        ->setViewer($viewer)
-        ->setObject($application)
-        ->execute();
-    }
-
-    return $this->policies;
-  }
-
   private function renderCapability() {
     $application = $this->getObject();
     $capability = $this->getCapabilityName();
-    return $application->getCapabilityLabel($capability);
+    $label = $application->getCapabilityLabel($capability);
+    return $this->renderValue($label);
   }
 
   private function getCapabilityName() {
diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php
index bef6fef5a8..9c82ff99c8 100644
--- a/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php
+++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php
@@ -33,6 +33,7 @@ final class PhabricatorApplicationTransactionValueController
       case PhabricatorTransactions::TYPE_EDIT_POLICY:
       case PhabricatorTransactions::TYPE_JOIN_POLICY:
       case PhabricatorRepositoryPushPolicyTransaction::TRANSACTIONTYPE:
+      case PhabricatorApplicationPolicyChangeTransaction::TRANSACTIONTYPE:
         break;
       default:
         return new Aphront404Response();

From 9c6969e810227aed3b75de293b43a4002e22b495 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 11 Sep 2019 21:51:27 -0700
Subject: [PATCH 13/23] Remove "Editable By" description fields in Passphrase,
 Phame, and Spaces

Summary:
Ref T13411. These three applications render an "Editable By: <policy>" field in their descriptions.

The pages that these appear on all have "Edit <thing>" actions which either tell you the policy or allow you to discover the policy, and this field is unusual (the vast majority of objects don't have it). I think it largely got copy/pasted or used as space-filler and doesn't offer much of value.

Remove it to simplify/standardize these pages and make changes to how this field works simpler to implement.

Test Plan: Viewed a Credential, Blog, and Space; no longer saw the "Editable By" field.

Maniphest Tasks: T13411

Differential Revision: https://secure.phabricator.com/D20802
---
 .../controller/PassphraseCredentialViewController.php     | 8 --------
 .../phame/controller/blog/PhameBlogManageController.php   | 8 --------
 .../spaces/controller/PhabricatorSpacesViewController.php | 8 --------
 3 files changed, 24 deletions(-)

diff --git a/src/applications/passphrase/controller/PassphraseCredentialViewController.php b/src/applications/passphrase/controller/PassphraseCredentialViewController.php
index aabb3821e0..6688bef285 100644
--- a/src/applications/passphrase/controller/PassphraseCredentialViewController.php
+++ b/src/applications/passphrase/controller/PassphraseCredentialViewController.php
@@ -190,14 +190,6 @@ final class PassphraseCredentialViewController extends PassphraseController {
       pht('Credential Type'),
       $type->getCredentialTypeName());
 
-    $descriptions = PhabricatorPolicyQuery::renderPolicyDescriptions(
-      $viewer,
-      $credential);
-
-    $properties->addProperty(
-      pht('Editable By'),
-      $descriptions[PhabricatorPolicyCapability::CAN_EDIT]);
-
     if ($type->shouldRequireUsername()) {
       $properties->addProperty(
         pht('Username'),
diff --git a/src/applications/phame/controller/blog/PhameBlogManageController.php b/src/applications/phame/controller/blog/PhameBlogManageController.php
index 2bdcbf7635..65378d91cb 100644
--- a/src/applications/phame/controller/blog/PhameBlogManageController.php
+++ b/src/applications/phame/controller/blog/PhameBlogManageController.php
@@ -143,14 +143,6 @@ final class PhameBlogManageController extends PhameBlogController {
         ),
         $feed_uri));
 
-    $descriptions = PhabricatorPolicyQuery::renderPolicyDescriptions(
-      $viewer,
-      $blog);
-
-    $properties->addProperty(
-      pht('Editable By'),
-      $descriptions[PhabricatorPolicyCapability::CAN_EDIT]);
-
     $engine = id(new PhabricatorMarkupEngine())
       ->setViewer($viewer)
       ->addObject($blog, PhameBlog::MARKUP_FIELD_DESCRIPTION)
diff --git a/src/applications/spaces/controller/PhabricatorSpacesViewController.php b/src/applications/spaces/controller/PhabricatorSpacesViewController.php
index 5fa1c01143..ba55ba90a7 100644
--- a/src/applications/spaces/controller/PhabricatorSpacesViewController.php
+++ b/src/applications/spaces/controller/PhabricatorSpacesViewController.php
@@ -80,14 +80,6 @@ final class PhabricatorSpacesViewController
         ? pht('Yes')
         : pht('No'));
 
-    $descriptions = PhabricatorPolicyQuery::renderPolicyDescriptions(
-      $viewer,
-      $space);
-
-    $list->addProperty(
-      pht('Editable By'),
-      $descriptions[PhabricatorPolicyCapability::CAN_EDIT]);
-
     $description = $space->getDescription();
     if (strlen($description)) {
       $description = new PHUIRemarkupView($viewer, $description);

From c9b0d107f079583e72b273e4df572b0f2dde1447 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 11 Sep 2019 22:01:20 -0700
Subject: [PATCH 14/23] Remove unused "icon" parameter from policy name
 rendering

Summary: Ref T13411. This pathway has an unused "icon" parameter with no callsites. Throw it away to ease refactoring.

Test Plan: Grepped for callsites, found none using this parameter.

Maniphest Tasks: T13411

Differential Revision: https://secure.phabricator.com/D20803
---
 .../PhabricatorPolicyExplainController.php    |  4 ++--
 .../policy/query/PhabricatorPolicyQuery.php   |  5 ++---
 .../policy/storage/PhabricatorPolicy.php      | 19 +++----------------
 3 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/src/applications/policy/controller/PhabricatorPolicyExplainController.php b/src/applications/policy/controller/PhabricatorPolicyExplainController.php
index a4ba355b16..8b41ef7357 100644
--- a/src/applications/policy/controller/PhabricatorPolicyExplainController.php
+++ b/src/applications/policy/controller/PhabricatorPolicyExplainController.php
@@ -333,8 +333,8 @@ final class PhabricatorPolicyExplainController
       ->appendList(
         array(
           PhabricatorPolicy::getPolicyExplanation(
-          $viewer,
-          $policy->getPHID()),
+            $viewer,
+            $policy->getPHID()),
         ));
 
     $strength = $this->getStrengthInformation($object, $policy, $capability);
diff --git a/src/applications/policy/query/PhabricatorPolicyQuery.php b/src/applications/policy/query/PhabricatorPolicyQuery.php
index e51b2ca401..3337a912b0 100644
--- a/src/applications/policy/query/PhabricatorPolicyQuery.php
+++ b/src/applications/policy/query/PhabricatorPolicyQuery.php
@@ -43,13 +43,12 @@ final class PhabricatorPolicyQuery
 
   public static function renderPolicyDescriptions(
     PhabricatorUser $viewer,
-    PhabricatorPolicyInterface $object,
-    $icon = false) {
+    PhabricatorPolicyInterface $object) {
 
     $policies = self::loadPolicies($viewer, $object);
 
     foreach ($policies as $capability => $policy) {
-      $policies[$capability] = $policy->renderDescription($icon);
+      $policies[$capability] = $policy->renderDescription();
     }
 
     return $policies;
diff --git a/src/applications/policy/storage/PhabricatorPolicy.php b/src/applications/policy/storage/PhabricatorPolicy.php
index 3d6b4ca609..834ca7a798 100644
--- a/src/applications/policy/storage/PhabricatorPolicy.php
+++ b/src/applications/policy/storage/PhabricatorPolicy.php
@@ -276,13 +276,7 @@ final class PhabricatorPolicy
     }
   }
 
-  public function renderDescription($icon = false) {
-    $img = null;
-    if ($icon) {
-      $img = id(new PHUIIconView())
-        ->setIcon($this->getIcon());
-    }
-
+  public function renderDescription() {
     if ($this->getHref()) {
       $desc = javelin_tag(
         'a',
@@ -291,16 +285,9 @@ final class PhabricatorPolicy
           'class' => 'policy-link',
           'sigil' => $this->getWorkflow() ? 'workflow' : null,
         ),
-        array(
-          $img,
-          $this->getName(),
-        ));
+        $this->getName());
     } else {
-      if ($img) {
-        $desc = array($img, $this->getName());
-      } else {
-        $desc = $this->getName();
-      }
+      $desc = $this->getName();
     }
 
     switch ($this->getType()) {

From 506f93b4a39007b50b2daa5ded3223b19c85f490 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 12 Sep 2019 07:23:43 -0700
Subject: [PATCH 15/23] Give policy name rendering explicit "text name",
 "capability link", and "transaction link" pathways

Summary:
Ref T13411. This cleans up policy name rendering. We ultimately render into three contexts:

  - Plain text contexts, like `bin/policy show`.
  - Transaction contexts, where we're showing a policy change. In these cases, we link some policies (like project policies and custom policies) but the links go directly to the relevant object or a minimal explanation of the change. We don't link policies like "All Users".
  - Capability contexts, where we're describing a capability, like "Can Push" or cases in Applicaitons. In these cases, we link all policies to the full policy explanation flow.

Test Plan:
  - Used `bin/policy show` to examine the policy of an object with a project policy, no longer saw HTML.
  - Viewed the transaction logs of Applications (ModularTransactions) and Tasks (not ModularTransactions) with policy edits, including project and custom policies.
  - Clicked "Custom Policy" in both logs, got consistent dialogs.
  - Viewed application detail pages, saw all capabities linked to explanatory capability dialogs. The value of having this dialog is that the user can get a full explanation of special rules even if the policy is something mundane like "All Users".

Maniphest Tasks: T13411

Differential Revision: https://secure.phabricator.com/D20804
---
 src/__phutil_library_map__.php                |  2 +
 ...habricatorPolicyManagementShowWorkflow.php |  4 +-
 .../policy/query/PhabricatorPolicyQuery.php   |  3 +-
 .../policy/storage/PhabricatorPolicy.php      | 40 +++-----
 .../policy/view/PhabricatorPolicyRef.php      | 99 +++++++++++++++++++
 .../PhabricatorApplicationTransaction.php     | 16 ++-
 .../PhabricatorModularTransactionType.php     | 15 ++-
 src/docs/user/userguide/unlocking.diviner     | 17 +++-
 8 files changed, 146 insertions(+), 50 deletions(-)
 create mode 100644 src/applications/policy/view/PhabricatorPolicyRef.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index fad919f4dd..d99427dcab 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -4195,6 +4195,7 @@ phutil_register_library_map(array(
     'PhabricatorPolicyManagementWorkflow' => 'applications/policy/management/PhabricatorPolicyManagementWorkflow.php',
     'PhabricatorPolicyPHIDTypePolicy' => 'applications/policy/phid/PhabricatorPolicyPHIDTypePolicy.php',
     'PhabricatorPolicyQuery' => 'applications/policy/query/PhabricatorPolicyQuery.php',
+    'PhabricatorPolicyRef' => 'applications/policy/view/PhabricatorPolicyRef.php',
     'PhabricatorPolicyRequestExceptionHandler' => 'aphront/handler/PhabricatorPolicyRequestExceptionHandler.php',
     'PhabricatorPolicyRule' => 'applications/policy/rule/PhabricatorPolicyRule.php',
     'PhabricatorPolicySearchEngineExtension' => 'applications/policy/engineextension/PhabricatorPolicySearchEngineExtension.php',
@@ -10675,6 +10676,7 @@ phutil_register_library_map(array(
     'PhabricatorPolicyManagementWorkflow' => 'PhabricatorManagementWorkflow',
     'PhabricatorPolicyPHIDTypePolicy' => 'PhabricatorPHIDType',
     'PhabricatorPolicyQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
+    'PhabricatorPolicyRef' => 'Phobject',
     'PhabricatorPolicyRequestExceptionHandler' => 'PhabricatorRequestExceptionHandler',
     'PhabricatorPolicyRule' => 'Phobject',
     'PhabricatorPolicySearchEngineExtension' => 'PhabricatorSearchEngineExtension',
diff --git a/src/applications/policy/management/PhabricatorPolicyManagementShowWorkflow.php b/src/applications/policy/management/PhabricatorPolicyManagementShowWorkflow.php
index 208f1ae964..a378ecc076 100644
--- a/src/applications/policy/management/PhabricatorPolicyManagementShowWorkflow.php
+++ b/src/applications/policy/management/PhabricatorPolicyManagementShowWorkflow.php
@@ -60,8 +60,10 @@ final class PhabricatorPolicyManagementShowWorkflow
 
     $console->writeOut("__%s__\n\n", pht('CAPABILITIES'));
     foreach ($policies as $capability => $policy) {
+      $ref = $policy->newRef($viewer);
+
       $console->writeOut("  **%s**\n", $capability);
-      $console->writeOut("    %s\n", $policy->renderDescription());
+      $console->writeOut("    %s\n", $ref->getPolicyDisplayName());
       $console->writeOut("    %s\n",
         PhabricatorPolicy::getPolicyExplanation($viewer, $policy->getPHID()));
       $console->writeOut("\n");
diff --git a/src/applications/policy/query/PhabricatorPolicyQuery.php b/src/applications/policy/query/PhabricatorPolicyQuery.php
index 3337a912b0..018007db28 100644
--- a/src/applications/policy/query/PhabricatorPolicyQuery.php
+++ b/src/applications/policy/query/PhabricatorPolicyQuery.php
@@ -48,7 +48,8 @@ final class PhabricatorPolicyQuery
     $policies = self::loadPolicies($viewer, $object);
 
     foreach ($policies as $capability => $policy) {
-      $policies[$capability] = $policy->renderDescription();
+      $policies[$capability] = $policy->newRef($viewer)
+        ->newCapabilityLink($object, $capability);
     }
 
     return $policies;
diff --git a/src/applications/policy/storage/PhabricatorPolicy.php b/src/applications/policy/storage/PhabricatorPolicy.php
index 834ca7a798..82d4f355bc 100644
--- a/src/applications/policy/storage/PhabricatorPolicy.php
+++ b/src/applications/policy/storage/PhabricatorPolicy.php
@@ -276,32 +276,22 @@ final class PhabricatorPolicy
     }
   }
 
-  public function renderDescription() {
-    if ($this->getHref()) {
-      $desc = javelin_tag(
-        'a',
-        array(
-          'href' => $this->getHref(),
-          'class' => 'policy-link',
-          'sigil' => $this->getWorkflow() ? 'workflow' : null,
-        ),
-        $this->getName());
-    } else {
-      $desc = $this->getName();
-    }
+  public function newRef(PhabricatorUser $viewer) {
+    return id(new PhabricatorPolicyRef())
+      ->setViewer($viewer)
+      ->setPolicy($this);
+  }
 
-    switch ($this->getType()) {
-      case PhabricatorPolicyType::TYPE_PROJECT:
-        return pht('%s (Project)', $desc);
-      case PhabricatorPolicyType::TYPE_CUSTOM:
-        return $desc;
-      case PhabricatorPolicyType::TYPE_MASKED:
-        return pht(
-          '%s (You do not have permission to view policy details.)',
-          $desc);
-      default:
-        return $desc;
-    }
+  public function isProjectPolicy() {
+    return ($this->getType() === PhabricatorPolicyType::TYPE_PROJECT);
+  }
+
+  public function isCustomPolicy() {
+    return ($this->getType() === PhabricatorPolicyType::TYPE_CUSTOM);
+  }
+
+  public function isMaskedPolicy() {
+    return ($this->getType() === PhabricatorPolicyType::TYPE_MASKED);
   }
 
   /**
diff --git a/src/applications/policy/view/PhabricatorPolicyRef.php b/src/applications/policy/view/PhabricatorPolicyRef.php
new file mode 100644
index 0000000000..605d59ee45
--- /dev/null
+++ b/src/applications/policy/view/PhabricatorPolicyRef.php
@@ -0,0 +1,99 @@
+<?php
+
+final class PhabricatorPolicyRef
+  extends Phobject {
+
+  private $viewer;
+  private $policy;
+
+  public function setViewer(PhabricatorUser $viewer) {
+    $this->viewer = $viewer;
+    return $this;
+  }
+
+  public function getViewer() {
+    return $this->viewer;
+  }
+
+  public function setPolicy(PhabricatorPolicy $policy) {
+    $this->policy = $policy;
+    return $this;
+  }
+
+  public function getPolicy() {
+    return $this->policy;
+  }
+
+  public function getPolicyDisplayName() {
+    $policy = $this->getPolicy();
+    return $policy->getFullName();
+  }
+
+  public function newTransactionLink(
+    $mode,
+    PhabricatorApplicationTransaction $xaction) {
+
+    $policy = $this->getPolicy();
+
+    if ($policy->isCustomPolicy()) {
+      $uri = urisprintf(
+        '/transactions/%s/%s/',
+        $mode,
+        $xaction->getPHID());
+      $workflow = true;
+    } else {
+      $uri = $policy->getHref();
+      $workflow = false;
+    }
+
+    return $this->newLink($uri, $workflow);
+  }
+
+  public function newCapabilityLink($object, $capability) {
+    $policy = $this->getPolicy();
+
+    $uri = urisprintf(
+      '/policy/explain/%s/%s/',
+      $object->getPHID(),
+      $capability);
+
+    return $this->newLink($uri, true);
+  }
+
+  private function newLink($uri, $workflow) {
+    $policy = $this->getPolicy();
+    $name = $policy->getName();
+
+    if ($uri !== null) {
+      $name = javelin_tag(
+        'a',
+        array(
+          'href' => $uri,
+          'sigil' => ($workflow ? 'workflow' : null),
+        ),
+        $name);
+    }
+
+    $hint = $this->getPolicyTypeHint();
+    if ($hint !== null) {
+      $name = pht('%s (%s)', $name, $hint);
+    }
+
+    return $name;
+  }
+
+  private function getPolicyTypeHint() {
+    $policy = $this->getPolicy();
+
+    if ($policy->isProjectPolicy()) {
+      return pht('Project');
+    }
+
+    if ($policy->isMaskedPolicy()) {
+      return pht('You do not have permission to view policy details.');
+    }
+
+    return null;
+  }
+
+}
diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
index 5fa4562164..7e65ac0a09 100644
--- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
+++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
@@ -445,19 +445,15 @@ abstract class PhabricatorApplicationTransaction
     $policy = PhabricatorPolicy::newFromPolicyAndHandle(
       $phid,
       $this->getHandleIfExists($phid));
+
+    $ref = $policy->newRef($this->getViewer());
+
     if ($this->renderingTarget == self::TARGET_HTML) {
-      switch ($policy->getType()) {
-        case PhabricatorPolicyType::TYPE_CUSTOM:
-          $policy->setHref('/transactions/'.$state.'/'.$this->getPHID().'/');
-          $policy->setWorkflow(true);
-          break;
-        default:
-          break;
-      }
-      $output = $policy->renderDescription();
+      $output = $ref->newTransactionLink($state, $this);
     } else {
-      $output = hsprintf('%s', $policy->getFullName());
+      $output = $ref->getPolicyDisplayName();
     }
+
     return $output;
   }
 
diff --git a/src/applications/transactions/storage/PhabricatorModularTransactionType.php b/src/applications/transactions/storage/PhabricatorModularTransactionType.php
index 2d0cb8e7c1..7d5e3c533e 100644
--- a/src/applications/transactions/storage/PhabricatorModularTransactionType.php
+++ b/src/applications/transactions/storage/PhabricatorModularTransactionType.php
@@ -215,17 +215,16 @@ abstract class PhabricatorModularTransactionType
       $phid,
       $handles[$phid]);
 
+    $ref = $policy->newRef($viewer);
+
     if ($this->isTextMode()) {
-      return $this->renderValue($policy->getFullName());
+      $name = $ref->getPolicyDisplayName();
+    } else {
+      $storage = $this->getStorage();
+      $name = $ref->newTransactionLink($mode, $storage);
     }
 
-    $storage = $this->getStorage();
-    if ($policy->getType() == PhabricatorPolicyType::TYPE_CUSTOM) {
-      $policy->setHref('/transactions/'.$mode.'/'.$storage->getPHID().'/');
-      $policy->setWorkflow(true);
-    }
-
-    return $this->renderValue($policy->renderDescription());
+    return $this->renderValue($name);
   }
 
   final protected function renderHandleList(array $phids) {
diff --git a/src/docs/user/userguide/unlocking.diviner b/src/docs/user/userguide/unlocking.diviner
index 7dc29f69bd..456655a393 100644
--- a/src/docs/user/userguide/unlocking.diviner
+++ b/src/docs/user/userguide/unlocking.diviner
@@ -63,16 +63,23 @@ For detailed help on managing and stripping MFA, see the instructions in
 Unlocking Objects
 =================
 
-If you aren't sure who owns an object, or no user account has access to an
-object, you can directly change object policies from the CLI:
+If you aren't sure who owns an object, you can inspect the policies from the
+CLI:
+
+```
+$ ./bin/policy show <object>
+```
+
+To identify the object you want to examine, you can specify an object
+name (like `T123`) or a PHID as the `<object>` parameter.
+
+If examining the policy isn't helpful, or no user account has access to an
+object, you can then directly change object policies from the CLI:
 
 ```
 $ ./bin/policy unlock <object> [--view ...] [--edit ...] [--owner ...]
 ```
 
-To identify the object you want to unlock, you can specify an object name (like
-`T123`) or a PHID as the `<object>` parameter.
-
 Use the `--view` and `--edit` flags (and, for some objects, the `--owner`
 flag) to specify new policies for the object.
 

From 9a36e6931cf737e94ceec70492ef435eec7ec0fd Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 12 Sep 2019 08:20:04 -0700
Subject: [PATCH 16/23] Inline custom policy rules inside policy capability
 explanation dialogs

Summary: Ref T13411. When users click a link to explain a capability (like the policy header on many objects, or the link next to specific capabilities in "Applications", "Diffusion", etc), inline the full ruleset for the custom policy into the dialog if the object has a custom policy.

Test Plan: {F6856365}

Maniphest Tasks: T13411

Differential Revision: https://secure.phabricator.com/D20805
---
 resources/celerity/map.php                    |  8 +-
 src/__phutil_library_map__.php                |  2 +
 .../PhabricatorPolicyExplainController.php    |  9 +-
 .../policy/view/PHUIPolicySectionView.php     | 12 ++-
 .../view/PhabricatorPolicyRulesView.php       | 84 +++++++++++++++++++
 ...rApplicationTransactionValueController.php | 81 +-----------------
 webroot/rsrc/css/phui/phui-header-view.css    | 42 ----------
 .../css/phui/phui-policy-section-view.css     | 62 ++++++++++++++
 8 files changed, 176 insertions(+), 124 deletions(-)
 create mode 100644 src/applications/policy/view/PhabricatorPolicyRulesView.php
 create mode 100644 webroot/rsrc/css/phui/phui-policy-section-view.css

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index 37794cfb81..651714b117 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -9,7 +9,7 @@ return array(
   'names' => array(
     'conpherence.pkg.css' => '3c8a0668',
     'conpherence.pkg.js' => '020aebcf',
-    'core.pkg.css' => '5a4a5010',
+    'core.pkg.css' => 'c69171e6',
     'core.pkg.js' => '73a06a9f',
     'differential.pkg.css' => '8d8360fb',
     'differential.pkg.js' => '0b037a4f',
@@ -155,7 +155,7 @@ return array(
     'rsrc/css/phui/phui-form-view.css' => '01b796c0',
     'rsrc/css/phui/phui-form.css' => '159e2d9c',
     'rsrc/css/phui/phui-head-thing.css' => 'd7f293df',
-    'rsrc/css/phui/phui-header-view.css' => '285c9139',
+    'rsrc/css/phui/phui-header-view.css' => 'b500eeea',
     'rsrc/css/phui/phui-hovercard.css' => '6ca90fa0',
     'rsrc/css/phui/phui-icon-set-selector.css' => '7aa5f3ec',
     'rsrc/css/phui/phui-icon.css' => '4cbc684a',
@@ -168,6 +168,7 @@ return array(
     'rsrc/css/phui/phui-object-box.css' => 'f434b6be',
     'rsrc/css/phui/phui-pager.css' => 'd022c7ad',
     'rsrc/css/phui/phui-pinboard-view.css' => '1f08f5d8',
+    'rsrc/css/phui/phui-policy-section-view.css' => '139fdc64',
     'rsrc/css/phui/phui-property-list-view.css' => 'cad62236',
     'rsrc/css/phui/phui-remarkup-preview.css' => '91767007',
     'rsrc/css/phui/phui-segment-bar-view.css' => '5166b370',
@@ -842,7 +843,7 @@ return array(
     'phui-form-css' => '159e2d9c',
     'phui-form-view-css' => '01b796c0',
     'phui-head-thing-view-css' => 'd7f293df',
-    'phui-header-view-css' => '285c9139',
+    'phui-header-view-css' => 'b500eeea',
     'phui-hovercard' => '074f0783',
     'phui-hovercard-view-css' => '6ca90fa0',
     'phui-icon-set-selector-css' => '7aa5f3ec',
@@ -863,6 +864,7 @@ return array(
     'phui-oi-simple-ui-css' => '6a30fa46',
     'phui-pager-css' => 'd022c7ad',
     'phui-pinboard-view-css' => '1f08f5d8',
+    'phui-policy-section-view-css' => '139fdc64',
     'phui-property-list-view-css' => 'cad62236',
     'phui-remarkup-preview-css' => '91767007',
     'phui-segment-bar-view-css' => '5166b370',
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index d99427dcab..1f844f9695 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -4198,6 +4198,7 @@ phutil_register_library_map(array(
     'PhabricatorPolicyRef' => 'applications/policy/view/PhabricatorPolicyRef.php',
     'PhabricatorPolicyRequestExceptionHandler' => 'aphront/handler/PhabricatorPolicyRequestExceptionHandler.php',
     'PhabricatorPolicyRule' => 'applications/policy/rule/PhabricatorPolicyRule.php',
+    'PhabricatorPolicyRulesView' => 'applications/policy/view/PhabricatorPolicyRulesView.php',
     'PhabricatorPolicySearchEngineExtension' => 'applications/policy/engineextension/PhabricatorPolicySearchEngineExtension.php',
     'PhabricatorPolicyStrengthConstants' => 'applications/policy/constants/PhabricatorPolicyStrengthConstants.php',
     'PhabricatorPolicyTestCase' => 'applications/policy/__tests__/PhabricatorPolicyTestCase.php',
@@ -10679,6 +10680,7 @@ phutil_register_library_map(array(
     'PhabricatorPolicyRef' => 'Phobject',
     'PhabricatorPolicyRequestExceptionHandler' => 'PhabricatorRequestExceptionHandler',
     'PhabricatorPolicyRule' => 'Phobject',
+    'PhabricatorPolicyRulesView' => 'AphrontView',
     'PhabricatorPolicySearchEngineExtension' => 'PhabricatorSearchEngineExtension',
     'PhabricatorPolicyStrengthConstants' => 'PhabricatorPolicyConstants',
     'PhabricatorPolicyTestCase' => 'PhabricatorTestCase',
diff --git a/src/applications/policy/controller/PhabricatorPolicyExplainController.php b/src/applications/policy/controller/PhabricatorPolicyExplainController.php
index 8b41ef7357..cb9bd20088 100644
--- a/src/applications/policy/controller/PhabricatorPolicyExplainController.php
+++ b/src/applications/policy/controller/PhabricatorPolicyExplainController.php
@@ -318,7 +318,7 @@ final class PhabricatorPolicyExplainController
       ->setViewer($viewer)
       ->setIcon($handle->getIcon().' bluegrey')
       ->setHeader(pht('Object Policy'))
-      ->appendList(
+      ->appendParagraph(
         array(
           array(
             phutil_tag('strong', array(), pht('%s:', $capability_name)),
@@ -337,6 +337,13 @@ final class PhabricatorPolicyExplainController
             $policy->getPHID()),
         ));
 
+    if ($policy->isCustomPolicy()) {
+      $rules_view = id(new PhabricatorPolicyRulesView())
+        ->setViewer($viewer)
+        ->setPolicy($policy);
+      $object_section->appendRulesView($rules_view);
+    }
+
     $strength = $this->getStrengthInformation($object, $policy, $capability);
     if ($strength) {
       $object_section->appendHint($strength);
diff --git a/src/applications/policy/view/PHUIPolicySectionView.php b/src/applications/policy/view/PHUIPolicySectionView.php
index 471e5035fb..14d97fb17e 100644
--- a/src/applications/policy/view/PHUIPolicySectionView.php
+++ b/src/applications/policy/view/PHUIPolicySectionView.php
@@ -93,6 +93,16 @@ final class PHUIPolicySectionView
     return $this->appendChild(phutil_tag('p', array(), $content));
   }
 
+  public function appendRulesView(PhabricatorPolicyRulesView $rules_view) {
+    return $this->appendChild(
+      phutil_tag(
+        'div',
+        array(
+          'class' => 'phui-policy-section-view-rules',
+        ),
+        $rules_view));
+  }
+
   protected function getTagAttributes() {
     return array(
       'class' => 'phui-policy-section-view',
@@ -100,7 +110,7 @@ final class PHUIPolicySectionView
   }
 
   protected function getTagContent() {
-    require_celerity_resource('phui-header-view-css');
+    require_celerity_resource('phui-policy-section-view-css');
 
     $icon_view = null;
     $icon = $this->getIcon();
diff --git a/src/applications/policy/view/PhabricatorPolicyRulesView.php b/src/applications/policy/view/PhabricatorPolicyRulesView.php
new file mode 100644
index 0000000000..657b612f16
--- /dev/null
+++ b/src/applications/policy/view/PhabricatorPolicyRulesView.php
@@ -0,0 +1,84 @@
+<?php
+
+final class PhabricatorPolicyRulesView
+  extends AphrontView {
+
+  private $policy;
+
+  public function setPolicy(PhabricatorPolicy $policy) {
+    $this->policy = $policy;
+    return $this;
+  }
+
+  public function getPolicy() {
+    return $this->policy;
+  }
+
+  public function render() {
+    $policy = $this->getPolicy();
+
+    require_celerity_resource('policy-transaction-detail-css');
+
+    $rule_objects = array();
+    foreach ($policy->getCustomRuleClasses() as $class) {
+      $rule_objects[$class] = newv($class, array());
+    }
+
+    $policy = clone $policy;
+    $policy->attachRuleObjects($rule_objects);
+
+    $details = array();
+    $details[] = phutil_tag(
+      'p',
+      array(
+        'class' => 'policy-transaction-detail-intro',
+      ),
+      pht('These rules are processed in order:'));
+
+    foreach ($policy->getRules() as $index => $rule) {
+      $rule_object = $rule_objects[$rule['rule']];
+      if ($rule['action'] == 'allow') {
+        $icon = 'fa-check-circle green';
+      } else {
+        $icon = 'fa-minus-circle red';
+      }
+      $icon = id(new PHUIIconView())
+        ->setIcon($icon)
+        ->setText(
+          ucfirst($rule['action']).' '.$rule_object->getRuleDescription());
+
+      $handle_phids = $rule_object->getRequiredHandlePHIDsForSummary(
+        $rule['value']);
+      if ($handle_phids) {
+        $value = $this->getViewer()
+          ->renderHandleList($handle_phids)
+          ->setAsInline(true);
+      } else {
+        $value = $rule['value'];
+      }
+
+      $details[] = phutil_tag('div',
+        array(
+          'class' => 'policy-transaction-detail-row',
+        ),
+        array(
+          $icon,
+          $value,
+        ));
+    }
+
+    $details[] = phutil_tag(
+      'p',
+      array(
+        'class' => 'policy-transaction-detail-end',
+      ),
+      pht(
+        'If no rules match, %s all other users.',
+        phutil_tag('b',
+        array(),
+        $policy->getDefaultAction())));
+
+    return $details;
+  }
+
+}
diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php
index 9c82ff99c8..ef5e168898 100644
--- a/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php
+++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php
@@ -58,89 +58,16 @@ final class PhabricatorApplicationTransactionValueController
       return new Aphront404Response();
     }
 
-    $rule_objects = array();
-    foreach ($policy->getCustomRuleClasses() as $class) {
-      $rule_objects[$class] = newv($class, array());
-    }
-    $policy->attachRuleObjects($rule_objects);
+    $rules_view = id(new PhabricatorPolicyRulesView())
+      ->setViewer($viewer)
+      ->setPolicy($policy);
 
-    $this->requireResource('policy-transaction-detail-css');
     $cancel_uri = $this->guessCancelURI($viewer, $xaction);
 
     return $this->newDialog()
       ->setTitle($policy->getFullName())
       ->setWidth(AphrontDialogView::WIDTH_FORM)
-      ->appendChild($this->renderPolicyDetails($policy, $rule_objects))
+      ->appendChild($rules_view)
       ->addCancelButton($cancel_uri, pht('Close'));
   }
-
-  private function extractPHIDs(
-    PhabricatorPolicy $policy,
-    array $rule_objects) {
-
-    $phids = array();
-    foreach ($policy->getRules() as $rule) {
-      $rule_object = $rule_objects[$rule['rule']];
-      $phids[] =
-        $rule_object->getRequiredHandlePHIDsForSummary($rule['value']);
-    }
-    return array_filter(array_mergev($phids));
-  }
-
-  private function renderPolicyDetails(
-    PhabricatorPolicy $policy,
-    array $rule_objects) {
-    $details = array();
-    $details[] = phutil_tag(
-      'p',
-      array(
-        'class' => 'policy-transaction-detail-intro',
-      ),
-      pht('These rules are processed in order:'));
-
-    foreach ($policy->getRules() as $index => $rule) {
-      $rule_object = $rule_objects[$rule['rule']];
-      if ($rule['action'] == 'allow') {
-        $icon = 'fa-check-circle green';
-      } else {
-        $icon = 'fa-minus-circle red';
-      }
-      $icon = id(new PHUIIconView())
-        ->setIcon($icon)
-        ->setText(
-          ucfirst($rule['action']).' '.$rule_object->getRuleDescription());
-
-      $handle_phids = $rule_object->getRequiredHandlePHIDsForSummary(
-        $rule['value']);
-      if ($handle_phids) {
-        $value = $this->getViewer()
-          ->renderHandleList($handle_phids)
-          ->setAsInline(true);
-      } else {
-        $value = $rule['value'];
-      }
-
-      $details[] = phutil_tag('div',
-        array(
-          'class' => 'policy-transaction-detail-row',
-        ),
-        array(
-          $icon,
-          $value,
-        ));
-    }
-
-    $details[] = phutil_tag(
-      'p',
-      array(
-        'class' => 'policy-transaction-detail-end',
-      ),
-      pht(
-        'If no rules match, %s all other users.',
-        phutil_tag('b',
-        array(),
-        $policy->getDefaultAction())));
-    return $details;
-  }
-
 }
diff --git a/webroot/rsrc/css/phui/phui-header-view.css b/webroot/rsrc/css/phui/phui-header-view.css
index 6a096af76d..1d851f04ee 100644
--- a/webroot/rsrc/css/phui/phui-header-view.css
+++ b/webroot/rsrc/css/phui/phui-header-view.css
@@ -354,45 +354,3 @@ body .phui-header-shell.phui-bleed-header
 .phui-header-view .phui-tag-indigo a {
   color: {$sh-indigotext};
 }
-
-.phui-policy-section-view {
-  margin-bottom: 24px;
-}
-
-.phui-policy-section-view-header {
-  background: {$bluebackground};
-  border-bottom: 1px solid {$lightblueborder};
-  padding: 4px 8px;
-  color: {$darkbluetext};
-  margin-bottom: 8px;
-}
-
-.phui-policy-section-view-header-text {
-  font-weight: bold;
-}
-
-.phui-policy-section-view-header .phui-icon-view {
-  margin-right: 8px;
-}
-
-.phui-policy-section-view-link {
-  float: right;
-}
-
-.phui-policy-section-view-link .phui-icon-view {
-  color: {$bluetext};
-}
-
-.phui-policy-section-view-hint {
-  color: {$greytext};
-  background: {$lightbluebackground};
-  padding: 8px;
-}
-
-.phui-policy-section-view-body {
-  padding: 0 12px;
-}
-
-.phui-policy-section-view-inactive-rule {
-  color: {$greytext};
-}
diff --git a/webroot/rsrc/css/phui/phui-policy-section-view.css b/webroot/rsrc/css/phui/phui-policy-section-view.css
new file mode 100644
index 0000000000..4325b34867
--- /dev/null
+++ b/webroot/rsrc/css/phui/phui-policy-section-view.css
@@ -0,0 +1,62 @@
+/**
+ * @provides phui-policy-section-view-css
+ */
+
+.phui-policy-section-view {
+  margin-bottom: 24px;
+}
+
+.phui-policy-section-view-header {
+  background: {$bluebackground};
+  border-bottom: 1px solid {$lightblueborder};
+  padding: 4px 8px;
+  color: {$darkbluetext};
+  margin-bottom: 8px;
+}
+
+.phui-policy-section-view-header-text {
+  font-weight: bold;
+}
+
+.phui-policy-section-view-header .phui-icon-view {
+  margin-right: 8px;
+}
+
+.phui-policy-section-view-link {
+  float: right;
+}
+
+.phui-policy-section-view-link .phui-icon-view {
+  color: {$bluetext};
+}
+
+.phui-policy-section-view-hint {
+  color: {$greytext};
+  background: {$lightbluebackground};
+  padding: 8px;
+}
+
+.phui-policy-section-view-body {
+  padding: 0 12px;
+}
+
+.phui-policy-section-view-inactive-rule {
+  color: {$greytext};
+}
+
+.phui-policy-section-view-rules {
+  margin: 8px 0;
+  padding: 8px;
+  background: {$lightbluebackground};
+  border: 1px solid {$lightblueborder};
+}
+
+.phui-policy-section-view .phui-policy-section-view-body ul {
+  margin: 8px 0;
+  padding: 0 16px 0 24px;
+  list-style: disc;
+}
+
+.phui-policy-section-view .phui-policy-section-view-body p + p {
+  margin-top: 8px;
+}

From 0c7ea8c94215eadae8d6082384709bf1988039e2 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 12 Sep 2019 08:53:14 -0700
Subject: [PATCH 17/23] When users fail a "CAN_SEE" check, give them an
 "opaque" policy explanation

Summary:
Ref T13411. Currently, if you hit a policy exception because you can't view an object, we disclose details about the view policy of the object, particularly which project's members can see the object for project policies.

Although there's a large amount of grey area here, this feels like a more substantial disclosure than we offer in other contexts. Instead, if you encounter a policy exception while testing "CAN_VIEW" or don't have "CAN_VIEW", present an "opaque" explanation which omits details that viewers who can't view the object shouldn't have access to. Today, this is the name of "Project" policies (and, implicitly, the rulesets of custom policies, which we now disclose in other similar contexts).

Test Plan:
  - Hit policy exceptions for "CAN_VIEW" on an object with a project view policy, saw an opaque explanation.
  - Hit policy exceptions for "CAN_EDIT" on an object with a project edit policy and a view policy I satisfied, saw a more detailed explanation.

Maniphest Tasks: T13411

Differential Revision: https://secure.phabricator.com/D20806
---
 .../policy/filter/PhabricatorPolicyFilter.php | 28 +++++++++++++++++--
 .../policy/storage/PhabricatorPolicy.php      | 23 ++++++++++++++-
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/src/applications/policy/filter/PhabricatorPolicyFilter.php b/src/applications/policy/filter/PhabricatorPolicyFilter.php
index 4ea7ce1549..63f3f98cec 100644
--- a/src/applications/policy/filter/PhabricatorPolicyFilter.php
+++ b/src/applications/policy/filter/PhabricatorPolicyFilter.php
@@ -602,12 +602,13 @@ final class PhabricatorPolicyFilter extends Phobject {
     PhabricatorPolicyInterface $object,
     $policy,
     $capability) {
+    $viewer = $this->viewer;
 
     if (!$this->raisePolicyExceptions) {
       return;
     }
 
-    if ($this->viewer->isOmnipotent()) {
+    if ($viewer->isOmnipotent()) {
       // Never raise policy exceptions for the omnipotent viewer. Although we
       // will never normally issue a policy rejection for the omnipotent
       // viewer, we can end up here when queries blanket reject objects that
@@ -634,7 +635,30 @@ final class PhabricatorPolicyFilter extends Phobject {
         $capability);
     }
 
-    $more = PhabricatorPolicy::getPolicyExplanation($this->viewer, $policy);
+    // See T13411. If you receive a policy exception because you can't view
+    // an object, we also want to avoid disclosing too many details about the
+    // actual policy (for example, the names of projects in the policy).
+
+    // If you failed a "CAN_VIEW" check, or failed some other check and don't
+    // have "CAN_VIEW" on the object, we give you an "opaque" explanation.
+    // Otherwise, we give you a more detailed explanation.
+
+    $view_capability = PhabricatorPolicyCapability::CAN_VIEW;
+    if ($capability === $view_capability) {
+      $show_details = false;
+    } else {
+      $show_details = self::hasCapability(
+        $viewer,
+        $object,
+        $view_capability);
+    }
+
+    if ($show_details) {
+      $more = PhabricatorPolicy::getPolicyExplanation($viewer, $policy);
+    } else {
+      $more = PhabricatorPolicy::getOpaquePolicyExplanation($viewer, $policy);
+    }
+
     $more = (array)$more;
     $more = array_filter($more);
 
diff --git a/src/applications/policy/storage/PhabricatorPolicy.php b/src/applications/policy/storage/PhabricatorPolicy.php
index 82d4f355bc..66a7d9e3be 100644
--- a/src/applications/policy/storage/PhabricatorPolicy.php
+++ b/src/applications/policy/storage/PhabricatorPolicy.php
@@ -220,6 +220,25 @@ final class PhabricatorPolicy
     PhabricatorUser $viewer,
     $policy) {
 
+    $type = phid_get_type($policy);
+    if ($type === PhabricatorProjectProjectPHIDType::TYPECONST) {
+      $handle = id(new PhabricatorHandleQuery())
+        ->setViewer($viewer)
+        ->withPHIDs(array($policy))
+        ->executeOne();
+
+      return pht(
+        'Members of the project "%s" can take this action.',
+        $handle->getFullName());
+    }
+
+    return self::getOpaquePolicyExplanation($viewer, $policy);
+  }
+
+  public static function getOpaquePolicyExplanation(
+    PhabricatorUser $viewer,
+    $policy) {
+
     $rule = PhabricatorPolicyQuery::getObjectPolicyRule($policy);
     if ($rule) {
       return $rule->getPolicyExplanation();
@@ -245,7 +264,9 @@ final class PhabricatorPolicy
         $type = phid_get_type($policy);
         if ($type == PhabricatorProjectProjectPHIDType::TYPECONST) {
           return pht(
-            'Members of the project "%s" can take this action.',
+            'Members of a particular project can take this action. (You '.
+            'can not see this object, so the name of this project is '.
+            'restricted.)',
             $handle->getFullName());
         } else if ($type == PhabricatorPeopleUserPHIDType::TYPECONST) {
           return pht(

From 4f845d8f8c7749b536f32c2f26f1fdb7c32e2766 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 12 Sep 2019 09:28:56 -0700
Subject: [PATCH 18/23] When users encounter a policy exception for a non-view
 capability with a custom policy, inline the policy rules

Summary:
Fixes T13411. This looks like the last case where you hit a policy explanation and have permission to see the policy, but we don't currently show you the policy rules.

This implementation is slightly clumsy, but likely harmless.

Test Plan: {F6856421}

Maniphest Tasks: T13411

Differential Revision: https://secure.phabricator.com/D20807
---
 .../policy/filter/PhabricatorPolicyFilter.php | 45 ++++++++++++++++---
 1 file changed, 38 insertions(+), 7 deletions(-)

diff --git a/src/applications/policy/filter/PhabricatorPolicyFilter.php b/src/applications/policy/filter/PhabricatorPolicyFilter.php
index 63f3f98cec..d8f239e51a 100644
--- a/src/applications/policy/filter/PhabricatorPolicyFilter.php
+++ b/src/applications/policy/filter/PhabricatorPolicyFilter.php
@@ -653,14 +653,42 @@ final class PhabricatorPolicyFilter extends Phobject {
         $view_capability);
     }
 
+    // TODO: This is a bit clumsy. We're producing HTML and text versions of
+    // this message, but can't render the full policy rules in text today.
+    // Users almost never get a text-only version of this exception anyway.
+
+    $head = null;
+    $more = null;
+
     if ($show_details) {
-      $more = PhabricatorPolicy::getPolicyExplanation($viewer, $policy);
+      $head = PhabricatorPolicy::getPolicyExplanation($viewer, $policy);
+
+      $policy_type = PhabricatorPolicyPHIDTypePolicy::TYPECONST;
+      $is_custom = (phid_get_type($policy) === $policy_type);
+      if ($is_custom) {
+        $policy_map = PhabricatorPolicyQuery::loadPolicies(
+          $viewer,
+          $object);
+        if (isset($policy_map[$capability])) {
+          require_celerity_resource('phui-policy-section-view-css');
+
+          $more = id(new PhabricatorPolicyRulesView())
+            ->setViewer($viewer)
+            ->setPolicy($policy_map[$capability]);
+
+          $more = phutil_tag(
+            'div',
+            array(
+              'class' => 'phui-policy-section-view-rules',
+            ),
+            $more);
+        }
+      }
     } else {
-      $more = PhabricatorPolicy::getOpaquePolicyExplanation($viewer, $policy);
+      $head = PhabricatorPolicy::getOpaquePolicyExplanation($viewer, $policy);
     }
 
-    $more = (array)$more;
-    $more = array_filter($more);
+    $head = (array)$head;
 
     $exceptions = PhabricatorPolicy::getSpecialRules(
       $object,
@@ -668,7 +696,10 @@ final class PhabricatorPolicyFilter extends Phobject {
       $capability,
       true);
 
-    $details = array_filter(array_merge($more, $exceptions));
+    $text_details = array_filter(array_merge($head, $exceptions));
+    $text_details = implode(' ', $text_details);
+
+    $html_details = array($head, $more, $exceptions);
 
     $access_denied = $this->renderAccessDenied($object);
 
@@ -677,7 +708,7 @@ final class PhabricatorPolicyFilter extends Phobject {
       $access_denied,
       $capability_name,
       $rejection,
-      implode(' ', $details));
+      $text_details);
 
     $exception = id(new PhabricatorPolicyException($full_message))
       ->setTitle($access_denied)
@@ -685,7 +716,7 @@ final class PhabricatorPolicyFilter extends Phobject {
       ->setRejection($rejection)
       ->setCapability($capability)
       ->setCapabilityName($capability_name)
-      ->setMoreInfo($details);
+      ->setMoreInfo($html_details);
 
     throw $exception;
   }

From d60d4e6a05212f5e31c93c36ee39c775eafba56a Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 12 Sep 2019 12:54:08 -0700
Subject: [PATCH 19/23] Don't present users with Herald fields/actions for
 uninstalled applications, unless the rule already uses them

Summary:
Fixes T7961. Currently, we present Herald users with actions like "Require legalpad signatures" and "Run build plans" even if Legalpad and Harbormaster are not installed.

Instead, allow fields and actions to be made "unavailable", which means that we won't present them as options when adding to new or existing rules.

If you edit a rule which already uses one of these fields or actions, it isn't affected.

Test Plan:
  - Created a rule with a legalpad action, uninstalled legalpad, edited the rule. Action remained untouched.
  - Created a new rule, wasn't offered the legalpad action.
  - Reinstalled the application, saw the action again.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T7961

Differential Revision: https://secure.phabricator.com/D20808
---
 resources/celerity/map.php                    |  6 +--
 .../HarbormasterRunBuildPlansHeraldAction.php |  4 ++
 .../herald/action/HeraldAction.php            |  4 ++
 .../herald/adapter/HeraldAdapter.php          | 20 ++++++++++
 .../controller/HeraldRuleController.php       | 10 ++++-
 src/applications/herald/field/HeraldField.php |  4 ++
 .../LegalpadRequireSignatureHeraldAction.php  |  5 +++
 .../js/application/herald/HeraldRuleEditor.js | 40 +++++++++++++++----
 8 files changed, 80 insertions(+), 13 deletions(-)

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index 651714b117..f5ff8827ae 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -397,7 +397,7 @@ return array(
     'rsrc/js/application/files/behavior-icon-composer.js' => '38a6cedb',
     'rsrc/js/application/files/behavior-launch-icon-composer.js' => 'a17b84f1',
     'rsrc/js/application/harbormaster/behavior-harbormaster-log.js' => 'b347a301',
-    'rsrc/js/application/herald/HeraldRuleEditor.js' => '27daef73',
+    'rsrc/js/application/herald/HeraldRuleEditor.js' => '2633bef7',
     'rsrc/js/application/herald/PathTypeahead.js' => 'ad486db3',
     'rsrc/js/application/herald/herald-rule-editor.js' => '0922e81d',
     'rsrc/js/application/maniphest/behavior-batch-selector.js' => '139ef688',
@@ -571,7 +571,7 @@ return array(
     'global-drag-and-drop-css' => '1d2713a4',
     'harbormaster-css' => '8dfe16b2',
     'herald-css' => '648d39e2',
-    'herald-rule-editor' => '27daef73',
+    'herald-rule-editor' => '2633bef7',
     'herald-test-css' => 'e004176f',
     'inline-comment-summary-css' => '81eb368d',
     'javelin-aphlict' => '022516b4',
@@ -1117,7 +1117,7 @@ return array(
       'javelin-json',
       'phabricator-draggable-list',
     ),
-    '27daef73' => array(
+    '2633bef7' => array(
       'multirow-row-manager',
       'javelin-install',
       'javelin-util',
diff --git a/src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php b/src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php
index 9fc053e8ae..6934a2e3d9 100644
--- a/src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php
+++ b/src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php
@@ -96,4 +96,8 @@ final class HarbormasterRunBuildPlansHeraldAction
     return $record->getTarget();
   }
 
+  public function isActionAvailable() {
+    return id(new PhabricatorHarbormasterApplication())->isInstalled();
+  }
+
 }
diff --git a/src/applications/herald/action/HeraldAction.php b/src/applications/herald/action/HeraldAction.php
index a9740d1736..d914c97a1c 100644
--- a/src/applications/herald/action/HeraldAction.php
+++ b/src/applications/herald/action/HeraldAction.php
@@ -405,4 +405,8 @@ abstract class HeraldAction extends Phobject {
     return array();
   }
 
+  public function isActionAvailable() {
+    return true;
+  }
+
 }
diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php
index cb8545b71d..2832c6d9f4 100644
--- a/src/applications/herald/adapter/HeraldAdapter.php
+++ b/src/applications/herald/adapter/HeraldAdapter.php
@@ -373,6 +373,16 @@ abstract class HeraldAdapter extends Phobject {
     return $field->getFieldGroupKey();
   }
 
+  public function isFieldAvailable($field_key) {
+    $field = $this->getFieldImplementation($field_key);
+
+    if (!$field) {
+      return null;
+    }
+
+    return $field->isFieldAvailable();
+  }
+
 
 /* -(  Conditions  )--------------------------------------------------------- */
 
@@ -765,6 +775,16 @@ abstract class HeraldAdapter extends Phobject {
     return $action->getActionGroupKey();
   }
 
+  public function isActionAvailable($action_key) {
+    $action = $this->getActionImplementation($action_key);
+
+    if (!$action) {
+      return null;
+    }
+
+    return $action->isActionAvailable();
+  }
+
   public function getActions($rule_type) {
     $actions = array();
     foreach ($this->getActionsForRuleType($rule_type) as $key => $action) {
diff --git a/src/applications/herald/controller/HeraldRuleController.php b/src/applications/herald/controller/HeraldRuleController.php
index 73c1e8c39c..5e13522b6e 100644
--- a/src/applications/herald/controller/HeraldRuleController.php
+++ b/src/applications/herald/controller/HeraldRuleController.php
@@ -684,7 +684,10 @@ final class HeraldRuleController extends HeraldController {
     $group_map = array();
     foreach ($field_map as $field_key => $field_name) {
       $group_key = $adapter->getFieldGroupKey($field_key);
-      $group_map[$group_key][$field_key] = $field_name;
+      $group_map[$group_key][$field_key] = array(
+        'name' => $field_name,
+        'available' => $adapter->isFieldAvailable($field_key),
+      );
     }
 
     return $this->getGroups(
@@ -696,7 +699,10 @@ final class HeraldRuleController extends HeraldController {
     $group_map = array();
     foreach ($action_map as $action_key => $action_name) {
       $group_key = $adapter->getActionGroupKey($action_key);
-      $group_map[$group_key][$action_key] = $action_name;
+      $group_map[$group_key][$action_key] = array(
+        'name' => $action_name,
+        'available' => $adapter->isActionAvailable($action_key),
+      );
     }
 
     return $this->getGroups(
diff --git a/src/applications/herald/field/HeraldField.php b/src/applications/herald/field/HeraldField.php
index 6f14091aff..2a1e89d558 100644
--- a/src/applications/herald/field/HeraldField.php
+++ b/src/applications/herald/field/HeraldField.php
@@ -241,4 +241,8 @@ abstract class HeraldField extends Phobject {
     return false;
   }
 
+  public function isFieldAvailable() {
+    return true;
+  }
+
 }
diff --git a/src/applications/legalpad/herald/LegalpadRequireSignatureHeraldAction.php b/src/applications/legalpad/herald/LegalpadRequireSignatureHeraldAction.php
index 7ff69d37d5..b477c2ce35 100644
--- a/src/applications/legalpad/herald/LegalpadRequireSignatureHeraldAction.php
+++ b/src/applications/legalpad/herald/LegalpadRequireSignatureHeraldAction.php
@@ -130,4 +130,9 @@ final class LegalpadRequireSignatureHeraldAction
       'Require document signatures: %s.',
       $this->renderHandleList($value));
   }
+
+  public function isActionAvailable() {
+    return id(new PhabricatorLegalpadApplication())->isInstalled();
+  }
+
 }
diff --git a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js
index 443fe9811e..254061533a 100644
--- a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js
+++ b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js
@@ -350,8 +350,10 @@ JX.install('HeraldRuleEditor', {
         sigil: 'field-select'
       };
 
-      var field_select = this._renderGroupSelect(groups, attrs);
-      field_select.value = this._config.conditions[row_id][0];
+      var field_select = this._renderGroupSelect(
+        groups,
+        attrs,
+        this._config.conditions[row_id][0]);
 
       var field_cell = JX.$N('td', {sigil: 'field-cell'}, field_select);
 
@@ -367,18 +369,38 @@ JX.install('HeraldRuleEditor', {
       }
     },
 
-    _renderGroupSelect: function(groups, attrs) {
+    _renderGroupSelect: function(groups, attrs, value) {
       var optgroups = [];
       for (var ii = 0; ii < groups.length; ii++) {
         var group = groups[ii];
         var options = [];
         for (var k in group.options) {
-          options.push(JX.$N('option', {value: k}, group.options[k]));
+          var option = group.options[k];
+
+          var name = option.name;
+          var available = option.available;
+
+          // See T7961. If the option is not marked as "available", we only
+          // include it in the dropdown if the dropdown already has it as a
+          // value. We want to hide options provided by applications which are
+          // not installed, but do not want to break existing rules.
+
+          if (available || (k === value)) {
+            options.push(JX.$N('option', {value: k}, name));
+          }
+        }
+        if (options.length) {
+          optgroups.push(JX.$N('optgroup', {label: group.label}, options));
         }
-        optgroups.push(JX.$N('optgroup', {label: group.label}, options));
       }
 
-      return JX.$N('select', attrs, optgroups);
+      var select = JX.$N('select', attrs, optgroups);
+
+      if (value !== undefined) {
+        select.value = value;
+      }
+
+      return select;
     },
 
     _newAction : function(data) {
@@ -402,8 +424,10 @@ JX.install('HeraldRuleEditor', {
         sigil: 'action-select'
       };
 
-      var action_select = this._renderGroupSelect(groups, attrs);
-      action_select.value = action[0];
+      var action_select = this._renderGroupSelect(
+        groups,
+        attrs,
+        action[0]);
 
       var action_cell = JX.$N('td', {sigil: 'action-cell'}, action_select);
 

From 3e60128037252fb62067315eaeff67f517967119 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 12 Sep 2019 13:11:18 -0700
Subject: [PATCH 20/23] Support "Subtype" in Herald

Summary: See PHI1434. For objects that support subtypes and have subtypes configured, allow Herald rules to act on subtypes.

Test Plan:
  - Configured task and project subtypes, wrote Herald rules, saw "Subtypes" as an option, saw appropriate typeahead values and detail page rendering.
  - Unconfigured project subtypes, saw field vanish from UI for new rules.
  - Wrote a "subtype"-depenent rule that added a comment, interacted with tasks of that subtype and a different subtype. Saw Herald act only on tasks with the correct subtype.

Differential Revision: https://secure.phabricator.com/D20809
---
 src/__phutil_library_map__.php                |  2 +
 .../maniphest/storage/ManiphestTask.php       |  3 +-
 .../project/storage/PhabricatorProject.php    |  3 +-
 .../PhabricatorEditEngineSubtypeMap.php       | 14 +++++
 ...habricatorEditEngineSubtypeHeraldField.php | 52 +++++++++++++++++++
 5 files changed, 72 insertions(+), 2 deletions(-)
 create mode 100644 src/applications/transactions/herald/PhabricatorEditEngineSubtypeHeraldField.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 1f844f9695..95c5059d06 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -3217,6 +3217,7 @@ phutil_register_library_map(array(
     'PhabricatorEditEngineSettingsPanel' => 'applications/settings/panel/PhabricatorEditEngineSettingsPanel.php',
     'PhabricatorEditEngineStaticCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEngineStaticCommentAction.php',
     'PhabricatorEditEngineSubtype' => 'applications/transactions/editengine/PhabricatorEditEngineSubtype.php',
+    'PhabricatorEditEngineSubtypeHeraldField' => 'applications/transactions/herald/PhabricatorEditEngineSubtypeHeraldField.php',
     'PhabricatorEditEngineSubtypeInterface' => 'applications/transactions/editengine/PhabricatorEditEngineSubtypeInterface.php',
     'PhabricatorEditEngineSubtypeMap' => 'applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php',
     'PhabricatorEditEngineSubtypeTestCase' => 'applications/transactions/editengine/__tests__/PhabricatorEditEngineSubtypeTestCase.php',
@@ -9550,6 +9551,7 @@ phutil_register_library_map(array(
     'PhabricatorEditEngineSettingsPanel' => 'PhabricatorSettingsPanel',
     'PhabricatorEditEngineStaticCommentAction' => 'PhabricatorEditEngineCommentAction',
     'PhabricatorEditEngineSubtype' => 'Phobject',
+    'PhabricatorEditEngineSubtypeHeraldField' => 'HeraldField',
     'PhabricatorEditEngineSubtypeMap' => 'Phobject',
     'PhabricatorEditEngineSubtypeTestCase' => 'PhabricatorTestCase',
     'PhabricatorEditEngineSubtypeTransaction' => 'PhabricatorEditEngineTransactionType',
diff --git a/src/applications/maniphest/storage/ManiphestTask.php b/src/applications/maniphest/storage/ManiphestTask.php
index d2700895ce..c56d8fe57a 100644
--- a/src/applications/maniphest/storage/ManiphestTask.php
+++ b/src/applications/maniphest/storage/ManiphestTask.php
@@ -564,7 +564,8 @@ final class ManiphestTask extends ManiphestDAO
 
   public function newEditEngineSubtypeMap() {
     $config = PhabricatorEnv::getEnvConfig('maniphest.subtypes');
-    return PhabricatorEditEngineSubtype::newSubtypeMap($config);
+    return PhabricatorEditEngineSubtype::newSubtypeMap($config)
+      ->setDatasource(new ManiphestTaskSubtypeDatasource());
   }
 
 
diff --git a/src/applications/project/storage/PhabricatorProject.php b/src/applications/project/storage/PhabricatorProject.php
index 54267829d3..1d234d4285 100644
--- a/src/applications/project/storage/PhabricatorProject.php
+++ b/src/applications/project/storage/PhabricatorProject.php
@@ -904,7 +904,8 @@ final class PhabricatorProject extends PhabricatorProjectDAO
 
   public function newEditEngineSubtypeMap() {
     $config = PhabricatorEnv::getEnvConfig('projects.subtypes');
-    return PhabricatorEditEngineSubtype::newSubtypeMap($config);
+    return PhabricatorEditEngineSubtype::newSubtypeMap($config)
+      ->setDatasource(new PhabricatorProjectSubtypeDatasource());
   }
 
   public function newSubtypeObject() {
diff --git a/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php b/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php
index 638b665184..dc1ee2842a 100644
--- a/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php
+++ b/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php
@@ -5,6 +5,7 @@ final class PhabricatorEditEngineSubtypeMap
   extends Phobject {
 
   private $subtypes;
+  private $datasource;
 
   public function __construct(array $subtypes) {
     assert_instances_of($subtypes, 'PhabricatorEditEngineSubtype');
@@ -39,6 +40,19 @@ final class PhabricatorEditEngineSubtypeMap
     return $this->subtypes[$subtype_key];
   }
 
+  public function setDatasource(PhabricatorTypeaheadDatasource $datasource) {
+    $this->datasource = $datasource;
+    return $this;
+  }
+
+  public function newDatasource() {
+    if (!$this->datasource) {
+      throw new PhutilInvalidStateException('setDatasource');
+    }
+
+    return clone($this->datasource);
+  }
+
   public function getCreateFormsForSubtype(
     PhabricatorEditEngine $edit_engine,
     PhabricatorEditEngineSubtypeInterface $object) {
diff --git a/src/applications/transactions/herald/PhabricatorEditEngineSubtypeHeraldField.php b/src/applications/transactions/herald/PhabricatorEditEngineSubtypeHeraldField.php
new file mode 100644
index 0000000000..be15540fc9
--- /dev/null
+++ b/src/applications/transactions/herald/PhabricatorEditEngineSubtypeHeraldField.php
@@ -0,0 +1,52 @@
+<?php
+
+final class PhabricatorEditEngineSubtypeHeraldField
+  extends HeraldField {
+
+  const FIELDCONST = 'subtype';
+
+  public function getHeraldFieldName() {
+    return pht('Subtype');
+  }
+
+  public function getFieldGroupKey() {
+    return HeraldSupportFieldGroup::FIELDGROUPKEY;
+  }
+
+  public function supportsObject($object) {
+    return ($object instanceof PhabricatorEditEngineSubtypeInterface);
+  }
+
+  public function getHeraldFieldValue($object) {
+    return $object->getEditEngineSubtype();
+  }
+
+  protected function getHeraldFieldStandardType() {
+    return self::STANDARD_PHID;
+  }
+
+  protected function getDatasource() {
+    $object = $this->getAdapter()->getObject();
+    $map = $object->newEditEngineSubtypeMap();
+    return $map->newDatasource();
+  }
+
+  protected function getDatasourceValueMap() {
+    $object = $this->getAdapter()->getObject();
+    $map = $object->newEditEngineSubtypeMap();
+
+    $result = array();
+    foreach ($map->getSubtypes() as $subtype) {
+      $result[$subtype->getKey()] = $subtype->getName();
+    }
+
+    return $result;
+  }
+
+  public function isFieldAvailable() {
+    $object = $this->getAdapter()->getObject();
+    $map = $object->newEditEngineSubtypeMap();
+    return ($map->getCount() > 1);
+  }
+
+}

From 41f0b8b0a3a10b713017ed1356d5d0a89432c29d Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 12 Sep 2019 16:05:40 -0700
Subject: [PATCH 21/23] Allow subtypes to specify "mutations", to control the
 behavior of the "Change Subtype" action

Summary:
Fixes T13415. Provide a way for subtypes to customize the behavior of "Change Subtype" actions that appear above comment areas.

Subtypes may disable this action by specifying `"mutations": []`, or provide a list of subtypes.

The bulk editor and API can still perform any change.

Test Plan:
  - Tried to define an invalid "mutations" list with a bad subtype, got a sensible error.
  - Specified a limited mutations list and an empty mutations list, verified that corresponding tasks got corresponding actions.
  - Used the bulk editor to perform a freeform mutation.
  - Verified that tasks of a subtype with no "mutations" still work the same way they used to (allow mutation into any subtype).

Maniphest Tasks: T13415

Differential Revision: https://secure.phabricator.com/D20810
---
 .../PhabricatorManiphestConfigOptions.php     | 41 +++++++++++++++----
 .../PhabricatorEditEngineSubtype.php          | 35 ++++++++++++++++
 .../PhabricatorEditEngineSubtypeMap.php       | 38 +++++++++++++++++
 .../PhabricatorSubtypeEditEngineExtension.php | 18 +++++---
 4 files changed, 120 insertions(+), 12 deletions(-)

diff --git a/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php b/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php
index 077fc511ce..60a41a26ca 100644
--- a/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php
+++ b/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php
@@ -344,6 +344,8 @@ dictionary with these keys:
   - `children` //Optional map.// Configure options shown to the user when
      they "Create Subtask". See below.
   - `fields` //Optional map.// Configure field behaviors. See below.
+  - `mutations` //Optional list.// Configure which subtypes this subtype
+    can easily be converted to by using the "Change Subtype" action. See below.
 
 Each subtype must have a unique key, and you must define a subtype with
 the key "%s", which is used as a default subtype.
@@ -404,15 +406,15 @@ The `fields` key can configure the behavior of custom fields on specific
 task subtypes. For example:
 
 ```
-{
-  ...
-  "fields": {
-    "custom.some-field": {
-      "disabled": true
+  {
+    ...
+    "fields": {
+      "custom.some-field": {
+        "disabled": true
+      }
     }
+    ...
   }
-  ...
-}
 ```
 
 Each field supports these options:
@@ -421,6 +423,31 @@ Each field supports these options:
     subtypes.
   - `name` //Optional string.// Custom name of this field for the subtype.
 
+
+The `mutations` key allows you to control the behavior of the "Change Subtype"
+action above the comment area. By default, this action allows users to change
+the task subtype into any other subtype.
+
+If you'd prefer to make it more difficult to change subtypes or offer only a
+subset of subtypes, you can specify the list of subtypes that "Change Subtypes"
+offers. For example, if you have several similar subtypes and want to allow
+tasks to be converted between them but not easily converted to other types,
+you can make the "Change Subtypes" control show only these options like this:
+
+```
+  {
+    ...
+    "mutations": ["bug", "issue", "defect"]
+    ...
+  }
+```
+
+If you specify an empty list, the "Change Subtypes" action will be completely
+hidden.
+
+This mutation list is advisory and only configures the UI. Tasks may still be
+converted across subtypes freely by using the Bulk Editor or API.
+
 EOTEXT
       ,
       $subtype_default_key));
diff --git a/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php b/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php
index 6e1d1de115..f471fcd92f 100644
--- a/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php
+++ b/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php
@@ -14,6 +14,7 @@ final class PhabricatorEditEngineSubtype
   private $childSubtypes = array();
   private $childIdentifiers = array();
   private $fieldConfiguration = array();
+  private $mutations;
 
   public function setKey($key) {
     $this->key = $key;
@@ -78,6 +79,15 @@ final class PhabricatorEditEngineSubtype
     return $this->childIdentifiers;
   }
 
+  public function setMutations($mutations) {
+    $this->mutations = $mutations;
+    return $this;
+  }
+
+  public function getMutations() {
+    return $this->mutations;
+  }
+
   public function hasTagView() {
     return (bool)strlen($this->getTagText());
   }
@@ -152,6 +162,7 @@ final class PhabricatorEditEngineSubtype
           'icon' => 'optional string',
           'children' => 'optional map<string, wild>',
           'fields' => 'optional map<string, wild>',
+          'mutations' => 'optional list<string>',
         ));
 
       $key = $value['key'];
@@ -217,6 +228,28 @@ final class PhabricatorEditEngineSubtype
           'with key "%s". This subtype is required and must be defined.',
           self::SUBTYPE_DEFAULT));
     }
+
+    foreach ($config as $value) {
+      $key = idx($value, 'key');
+
+      $mutations = idx($value, 'mutations');
+      if (!$mutations) {
+        continue;
+      }
+
+      foreach ($mutations as $mutation) {
+        if (!isset($map[$mutation])) {
+          throw new Exception(
+            pht(
+              'Subtype configuration is invalid: subtype with key "%s" '.
+              'specifies that it can mutate into subtype "%s", but that is '.
+              'not a valid subtype.',
+              $key,
+              $mutation));
+        }
+      }
+    }
+
   }
 
   public static function newSubtypeMap(array $config) {
@@ -267,6 +300,8 @@ final class PhabricatorEditEngineSubtype
         }
       }
 
+      $subtype->setMutations(idx($entry, 'mutations'));
+
       $map[$key] = $subtype;
     }
 
diff --git a/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php b/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php
index dc1ee2842a..edf2d2045a 100644
--- a/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php
+++ b/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php
@@ -53,6 +53,44 @@ final class PhabricatorEditEngineSubtypeMap
     return clone($this->datasource);
   }
 
+  public function getMutationMap($source_key) {
+    return mpull($this->getMutations($source_key), 'getName');
+  }
+
+  public function getMutations($source_key) {
+    $mutations = $this->subtypes;
+
+    $subtype = idx($this->subtypes, $source_key);
+    if ($subtype) {
+      $map = $subtype->getMutations();
+      if ($map !== null) {
+        $map = array_fuse($map);
+        foreach ($mutations as $key => $mutation) {
+          if ($key === $source_key) {
+            // This is the current subtype, so we always want to show it.
+            continue;
+          }
+
+          if (isset($map[$key])) {
+            // This is an allowed mutation, so keep it.
+            continue;
+          }
+
+          // Discard other subtypes as mutation options.
+          unset($mutations[$key]);
+        }
+      }
+    }
+
+    // If the only available mutation is the current subtype, treat this like
+    // no mutations are available.
+    if (array_keys($mutations) === array($source_key)) {
+      $mutations = array();
+    }
+
+    return $mutations;
+  }
+
   public function getCreateFormsForSubtype(
     PhabricatorEditEngine $edit_engine,
     PhabricatorEditEngineSubtypeInterface $object) {
diff --git a/src/applications/transactions/engineextension/PhabricatorSubtypeEditEngineExtension.php b/src/applications/transactions/engineextension/PhabricatorSubtypeEditEngineExtension.php
index d0b6d017f3..e73d476d74 100644
--- a/src/applications/transactions/engineextension/PhabricatorSubtypeEditEngineExtension.php
+++ b/src/applications/transactions/engineextension/PhabricatorSubtypeEditEngineExtension.php
@@ -29,9 +29,17 @@ final class PhabricatorSubtypeEditEngineExtension
     PhabricatorApplicationTransactionInterface $object) {
 
     $subtype_type = PhabricatorTransactions::TYPE_SUBTYPE;
+    $subtype_value = $object->getEditEngineSubtype();
 
     $map = $object->newEditEngineSubtypeMap();
-    $options = $map->getDisplayMap();
+
+    if ($object->getID()) {
+      $options = $map->getMutationMap($subtype_value);
+    } else {
+      // NOTE: This is a crude proxy for "are we in the bulk edit workflow".
+      // We want to allow any mutation.
+      $options = $map->getDisplayMap();
+    }
 
     $subtype_field = id(new PhabricatorSelectEditField())
       ->setKey(self::EDITKEY)
@@ -40,12 +48,12 @@ final class PhabricatorSubtypeEditEngineExtension
       ->setTransactionType($subtype_type)
       ->setConduitDescription(pht('Change the object subtype.'))
       ->setConduitTypeDescription(pht('New object subtype key.'))
-      ->setValue($object->getEditEngineSubtype())
+      ->setValue($subtype_value)
       ->setOptions($options);
 
-    // If subtypes are configured, enable changing them from the bulk editor
-    // and comment action stack.
-    if ($map->getCount() > 1) {
+    // If subtypes are configured, enable changing them from the bulk editor.
+    // Bulk editor
+    if ($options) {
       $subtype_field
         ->setBulkEditLabel(pht('Change subtype to'))
         ->setCommentActionLabel(pht('Change Subtype'))

From 3f6620336216840028b1c44cd8c5171e4914fb58 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 12 Sep 2019 16:20:33 -0700
Subject: [PATCH 22/23] Fix a straggling callsite to
 "renderApplicationPolicy()"

Summary: Ref T13411. This is a leftover from recent policy rendering changes.

Test Plan: Viewed feed with application policy stories, no more fatal.

Maniphest Tasks: T13411

Differential Revision: https://secure.phabricator.com/D20811
---
 .../xactions/PhabricatorApplicationPolicyChangeTransaction.php | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php b/src/applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php
index 3c58b42ab5..94e1cc495d 100644
--- a/src/applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php
+++ b/src/applications/meta/xactions/PhabricatorApplicationPolicyChangeTransaction.php
@@ -61,9 +61,6 @@ final class PhabricatorApplicationPolicyChangeTransaction
   }
 
   public function getTitleForFeed() {
-    $old = $this->renderApplicationPolicy($this->getOldValue());
-    $new = $this->renderApplicationPolicy($this->getNewValue());
-
     return pht(
       '%s changed the %s policy for application %s from %s to %s.',
       $this->renderAuthor(),

From 3dcb4a7b5036f627548a9a42ae3ef916130ccfea Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 12 Sep 2019 18:55:18 -0700
Subject: [PATCH 23/23] Work around rendering engine freeze in Chrome 77
 affecting workboards

Summary:
Ref T13413. In Chrome 77, workboard cards with titles that must break in the middle of words cause the browser to completely lock up.

Work around the major known instance of this by overriding the "break-word" behavior. This gives us worse rendering for tasks with very long "words" in their titles (they are truncated instead of broken) but fixes the freezing.

Once Chrome is fixed, this can be reverted.

Test Plan:
  - Created a task named "MMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMM" on a workboard.
  - Loaded the board in Chrome 77.
  - Before: entire page locks up.
  - After: smooth sailing, except the "MMMMMM..." is truncated.

Maniphest Tasks: T13413

Differential Revision: https://secure.phabricator.com/D20812
---
 resources/celerity/map.php                         | 4 ++--
 webroot/rsrc/css/phui/workboards/phui-workcard.css | 4 ++++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index f5ff8827ae..d1233b3684 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -179,7 +179,7 @@ return array(
     'rsrc/css/phui/phui-two-column-view.css' => '01e6991e',
     'rsrc/css/phui/workboards/phui-workboard-color.css' => 'e86de308',
     'rsrc/css/phui/workboards/phui-workboard.css' => '74fc9d98',
-    'rsrc/css/phui/workboards/phui-workcard.css' => '9e9eb0df',
+    'rsrc/css/phui/workboards/phui-workcard.css' => '913441b6',
     'rsrc/css/phui/workboards/phui-workpanel.css' => '3ae89b20',
     'rsrc/css/sprite-login.css' => '18b368a6',
     'rsrc/css/sprite-tokens.css' => 'f1896dc5',
@@ -876,7 +876,7 @@ return array(
     'phui-two-column-view-css' => '01e6991e',
     'phui-workboard-color-css' => 'e86de308',
     'phui-workboard-view-css' => '74fc9d98',
-    'phui-workcard-view-css' => '9e9eb0df',
+    'phui-workcard-view-css' => '913441b6',
     'phui-workpanel-view-css' => '3ae89b20',
     'phuix-action-list-view' => 'c68f183f',
     'phuix-action-view' => 'aaa08f3b',
diff --git a/webroot/rsrc/css/phui/workboards/phui-workcard.css b/webroot/rsrc/css/phui/workboards/phui-workcard.css
index 3c6a798fc8..d9acef12ef 100644
--- a/webroot/rsrc/css/phui/workboards/phui-workcard.css
+++ b/webroot/rsrc/css/phui/workboards/phui-workcard.css
@@ -36,6 +36,10 @@
 
 .phui-workcard .phui-oi-link {
   white-space: normal;
+
+  /* See T13413. This works around a Chrome 77 rendering engine freeze. */
+  word-wrap: normal;
+
   font-weight: normal;
   color: {$blacktext};
   margin-left: 2px;