From 9f3cde4db7654499c1ed565fbf048b8e8710f70a Mon Sep 17 00:00:00 2001
From: Jakub Vrana <jakub@vrana.cz>
Date: Sat, 18 Feb 2017 09:24:56 +0000
Subject: [PATCH 01/25] Fix errors found by PHPStan

Test Plan: None.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D17377
---
 .../calendar/util/CalendarTimeUtil.php          |  4 ++--
 ...abricatorDaemonManagementRestartWorkflow.php |  2 +-
 ...PhabricatorDaemonManagementStartWorkflow.php |  2 +-
 .../DiffusionBrowseQueryConduitAPIMethod.php    |  2 +-
 .../DiffusionRepositoryController.php           |  4 ++--
 .../editor/DiffusionCommitEditEngine.php        |  1 -
 .../herald/HeraldPreCommitContentAdapter.php    |  2 +-
 .../DiffusionLowLevelResolveRefsQuery.php       |  4 ++--
 .../diviner/atomizer/DivinerPHPAtomizer.php     |  2 +-
 .../metamta/storage/PhabricatorMetaMTAMail.php  |  3 +++
 ...ricatorPhortuneManagementInvoiceWorkflow.php |  2 +-
 .../PhabricatorRepositoryDiscoveryEngine.php    |  2 +-
 .../PhabricatorRepositoryMirrorEngine.php       |  2 +-
 .../engine/PhabricatorRepositoryPullEngine.php  |  2 +-
 .../markup/PhabricatorMarkupEngine.php          |  2 +-
 .../PhabricatorCursorPagedPolicyAwareQuery.php  |  1 +
 src/infrastructure/storage/lisk/LiskDAO.php     | 17 +++++++++--------
 src/infrastructure/time/PhabricatorTime.php     |  4 ++--
 .../phui/calendar/PHUICalendarMonthView.php     |  2 +-
 src/view/viewutils.php                          |  2 +-
 20 files changed, 33 insertions(+), 29 deletions(-)

diff --git a/src/applications/calendar/util/CalendarTimeUtil.php b/src/applications/calendar/util/CalendarTimeUtil.php
index 6051ade059..781f6adf7a 100644
--- a/src/applications/calendar/util/CalendarTimeUtil.php
+++ b/src/applications/calendar/util/CalendarTimeUtil.php
@@ -69,7 +69,7 @@ final class CalendarTimeUtil extends Phobject {
 
     $today_epoch = PhabricatorTime::parseLocalTime('today', $user);
     $today = new DateTime('@'.$today_epoch);
-    $today->setTimeZone($timezone);
+    $today->setTimezone($timezone);
 
     if (strtolower($start_day_str) == 'today' ||
         $today->format('l') == $start_day_str) {
@@ -79,7 +79,7 @@ final class CalendarTimeUtil extends Phobject {
         'last '.$start_day_str,
         $user);
       $start_day = new DateTime('@'.$start_epoch);
-      $start_day->setTimeZone($timezone);
+      $start_day->setTimezone($timezone);
     }
     return array(
       'today' => $today,
diff --git a/src/applications/daemon/management/PhabricatorDaemonManagementRestartWorkflow.php b/src/applications/daemon/management/PhabricatorDaemonManagementRestartWorkflow.php
index adb6490f08..3e9f0af8c3 100644
--- a/src/applications/daemon/management/PhabricatorDaemonManagementRestartWorkflow.php
+++ b/src/applications/daemon/management/PhabricatorDaemonManagementRestartWorkflow.php
@@ -47,7 +47,7 @@ final class PhabricatorDaemonManagementRestartWorkflow
 
     return $this->executeStartCommand(
       array(
-        'reserve' => (float)$args->getArg('autoscale-reserve', 0.0),
+        'reserve' => (float)$args->getArg('autoscale-reserve'),
       ));
   }
 
diff --git a/src/applications/daemon/management/PhabricatorDaemonManagementStartWorkflow.php b/src/applications/daemon/management/PhabricatorDaemonManagementStartWorkflow.php
index 9a807357a7..bd23d3bc7d 100644
--- a/src/applications/daemon/management/PhabricatorDaemonManagementStartWorkflow.php
+++ b/src/applications/daemon/management/PhabricatorDaemonManagementStartWorkflow.php
@@ -34,7 +34,7 @@ final class PhabricatorDaemonManagementStartWorkflow
       array(
         'keep-leases' => $args->getArg('keep-leases'),
         'force' => $args->getArg('force'),
-        'reserve' => (float)$args->getArg('autoscale-reserve', 0.0),
+        'reserve' => (float)$args->getArg('autoscale-reserve'),
       ));
   }
 
diff --git a/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php
index ad5aaeb4eb..7ff0f6725a 100644
--- a/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php
+++ b/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php
@@ -52,7 +52,7 @@ final class DiffusionBrowseQueryConduitAPIMethod
           $commit,
           $path);
       } catch (CommandException $e) {
-        $stderr = $e->getStdErr();
+        $stderr = $e->getStderr();
         if (preg_match('/^fatal: Not a valid object name/', $stderr)) {
           // Grab two logs, since the first one is when the object was deleted.
           list($stdout) = $repository->execxLocalCommand(
diff --git a/src/applications/diffusion/controller/DiffusionRepositoryController.php b/src/applications/diffusion/controller/DiffusionRepositoryController.php
index ae6a656bd2..a18f2da1d7 100644
--- a/src/applications/diffusion/controller/DiffusionRepositoryController.php
+++ b/src/applications/diffusion/controller/DiffusionRepositoryController.php
@@ -446,7 +446,7 @@ final class DiffusionRepositoryController extends DiffusionController {
     $header->setHeader(pht('Branches'));
 
     if ($more_branches) {
-      $header->setSubHeader(pht('Showing %d branches.', $limit));
+      $header->setSubheader(pht('Showing %d branches.', $limit));
     }
 
     $button = new PHUIButtonView();
@@ -505,7 +505,7 @@ final class DiffusionRepositoryController extends DiffusionController {
     $header->setHeader(pht('Tags'));
 
     if ($more_tags) {
-      $header->setSubHeader(
+      $header->setSubheader(
         pht('Showing the %d most recent tags.', $tag_limit));
     }
 
diff --git a/src/applications/diffusion/editor/DiffusionCommitEditEngine.php b/src/applications/diffusion/editor/DiffusionCommitEditEngine.php
index 28614579a0..3470d02f92 100644
--- a/src/applications/diffusion/editor/DiffusionCommitEditEngine.php
+++ b/src/applications/diffusion/editor/DiffusionCommitEditEngine.php
@@ -111,7 +111,6 @@ final class DiffusionCommitEditEngine
       ->setValue($object->getAuditorPHIDsForEdit());
 
     $reason = $data->getCommitDetail('autocloseReason', false);
-    $reason = PhabricatorRepository::BECAUSE_AUTOCLOSE_FORCED;
     if ($reason !== false) {
       switch ($reason) {
         case PhabricatorRepository::BECAUSE_REPOSITORY_IMPORTING:
diff --git a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
index 3756651c7e..13c2695fed 100644
--- a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
+++ b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
@@ -46,7 +46,7 @@ final class HeraldPreCommitContentAdapter extends HeraldPreCommitAdapter {
 
     if ($this->changesets instanceof Exception) {
       $ex_class = get_class($this->changesets);
-      $ex_message = $this->changesets->getmessage();
+      $ex_message = $this->changesets->getMessage();
       if ($type === 'name') {
         return array("<{$ex_class}: {$ex_message}>");
       } else {
diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php
index f12dfcf4c0..b649ca65a8 100644
--- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php
+++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php
@@ -271,13 +271,13 @@ final class DiffusionLowLevelResolveRefsQuery
       try {
         list($stdout) = $future->resolvex();
       } catch (CommandException $ex) {
-        if (preg_match('/ambiguous identifier/', $ex->getStdErr())) {
+        if (preg_match('/ambiguous identifier/', $ex->getStderr())) {
           // This indicates that the ref ambiguously matched several things.
           // Eventually, it would be nice to return all of them, but it is
           // unclear how to best do that. For now, treat it as a miss instead.
           continue;
         }
-        if (preg_match('/unknown revision/', $ex->getStdErr())) {
+        if (preg_match('/unknown revision/', $ex->getStderr())) {
           // No matches for this ref.
           continue;
         }
diff --git a/src/applications/diviner/atomizer/DivinerPHPAtomizer.php b/src/applications/diviner/atomizer/DivinerPHPAtomizer.php
index 201055a15c..0a505a8d6b 100644
--- a/src/applications/diviner/atomizer/DivinerPHPAtomizer.php
+++ b/src/applications/diviner/atomizer/DivinerPHPAtomizer.php
@@ -128,7 +128,7 @@ final class DivinerPHPAtomizer extends DivinerAtomizer {
 
   private function parseParams(DivinerAtom $atom, AASTNode $func) {
     $params = $func
-      ->getChildByIndex(3, 'n_DECLARATAION_PARAMETER_LIST')
+      ->getChildOfType(3, 'n_DECLARATAION_PARAMETER_LIST')
       ->selectDescendantsOfType('n_DECLARATION_PARAMETER');
 
     $param_spec = array();
diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php
index 7d8a638401..f2ed8c7721 100644
--- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php
+++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php
@@ -340,6 +340,9 @@ final class PhabricatorMetaMTAMail
     return $this->save();
   }
 
+  /**
+   * @return this
+   */
   public function save() {
     if ($this->getID()) {
       return parent::save();
diff --git a/src/applications/phortune/management/PhabricatorPhortuneManagementInvoiceWorkflow.php b/src/applications/phortune/management/PhabricatorPhortuneManagementInvoiceWorkflow.php
index 673bf812d7..d1fce6dae0 100644
--- a/src/applications/phortune/management/PhabricatorPhortuneManagementInvoiceWorkflow.php
+++ b/src/applications/phortune/management/PhabricatorPhortuneManagementInvoiceWorkflow.php
@@ -86,7 +86,7 @@ final class PhabricatorPhortuneManagementInvoiceWorkflow
 
     $auto_range = $args->getArg('auto-range');
     $last_arg = $args->getArg('last');
-    $next_arg = $args->getARg('next');
+    $next_arg = $args->getArg('next');
 
     if (!$auto_range && !$last_arg && !$next_arg) {
       throw new PhutilArgumentUsageException(
diff --git a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
index d7d72e7710..5b33a9a097 100644
--- a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
+++ b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
@@ -233,7 +233,7 @@ final class PhabricatorRepositoryDiscoveryEngine
           $limit,
           $repository->getSubversionBaseURI($at_rev));
       } catch (CommandException $ex) {
-        $stderr = $ex->getStdErr();
+        $stderr = $ex->getStderr();
         if (preg_match('/(path|File) not found/', $stderr)) {
           // We've gone all the way back through history and this path was not
           // affected by earlier commits.
diff --git a/src/applications/repository/engine/PhabricatorRepositoryMirrorEngine.php b/src/applications/repository/engine/PhabricatorRepositoryMirrorEngine.php
index f07c301bf0..b18f6b1bf8 100644
--- a/src/applications/repository/engine/PhabricatorRepositoryMirrorEngine.php
+++ b/src/applications/repository/engine/PhabricatorRepositoryMirrorEngine.php
@@ -108,7 +108,7 @@ final class PhabricatorRepositoryMirrorEngine
         ->setCWD($repository->getLocalPath())
         ->resolvex();
     } catch (CommandException $ex) {
-      if (preg_match('/no changes found/', $ex->getStdOut())) {
+      if (preg_match('/no changes found/', $ex->getStdout())) {
         // mercurial says nothing changed, but that's good
       } else {
         throw $ex;
diff --git a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php
index d7a403acf8..610bbbfc55 100644
--- a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php
+++ b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php
@@ -471,7 +471,7 @@ final class PhabricatorRepositoryPullEngine
       $future->resolvex();
     } catch (CommandException $ex) {
       $err = $ex->getError();
-      $stdout = $ex->getStdOut();
+      $stdout = $ex->getStdout();
 
       // NOTE: Between versions 2.1 and 2.1.1, Mercurial changed the behavior
       // of "hg pull" to return 1 in case of a successful pull with no changes.
diff --git a/src/infrastructure/markup/PhabricatorMarkupEngine.php b/src/infrastructure/markup/PhabricatorMarkupEngine.php
index 3ce5578f82..3fbf29c294 100644
--- a/src/infrastructure/markup/PhabricatorMarkupEngine.php
+++ b/src/infrastructure/markup/PhabricatorMarkupEngine.php
@@ -111,7 +111,7 @@ final class PhabricatorMarkupEngine extends Phobject {
     }
 
     if (!$keys) {
-      return;
+      return $this;
     }
 
     $objects = array_select_keys($this->objects, $keys);
diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php
index 8d43980122..efb08da223 100644
--- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php
+++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php
@@ -1521,6 +1521,7 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
 
 
   /**
+   * @return this
    * @task edgelogic
    */
   public function withEdgeLogicConstraints($edge_type, array $constraints) {
diff --git a/src/infrastructure/storage/lisk/LiskDAO.php b/src/infrastructure/storage/lisk/LiskDAO.php
index c6712ae873..94eef7d163 100644
--- a/src/infrastructure/storage/lisk/LiskDAO.php
+++ b/src/infrastructure/storage/lisk/LiskDAO.php
@@ -1094,12 +1094,12 @@ abstract class LiskDAO extends Phobject {
 
 
   /**
-   *  Save this object, forcing the query to use INSERT regardless of object
-   *  state.
+   * Save this object, forcing the query to use INSERT regardless of object
+   * state.
    *
-   *  @return this
+   * @return this
    *
-   *  @task   save
+   * @task   save
    */
   public function insert() {
     $this->isEphemeralCheck();
@@ -1108,12 +1108,12 @@ abstract class LiskDAO extends Phobject {
 
 
   /**
-   *  Save this object, forcing the query to use UPDATE regardless of object
-   *  state.
+   * Save this object, forcing the query to use UPDATE regardless of object
+   * state.
    *
-   *  @return this
+   * @return this
    *
-   *  @task   save
+   * @task   save
    */
   public function update() {
     $this->isEphemeralCheck();
@@ -1192,6 +1192,7 @@ abstract class LiskDAO extends Phobject {
    * Internal implementation of INSERT and REPLACE.
    *
    * @param  const   Either "INSERT" or "REPLACE", to force the desired mode.
+   * @return this
    *
    * @task   save
    */
diff --git a/src/infrastructure/time/PhabricatorTime.php b/src/infrastructure/time/PhabricatorTime.php
index 250d8f1e23..b221285d13 100644
--- a/src/infrastructure/time/PhabricatorTime.php
+++ b/src/infrastructure/time/PhabricatorTime.php
@@ -64,7 +64,7 @@ final class PhabricatorTime extends Phobject {
   public static function getTodayMidnightDateTime($viewer) {
     $timezone = new DateTimeZone($viewer->getTimezoneIdentifier());
     $today = new DateTime('@'.time());
-    $today->setTimeZone($timezone);
+    $today->setTimezone($timezone);
     $year = $today->format('Y');
     $month = $today->format('m');
     $day = $today->format('d');
@@ -74,7 +74,7 @@ final class PhabricatorTime extends Phobject {
 
   public static function getDateTimeFromEpoch($epoch, PhabricatorUser $viewer) {
     $datetime = new DateTime('@'.$epoch);
-    $datetime->setTimeZone($viewer->getTimeZone());
+    $datetime->setTimezone($viewer->getTimeZone());
     return $datetime;
   }
 
diff --git a/src/view/phui/calendar/PHUICalendarMonthView.php b/src/view/phui/calendar/PHUICalendarMonthView.php
index 8272eb007f..752a9afa15 100644
--- a/src/view/phui/calendar/PHUICalendarMonthView.php
+++ b/src/view/phui/calendar/PHUICalendarMonthView.php
@@ -577,7 +577,7 @@ final class PHUICalendarMonthView extends AphrontView {
   private function getTodayMidnight() {
     $viewer = $this->getUser();
     $today = new DateTime('@'.time());
-    $today->setTimeZone($viewer->getTimeZone());
+    $today->setTimezone($viewer->getTimeZone());
     $today->setTime(0, 0, 0);
 
     return $today;
diff --git a/src/view/viewutils.php b/src/view/viewutils.php
index ea89399e22..e0629800c7 100644
--- a/src/view/viewutils.php
+++ b/src/view/viewutils.php
@@ -88,7 +88,7 @@ function phabricator_format_local_time($epoch, $user, $format) {
       "raised an exception.", $epoch));
   }
 
-  $date->setTimeZone($zone);
+  $date->setTimezone($zone);
 
   return PhutilTranslator::getInstance()->translateDate($format, $date);
 }

From ab9c1b73b532b761dc3df9fa52c179ce2fade4bf Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 20 Feb 2017 12:48:37 -0800
Subject: [PATCH 02/25] Fix bad JS rendering in "Allow Desktop Notifications"
 workflow

Summary:
See downstream <https://phabricator.kde.org/T5404>. This code was doing some `.firstChild` shenanigans which didn't survive some UI refactoring.

This whole UI is a little iffy but just unbreak it for now.

Test Plan: Allowed and rejected desktop notifications, got largely reasonable UI rendering.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17388
---
 resources/celerity/map.php                     | 18 +++++++++---------
 ...icatorDesktopNotificationsSettingsPanel.php | 12 +++++++++++-
 .../behavior-desktop-notifications-control.js  | 16 ++++++++++------
 3 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index d2953c72dd..6666b7b5a0 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -376,7 +376,7 @@ return array(
     'rsrc/js/application/aphlict/behavior-aphlict-dropdown.js' => 'caade6f2',
     'rsrc/js/application/aphlict/behavior-aphlict-listen.js' => 'fb20ac8d',
     'rsrc/js/application/aphlict/behavior-aphlict-status.js' => '5e2634b9',
-    'rsrc/js/application/aphlict/behavior-desktop-notifications-control.js' => 'edd1ba66',
+    'rsrc/js/application/aphlict/behavior-desktop-notifications-control.js' => 'd5a2d665',
     'rsrc/js/application/calendar/behavior-day-view.js' => '4b3c4443',
     'rsrc/js/application/calendar/behavior-event-all-day.js' => 'b41537c9',
     'rsrc/js/application/calendar/behavior-month-view.js' => 'fe33e256',
@@ -621,7 +621,7 @@ return array(
     'javelin-behavior-dashboard-query-panel-select' => '453c5375',
     'javelin-behavior-dashboard-tab-panel' => 'd4eecc63',
     'javelin-behavior-day-view' => '4b3c4443',
-    'javelin-behavior-desktop-notifications-control' => 'edd1ba66',
+    'javelin-behavior-desktop-notifications-control' => 'd5a2d665',
     'javelin-behavior-detect-timezone' => '4c193c96',
     'javelin-behavior-device' => 'bb1dd507',
     'javelin-behavior-diff-preview-link' => '051c7832',
@@ -2060,6 +2060,13 @@ return array(
       'javelin-dom',
       'javelin-stratcom',
     ),
+    'd5a2d665' => array(
+      'javelin-behavior',
+      'javelin-stratcom',
+      'javelin-dom',
+      'javelin-uri',
+      'phabricator-notification',
+    ),
     'd6a7e717' => array(
       'multirow-row-manager',
       'javelin-install',
@@ -2154,13 +2161,6 @@ return array(
       'javelin-dom',
       'phabricator-draggable-list',
     ),
-    'edd1ba66' => array(
-      'javelin-behavior',
-      'javelin-stratcom',
-      'javelin-dom',
-      'javelin-uri',
-      'phabricator-notification',
-    ),
     'edf8a145' => array(
       'javelin-behavior',
       'javelin-uri',
diff --git a/src/applications/settings/panel/PhabricatorDesktopNotificationsSettingsPanel.php b/src/applications/settings/panel/PhabricatorDesktopNotificationsSettingsPanel.php
index e987fed4ab..c08a833f1b 100644
--- a/src/applications/settings/panel/PhabricatorDesktopNotificationsSettingsPanel.php
+++ b/src/applications/settings/panel/PhabricatorDesktopNotificationsSettingsPanel.php
@@ -88,15 +88,25 @@ final class PhabricatorDesktopNotificationsSettingsPanel
           'for this Phabricator instance. Consult your browser settings / '.
           'documentation to figure out how to clear this setting, do so, '.
           'and then re-visit this page to grant permission.'));
+
+    $message_id = celerity_generate_unique_node_id();
+
+    $message_container = phutil_tag(
+      'span',
+      array(
+        'id' => $message_id,
+      ));
+
     $status_box = id(new PHUIInfoView())
       ->setSeverity(PHUIInfoView::SEVERITY_NOTICE)
       ->setID($status_id)
       ->setIsHidden(true)
-      ->appendChild($accept_ask);
+      ->appendChild($message_container);
 
     $control_config = array(
        'controlID' => $control_id,
        'statusID' => $status_id,
+       'messageID' => $message_id,
        'browserStatusID' => $browser_status_id,
        'defaultMode' => 0,
        'desktopMode' => 1,
diff --git a/webroot/rsrc/js/application/aphlict/behavior-desktop-notifications-control.js b/webroot/rsrc/js/application/aphlict/behavior-desktop-notifications-control.js
index d3cedc8615..3a9b028ca3 100644
--- a/webroot/rsrc/js/application/aphlict/behavior-desktop-notifications-control.js
+++ b/webroot/rsrc/js/application/aphlict/behavior-desktop-notifications-control.js
@@ -19,22 +19,26 @@ JX.behavior('desktop-notifications-control', function(config, statics) {
     return el;
   }
   function updateFormStatus(permission) {
-    var statusEl = findEl(config.statusID);
-    if (!statusEl) {
+    var status_node = findEl(config.statusID);
+    if (!status_node) {
       return;
     }
+
+    var message_node = JX.$(config.messageID);
+
     switch (permission) {
       case 'default':
-        JX.DOM.setContent(statusEl.firstChild, config.cancelAsk);
+        JX.DOM.setContent(message_node, config.cancelAsk);
         break;
       case 'granted':
-        JX.DOM.setContent(statusEl.firstChild, config.grantedAsk);
+        JX.DOM.setContent(message_node, config.grantedAsk);
         break;
       case 'denied':
-        JX.DOM.setContent(statusEl.firstChild, config.deniedAsk);
+        JX.DOM.setContent(message_node, config.deniedAsk);
         break;
     }
-    JX.DOM.show(statusEl);
+
+    JX.DOM.show(status_node);
   }
 
   function updateBrowserStatus(permission) {

From 89ce42c15c0352635ef401a6fc2032e3dab215a1 Mon Sep 17 00:00:00 2001
From: Chad Little <chad@phacility.com>
Date: Tue, 21 Feb 2017 14:18:17 -0800
Subject: [PATCH 03/25] Update people hovercard UI

Summary: Removes Badges, they felt awkward. Updates UI, larger image, better layout, more icons.

Test Plan: Review numerous layouts with fancy new search tool.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17391
---
 resources/celerity/map.php                    |  18 +--
 .../PeopleHovercardEngineExtension.php        |   1 -
 ...habricatorPeoplePictureProfileMenuItem.php |   9 +-
 .../people/view/PhabricatorUserCardView.php   | 112 ++++++++----------
 .../application/project/project-card-view.css |  52 +++++++-
 webroot/rsrc/css/phui/phui-badge.css          |   5 -
 webroot/rsrc/css/phui/phui-hovercard.css      |   6 +-
 webroot/rsrc/css/phui/phui-icon.css           |   8 +-
 8 files changed, 130 insertions(+), 81 deletions(-)

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index 6666b7b5a0..3959c984e8 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -9,7 +9,7 @@ return array(
   'names' => array(
     'conpherence.pkg.css' => 'a520d619',
     'conpherence.pkg.js' => '6249a1cf',
-    'core.pkg.css' => '0d7ecd3b',
+    'core.pkg.css' => 'cc0250e8',
     'core.pkg.js' => '1fa7c0c5',
     'darkconsole.pkg.js' => 'e7393ebb',
     'differential.pkg.css' => '4815647b',
@@ -96,7 +96,7 @@ return array(
     'rsrc/css/application/policy/policy-transaction-detail.css' => '82100a43',
     'rsrc/css/application/policy/policy.css' => '957ea14c',
     'rsrc/css/application/ponder/ponder-view.css' => 'fbd45f96',
-    'rsrc/css/application/project/project-card-view.css' => 'f25746f5',
+    'rsrc/css/application/project/project-card-view.css' => '77219296',
     'rsrc/css/application/project/project-view.css' => '9f6ce0e1',
     'rsrc/css/application/releeph/releeph-core.css' => '9b3c5733',
     'rsrc/css/application/releeph/releeph-preview-branch.css' => 'b7a6f4a5',
@@ -130,7 +130,7 @@ return array(
     'rsrc/css/phui/object-item/phui-oi-simple-ui.css' => 'a8beebea',
     'rsrc/css/phui/phui-action-list.css' => 'f980c059',
     'rsrc/css/phui/phui-action-panel.css' => '91c7b835',
-    'rsrc/css/phui/phui-badge.css' => '3baef8db',
+    'rsrc/css/phui/phui-badge.css' => '22fe77f8',
     'rsrc/css/phui/phui-basic-nav-view.css' => 'a0705f53',
     'rsrc/css/phui/phui-big-info-view.css' => 'bd903741',
     'rsrc/css/phui/phui-box.css' => '269cbc99',
@@ -150,9 +150,9 @@ return array(
     'rsrc/css/phui/phui-form.css' => '5815af7b',
     'rsrc/css/phui/phui-head-thing.css' => 'fd311e5f',
     'rsrc/css/phui/phui-header-view.css' => 'fef6a54e',
-    'rsrc/css/phui/phui-hovercard.css' => 'e904f5dc',
+    'rsrc/css/phui/phui-hovercard.css' => 'ae091fc5',
     'rsrc/css/phui/phui-icon-set-selector.css' => '87db8fee',
-    'rsrc/css/phui/phui-icon.css' => '09f46dd9',
+    'rsrc/css/phui/phui-icon.css' => '12b387a1',
     'rsrc/css/phui/phui-image-mask.css' => 'a8498f9c',
     'rsrc/css/phui/phui-info-panel.css' => '27ea50a1',
     'rsrc/css/phui/phui-info-view.css' => 'ec92802a',
@@ -837,7 +837,7 @@ return array(
     'phrequent-css' => 'ffc185ad',
     'phriction-document-css' => '4282e4ad',
     'phui-action-panel-css' => '91c7b835',
-    'phui-badge-view-css' => '3baef8db',
+    'phui-badge-view-css' => '22fe77f8',
     'phui-basic-nav-view-css' => 'a0705f53',
     'phui-big-info-view-css' => 'bd903741',
     'phui-box-css' => '269cbc99',
@@ -863,9 +863,9 @@ return array(
     'phui-head-thing-view-css' => 'fd311e5f',
     'phui-header-view-css' => 'fef6a54e',
     'phui-hovercard' => '1bd28176',
-    'phui-hovercard-view-css' => 'e904f5dc',
+    'phui-hovercard-view-css' => 'ae091fc5',
     'phui-icon-set-selector-css' => '87db8fee',
-    'phui-icon-view-css' => '09f46dd9',
+    'phui-icon-view-css' => '12b387a1',
     'phui-image-mask-css' => 'a8498f9c',
     'phui-info-panel-css' => '27ea50a1',
     'phui-info-view-css' => 'ec92802a',
@@ -905,7 +905,7 @@ return array(
     'policy-edit-css' => '815c66f7',
     'policy-transaction-detail-css' => '82100a43',
     'ponder-view-css' => 'fbd45f96',
-    'project-card-view-css' => 'f25746f5',
+    'project-card-view-css' => '77219296',
     'project-view-css' => '9f6ce0e1',
     'releeph-core' => '9b3c5733',
     'releeph-preview-branch' => 'b7a6f4a5',
diff --git a/src/applications/people/engineextension/PeopleHovercardEngineExtension.php b/src/applications/people/engineextension/PeopleHovercardEngineExtension.php
index 22d63eda8a..507715d21a 100644
--- a/src/applications/people/engineextension/PeopleHovercardEngineExtension.php
+++ b/src/applications/people/engineextension/PeopleHovercardEngineExtension.php
@@ -27,7 +27,6 @@ final class PeopleHovercardEngineExtension
       ->needAvailability(true)
       ->needProfileImage(true)
       ->needProfile(true)
-      ->needBadges(true)
       ->execute();
     $users = mpull($users, null, 'getPHID');
 
diff --git a/src/applications/people/menuitem/PhabricatorPeoplePictureProfileMenuItem.php b/src/applications/people/menuitem/PhabricatorPeoplePictureProfileMenuItem.php
index 89fb7b9681..938b7cf60a 100644
--- a/src/applications/people/menuitem/PhabricatorPeoplePictureProfileMenuItem.php
+++ b/src/applications/people/menuitem/PhabricatorPeoplePictureProfileMenuItem.php
@@ -36,6 +36,13 @@ final class PhabricatorPeoplePictureProfileMenuItem
 
     $picture = $user->getProfileImageURI();
     $name = $user->getUsername();
+
+    $classes = array();
+    $classes[] = 'people-menu-image';
+    if ($user->getIsDisabled()) {
+      $classes[] = 'phui-image-disabled';
+    }
+
     $href = urisprintf(
       '/p/%s/',
       $user->getUsername());
@@ -44,7 +51,7 @@ final class PhabricatorPeoplePictureProfileMenuItem
       'img',
       array(
         'src' => $picture,
-        'class' => 'people-menu-image',
+        'class' => implode(' ', $classes),
       ));
 
     $can_edit = PhabricatorPolicyFilter::hasCapability(
diff --git a/src/applications/people/view/PhabricatorUserCardView.php b/src/applications/people/view/PhabricatorUserCardView.php
index bb0ab9eebc..4c45549b77 100644
--- a/src/applications/people/view/PhabricatorUserCardView.php
+++ b/src/applications/people/view/PhabricatorUserCardView.php
@@ -33,9 +33,7 @@ final class PhabricatorUserCardView extends AphrontTagView {
     $classes[] = 'project-card-view';
 
     if ($this->profile->getIsDisabled()) {
-      $classes[] = 'project-card-grey';
-    } else {
-      $classes[] = 'project-card-blue';
+      $classes[] = 'project-card-disabled';
     }
 
     return array(
@@ -56,10 +54,12 @@ final class PhabricatorUserCardView extends AphrontTagView {
     // the most important tag. Users can click through to the profile to get
     // more details.
 
+    $classes = array();
     if ($user->getIsDisabled()) {
       $tag_icon = 'fa-ban';
       $tag_title = pht('Disabled');
       $tag_shade = PHUITagView::COLOR_RED;
+      $classes[] = 'phui-image-disabled';
     } else if (!$user->getIsApproved()) {
       $tag_icon = 'fa-ban';
       $tag_title = pht('Unapproved Account');
@@ -87,47 +87,60 @@ final class PhabricatorUserCardView extends AphrontTagView {
       $tag->setShade($tag_shade);
     }
 
-    $header = id(new PHUIHeaderView())
-      ->setHeader($user->getFullName())
-      ->addTag($tag)
-      ->setUser($viewer)
-      ->setImage($picture);
-
     $body = array();
 
+    /* TODO: Replace with Conpherence Availability if we ship it */
     $body[] = $this->addItem(
-      pht('User Since'),
+      'fa-user-plus',
       phabricator_date($user->getDateCreated(), $viewer));
 
     if (PhabricatorApplication::isClassInstalledForViewer(
         'PhabricatorCalendarApplication',
         $viewer)) {
       $body[] = $this->addItem(
-        pht('Availability'),
+        'fa-calendar-o',
         id(new PHUIUserAvailabilityView())
           ->setViewer($viewer)
           ->setAvailableUser($user));
     }
 
-    $badges = $this->buildBadges($user, $viewer);
-    if ($badges) {
-      $badges = id(new PHUIBadgeBoxView())
-        ->addItems($badges)
-        ->setCollapsed(true);
-      $body[] = phutil_tag(
-        'div',
-        array(
-          'class' => 'phui-hovercard-body-item hovercard-badges',
-        ),
-        $badges);
-    }
+    $classes[] = 'project-card-image';
+    $image = phutil_tag(
+      'img',
+      array(
+        'src' => $picture,
+        'class' => implode(' ', $classes),
+      ));
 
-    $body = phutil_tag(
+    $href = urisprintf(
+      '/p/%s/',
+      $user->getUsername());
+
+    $image = phutil_tag(
+      'a',
+      array(
+        'href' => $href,
+      ),
+      $image);
+
+    $name = phutil_tag_div('project-card-name',
+      $user->getRealname());
+    $username = phutil_tag_div('project-card-username',
+      '@'.$user->getUsername());
+    $tag = phutil_tag_div('phui-header-subheader',
+      $tag);
+
+    $header = phutil_tag(
       'div',
       array(
-        'class' => 'project-card-body',
+        'class' => 'project-card-header',
       ),
-      $body);
+      array(
+        $name,
+        $username,
+        $tag,
+        $body,
+      ));
 
     $card = phutil_tag(
       'div',
@@ -135,47 +148,24 @@ final class PhabricatorUserCardView extends AphrontTagView {
         'class' => 'project-card-inner',
       ),
       array(
+        $image,
         $header,
-        $body,
       ));
 
     return $card;
   }
 
-  private function addItem($label, $value) {
-    $item = array(
-      phutil_tag('strong', array(), $label),
-      ': ',
-      phutil_tag('span', array(), $value),
-    );
-    return phutil_tag_div('project-card-item', $item);
-  }
-
-  private function buildBadges(
-    PhabricatorUser $user,
-    $viewer) {
-
-    $class = 'PhabricatorBadgesApplication';
-    $items = array();
-
-    if (PhabricatorApplication::isClassInstalledForViewer($class, $viewer)) {
-      $badge_phids = $user->getBadgePHIDs();
-      if ($badge_phids) {
-        $badges = id(new PhabricatorBadgesQuery())
-          ->setViewer($viewer)
-          ->withPHIDs($badge_phids)
-          ->withStatuses(array(PhabricatorBadgesBadge::STATUS_ACTIVE))
-          ->execute();
-
-        foreach ($badges as $badge) {
-          $items[] = id(new PHUIBadgeMiniView())
-            ->setIcon($badge->getIcon())
-            ->setHeader($badge->getName())
-            ->setQuality($badge->getQuality());
-        }
-      }
-    }
-    return $items;
+  private function addItem($icon, $value) {
+    $icon = id(new PHUIIconView())
+      ->addClass('project-card-item-icon')
+      ->setIcon($icon);
+    $text = phutil_tag(
+      'span',
+      array(
+        'class' => 'project-card-item-text',
+      ),
+      $value);
+    return phutil_tag_div('project-card-item', array($icon, $text));
   }
 
 }
diff --git a/webroot/rsrc/css/application/project/project-card-view.css b/webroot/rsrc/css/application/project/project-card-view.css
index 5f5155c130..0ad25c5420 100644
--- a/webroot/rsrc/css/application/project/project-card-view.css
+++ b/webroot/rsrc/css/application/project/project-card-view.css
@@ -9,7 +9,8 @@
   border: 1px solid {$lightblueborder};
   border-radius: 3px;
   box-shadow: {$dropshadow};
-  width: 380px;
+  width: 420px;
+  position: relative;
 }
 
 .project-card-view .phui-header-shell {
@@ -35,6 +36,53 @@
   display: block;
 }
 
+.project-card-view .project-card-image {
+  height: 140px;
+  width: 140px;
+  margin: 6px;
+  border-radius: 3px;
+}
+
+.project-card-view .project-card-item div {
+  display: inline;
+}
+
+.project-card-view .project-card-item {
+  margin-bottom: 2px;
+}
+
+.project-card-view .project-card-item-text {
+  color: {$greytext};
+}
+
+.project-card-view .project-card-item-icon {
+  width: 20px;
+}
+
+.project-card-view .project-card-header {
+  position: absolute;
+  top: 12px;
+  left: 158px;
+}
+
+.project-card-header .project-card-name {
+  font-size: 20px;
+  font-family: 'Aleo', {$fontfamily};
+  font-weight: bold;
+  color: #000;
+  margin-bottom: 2px;
+  text-overflow: ellipsis;
+  white-space: nowrap;
+  width: 250px;
+  overflow: hidden;
+}
+
+.project-card-header .project-card-username {
+  font-size: 14px;
+  color: {$bluetext};
+  margin-bottom: 12px;
+}
+
 .project-card-view .phui-header-shell .phui-header-col1 {
   vertical-align: top;
   width: 64px;
@@ -65,7 +113,7 @@
 }
 
 .project-card-view .project-card-body {
-  padding: 0 12px 12px 76px;
+  padding: 0 12px 12px 12px;
   color: {$darkbluetext};
 }
 
diff --git a/webroot/rsrc/css/phui/phui-badge.css b/webroot/rsrc/css/phui/phui-badge.css
index b702d3e6c5..a38dff9154 100644
--- a/webroot/rsrc/css/phui/phui-badge.css
+++ b/webroot/rsrc/css/phui/phui-badge.css
@@ -183,11 +183,6 @@
   line-height: 24px;
   text-align: center;
   display: inline-block;
-  opacity: 0.7;
-}
-
-.phui-badge-mini:hover {
-  opacity: 1;
 }
 
 .phui-badge-mini .phui-icon-view {
diff --git a/webroot/rsrc/css/phui/phui-hovercard.css b/webroot/rsrc/css/phui/phui-hovercard.css
index 6431c01cdf..bf81096a71 100644
--- a/webroot/rsrc/css/phui/phui-hovercard.css
+++ b/webroot/rsrc/css/phui/phui-hovercard.css
@@ -81,7 +81,11 @@
 }
 
 .hovercard-badges {
-  margin: 8px 0 0 0;
+  margin: 6px 0 0 0;
+  padding: 4px;
+  background: {$page.background};
+  border-bottom-left-radius: 3px;
+  border-bottom-right-radius: 3px;
 }
 
 .hovercard-badges .phui-badge-flex-item {
diff --git a/webroot/rsrc/css/phui/phui-icon.css b/webroot/rsrc/css/phui/phui-icon.css
index 142208c110..22b33bca11 100644
--- a/webroot/rsrc/css/phui/phui-icon.css
+++ b/webroot/rsrc/css/phui/phui-icon.css
@@ -31,7 +31,7 @@
 }
 
 .phui-icon-has-text:before {
-  margin-right: 4px;
+  margin-right: 6px;
 }
 
 a.phui-icon-view:hover {
@@ -39,6 +39,12 @@ a.phui-icon-view:hover {
   color: {$sky};
 }
 
+img.phui-image-disabled {
+  opacity: .8;
+  -webkit-filter: grayscale(100%);
+  filter: grayscale(100%);
+}
+
 /* - Icon in a Circle ------------------------------------------------------- */
 
 .phui-icon-circle {

From e2868a0da24c96059211d1ed54df3ebad86b53bd Mon Sep 17 00:00:00 2001
From: Chad Little <chad@phacility.com>
Date: Tue, 21 Feb 2017 14:50:31 -0800
Subject: [PATCH 04/25] Remove ability to edit Badge forms

Summary: Ref T12270. Remove the EditEngine form configuration option on Badges.

Test Plan: View edit page, don't see configure form.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12270

Differential Revision: https://secure.phabricator.com/D17392
---
 .../badges/editor/PhabricatorBadgesEditEngine.php             | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/applications/badges/editor/PhabricatorBadgesEditEngine.php b/src/applications/badges/editor/PhabricatorBadgesEditEngine.php
index 937dc1bd0e..58cccb1a89 100644
--- a/src/applications/badges/editor/PhabricatorBadgesEditEngine.php
+++ b/src/applications/badges/editor/PhabricatorBadgesEditEngine.php
@@ -21,6 +21,10 @@ final class PhabricatorBadgesEditEngine
     return pht('Configure creation and editing forms in Badges.');
   }
 
+  public function isEngineConfigurable() {
+    return false;
+  }
+
   protected function newEditableObject() {
     return PhabricatorBadgesBadge::initializeNewBadge($this->getViewer());
   }

From aaa81b48349d4ec54d178647a6728d9dc96e89d5 Mon Sep 17 00:00:00 2001
From: Chad Little <chad@phacility.com>
Date: Wed, 22 Feb 2017 08:28:24 -0800
Subject: [PATCH 05/25] Center Pager buttons

Summary: Fixes T12305. Centers the buttons, which I prefer anyways.

Test Plan: Review buttons, now centered.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12305

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

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index 3959c984e8..4abc5e307c 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -9,7 +9,7 @@ return array(
   'names' => array(
     'conpherence.pkg.css' => 'a520d619',
     'conpherence.pkg.js' => '6249a1cf',
-    'core.pkg.css' => 'cc0250e8',
+    'core.pkg.css' => '12c56bd9',
     'core.pkg.js' => '1fa7c0c5',
     'darkconsole.pkg.js' => 'e7393ebb',
     'differential.pkg.css' => '4815647b',
@@ -160,7 +160,7 @@ return array(
     'rsrc/css/phui/phui-lightbox.css' => '0a035e40',
     'rsrc/css/phui/phui-list.css' => '9da2aa00',
     'rsrc/css/phui/phui-object-box.css' => '8b289e3d',
-    'rsrc/css/phui/phui-pager.css' => 'bea33d23',
+    'rsrc/css/phui/phui-pager.css' => '77d8a794',
     'rsrc/css/phui/phui-pinboard-view.css' => '2495140e',
     'rsrc/css/phui/phui-property-list-view.css' => '2dc7993f',
     'rsrc/css/phui/phui-remarkup-preview.css' => '1a8f2591',
@@ -880,7 +880,7 @@ return array(
     'phui-oi-flush-ui-css' => '9d9685d6',
     'phui-oi-list-view-css' => '5c383524',
     'phui-oi-simple-ui-css' => 'a8beebea',
-    'phui-pager-css' => 'bea33d23',
+    'phui-pager-css' => '77d8a794',
     'phui-pinboard-view-css' => '2495140e',
     'phui-property-list-view-css' => '2dc7993f',
     'phui-remarkup-preview-css' => '1a8f2591',
diff --git a/webroot/rsrc/css/phui/phui-pager.css b/webroot/rsrc/css/phui/phui-pager.css
index ff59ad0c77..590b8514ce 100644
--- a/webroot/rsrc/css/phui/phui-pager.css
+++ b/webroot/rsrc/css/phui/phui-pager.css
@@ -4,5 +4,5 @@
 
 .phui-pager-view {
   clear: both;
-  text-align: right;
+  text-align: center;
 }

From 254ee82a0c9d4bbc0d0917aa892e77c55116aa56 Mon Sep 17 00:00:00 2001
From: Chad Little <chad@phacility.com>
Date: Wed, 22 Feb 2017 12:30:57 -0800
Subject: [PATCH 06/25] Hide Conpherence durable column when printing

Summary: Fixes T12303. Hides column.

Test Plan: Print a page with `__print__=1`

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12303

Differential Revision: https://secure.phabricator.com/D17395
---
 resources/celerity/map.php                                  | 6 +++---
 webroot/rsrc/css/application/conpherence/durable-column.css | 4 +++-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index 4abc5e307c..836194f420 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -7,7 +7,7 @@
  */
 return array(
   'names' => array(
-    'conpherence.pkg.css' => 'a520d619',
+    'conpherence.pkg.css' => '6875302f',
     'conpherence.pkg.js' => '6249a1cf',
     'core.pkg.css' => '12c56bd9',
     'core.pkg.js' => '1fa7c0c5',
@@ -45,7 +45,7 @@ return array(
     'rsrc/css/application/config/config-template.css' => '8f18fa41',
     'rsrc/css/application/config/setup-issue.css' => 'f794cfc3',
     'rsrc/css/application/config/unhandled-exception.css' => '4c96257a',
-    'rsrc/css/application/conpherence/durable-column.css' => 'd82e130c',
+    'rsrc/css/application/conpherence/durable-column.css' => '292c71f0',
     'rsrc/css/application/conpherence/header-pane.css' => 'db93ebc6',
     'rsrc/css/application/conpherence/menu.css' => '3d8e5c9c',
     'rsrc/css/application/conpherence/message-pane.css' => 'b085d40d',
@@ -563,7 +563,7 @@ return array(
     'conduit-api-css' => '7bc725c4',
     'config-options-css' => '0ede4c9b',
     'config-page-css' => 'c1d5121b',
-    'conpherence-durable-column-view' => 'd82e130c',
+    'conpherence-durable-column-view' => '292c71f0',
     'conpherence-header-pane-css' => 'db93ebc6',
     'conpherence-menu-css' => '3d8e5c9c',
     'conpherence-message-pane-css' => 'b085d40d',
diff --git a/webroot/rsrc/css/application/conpherence/durable-column.css b/webroot/rsrc/css/application/conpherence/durable-column.css
index d141115b73..2b0ab85e44 100644
--- a/webroot/rsrc/css/application/conpherence/durable-column.css
+++ b/webroot/rsrc/css/application/conpherence/durable-column.css
@@ -18,7 +18,9 @@
   box-shadow: 0px 1px 8px rgba(55,55,55, .3);
 }
 
-.device .conpherence-durable-column {
+.device .conpherence-durable-column,
+.printable .conpherence-durable-column,
+!print .conpherence-durable-column {
   display: none;
 }
 

From bf44210dc8f1e2bd437976161990721e91c4a6e8 Mon Sep 17 00:00:00 2001
From: Chad Little <chad@phacility.com>
Date: Wed, 22 Feb 2017 09:25:36 -0800
Subject: [PATCH 07/25] Reduce application search engine results list for
 Dashboards

Summary: Ref T10390. Simplifies dropdown by rolling out canUseInPanel in useless panels

Test Plan: Add a query panel, see less options.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T10390

Differential Revision: https://secure.phabricator.com/D17341
---
 .../audit/query/PhabricatorCommitSearchEngine.php           | 2 +-
 .../auth/query/PhabricatorAuthInviteSearchEngine.php        | 6 +++++-
 .../badges/query/PhabricatorBadgesSearchEngine.php          | 2 +-
 .../query/PhabricatorCalendarExportSearchEngine.php         | 4 ++++
 .../query/PhabricatorCalendarImportLogSearchEngine.php      | 4 ++++
 .../query/PhabricatorCalendarImportSearchEngine.php         | 4 ++++
 .../conduit/query/PhabricatorConduitLogSearchEngine.php     | 4 ++++
 .../conduit/query/PhabricatorConduitSearchEngine.php        | 4 ++++
 .../conpherence/query/ConpherenceThreadSearchEngine.php     | 2 +-
 .../query/PhabricatorDashboardPanelSearchEngine.php         | 4 ++++
 .../dashboard/query/PhabricatorDashboardSearchEngine.php    | 4 ++++
 src/applications/diviner/query/DivinerAtomSearchEngine.php  | 4 ++++
 .../files/query/PhabricatorFileSearchEngine.php             | 4 ++++
 .../herald/query/HeraldTranscriptSearchEngine.php           | 4 ++++
 .../maniphest/query/ManiphestTaskSearchEngine.php           | 2 +-
 .../metamta/query/PhabricatorMetaMTAMailSearchEngine.php    | 4 ++++
 .../query/PhabricatorOAuthServerClientSearchEngine.php      | 4 ++++
 .../owners/query/PhabricatorOwnersPackageSearchEngine.php   | 4 ++++
 .../query/PhabricatorPackagesPackageSearchEngine.php        | 4 ++++
 .../query/PhabricatorPackagesPublisherSearchEngine.php      | 4 ++++
 .../query/PhabricatorPackagesVersionSearchEngine.php        | 4 ++++
 src/applications/phrequent/query/PhrequentSearchEngine.php  | 5 +++++
 .../phurl/query/PhabricatorPhurlURLSearchEngine.php         | 2 +-
 .../project/query/PhabricatorProjectColumnSearchEngine.php  | 5 ++++-
 .../releeph/query/ReleephBranchSearchEngine.php             | 4 ++++
 .../releeph/query/ReleephProductSearchEngine.php            | 4 ++++
 .../releeph/query/ReleephRequestSearchEngine.php            | 4 ++++
 .../query/PhabricatorSearchApplicationSearchEngine.php      | 4 ++++
 .../query/PhabricatorEditEngineSearchEngine.php             | 4 ++++
 .../workers/query/PhabricatorWorkerBulkJobSearchEngine.php  | 6 +++++-
 30 files changed, 108 insertions(+), 8 deletions(-)

diff --git a/src/applications/audit/query/PhabricatorCommitSearchEngine.php b/src/applications/audit/query/PhabricatorCommitSearchEngine.php
index 7f429dd596..e1c1120608 100644
--- a/src/applications/audit/query/PhabricatorCommitSearchEngine.php
+++ b/src/applications/audit/query/PhabricatorCommitSearchEngine.php
@@ -4,7 +4,7 @@ final class PhabricatorCommitSearchEngine
   extends PhabricatorApplicationSearchEngine {
 
   public function getResultTypeDescription() {
-    return pht('Commits');
+    return pht('Diffusion Commits');
   }
 
   public function getApplicationClassName() {
diff --git a/src/applications/auth/query/PhabricatorAuthInviteSearchEngine.php b/src/applications/auth/query/PhabricatorAuthInviteSearchEngine.php
index d07ed15d41..e439dd9fb8 100644
--- a/src/applications/auth/query/PhabricatorAuthInviteSearchEngine.php
+++ b/src/applications/auth/query/PhabricatorAuthInviteSearchEngine.php
@@ -4,13 +4,17 @@ final class PhabricatorAuthInviteSearchEngine
   extends PhabricatorApplicationSearchEngine {
 
   public function getResultTypeDescription() {
-    return pht('Email Invites');
+    return pht('Auth Email Invites');
   }
 
   public function getApplicationClassName() {
     return 'PhabricatorAuthApplication';
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   public function buildSavedQueryFromRequest(AphrontRequest $request) {
     $saved = new PhabricatorSavedQuery();
 
diff --git a/src/applications/badges/query/PhabricatorBadgesSearchEngine.php b/src/applications/badges/query/PhabricatorBadgesSearchEngine.php
index fc2bf7ef1e..b57eae5910 100644
--- a/src/applications/badges/query/PhabricatorBadgesSearchEngine.php
+++ b/src/applications/badges/query/PhabricatorBadgesSearchEngine.php
@@ -4,7 +4,7 @@ final class PhabricatorBadgesSearchEngine
   extends PhabricatorApplicationSearchEngine {
 
   public function getResultTypeDescription() {
-    return pht('Badge');
+    return pht('Badges');
   }
 
   public function getApplicationClassName() {
diff --git a/src/applications/calendar/query/PhabricatorCalendarExportSearchEngine.php b/src/applications/calendar/query/PhabricatorCalendarExportSearchEngine.php
index 4a65bfd099..032cab0c02 100644
--- a/src/applications/calendar/query/PhabricatorCalendarExportSearchEngine.php
+++ b/src/applications/calendar/query/PhabricatorCalendarExportSearchEngine.php
@@ -11,6 +11,10 @@ final class PhabricatorCalendarExportSearchEngine
     return 'PhabricatorCalendarApplication';
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   public function newQuery() {
     $viewer = $this->requireViewer();
 
diff --git a/src/applications/calendar/query/PhabricatorCalendarImportLogSearchEngine.php b/src/applications/calendar/query/PhabricatorCalendarImportLogSearchEngine.php
index 81a1256fca..99f292f9a8 100644
--- a/src/applications/calendar/query/PhabricatorCalendarImportLogSearchEngine.php
+++ b/src/applications/calendar/query/PhabricatorCalendarImportLogSearchEngine.php
@@ -11,6 +11,10 @@ final class PhabricatorCalendarImportLogSearchEngine
     return 'PhabricatorCalendarApplication';
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   public function newQuery() {
     return new PhabricatorCalendarImportLogQuery();
   }
diff --git a/src/applications/calendar/query/PhabricatorCalendarImportSearchEngine.php b/src/applications/calendar/query/PhabricatorCalendarImportSearchEngine.php
index 75252b6dac..a5e44812ea 100644
--- a/src/applications/calendar/query/PhabricatorCalendarImportSearchEngine.php
+++ b/src/applications/calendar/query/PhabricatorCalendarImportSearchEngine.php
@@ -11,6 +11,10 @@ final class PhabricatorCalendarImportSearchEngine
     return 'PhabricatorCalendarApplication';
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   public function newQuery() {
     return new PhabricatorCalendarImportQuery();
   }
diff --git a/src/applications/conduit/query/PhabricatorConduitLogSearchEngine.php b/src/applications/conduit/query/PhabricatorConduitLogSearchEngine.php
index 68f7060507..20d034168d 100644
--- a/src/applications/conduit/query/PhabricatorConduitLogSearchEngine.php
+++ b/src/applications/conduit/query/PhabricatorConduitLogSearchEngine.php
@@ -11,6 +11,10 @@ final class PhabricatorConduitLogSearchEngine
     return 'PhabricatorConduitApplication';
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   public function newQuery() {
     return new PhabricatorConduitLogQuery();
   }
diff --git a/src/applications/conduit/query/PhabricatorConduitSearchEngine.php b/src/applications/conduit/query/PhabricatorConduitSearchEngine.php
index 787c2154d5..9d244401e6 100644
--- a/src/applications/conduit/query/PhabricatorConduitSearchEngine.php
+++ b/src/applications/conduit/query/PhabricatorConduitSearchEngine.php
@@ -11,6 +11,10 @@ final class PhabricatorConduitSearchEngine
     return 'PhabricatorConduitApplication';
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   public function getPageSize(PhabricatorSavedQuery $saved) {
     return PHP_INT_MAX - 1;
   }
diff --git a/src/applications/conpherence/query/ConpherenceThreadSearchEngine.php b/src/applications/conpherence/query/ConpherenceThreadSearchEngine.php
index 3129028c2e..4e0a89d266 100644
--- a/src/applications/conpherence/query/ConpherenceThreadSearchEngine.php
+++ b/src/applications/conpherence/query/ConpherenceThreadSearchEngine.php
@@ -4,7 +4,7 @@ final class ConpherenceThreadSearchEngine
   extends PhabricatorApplicationSearchEngine {
 
   public function getResultTypeDescription() {
-    return pht('Rooms');
+    return pht('Conpherence Rooms');
   }
 
   public function getApplicationClassName() {
diff --git a/src/applications/dashboard/query/PhabricatorDashboardPanelSearchEngine.php b/src/applications/dashboard/query/PhabricatorDashboardPanelSearchEngine.php
index 89e77997d8..87e908b9c2 100644
--- a/src/applications/dashboard/query/PhabricatorDashboardPanelSearchEngine.php
+++ b/src/applications/dashboard/query/PhabricatorDashboardPanelSearchEngine.php
@@ -15,6 +15,10 @@ final class PhabricatorDashboardPanelSearchEngine
     return new PhabricatorDashboardPanelQuery();
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   protected function buildQueryFromParameters(array $map) {
     $query = $this->newQuery();
     if ($map['status']) {
diff --git a/src/applications/dashboard/query/PhabricatorDashboardSearchEngine.php b/src/applications/dashboard/query/PhabricatorDashboardSearchEngine.php
index 6061a0ab8a..a854b066f8 100644
--- a/src/applications/dashboard/query/PhabricatorDashboardSearchEngine.php
+++ b/src/applications/dashboard/query/PhabricatorDashboardSearchEngine.php
@@ -16,6 +16,10 @@ final class PhabricatorDashboardSearchEngine
       ->needPanels(true);
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   protected function buildCustomSearchFields() {
     return array(
       id(new PhabricatorSearchTextField())
diff --git a/src/applications/diviner/query/DivinerAtomSearchEngine.php b/src/applications/diviner/query/DivinerAtomSearchEngine.php
index cffdd3592d..35caf20a63 100644
--- a/src/applications/diviner/query/DivinerAtomSearchEngine.php
+++ b/src/applications/diviner/query/DivinerAtomSearchEngine.php
@@ -10,6 +10,10 @@ final class DivinerAtomSearchEngine extends PhabricatorApplicationSearchEngine {
     return 'PhabricatorDivinerApplication';
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   public function buildSavedQueryFromRequest(AphrontRequest $request) {
     $saved = new PhabricatorSavedQuery();
 
diff --git a/src/applications/files/query/PhabricatorFileSearchEngine.php b/src/applications/files/query/PhabricatorFileSearchEngine.php
index f2193d3beb..1f2f2de4b3 100644
--- a/src/applications/files/query/PhabricatorFileSearchEngine.php
+++ b/src/applications/files/query/PhabricatorFileSearchEngine.php
@@ -11,6 +11,10 @@ final class PhabricatorFileSearchEngine
     return 'PhabricatorFilesApplication';
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   public function newQuery() {
     return new PhabricatorFileQuery();
   }
diff --git a/src/applications/herald/query/HeraldTranscriptSearchEngine.php b/src/applications/herald/query/HeraldTranscriptSearchEngine.php
index cc0d620394..e35620f0da 100644
--- a/src/applications/herald/query/HeraldTranscriptSearchEngine.php
+++ b/src/applications/herald/query/HeraldTranscriptSearchEngine.php
@@ -11,6 +11,10 @@ final class HeraldTranscriptSearchEngine
     return 'PhabricatorHeraldApplication';
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   public function buildSavedQueryFromRequest(AphrontRequest $request) {
     $saved = new PhabricatorSavedQuery();
 
diff --git a/src/applications/maniphest/query/ManiphestTaskSearchEngine.php b/src/applications/maniphest/query/ManiphestTaskSearchEngine.php
index 5246f32a0e..d8f5ab493a 100644
--- a/src/applications/maniphest/query/ManiphestTaskSearchEngine.php
+++ b/src/applications/maniphest/query/ManiphestTaskSearchEngine.php
@@ -31,7 +31,7 @@ final class ManiphestTaskSearchEngine
   }
 
   public function getResultTypeDescription() {
-    return pht('Tasks');
+    return pht('Maniphest Tasks');
   }
 
   public function getApplicationClassName() {
diff --git a/src/applications/metamta/query/PhabricatorMetaMTAMailSearchEngine.php b/src/applications/metamta/query/PhabricatorMetaMTAMailSearchEngine.php
index 11ca1116ea..df7774aae2 100644
--- a/src/applications/metamta/query/PhabricatorMetaMTAMailSearchEngine.php
+++ b/src/applications/metamta/query/PhabricatorMetaMTAMailSearchEngine.php
@@ -11,6 +11,10 @@ final class PhabricatorMetaMTAMailSearchEngine
     return 'PhabricatorMetaMTAApplication';
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   public function newQuery() {
     return new PhabricatorMetaMTAMailQuery();
   }
diff --git a/src/applications/oauthserver/query/PhabricatorOAuthServerClientSearchEngine.php b/src/applications/oauthserver/query/PhabricatorOAuthServerClientSearchEngine.php
index 3cb027fe4f..e07b1ea2c2 100644
--- a/src/applications/oauthserver/query/PhabricatorOAuthServerClientSearchEngine.php
+++ b/src/applications/oauthserver/query/PhabricatorOAuthServerClientSearchEngine.php
@@ -11,6 +11,10 @@ final class PhabricatorOAuthServerClientSearchEngine
     return 'PhabricatorOAuthServerApplication';
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   public function newQuery() {
     return id(new PhabricatorOAuthServerClientQuery());
   }
diff --git a/src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php b/src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php
index 728c3f42a8..d6001419b4 100644
--- a/src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php
+++ b/src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php
@@ -15,6 +15,10 @@ final class PhabricatorOwnersPackageSearchEngine
     return new PhabricatorOwnersPackageQuery();
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   protected function buildCustomSearchFields() {
     return array(
       id(new PhabricatorSearchDatasourceField())
diff --git a/src/applications/packages/query/PhabricatorPackagesPackageSearchEngine.php b/src/applications/packages/query/PhabricatorPackagesPackageSearchEngine.php
index e01c8db503..817321dec1 100644
--- a/src/applications/packages/query/PhabricatorPackagesPackageSearchEngine.php
+++ b/src/applications/packages/query/PhabricatorPackagesPackageSearchEngine.php
@@ -15,6 +15,10 @@ final class PhabricatorPackagesPackageSearchEngine
     return id(new PhabricatorPackagesPackageQuery());
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   protected function buildQueryFromParameters(array $map) {
     $query = $this->newQuery();
 
diff --git a/src/applications/packages/query/PhabricatorPackagesPublisherSearchEngine.php b/src/applications/packages/query/PhabricatorPackagesPublisherSearchEngine.php
index f11738f730..be7b83f5fc 100644
--- a/src/applications/packages/query/PhabricatorPackagesPublisherSearchEngine.php
+++ b/src/applications/packages/query/PhabricatorPackagesPublisherSearchEngine.php
@@ -15,6 +15,10 @@ final class PhabricatorPackagesPublisherSearchEngine
     return id(new PhabricatorPackagesPublisherQuery());
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   protected function buildQueryFromParameters(array $map) {
     $query = $this->newQuery();
 
diff --git a/src/applications/packages/query/PhabricatorPackagesVersionSearchEngine.php b/src/applications/packages/query/PhabricatorPackagesVersionSearchEngine.php
index da1581bf80..b3592ffd70 100644
--- a/src/applications/packages/query/PhabricatorPackagesVersionSearchEngine.php
+++ b/src/applications/packages/query/PhabricatorPackagesVersionSearchEngine.php
@@ -15,6 +15,10 @@ final class PhabricatorPackagesVersionSearchEngine
     return id(new PhabricatorPackagesVersionQuery());
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   protected function buildQueryFromParameters(array $map) {
     $query = $this->newQuery();
 
diff --git a/src/applications/phrequent/query/PhrequentSearchEngine.php b/src/applications/phrequent/query/PhrequentSearchEngine.php
index d137c40b64..46f7ae769c 100644
--- a/src/applications/phrequent/query/PhrequentSearchEngine.php
+++ b/src/applications/phrequent/query/PhrequentSearchEngine.php
@@ -10,6 +10,11 @@ final class PhrequentSearchEngine extends PhabricatorApplicationSearchEngine {
     return 'PhabricatorPhrequentApplication';
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
+
   public function getPageSize(PhabricatorSavedQuery $saved) {
     return $saved->getParameter('limit', 1000);
   }
diff --git a/src/applications/phurl/query/PhabricatorPhurlURLSearchEngine.php b/src/applications/phurl/query/PhabricatorPhurlURLSearchEngine.php
index f2e2c99399..8814b25b9c 100644
--- a/src/applications/phurl/query/PhabricatorPhurlURLSearchEngine.php
+++ b/src/applications/phurl/query/PhabricatorPhurlURLSearchEngine.php
@@ -4,7 +4,7 @@ final class PhabricatorPhurlURLSearchEngine
   extends PhabricatorApplicationSearchEngine {
 
   public function getResultTypeDescription() {
-    return pht('Shortened URLs');
+    return pht('Phurl URLs');
   }
 
   public function getApplicationClassName() {
diff --git a/src/applications/project/query/PhabricatorProjectColumnSearchEngine.php b/src/applications/project/query/PhabricatorProjectColumnSearchEngine.php
index 0658ec75b0..68fe1e7eb0 100644
--- a/src/applications/project/query/PhabricatorProjectColumnSearchEngine.php
+++ b/src/applications/project/query/PhabricatorProjectColumnSearchEngine.php
@@ -11,6 +11,10 @@ final class PhabricatorProjectColumnSearchEngine
     return 'PhabricatorProjectApplication';
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   public function newQuery() {
     return new PhabricatorProjectColumnQuery();
   }
@@ -25,7 +29,6 @@ final class PhabricatorProjectColumnSearchEngine
     );
   }
 
-
   protected function buildQueryFromParameters(array $map) {
     $query = $this->newQuery();
 
diff --git a/src/applications/releeph/query/ReleephBranchSearchEngine.php b/src/applications/releeph/query/ReleephBranchSearchEngine.php
index 68ec126eb5..441f70e992 100644
--- a/src/applications/releeph/query/ReleephBranchSearchEngine.php
+++ b/src/applications/releeph/query/ReleephBranchSearchEngine.php
@@ -9,6 +9,10 @@ final class ReleephBranchSearchEngine
     return pht('Releeph Branches');
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   public function getApplicationClassName() {
     return 'PhabricatorReleephApplication';
   }
diff --git a/src/applications/releeph/query/ReleephProductSearchEngine.php b/src/applications/releeph/query/ReleephProductSearchEngine.php
index f56125a687..d58ee735f1 100644
--- a/src/applications/releeph/query/ReleephProductSearchEngine.php
+++ b/src/applications/releeph/query/ReleephProductSearchEngine.php
@@ -11,6 +11,10 @@ final class ReleephProductSearchEngine
     return 'PhabricatorReleephApplication';
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   public function buildSavedQueryFromRequest(AphrontRequest $request) {
     $saved = new PhabricatorSavedQuery();
 
diff --git a/src/applications/releeph/query/ReleephRequestSearchEngine.php b/src/applications/releeph/query/ReleephRequestSearchEngine.php
index 20aaa38ed9..7f866405c1 100644
--- a/src/applications/releeph/query/ReleephRequestSearchEngine.php
+++ b/src/applications/releeph/query/ReleephRequestSearchEngine.php
@@ -14,6 +14,10 @@ final class ReleephRequestSearchEngine
     return 'PhabricatorReleephApplication';
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   public function setBranch(ReleephBranch $branch) {
     $this->branch = $branch;
     return $this;
diff --git a/src/applications/search/query/PhabricatorSearchApplicationSearchEngine.php b/src/applications/search/query/PhabricatorSearchApplicationSearchEngine.php
index 835e483cff..cf7dc0f6ba 100644
--- a/src/applications/search/query/PhabricatorSearchApplicationSearchEngine.php
+++ b/src/applications/search/query/PhabricatorSearchApplicationSearchEngine.php
@@ -11,6 +11,10 @@ final class PhabricatorSearchApplicationSearchEngine
     return 'PhabricatorSearchApplication';
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   public function buildSavedQueryFromRequest(AphrontRequest $request) {
     $saved = new PhabricatorSavedQuery();
 
diff --git a/src/applications/transactions/query/PhabricatorEditEngineSearchEngine.php b/src/applications/transactions/query/PhabricatorEditEngineSearchEngine.php
index 70cd4cd1a3..4ba1a0e879 100644
--- a/src/applications/transactions/query/PhabricatorEditEngineSearchEngine.php
+++ b/src/applications/transactions/query/PhabricatorEditEngineSearchEngine.php
@@ -15,6 +15,10 @@ final class PhabricatorEditEngineSearchEngine
     return id(new PhabricatorEditEngineQuery());
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   protected function buildQueryFromParameters(array $map) {
     $query = $this->newQuery();
     return $query;
diff --git a/src/infrastructure/daemon/workers/query/PhabricatorWorkerBulkJobSearchEngine.php b/src/infrastructure/daemon/workers/query/PhabricatorWorkerBulkJobSearchEngine.php
index 223db0e95a..3b9d6c9d48 100644
--- a/src/infrastructure/daemon/workers/query/PhabricatorWorkerBulkJobSearchEngine.php
+++ b/src/infrastructure/daemon/workers/query/PhabricatorWorkerBulkJobSearchEngine.php
@@ -4,7 +4,7 @@ final class PhabricatorWorkerBulkJobSearchEngine
   extends PhabricatorApplicationSearchEngine {
 
   public function getResultTypeDescription() {
-    return pht('Bulk Jobs');
+    return pht('Daemon Bulk Jobs');
   }
 
   public function getApplicationClassName() {
@@ -15,6 +15,10 @@ final class PhabricatorWorkerBulkJobSearchEngine
     return id(new PhabricatorWorkerBulkJobQuery());
   }
 
+  public function canUseInPanelContext() {
+    return false;
+  }
+
   protected function buildQueryFromParameters(array $map) {
     $query = $this->newQuery();
 

From 6f50729a91710f21ec8e323b27c8624303043410 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 20 Feb 2017 12:43:17 -0800
Subject: [PATCH 08/25] Update Phabricator for new daemon pool changes

Summary:
Ref T12298. This updates `bin/phd` for minor changes to daemon configuration. In particular:

  - Every daemon now has an autoscale pool (for trigger/pull, the maximum pool size is 1).
  - Pools now have labels to make debugging a little easier.
  - Some minor structural changes.

Test Plan: See D17389.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12298

Differential Revision: https://secure.phabricator.com/D17390
---
 ...abricatorDaemonManagementDebugWorkflow.php | 19 ++++----------
 .../PhabricatorDaemonManagementWorkflow.php   | 25 ++++++-------------
 2 files changed, 12 insertions(+), 32 deletions(-)

diff --git a/src/applications/daemon/management/PhabricatorDaemonManagementDebugWorkflow.php b/src/applications/daemon/management/PhabricatorDaemonManagementDebugWorkflow.php
index e2023ae143..9a535af617 100644
--- a/src/applications/daemon/management/PhabricatorDaemonManagementDebugWorkflow.php
+++ b/src/applications/daemon/management/PhabricatorDaemonManagementDebugWorkflow.php
@@ -28,10 +28,6 @@ final class PhabricatorDaemonManagementDebugWorkflow
               'instead of the configured %s',
               'phd.user'),
           ),
-          array(
-            'name' => 'autoscale',
-            'help' => pht('Put the daemon in an autoscale group.'),
-          ),
         ));
   }
 
@@ -44,16 +40,11 @@ final class PhabricatorDaemonManagementDebugWorkflow
         pht('You must specify which daemon to debug.'));
     }
 
-    $config = array();
-
-    $config['class'] = array_shift($argv);
-    $config['argv'] = $argv;
-
-    if ($args->getArg('autoscale')) {
-      $config['autoscale'] = array(
-        'group' => 'debug',
-      );
-    }
+    $config = array(
+      'class' => array_shift($argv),
+      'label' => 'debug',
+      'argv' => $argv,
+    );
 
     return $this->launchDaemons(
       array(
diff --git a/src/applications/daemon/management/PhabricatorDaemonManagementWorkflow.php b/src/applications/daemon/management/PhabricatorDaemonManagementWorkflow.php
index e0218a741d..77f32c293f 100644
--- a/src/applications/daemon/management/PhabricatorDaemonManagementWorkflow.php
+++ b/src/applications/daemon/management/PhabricatorDaemonManagementWorkflow.php
@@ -362,17 +362,17 @@ abstract class PhabricatorDaemonManagementWorkflow
     $daemons = array(
       array(
         'class' => 'PhabricatorRepositoryPullLocalDaemon',
+        'label' => 'pull',
       ),
       array(
         'class' => 'PhabricatorTriggerDaemon',
+        'label' => 'trigger',
       ),
       array(
         'class' => 'PhabricatorTaskmasterDaemon',
-        'autoscale' => array(
-          'group' => 'task',
-          'pool' => PhabricatorEnv::getEnvConfig('phd.taskmasters'),
-          'reserve' => idx($options, 'reserve', 0),
-        ),
+        'label' => 'task',
+        'pool' => PhabricatorEnv::getEnvConfig('phd.taskmasters'),
+        'reserve' => idx($options, 'reserve', 0),
       ),
     );
 
@@ -618,23 +618,12 @@ abstract class PhabricatorDaemonManagementWorkflow
       pht('(Logs will appear in "%s".)', $log_dir));
 
     foreach ($daemons as $daemon) {
-      $is_autoscale = isset($daemon['autoscale']['group']);
-      if ($is_autoscale) {
-        $autoscale = $daemon['autoscale'];
-        foreach ($autoscale as $key => $value) {
-          $autoscale[$key] = $key.'='.$value;
-        }
-        $autoscale = implode(', ', $autoscale);
-
-        $autoscale = pht('(Autoscaling: %s)', $autoscale);
-      } else {
-        $autoscale = pht('(Static)');
-      }
+      $pool_size = pht('(Pool: %s)', idx($daemon, 'pool', 1));
 
       $console->writeOut(
         "    %s %s\n",
+        $pool_size,
         $daemon['class'],
-        $autoscale,
         implode(' ', idx($daemon, 'argv', array())));
     }
     $console->writeOut("\n");

From 939fb69aa68b8010d7c857eaed1240203a4a25d9 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 22 Feb 2017 14:05:46 -0800
Subject: [PATCH 09/25] Be less strict when detecting dead daemons

Summary:
Fixes T12306. Currently, we warn about daemons not running even if they're in normal "alive" states, particularly "waiting to restart after a failure".

This check was made more strict in D12088, back when we tried to version check running daemons. Since we implemented auto-restart-after-config-change we don't do this anymore, so it should be fine to make this more lax again.

Test Plan:
  - Faked an exception for all tasks.
  - Before patch: reloading the daemon setup error sometimes raised a false positive ("waiting" daemon detected as dead).
  - After patch: daemon setup error no longer triggers.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12306

Differential Revision: https://secure.phabricator.com/D17397
---
 src/applications/config/check/PhabricatorDaemonsSetupCheck.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/applications/config/check/PhabricatorDaemonsSetupCheck.php b/src/applications/config/check/PhabricatorDaemonsSetupCheck.php
index 46ee4efe77..9fc6b587b0 100644
--- a/src/applications/config/check/PhabricatorDaemonsSetupCheck.php
+++ b/src/applications/config/check/PhabricatorDaemonsSetupCheck.php
@@ -10,7 +10,7 @@ final class PhabricatorDaemonsSetupCheck extends PhabricatorSetupCheck {
 
     $task_daemon = id(new PhabricatorDaemonLogQuery())
       ->setViewer(PhabricatorUser::getOmnipotentUser())
-      ->withStatus(PhabricatorDaemonLogQuery::STATUS_RUNNING)
+      ->withStatus(PhabricatorDaemonLogQuery::STATUS_ALIVE)
       ->withDaemonClasses(array('PhabricatorTaskmasterDaemon'))
       ->setLimit(1)
       ->execute();

From 4540ae028a6e20266699179758dddcbd0ef7a8e6 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 22 Feb 2017 14:26:38 -0800
Subject: [PATCH 10/25] Fix "Create Form" link destinations when editing edit
 forms

Summary:
Fixes T12301. In D17372, this changed to use generic EditEngines instead of the proper runtime engine. Normally this doesn't matter, but can in this case.

After loading the configurations normally, swap their attached engines for the specific configured runtime engine we're currently executing.

Test Plan: Clicked "Create Form" from the Maniphest form list, saw it go to "Create Maniphest Form", not "Create Generic Meta-Form".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12301

Differential Revision: https://secure.phabricator.com/D17398
---
 .../transactions/editengine/PhabricatorEditEngine.php      | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php
index eb4f607ca9..8bfde17fdd 100644
--- a/src/applications/transactions/editengine/PhabricatorEditEngine.php
+++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php
@@ -2126,6 +2126,13 @@ abstract class PhabricatorEditEngine
 
     $configs = msort($configs, 'getCreateSortKey');
 
+    // Attach this specific engine to configurations we load so they can access
+    // any runtime configuration. For example, this allows us to generate the
+    // correct "Create Form" buttons when editing forms, see T12301.
+    foreach ($configs as $config) {
+      $config->attachEngine($this);
+    }
+
     return $configs;
   }
 

From 568a3877d17b81140522b7d369360eaa609dbc32 Mon Sep 17 00:00:00 2001
From: Chad Little <chad@phacility.com>
Date: Tue, 21 Feb 2017 16:29:47 -0800
Subject: [PATCH 11/25] Simplify dashboard panel creation

Summary: Ref T10390. Basically hides policy controls when creating a panel on a dashboard. Shows when you edit them or through normal workflow. I think we should maybe also get rid of view policy? Not sure the benefit since results will be filtered anyways. Maybe Text panels? Not sure the use case.

Test Plan: Add a panel, edit a panel.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: hskiba, Korvin

Maniphest Tasks: T10390

Differential Revision: https://secure.phabricator.com/D17393
---
 ...habricatorDashboardPanelEditController.php | 30 +++++++++++--------
 .../storage/PhabricatorDashboard.php          |  2 +-
 .../storage/PhabricatorDashboardPanel.php     |  2 +-
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/src/applications/dashboard/controller/PhabricatorDashboardPanelEditController.php b/src/applications/dashboard/controller/PhabricatorDashboardPanelEditController.php
index cc7502afef..7aece8c603 100644
--- a/src/applications/dashboard/controller/PhabricatorDashboardPanelEditController.php
+++ b/src/applications/dashboard/controller/PhabricatorDashboardPanelEditController.php
@@ -193,19 +193,23 @@ final class PhabricatorDashboardPanelEditController
           ->setLabel(pht('Name'))
           ->setName('name')
           ->setValue($v_name)
-          ->setError($e_name))
-      ->appendChild(
-        id(new AphrontFormPolicyControl())
-          ->setName('viewPolicy')
-          ->setPolicyObject($panel)
-          ->setCapability(PhabricatorPolicyCapability::CAN_VIEW)
-          ->setPolicies($policies))
-      ->appendChild(
-        id(new AphrontFormPolicyControl())
-          ->setName('editPolicy')
-          ->setPolicyObject($panel)
-          ->setCapability(PhabricatorPolicyCapability::CAN_EDIT)
-          ->setPolicies($policies));
+          ->setError($e_name));
+
+      if (!$request->isAjax() || !$is_create) {
+        $form
+          ->appendChild(
+            id(new AphrontFormPolicyControl())
+              ->setName('viewPolicy')
+              ->setPolicyObject($panel)
+              ->setCapability(PhabricatorPolicyCapability::CAN_VIEW)
+              ->setPolicies($policies))
+          ->appendChild(
+            id(new AphrontFormPolicyControl())
+              ->setName('editPolicy')
+              ->setPolicyObject($panel)
+              ->setCapability(PhabricatorPolicyCapability::CAN_EDIT)
+              ->setPolicies($policies));
+    }
 
     $field_list->appendFieldsToForm($form);
 
diff --git a/src/applications/dashboard/storage/PhabricatorDashboard.php b/src/applications/dashboard/storage/PhabricatorDashboard.php
index 325e573715..b79fded93f 100644
--- a/src/applications/dashboard/storage/PhabricatorDashboard.php
+++ b/src/applications/dashboard/storage/PhabricatorDashboard.php
@@ -32,7 +32,7 @@ final class PhabricatorDashboard extends PhabricatorDashboardDAO
     return id(new PhabricatorDashboard())
       ->setName('')
       ->setIcon('fa-dashboard')
-      ->setViewPolicy(PhabricatorPolicies::POLICY_USER)
+      ->setViewPolicy(PhabricatorPolicies::getMostOpenPolicy())
       ->setEditPolicy($actor->getPHID())
       ->setStatus(self::STATUS_ACTIVE)
       ->setAuthorPHID($actor->getPHID())
diff --git a/src/applications/dashboard/storage/PhabricatorDashboardPanel.php b/src/applications/dashboard/storage/PhabricatorDashboardPanel.php
index 4e82aaa348..9f8875dc1b 100644
--- a/src/applications/dashboard/storage/PhabricatorDashboardPanel.php
+++ b/src/applications/dashboard/storage/PhabricatorDashboardPanel.php
@@ -27,7 +27,7 @@ final class PhabricatorDashboardPanel
     return id(new PhabricatorDashboardPanel())
       ->setName('')
       ->setAuthorPHID($actor->getPHID())
-      ->setViewPolicy(PhabricatorPolicies::POLICY_USER)
+      ->setViewPolicy(PhabricatorPolicies::getMostOpenPolicy())
       ->setEditPolicy($actor->getPHID());
   }
 

From 84aff44bcd644ef9bd8c5235ba89cf3a7be745fe Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 30 Jan 2017 17:18:55 -0800
Subject: [PATCH 12/25] Add a "Red/Green Colorblind" accessibility mode, make
 all web UIs and email respect it

Summary:
Fixes T12172. Fixes T12060. This allows runtime code building CSS for mail to read CSS variables, then makes all the code do that.

It reverts the non-colorblind red/green to the colors in use before T12060, which seem better for non-colorblind users since no one really complained?

Test Plan:
  - Viewed code diffs in Web UI.
  - Viewed prose diffs in Web UI.
  - Viewed code diffs in email.
  - Viewed prose diffs in email.

All modes respected the accessibility color scheme.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12172, T12060

Differential Revision: https://secure.phabricator.com/D17269
---
 resources/celerity/map.php                    | 12 +++++-----
 src/__phutil_library_map__.php                |  2 ++
 .../CelerityDefaultPostprocessor.php          |  8 +++----
 .../CelerityRedGreenPostprocessor.php         | 23 +++++++++++++++++++
 ...DifferentialChangesetOneUpMailRenderer.php |  8 +++++--
 .../people/storage/PhabricatorUser.php        | 18 +++++++++++++++
 ...plicationTransactionTextDiffDetailView.php |  8 +++++--
 7 files changed, 65 insertions(+), 14 deletions(-)
 create mode 100644 src/applications/celerity/postprocessor/CelerityRedGreenPostprocessor.php

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index 836194f420..7122e68f39 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -12,7 +12,7 @@ return array(
     'core.pkg.css' => '12c56bd9',
     'core.pkg.js' => '1fa7c0c5',
     'darkconsole.pkg.js' => 'e7393ebb',
-    'differential.pkg.css' => '4815647b',
+    'differential.pkg.css' => '90b30783',
     'differential.pkg.js' => 'ddfeb49b',
     'diffusion.pkg.css' => '91c5d3a6',
     'diffusion.pkg.js' => '84c8f8fd',
@@ -58,7 +58,7 @@ return array(
     'rsrc/css/application/dashboard/dashboard.css' => '0921c307',
     'rsrc/css/application/diff/inline-comment-summary.css' => '51efda3a',
     'rsrc/css/application/differential/add-comment.css' => 'c47f8c40',
-    'rsrc/css/application/differential/changeset-view.css' => '6a9bdf9c',
+    'rsrc/css/application/differential/changeset-view.css' => '41af6d25',
     'rsrc/css/application/differential/core.css' => '5b7b8ff4',
     'rsrc/css/application/differential/phui-inline-comment.css' => 'be663c95',
     'rsrc/css/application/differential/revision-comment.css' => '14b8565a',
@@ -572,7 +572,7 @@ return array(
     'conpherence-thread-manager' => 'c8b5ee6f',
     'conpherence-transaction-css' => '85129c68',
     'd3' => 'a11a5ff2',
-    'differential-changeset-view-css' => '6a9bdf9c',
+    'differential-changeset-view-css' => '41af6d25',
     'differential-core-view-css' => '5b7b8ff4',
     'differential-inline-comment-editor' => '2e3f9738',
     'differential-revision-add-comment-css' => 'c47f8c40',
@@ -1166,6 +1166,9 @@ return array(
       'javelin-dom',
       'javelin-reactor-dom',
     ),
+    '41af6d25' => array(
+      'phui-inline-comment-view-css',
+    ),
     42126667 => array(
       'javelin-behavior',
       'javelin-dom',
@@ -1386,9 +1389,6 @@ return array(
     '69adf288' => array(
       'javelin-install',
     ),
-    '6a9bdf9c' => array(
-      'phui-inline-comment-view-css',
-    ),
     '6ad39b6f' => array(
       'javelin-install',
       'javelin-event',
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index c8b3f97726..1cda447088 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -235,6 +235,7 @@ phutil_register_library_map(array(
     'CelerityPhysicalResourcesTestCase' => 'applications/celerity/resources/__tests__/CelerityPhysicalResourcesTestCase.php',
     'CelerityPostprocessor' => 'applications/celerity/postprocessor/CelerityPostprocessor.php',
     'CelerityPostprocessorTestCase' => 'applications/celerity/__tests__/CelerityPostprocessorTestCase.php',
+    'CelerityRedGreenPostprocessor' => 'applications/celerity/postprocessor/CelerityRedGreenPostprocessor.php',
     'CelerityResourceController' => 'applications/celerity/controller/CelerityResourceController.php',
     'CelerityResourceGraph' => 'applications/celerity/CelerityResourceGraph.php',
     'CelerityResourceMap' => 'applications/celerity/CelerityResourceMap.php',
@@ -4911,6 +4912,7 @@ phutil_register_library_map(array(
     'CelerityPhysicalResourcesTestCase' => 'PhabricatorTestCase',
     'CelerityPostprocessor' => 'Phobject',
     'CelerityPostprocessorTestCase' => 'PhabricatorTestCase',
+    'CelerityRedGreenPostprocessor' => 'CelerityPostprocessor',
     'CelerityResourceController' => 'PhabricatorController',
     'CelerityResourceGraph' => 'AbstractDirectedGraph',
     'CelerityResourceMap' => 'Phobject',
diff --git a/src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php b/src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php
index 19a4a363e1..62a4b4fd24 100644
--- a/src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php
+++ b/src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php
@@ -193,10 +193,10 @@ final class CelerityDefaultPostprocessor
       'sh-disabledbackground'   => '#f3f3f3',
 
       // Diffs
-      'new-background' => '#eaffea',
-      'new-bright' => '#a6f3a6',
-      'old-background' => '#ffecec',
-      'old-bright' => '#f8cbcb',
+      'new-background' => 'rgba(151, 234, 151, .3)',
+      'new-bright' => 'rgba(151, 234, 151, .6)',
+      'old-background' => 'rgba(251, 175, 175, .3)',
+      'old-bright' => 'rgba(251, 175, 175, .7)',
       'move-background' => '#fdf5d4',
       'copy-background' => '#f1c40f',
 
diff --git a/src/applications/celerity/postprocessor/CelerityRedGreenPostprocessor.php b/src/applications/celerity/postprocessor/CelerityRedGreenPostprocessor.php
new file mode 100644
index 0000000000..fc2f9d8e9a
--- /dev/null
+++ b/src/applications/celerity/postprocessor/CelerityRedGreenPostprocessor.php
@@ -0,0 +1,23 @@
+<?php
+
+final class CelerityRedGreenPostprocessor
+  extends CelerityPostprocessor {
+
+  public function getPostprocessorKey() {
+    return 'redgreen';
+  }
+
+  public function getPostprocessorName() {
+    return pht('Use Red/Green (Deuteranopia) Colors');
+  }
+
+  public function buildVariables() {
+    return array(
+      'new-background' => 'rgba(152, 207, 235, .15)',
+      'new-bright' => 'rgba(152, 207, 235, .35)',
+      'old-background' => 'rgba(250, 212, 175, .3)',
+      'old-bright' => 'rgba(250, 212, 175, .55)',
+    );
+  }
+
+}
diff --git a/src/applications/differential/render/DifferentialChangesetOneUpMailRenderer.php b/src/applications/differential/render/DifferentialChangesetOneUpMailRenderer.php
index e720422ecd..ad8939d275 100644
--- a/src/applications/differential/render/DifferentialChangesetOneUpMailRenderer.php
+++ b/src/applications/differential/render/DifferentialChangesetOneUpMailRenderer.php
@@ -51,6 +51,10 @@ final class DifferentialChangesetOneUpMailRenderer
   protected function renderPrimitives(array $primitives, $rows) {
     $out = array();
 
+    $viewer = $this->getUser();
+    $old_bright = $viewer->getCSSValue('old-bright');
+    $new_bright = $viewer->getCSSValue('new-bright');
+
     $context_style = array(
       'background: #F7F7F7;',
       'color: #74777D;',
@@ -72,13 +76,13 @@ final class DifferentialChangesetOneUpMailRenderer
 
           if ($is_old) {
             if ($p['htype']) {
-              $style = 'background: #ffd0d0;';
+              $style = "background: {$old_bright};";
             } else {
               $style = null;
             }
           } else {
             if ($p['htype']) {
-              $style = 'background: #d0ffd0;';
+              $style = "background: {$new_bright};";
             } else {
               $style =  null;
             }
diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php
index 813d11c868..f2ebd76210 100644
--- a/src/applications/people/storage/PhabricatorUser.php
+++ b/src/applications/people/storage/PhabricatorUser.php
@@ -1580,4 +1580,22 @@ final class PhabricatorUser
     return $this;
   }
 
+
+  public function getCSSValue($variable_key) {
+    $preference = PhabricatorAccessibilitySetting::SETTINGKEY;
+    $key = $this->getUserSetting($preference);
+
+    $postprocessor = CelerityPostprocessor::getPostprocessor($key);
+    $variables = $postprocessor->getVariables();
+
+    if (!isset($variables[$variable_key])) {
+      throw new Exception(
+        pht(
+          'Unknown CSS variable "%s"!',
+          $variable_key));
+    }
+
+    return $variables[$variable_key];
+  }
+
 }
diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php
index 158090b1a2..755d6a9fcf 100644
--- a/src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php
+++ b/src/applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php
@@ -19,17 +19,21 @@ final class PhabricatorApplicationTransactionTextDiffDetailView
   public function renderForMail() {
     $diff = $this->buildDiff();
 
+    $viewer = $this->getViewer();
+    $old_bright = $viewer->getCSSValue('old-bright');
+    $new_bright = $viewer->getCSSValue('new-bright');
+
     $old_styles = array(
       'padding: 0 2px;',
       'color: #333333;',
-      'background: #f8cbcb;',
+      "background: {$old_bright};",
     );
     $old_styles = implode(' ', $old_styles);
 
     $new_styles = array(
       'padding: 0 2px;',
       'color: #333333;',
-      'background: #a6f3a6;',
+      "background: {$new_bright};",
     );
     $new_styles = implode(' ', $new_styles);
 

From ad032e72caff66c413566bf7d8fa9f43a97d4620 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 23 Feb 2017 09:30:15 -0800
Subject: [PATCH 13/25] When the profiler is active, keep it active if the user
 submits forms

Summary: Ref T12297. When a page is generated with the profiler active, keep it active by adding a `__profile__` input to any forms we generate.

Test Plan: Hit Conduit API page with `__profile__` active, saw it reflected in forms.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12297

Differential Revision: https://secure.phabricator.com/D17399
---
 src/infrastructure/javelin/markup.php | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/infrastructure/javelin/markup.php b/src/infrastructure/javelin/markup.php
index c78de8b86a..1909a3bd2b 100644
--- a/src/infrastructure/javelin/markup.php
+++ b/src/infrastructure/javelin/markup.php
@@ -98,6 +98,19 @@ function phabricator_form(PhabricatorUser $user, $attributes, $content) {
           'name' => '__form__',
           'value' => true,
         ));
+
+      // If the profiler was active for this request, keep it active for any
+      // forms submitted from this page.
+      if (DarkConsoleXHProfPluginAPI::isProfilerRequested()) {
+        $body[] = phutil_tag(
+          'input',
+          array(
+            'type' => 'hidden',
+            'name' => '__profile__',
+            'value' => true,
+          ));
+      }
+
     }
   }
 

From 42547022719379a27211e630872b1ca5b77f7e26 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 23 Feb 2017 09:56:55 -0800
Subject: [PATCH 14/25] Use ApplicationSearch in XHProf

Summary:
Ref T12297. This slightly modernizes the XHProf UI. Not included here:

  - Some of the code acts like samples have PHIDs, but they currently do not. I plan to add them in the next change.
  - I've intentionally left the actual list untouched for now -- it has some old/buggy code (like `flag-6` is no longer an icon) that I'll fix in a future change.

Test Plan: {F3224264}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12297

Differential Revision: https://secure.phabricator.com/D17400
---
 src/__phutil_library_map__.php                |  9 +-
 .../PhabricatorXHProfSampleListController.php | 90 +----------------
 .../query/PhabricatorXHProfSampleQuery.php    | 51 ++++++++++
 .../PhabricatorXHProfSampleSearchEngine.php   | 97 +++++++++++++++++++
 .../storage/PhabricatorXHProfSample.php       | 25 ++++-
 5 files changed, 184 insertions(+), 88 deletions(-)
 create mode 100644 src/applications/xhprof/query/PhabricatorXHProfSampleQuery.php
 create mode 100644 src/applications/xhprof/query/PhabricatorXHProfSampleSearchEngine.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 1cda447088..1ced8a7b4c 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -4132,6 +4132,8 @@ phutil_register_library_map(array(
     'PhabricatorXHProfProfileView' => 'applications/xhprof/view/PhabricatorXHProfProfileView.php',
     'PhabricatorXHProfSample' => 'applications/xhprof/storage/PhabricatorXHProfSample.php',
     'PhabricatorXHProfSampleListController' => 'applications/xhprof/controller/PhabricatorXHProfSampleListController.php',
+    'PhabricatorXHProfSampleQuery' => 'applications/xhprof/query/PhabricatorXHProfSampleQuery.php',
+    'PhabricatorXHProfSampleSearchEngine' => 'applications/xhprof/query/PhabricatorXHProfSampleSearchEngine.php',
     'PhabricatorYoutubeRemarkupRule' => 'infrastructure/markup/rule/PhabricatorYoutubeRemarkupRule.php',
     'Phame404Response' => 'applications/phame/site/Phame404Response.php',
     'PhameBlog' => 'applications/phame/storage/PhameBlog.php',
@@ -9442,8 +9444,13 @@ phutil_register_library_map(array(
     'PhabricatorXHProfProfileSymbolView' => 'PhabricatorXHProfProfileView',
     'PhabricatorXHProfProfileTopLevelView' => 'PhabricatorXHProfProfileView',
     'PhabricatorXHProfProfileView' => 'AphrontView',
-    'PhabricatorXHProfSample' => 'PhabricatorXHProfDAO',
+    'PhabricatorXHProfSample' => array(
+      'PhabricatorXHProfDAO',
+      'PhabricatorPolicyInterface',
+    ),
     'PhabricatorXHProfSampleListController' => 'PhabricatorXHProfController',
+    'PhabricatorXHProfSampleQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
+    'PhabricatorXHProfSampleSearchEngine' => 'PhabricatorApplicationSearchEngine',
     'PhabricatorYoutubeRemarkupRule' => 'PhutilRemarkupRule',
     'Phame404Response' => 'AphrontHTMLResponse',
     'PhameBlog' => array(
diff --git a/src/applications/xhprof/controller/PhabricatorXHProfSampleListController.php b/src/applications/xhprof/controller/PhabricatorXHProfSampleListController.php
index 5faab70c7a..41d108b85f 100644
--- a/src/applications/xhprof/controller/PhabricatorXHProfSampleListController.php
+++ b/src/applications/xhprof/controller/PhabricatorXHProfSampleListController.php
@@ -8,91 +8,9 @@ final class PhabricatorXHProfSampleListController
   }
 
   public function handleRequest(AphrontRequest $request) {
-    $viewer = $request->getViewer();
-    $view = $request->getURIData('view');
-
-    if (!$view) {
-      $view = 'all';
-    }
-
-    $pager = new PHUIPagerView();
-    $pager->setOffset($request->getInt('page'));
-
-    switch ($view) {
-      case 'sampled':
-        $clause = 'sampleRate > 0';
-        $show_type = false;
-        break;
-      case 'my-runs':
-        $clause = qsprintf(
-          id(new PhabricatorXHProfSample())->establishConnection('r'),
-          'sampleRate = 0 AND userPHID = %s',
-          $request->getUser()->getPHID());
-        $show_type = false;
-        break;
-      case 'manual':
-        $clause = 'sampleRate = 0';
-        $show_type = false;
-        break;
-      case 'all':
-      default:
-        $clause = '1 = 1';
-        $show_type = true;
-        break;
-    }
-
-    $samples = id(new PhabricatorXHProfSample())->loadAllWhere(
-      '%Q ORDER BY id DESC LIMIT %d, %d',
-      $clause,
-      $pager->getOffset(),
-      $pager->getPageSize() + 1);
-
-    $samples = $pager->sliceResults($samples);
-    $pager->setURI($request->getRequestURI(), 'page');
-
-    $list = new PHUIObjectItemListView();
-    foreach ($samples as $sample) {
-      $file_phid = $sample->getFilePHID();
-
-      $item = id(new PHUIObjectItemView())
-        ->setObjectName($sample->getID())
-        ->setHeader($sample->getRequestPath())
-        ->setHref($this->getApplicationURI('profile/'.$file_phid.'/'))
-        ->addAttribute(
-          number_format($sample->getUsTotal())." \xCE\xBCs");
-
-      if ($sample->getController()) {
-        $item->addAttribute($sample->getController());
-      }
-
-      $item->addAttribute($sample->getHostName());
-
-      $rate = $sample->getSampleRate();
-      if ($rate == 0) {
-        $item->addIcon('flag-6', pht('Manual Run'));
-      } else {
-        $item->addIcon('flag-7', pht('Sampled (1/%d)', $rate));
-      }
-
-      $item->addIcon(
-        'none',
-        phabricator_datetime($sample->getDateCreated(), $viewer));
-
-      $list->addItem($item);
-    }
-
-    $list->setPager($pager);
-    $list->setNoDataString(pht('There are no profiling samples.'));
-
-    $crumbs = $this->buildApplicationCrumbs();
-    $crumbs->addTextCrumb(pht('XHProf Samples'));
-
-    $title = pht('XHProf Samples');
-
-    return $this->newPage()
-      ->setTitle($title)
-      ->setCrumbs($crumbs)
-      ->appendChild($list);
-
+    return id(new PhabricatorXHProfSampleSearchEngine())
+      ->setController($this)
+      ->buildResponse();
   }
+
 }
diff --git a/src/applications/xhprof/query/PhabricatorXHProfSampleQuery.php b/src/applications/xhprof/query/PhabricatorXHProfSampleQuery.php
new file mode 100644
index 0000000000..3e9643a7fc
--- /dev/null
+++ b/src/applications/xhprof/query/PhabricatorXHProfSampleQuery.php
@@ -0,0 +1,51 @@
+<?php
+
+final class PhabricatorXHProfSampleQuery
+  extends PhabricatorCursorPagedPolicyAwareQuery {
+
+  private $ids;
+  private $phids;
+
+  public function withIDs(array $ids) {
+    $this->ids = $ids;
+    return $this;
+  }
+
+  public function withPHIDs(array $phids) {
+    $this->phids = $phids;
+    return $this;
+  }
+
+  public function newResultObject() {
+    return new PhabricatorXHProfSample();
+  }
+
+  protected function loadPage() {
+    return $this->loadStandardPage($this->newResultObject());
+  }
+
+  protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
+    $where = parent::buildWhereClauseParts($conn);
+
+    if ($this->ids !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'id IN (%Ld)',
+        $this->ids);
+    }
+
+    if ($this->phids !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'phid IN (%Ls)',
+        $this->phids);
+    }
+
+    return $where;
+  }
+
+  public function getQueryApplicationClass() {
+    return 'PhabricatorXHProfApplication';
+  }
+
+}
diff --git a/src/applications/xhprof/query/PhabricatorXHProfSampleSearchEngine.php b/src/applications/xhprof/query/PhabricatorXHProfSampleSearchEngine.php
new file mode 100644
index 0000000000..7529ed59da
--- /dev/null
+++ b/src/applications/xhprof/query/PhabricatorXHProfSampleSearchEngine.php
@@ -0,0 +1,97 @@
+<?php
+
+final class PhabricatorXHProfSampleSearchEngine
+  extends PhabricatorApplicationSearchEngine {
+
+  public function getResultTypeDescription() {
+    return pht('XHProf Samples');
+  }
+
+  public function getApplicationClassName() {
+    return 'PhabricatorXHProfApplication';
+  }
+
+  public function newQuery() {
+    return id(new PhabricatorXHProfSampleQuery());
+  }
+
+  protected function buildQueryFromParameters(array $map) {
+    $query = $this->newQuery();
+
+    return $query;
+  }
+
+  protected function buildCustomSearchFields() {
+    return array();
+  }
+
+  protected function getURI($path) {
+    return '/xhprof/'.$path;
+  }
+
+  protected function getBuiltinQueryNames() {
+    $names = array(
+      'all' => pht('All Samples'),
+    );
+
+    return $names;
+  }
+
+  public function buildSavedQueryFromBuiltin($query_key) {
+    $query = $this->newSavedQuery();
+    $query->setQueryKey($query_key);
+
+    switch ($query_key) {
+      case 'all':
+        return $query;
+    }
+
+    return parent::buildSavedQueryFromBuiltin($query_key);
+  }
+
+  protected function renderResultList(
+    array $samples,
+    PhabricatorSavedQuery $query,
+    array $handles) {
+    assert_instances_of($samples, 'PhabricatorXHProfSample');
+
+    $viewer = $this->requireViewer();
+
+    $list = new PHUIObjectItemListView();
+    foreach ($samples as $sample) {
+      $file_phid = $sample->getFilePHID();
+
+      $item = id(new PHUIObjectItemView())
+        ->setObjectName($sample->getID())
+        ->setHeader($sample->getRequestPath())
+        ->setHref($this->getApplicationURI('profile/'.$file_phid.'/'))
+        ->addAttribute(
+          number_format($sample->getUsTotal())." \xCE\xBCs");
+
+      if ($sample->getController()) {
+        $item->addAttribute($sample->getController());
+      }
+
+      $item->addAttribute($sample->getHostName());
+
+      $rate = $sample->getSampleRate();
+      if ($rate == 0) {
+        $item->addIcon('flag-6', pht('Manual Run'));
+      } else {
+        $item->addIcon('flag-7', pht('Sampled (1/%d)', $rate));
+      }
+
+      $item->addIcon(
+        'none',
+        phabricator_datetime($sample->getDateCreated(), $viewer));
+
+      $list->addItem($item);
+    }
+
+    $result = new PhabricatorApplicationSearchResultView();
+    $result->setObjectList($list);
+
+    return $result;
+  }
+
+}
diff --git a/src/applications/xhprof/storage/PhabricatorXHProfSample.php b/src/applications/xhprof/storage/PhabricatorXHProfSample.php
index 662305429d..75b357db5f 100644
--- a/src/applications/xhprof/storage/PhabricatorXHProfSample.php
+++ b/src/applications/xhprof/storage/PhabricatorXHProfSample.php
@@ -1,6 +1,8 @@
 <?php
 
-final class PhabricatorXHProfSample extends PhabricatorXHProfDAO {
+final class PhabricatorXHProfSample
+  extends PhabricatorXHProfDAO
+  implements PhabricatorPolicyInterface {
 
   protected $filePHID;
   protected $usTotal;
@@ -29,4 +31,25 @@ final class PhabricatorXHProfSample extends PhabricatorXHProfDAO {
     ) + parent::getConfiguration();
   }
 
+
+/* -(  PhabricatorPolicyInterface  )----------------------------------------- */
+
+
+  public function getCapabilities() {
+    return array(
+      PhabricatorPolicyCapability::CAN_VIEW,
+    );
+  }
+
+  public function getPolicy($capability) {
+    switch ($capability) {
+      case PhabricatorPolicyCapability::CAN_VIEW:
+        return PhabricatorPolicies::getMostOpenPolicy();
+    }
+  }
+
+  public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
+    return false;
+  }
+
 }

From ee2d8ce94b4086646fa8a7543c3596983c07ef53 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 23 Feb 2017 10:39:04 -0800
Subject: [PATCH 15/25] Allow XHProf profiles to be drag-and-dropped to upload
 them

Summary: Ref T12297. This could be fancier, but should make pulling profiles off `admin.phacility.com` significantly more realistic.

Test Plan: Dragged and dropped some profiles to upload them, then reviewed them via web UI.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12297

Differential Revision: https://secure.phabricator.com/D17401
---
 src/__phutil_library_map__.php                |  2 +
 .../PhabricatorXHProfApplication.php          |  1 +
 .../PhabricatorXHProfDropController.php       | 54 +++++++++++++++++++
 .../PhabricatorXHProfSampleSearchEngine.php   | 37 ++++++++++---
 .../storage/PhabricatorXHProfSample.php       | 19 +++++++
 5 files changed, 106 insertions(+), 7 deletions(-)
 create mode 100644 src/applications/xhprof/controller/PhabricatorXHProfDropController.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 1ced8a7b4c..416654b782 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -4126,6 +4126,7 @@ phutil_register_library_map(array(
     'PhabricatorXHProfApplication' => 'applications/xhprof/application/PhabricatorXHProfApplication.php',
     'PhabricatorXHProfController' => 'applications/xhprof/controller/PhabricatorXHProfController.php',
     'PhabricatorXHProfDAO' => 'applications/xhprof/storage/PhabricatorXHProfDAO.php',
+    'PhabricatorXHProfDropController' => 'applications/xhprof/controller/PhabricatorXHProfDropController.php',
     'PhabricatorXHProfProfileController' => 'applications/xhprof/controller/PhabricatorXHProfProfileController.php',
     'PhabricatorXHProfProfileSymbolView' => 'applications/xhprof/view/PhabricatorXHProfProfileSymbolView.php',
     'PhabricatorXHProfProfileTopLevelView' => 'applications/xhprof/view/PhabricatorXHProfProfileTopLevelView.php',
@@ -9440,6 +9441,7 @@ phutil_register_library_map(array(
     'PhabricatorXHProfApplication' => 'PhabricatorApplication',
     'PhabricatorXHProfController' => 'PhabricatorController',
     'PhabricatorXHProfDAO' => 'PhabricatorLiskDAO',
+    'PhabricatorXHProfDropController' => 'PhabricatorXHProfController',
     'PhabricatorXHProfProfileController' => 'PhabricatorXHProfController',
     'PhabricatorXHProfProfileSymbolView' => 'PhabricatorXHProfProfileView',
     'PhabricatorXHProfProfileTopLevelView' => 'PhabricatorXHProfProfileView',
diff --git a/src/applications/xhprof/application/PhabricatorXHProfApplication.php b/src/applications/xhprof/application/PhabricatorXHProfApplication.php
index 999e0378a5..bdc299dd4e 100644
--- a/src/applications/xhprof/application/PhabricatorXHProfApplication.php
+++ b/src/applications/xhprof/application/PhabricatorXHProfApplication.php
@@ -32,6 +32,7 @@ final class PhabricatorXHProfApplication extends PhabricatorApplication {
         '' => 'PhabricatorXHProfSampleListController',
         'list/(?P<view>[^/]+)/' => 'PhabricatorXHProfSampleListController',
         'profile/(?P<phid>[^/]+)/' => 'PhabricatorXHProfProfileController',
+        'import/drop/' => 'PhabricatorXHProfDropController',
       ),
     );
   }
diff --git a/src/applications/xhprof/controller/PhabricatorXHProfDropController.php b/src/applications/xhprof/controller/PhabricatorXHProfDropController.php
new file mode 100644
index 0000000000..6c3ab9e22c
--- /dev/null
+++ b/src/applications/xhprof/controller/PhabricatorXHProfDropController.php
@@ -0,0 +1,54 @@
+<?php
+
+final class PhabricatorXHProfDropController
+  extends PhabricatorXHProfController {
+
+  public function handleRequest(AphrontRequest $request) {
+    $viewer = $request->getViewer();
+
+    if (!$request->validateCSRF()) {
+      return new Aphront400Response();
+    }
+
+    $cancel_uri = $this->getApplicationURI();
+
+    $ids = $request->getStrList('h');
+    if ($ids) {
+      $files = id(new PhabricatorFileQuery())
+        ->setViewer($viewer)
+        ->withIDs($ids)
+        ->setRaisePolicyExceptions(true)
+        ->execute();
+    } else {
+      $files = array();
+    }
+
+    if (!$files) {
+      return $this->newDialog()
+        ->setTitle(pht('Nothing Uploaded'))
+        ->appendParagraph(
+          pht('Drag and drop .xhprof files to import them.'))
+        ->addCancelButton($cancel_uri, pht('Done'));
+    }
+
+    $samples = array();
+    foreach ($files as $file) {
+      $sample = PhabricatorXHProfSample::initializeNewSample()
+        ->setFilePHID($file->getPHID())
+        ->setUserPHID($viewer->getPHID())
+        ->save();
+
+      $samples[] = $sample;
+    }
+
+    if (count($samples) == 1) {
+      $event = head($samples);
+      $next_uri = $event->getURI();
+    } else {
+      $next_uri = $this->getApplicationURI();
+    }
+
+    return id(new AphrontRedirectResponse())->setURI($next_uri);
+  }
+
+}
diff --git a/src/applications/xhprof/query/PhabricatorXHProfSampleSearchEngine.php b/src/applications/xhprof/query/PhabricatorXHProfSampleSearchEngine.php
index 7529ed59da..171f28f027 100644
--- a/src/applications/xhprof/query/PhabricatorXHProfSampleSearchEngine.php
+++ b/src/applications/xhprof/query/PhabricatorXHProfSampleSearchEngine.php
@@ -63,10 +63,13 @@ final class PhabricatorXHProfSampleSearchEngine
 
       $item = id(new PHUIObjectItemView())
         ->setObjectName($sample->getID())
-        ->setHeader($sample->getRequestPath())
-        ->setHref($this->getApplicationURI('profile/'.$file_phid.'/'))
-        ->addAttribute(
-          number_format($sample->getUsTotal())." \xCE\xBCs");
+        ->setHeader($sample->getDisplayName())
+        ->setHref($sample->getURI());
+
+      $us_total = $sample->getUsTotal();
+      if ($us_total) {
+        $item->addAttribute(pht("%s \xCE\xBCs", new PhutilNumber($us_total)));
+      }
 
       if ($sample->getController()) {
         $item->addAttribute($sample->getController());
@@ -88,10 +91,30 @@ final class PhabricatorXHProfSampleSearchEngine
       $list->addItem($item);
     }
 
-    $result = new PhabricatorApplicationSearchResultView();
-    $result->setObjectList($list);
+    return $this->newResultView()
+      ->setObjectList($list);
+  }
 
-    return $result;
+
+  private function newResultView($content = null) {
+    // If we aren't rendering a dashboard panel, activate global drag-and-drop
+    // so you can import profiles by dropping them into the list.
+
+    if (!$this->isPanelContext()) {
+      $drop_upload = id(new PhabricatorGlobalUploadTargetView())
+        ->setViewer($this->requireViewer())
+        ->setHintText("\xE2\x87\xAA ".pht('Drop .xhprof Files to Import'))
+        ->setSubmitURI('/xhprof/import/drop/')
+        ->setViewPolicy(PhabricatorPolicies::POLICY_NOONE);
+
+      $content = array(
+        $drop_upload,
+        $content,
+      );
+    }
+
+    return id(new PhabricatorApplicationSearchResultView())
+      ->setContent($content);
   }
 
 }
diff --git a/src/applications/xhprof/storage/PhabricatorXHProfSample.php b/src/applications/xhprof/storage/PhabricatorXHProfSample.php
index 75b357db5f..3384ef3be2 100644
--- a/src/applications/xhprof/storage/PhabricatorXHProfSample.php
+++ b/src/applications/xhprof/storage/PhabricatorXHProfSample.php
@@ -12,6 +12,12 @@ final class PhabricatorXHProfSample
   protected $controller;
   protected $userPHID;
 
+  public static function initializeNewSample() {
+    return id(new self())
+      ->setUsTotal(0)
+      ->setSampleRate(0);
+  }
+
   protected function getConfiguration() {
     return array(
       self::CONFIG_COLUMN_SCHEMA => array(
@@ -31,6 +37,19 @@ final class PhabricatorXHProfSample
     ) + parent::getConfiguration();
   }
 
+  public function getURI() {
+    return '/xhprof/profile/'.$this->getFilePHID().'/';
+  }
+
+  public function getDisplayName() {
+    $request_path = $this->getRequestPath();
+    if (strlen($request_path)) {
+      return $request_path;
+    }
+
+    return pht('Unnamed Sample');
+  }
+
 
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */
 

From 3eae9a368de0a41582b5c6496ee82fa863223338 Mon Sep 17 00:00:00 2001
From: Chad Little <chad@phacility.com>
Date: Thu, 23 Feb 2017 14:22:43 -0800
Subject: [PATCH 16/25] Modular Transactions for Badges

Summary: Ref T12270. This converts Badges to modular transactions for editing and awarding.

Test Plan: Add Badge, edit badge, award and revoke... Still going to test this some more but feel free to comment on anything obviously wrong?

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12270

Differential Revision: https://secure.phabricator.com/D17402
---
 src/__phutil_library_map__.php                |  20 +-
 .../PhabricatorBadgesArchiveController.php    |   3 +-
 .../PhabricatorBadgesAwardController.php      |   3 +-
 ...bricatorBadgesEditRecipientsController.php |   3 +-
 ...icatorBadgesRemoveRecipientsController.php |   3 +-
 .../editor/PhabricatorBadgesEditEngine.php    |  18 +-
 .../badges/editor/PhabricatorBadgesEditor.php | 171 +----------
 .../storage/PhabricatorBadgesTransaction.php  | 275 +-----------------
 ...PhabricatorBadgesBadgeAwardTransaction.php |  62 ++++
 ...catorBadgesBadgeDescriptionTransaction.php |  57 ++++
 ...habricatorBadgesBadgeFlavorTransaction.php |  33 +++
 .../PhabricatorBadgesBadgeIconTransaction.php |  56 ++++
 .../PhabricatorBadgesBadgeNameTransaction.php |  44 +++
 ...abricatorBadgesBadgeQualityTransaction.php |  70 +++++
 ...habricatorBadgesBadgeRevokeTransaction.php |  54 ++++
 ...habricatorBadgesBadgeStatusTransaction.php |  50 ++++
 .../PhabricatorBadgesBadgeTransactionType.php |   4 +
 17 files changed, 488 insertions(+), 438 deletions(-)
 create mode 100644 src/applications/badges/xaction/PhabricatorBadgesBadgeAwardTransaction.php
 create mode 100644 src/applications/badges/xaction/PhabricatorBadgesBadgeDescriptionTransaction.php
 create mode 100644 src/applications/badges/xaction/PhabricatorBadgesBadgeFlavorTransaction.php
 create mode 100644 src/applications/badges/xaction/PhabricatorBadgesBadgeIconTransaction.php
 create mode 100644 src/applications/badges/xaction/PhabricatorBadgesBadgeNameTransaction.php
 create mode 100644 src/applications/badges/xaction/PhabricatorBadgesBadgeQualityTransaction.php
 create mode 100644 src/applications/badges/xaction/PhabricatorBadgesBadgeRevokeTransaction.php
 create mode 100644 src/applications/badges/xaction/PhabricatorBadgesBadgeStatusTransaction.php
 create mode 100644 src/applications/badges/xaction/PhabricatorBadgesBadgeTransactionType.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 416654b782..e6e48571ff 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -2012,7 +2012,16 @@ phutil_register_library_map(array(
     'PhabricatorBadgesAwardController' => 'applications/badges/controller/PhabricatorBadgesAwardController.php',
     'PhabricatorBadgesAwardQuery' => 'applications/badges/query/PhabricatorBadgesAwardQuery.php',
     'PhabricatorBadgesBadge' => 'applications/badges/storage/PhabricatorBadgesBadge.php',
+    'PhabricatorBadgesBadgeAwardTransaction' => 'applications/badges/xaction/PhabricatorBadgesBadgeAwardTransaction.php',
+    'PhabricatorBadgesBadgeDescriptionTransaction' => 'applications/badges/xaction/PhabricatorBadgesBadgeDescriptionTransaction.php',
+    'PhabricatorBadgesBadgeFlavorTransaction' => 'applications/badges/xaction/PhabricatorBadgesBadgeFlavorTransaction.php',
+    'PhabricatorBadgesBadgeIconTransaction' => 'applications/badges/xaction/PhabricatorBadgesBadgeIconTransaction.php',
     'PhabricatorBadgesBadgeNameNgrams' => 'applications/badges/storage/PhabricatorBadgesBadgeNameNgrams.php',
+    'PhabricatorBadgesBadgeNameTransaction' => 'applications/badges/xaction/PhabricatorBadgesBadgeNameTransaction.php',
+    'PhabricatorBadgesBadgeQualityTransaction' => 'applications/badges/xaction/PhabricatorBadgesBadgeQualityTransaction.php',
+    'PhabricatorBadgesBadgeRevokeTransaction' => 'applications/badges/xaction/PhabricatorBadgesBadgeRevokeTransaction.php',
+    'PhabricatorBadgesBadgeStatusTransaction' => 'applications/badges/xaction/PhabricatorBadgesBadgeStatusTransaction.php',
+    'PhabricatorBadgesBadgeTransactionType' => 'applications/badges/xaction/PhabricatorBadgesBadgeTransactionType.php',
     'PhabricatorBadgesCommentController' => 'applications/badges/controller/PhabricatorBadgesCommentController.php',
     'PhabricatorBadgesController' => 'applications/badges/controller/PhabricatorBadgesController.php',
     'PhabricatorBadgesCreateCapability' => 'applications/badges/capability/PhabricatorBadgesCreateCapability.php',
@@ -6954,7 +6963,16 @@ phutil_register_library_map(array(
       'PhabricatorConduitResultInterface',
       'PhabricatorNgramsInterface',
     ),
+    'PhabricatorBadgesBadgeAwardTransaction' => 'PhabricatorBadgesBadgeTransactionType',
+    'PhabricatorBadgesBadgeDescriptionTransaction' => 'PhabricatorBadgesBadgeTransactionType',
+    'PhabricatorBadgesBadgeFlavorTransaction' => 'PhabricatorBadgesBadgeTransactionType',
+    'PhabricatorBadgesBadgeIconTransaction' => 'PhabricatorBadgesBadgeTransactionType',
     'PhabricatorBadgesBadgeNameNgrams' => 'PhabricatorSearchNgrams',
+    'PhabricatorBadgesBadgeNameTransaction' => 'PhabricatorBadgesBadgeTransactionType',
+    'PhabricatorBadgesBadgeQualityTransaction' => 'PhabricatorBadgesBadgeTransactionType',
+    'PhabricatorBadgesBadgeRevokeTransaction' => 'PhabricatorBadgesBadgeTransactionType',
+    'PhabricatorBadgesBadgeStatusTransaction' => 'PhabricatorBadgesBadgeTransactionType',
+    'PhabricatorBadgesBadgeTransactionType' => 'PhabricatorModularTransactionType',
     'PhabricatorBadgesCommentController' => 'PhabricatorBadgesController',
     'PhabricatorBadgesController' => 'PhabricatorController',
     'PhabricatorBadgesCreateCapability' => 'PhabricatorPolicyCapability',
@@ -6980,7 +6998,7 @@ phutil_register_library_map(array(
     'PhabricatorBadgesSchemaSpec' => 'PhabricatorConfigSchemaSpec',
     'PhabricatorBadgesSearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod',
     'PhabricatorBadgesSearchEngine' => 'PhabricatorApplicationSearchEngine',
-    'PhabricatorBadgesTransaction' => 'PhabricatorApplicationTransaction',
+    'PhabricatorBadgesTransaction' => 'PhabricatorModularTransaction',
     'PhabricatorBadgesTransactionComment' => 'PhabricatorApplicationTransactionComment',
     'PhabricatorBadgesTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
     'PhabricatorBadgesViewController' => 'PhabricatorBadgesProfileController',
diff --git a/src/applications/badges/controller/PhabricatorBadgesArchiveController.php b/src/applications/badges/controller/PhabricatorBadgesArchiveController.php
index 52e1b9f1c7..f109d9459a 100644
--- a/src/applications/badges/controller/PhabricatorBadgesArchiveController.php
+++ b/src/applications/badges/controller/PhabricatorBadgesArchiveController.php
@@ -32,7 +32,8 @@ final class PhabricatorBadgesArchiveController
       $xactions = array();
 
       $xactions[] = id(new PhabricatorBadgesTransaction())
-        ->setTransactionType(PhabricatorBadgesTransaction::TYPE_STATUS)
+        ->setTransactionType(
+          PhabricatorBadgesBadgeStatusTransaction::TRANSACTIONTYPE)
         ->setNewValue($new_status);
 
       id(new PhabricatorBadgesEditor())
diff --git a/src/applications/badges/controller/PhabricatorBadgesAwardController.php b/src/applications/badges/controller/PhabricatorBadgesAwardController.php
index f9294ec5c6..ad409f8297 100644
--- a/src/applications/badges/controller/PhabricatorBadgesAwardController.php
+++ b/src/applications/badges/controller/PhabricatorBadgesAwardController.php
@@ -37,7 +37,8 @@ final class PhabricatorBadgesAwardController
       foreach ($badges as $badge) {
         $xactions = array();
         $xactions[] = id(new PhabricatorBadgesTransaction())
-          ->setTransactionType(PhabricatorBadgesTransaction::TYPE_AWARD)
+          ->setTransactionType(
+            PhabricatorBadgesBadgeAwardTransaction::TRANSACTIONTYPE)
           ->setNewValue($award_phids);
 
         $editor = id(new PhabricatorBadgesEditor())
diff --git a/src/applications/badges/controller/PhabricatorBadgesEditRecipientsController.php b/src/applications/badges/controller/PhabricatorBadgesEditRecipientsController.php
index 546c41a1d4..9045869fc8 100644
--- a/src/applications/badges/controller/PhabricatorBadgesEditRecipientsController.php
+++ b/src/applications/badges/controller/PhabricatorBadgesEditRecipientsController.php
@@ -37,7 +37,8 @@ final class PhabricatorBadgesEditRecipientsController
       }
 
       $xactions[] = id(new PhabricatorBadgesTransaction())
-        ->setTransactionType(PhabricatorBadgesTransaction::TYPE_AWARD)
+        ->setTransactionType(
+          PhabricatorBadgesBadgeAwardTransaction::TRANSACTIONTYPE)
         ->setNewValue($award_phids);
 
       $editor = id(new PhabricatorBadgesEditor())
diff --git a/src/applications/badges/controller/PhabricatorBadgesRemoveRecipientsController.php b/src/applications/badges/controller/PhabricatorBadgesRemoveRecipientsController.php
index 0185698686..b26a2c2159 100644
--- a/src/applications/badges/controller/PhabricatorBadgesRemoveRecipientsController.php
+++ b/src/applications/badges/controller/PhabricatorBadgesRemoveRecipientsController.php
@@ -34,7 +34,8 @@ final class PhabricatorBadgesRemoveRecipientsController
     if ($request->isFormPost()) {
       $xactions = array();
       $xactions[] = id(new PhabricatorBadgesTransaction())
-        ->setTransactionType(PhabricatorBadgesTransaction::TYPE_REVOKE)
+        ->setTransactionType(
+          PhabricatorBadgesBadgeRevokeTransaction::TRANSACTIONTYPE)
         ->setNewValue(array($remove_phid));
 
       $editor = id(new PhabricatorBadgesEditor())
diff --git a/src/applications/badges/editor/PhabricatorBadgesEditEngine.php b/src/applications/badges/editor/PhabricatorBadgesEditEngine.php
index 58cccb1a89..597cf470f6 100644
--- a/src/applications/badges/editor/PhabricatorBadgesEditEngine.php
+++ b/src/applications/badges/editor/PhabricatorBadgesEditEngine.php
@@ -86,20 +86,24 @@ final class PhabricatorBadgesEditEngine
         ->setLabel(pht('Name'))
         ->setDescription(pht('Badge name.'))
         ->setConduitTypeDescription(pht('New badge name.'))
-        ->setTransactionType(PhabricatorBadgesTransaction::TYPE_NAME)
-        ->setValue($object->getName()),
+        ->setTransactionType(
+          PhabricatorBadgesBadgeNameTransaction::TRANSACTIONTYPE)
+        ->setValue($object->getName())
+        ->setIsRequired(true),
       id(new PhabricatorTextEditField())
         ->setKey('flavor')
         ->setLabel(pht('Flavor text'))
         ->setDescription(pht('Short description of the badge.'))
         ->setConduitTypeDescription(pht('New badge flavor.'))
         ->setValue($object->getFlavor())
-        ->setTransactionType(PhabricatorBadgesTransaction::TYPE_FLAVOR),
+        ->setTransactionType(
+          PhabricatorBadgesBadgeFlavorTransaction::TRANSACTIONTYPE),
       id(new PhabricatorIconSetEditField())
         ->setKey('icon')
         ->setLabel(pht('Icon'))
         ->setIconSet(new PhabricatorBadgesIconSet())
-        ->setTransactionType(PhabricatorBadgesTransaction::TYPE_ICON)
+        ->setTransactionType(
+          PhabricatorBadgesBadgeIconTransaction::TRANSACTIONTYPE)
         ->setConduitDescription(pht('Change the badge icon.'))
         ->setConduitTypeDescription(pht('New badge icon.'))
         ->setValue($object->getIcon()),
@@ -109,14 +113,16 @@ final class PhabricatorBadgesEditEngine
         ->setDescription(pht('Color and rarity of the badge.'))
         ->setConduitTypeDescription(pht('New badge quality.'))
         ->setValue($object->getQuality())
-        ->setTransactionType(PhabricatorBadgesTransaction::TYPE_QUALITY)
+        ->setTransactionType(
+          PhabricatorBadgesBadgeQualityTransaction::TRANSACTIONTYPE)
         ->setOptions(PhabricatorBadgesQuality::getDropdownQualityMap()),
       id(new PhabricatorRemarkupEditField())
         ->setKey('description')
         ->setLabel(pht('Description'))
         ->setDescription(pht('Badge long description.'))
         ->setConduitTypeDescription(pht('New badge description.'))
-        ->setTransactionType(PhabricatorBadgesTransaction::TYPE_DESCRIPTION)
+        ->setTransactionType(
+          PhabricatorBadgesBadgeDescriptionTransaction::TRANSACTIONTYPE)
         ->setValue($object->getDescription()),
     );
   }
diff --git a/src/applications/badges/editor/PhabricatorBadgesEditor.php b/src/applications/badges/editor/PhabricatorBadgesEditor.php
index 45a42a3362..65b2ca6cdf 100644
--- a/src/applications/badges/editor/PhabricatorBadgesEditor.php
+++ b/src/applications/badges/editor/PhabricatorBadgesEditor.php
@@ -11,22 +11,20 @@ final class PhabricatorBadgesEditor
     return pht('Badges');
   }
 
+  public function getCreateObjectTitle($author, $object) {
+    return pht('%s created this badge.', $author);
+  }
+
+  public function getCreateObjectTitleForFeed($author, $object) {
+    return pht('%s created %s.', $author, $object);
+  }
+
   protected function supportsSearch() {
     return true;
   }
 
   public function getTransactionTypes() {
     $types = parent::getTransactionTypes();
-
-    $types[] = PhabricatorBadgesTransaction::TYPE_NAME;
-    $types[] = PhabricatorBadgesTransaction::TYPE_FLAVOR;
-    $types[] = PhabricatorBadgesTransaction::TYPE_DESCRIPTION;
-    $types[] = PhabricatorBadgesTransaction::TYPE_ICON;
-    $types[] = PhabricatorBadgesTransaction::TYPE_STATUS;
-    $types[] = PhabricatorBadgesTransaction::TYPE_QUALITY;
-    $types[] = PhabricatorBadgesTransaction::TYPE_AWARD;
-    $types[] = PhabricatorBadgesTransaction::TYPE_REVOKE;
-
     $types[] = PhabricatorTransactions::TYPE_COMMENT;
     $types[] = PhabricatorTransactions::TYPE_EDGE;
     $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
@@ -34,159 +32,6 @@ final class PhabricatorBadgesEditor
     return $types;
   }
 
-  protected function getCustomTransactionOldValue(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-    switch ($xaction->getTransactionType()) {
-      case PhabricatorBadgesTransaction::TYPE_NAME:
-        return $object->getName();
-      case PhabricatorBadgesTransaction::TYPE_FLAVOR:
-        return $object->getFlavor();
-      case PhabricatorBadgesTransaction::TYPE_DESCRIPTION:
-        return $object->getDescription();
-      case PhabricatorBadgesTransaction::TYPE_ICON:
-        return $object->getIcon();
-      case PhabricatorBadgesTransaction::TYPE_QUALITY:
-        return (int)$object->getQuality();
-      case PhabricatorBadgesTransaction::TYPE_STATUS:
-        return $object->getStatus();
-      case PhabricatorBadgesTransaction::TYPE_AWARD:
-        $award_phids = mpull($object->getAwards(), 'getRecipientPHID');
-        return $award_phids;
-      case PhabricatorBadgesTransaction::TYPE_REVOKE:
-        return null;
-    }
-
-    return parent::getCustomTransactionOldValue($object, $xaction);
-  }
-
-  protected function getCustomTransactionNewValue(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-
-    switch ($xaction->getTransactionType()) {
-      case PhabricatorBadgesTransaction::TYPE_NAME:
-      case PhabricatorBadgesTransaction::TYPE_FLAVOR:
-      case PhabricatorBadgesTransaction::TYPE_DESCRIPTION:
-      case PhabricatorBadgesTransaction::TYPE_ICON:
-      case PhabricatorBadgesTransaction::TYPE_STATUS:
-      case PhabricatorBadgesTransaction::TYPE_AWARD:
-      case PhabricatorBadgesTransaction::TYPE_REVOKE:
-        return $xaction->getNewValue();
-      case PhabricatorBadgesTransaction::TYPE_QUALITY:
-        return (int)$xaction->getNewValue();
-    }
-
-    return parent::getCustomTransactionNewValue($object, $xaction);
-  }
-
-  protected function applyCustomInternalTransaction(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-
-    $type = $xaction->getTransactionType();
-    switch ($type) {
-      case PhabricatorBadgesTransaction::TYPE_NAME:
-        $object->setName($xaction->getNewValue());
-        return;
-      case PhabricatorBadgesTransaction::TYPE_FLAVOR:
-        $object->setFlavor($xaction->getNewValue());
-        return;
-      case PhabricatorBadgesTransaction::TYPE_DESCRIPTION:
-        $object->setDescription($xaction->getNewValue());
-        return;
-      case PhabricatorBadgesTransaction::TYPE_ICON:
-        $object->setIcon($xaction->getNewValue());
-        return;
-      case PhabricatorBadgesTransaction::TYPE_QUALITY:
-        $object->setQuality($xaction->getNewValue());
-        return;
-      case PhabricatorBadgesTransaction::TYPE_STATUS:
-        $object->setStatus($xaction->getNewValue());
-        return;
-      case PhabricatorBadgesTransaction::TYPE_AWARD:
-      case PhabricatorBadgesTransaction::TYPE_REVOKE:
-        return;
-    }
-
-    return parent::applyCustomInternalTransaction($object, $xaction);
-  }
-
-  protected function applyCustomExternalTransaction(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-
-    $type = $xaction->getTransactionType();
-    switch ($type) {
-      case PhabricatorBadgesTransaction::TYPE_NAME:
-      case PhabricatorBadgesTransaction::TYPE_FLAVOR:
-      case PhabricatorBadgesTransaction::TYPE_DESCRIPTION:
-      case PhabricatorBadgesTransaction::TYPE_ICON:
-      case PhabricatorBadgesTransaction::TYPE_STATUS:
-      case PhabricatorBadgesTransaction::TYPE_QUALITY:
-        return;
-      case PhabricatorBadgesTransaction::TYPE_REVOKE:
-        $revoked_recipient_phids = $xaction->getNewValue();
-        $awards = $object->getAwards();
-        $awards = mpull($awards, null, 'getRecipientPHID');
-
-        foreach ($revoked_recipient_phids as $phid) {
-          $awards[$phid]->delete();
-        }
-        $object->attachAwards($awards);
-        return;
-      case PhabricatorBadgesTransaction::TYPE_AWARD:
-        $recipient_phids = $xaction->getNewValue();
-        $awards = $object->getAwards();
-        $awards = mpull($awards, null, 'getRecipientPHID');
-
-        foreach ($recipient_phids as $phid) {
-          $award = idx($awards, $phid);
-          if (!$award) {
-            $award = PhabricatorBadgesAward::initializeNewBadgesAward(
-              $this->getActor(),
-              $object,
-              $phid);
-            $award->save();
-            $awards[] = $award;
-          }
-        }
-        $object->attachAwards($awards);
-        return;
-    }
-
-    return parent::applyCustomExternalTransaction($object, $xaction);
-  }
-
-  protected function validateTransaction(
-    PhabricatorLiskDAO $object,
-    $type,
-    array $xactions) {
-
-    $errors = parent::validateTransaction($object, $type, $xactions);
-
-    switch ($type) {
-      case PhabricatorBadgesTransaction::TYPE_NAME:
-        $missing = $this->validateIsEmptyTextField(
-          $object->getName(),
-          $xactions);
-
-        if ($missing) {
-          $error = new PhabricatorApplicationTransactionValidationError(
-            $type,
-            pht('Required'),
-            pht('Badge name is required.'),
-            nonempty(last($xactions), null));
-
-          $error->setIsMissingFieldError(true);
-          $errors[] = $error;
-        }
-      break;
-    }
-
-    return $errors;
-  }
-
   protected function shouldSendMail(
     PhabricatorLiskDAO $object,
     array $xactions) {
diff --git a/src/applications/badges/storage/PhabricatorBadgesTransaction.php b/src/applications/badges/storage/PhabricatorBadgesTransaction.php
index 24509f94bb..75d8f7fd59 100644
--- a/src/applications/badges/storage/PhabricatorBadgesTransaction.php
+++ b/src/applications/badges/storage/PhabricatorBadgesTransaction.php
@@ -1,16 +1,7 @@
 <?php
 
 final class PhabricatorBadgesTransaction
-  extends PhabricatorApplicationTransaction {
-
-  const TYPE_NAME = 'badges:name';
-  const TYPE_DESCRIPTION = 'badges:description';
-  const TYPE_QUALITY = 'badges:quality';
-  const TYPE_ICON = 'badges:icon';
-  const TYPE_STATUS = 'badges:status';
-  const TYPE_FLAVOR = 'badges:flavor';
-  const TYPE_AWARD = 'badges:award';
-  const TYPE_REVOKE = 'badges:revoke';
+  extends PhabricatorModularTransaction {
 
   const MAILTAG_DETAILS = 'badges:details';
   const MAILTAG_COMMENT = 'badges:comment';
@@ -28,209 +19,8 @@ final class PhabricatorBadgesTransaction
     return new PhabricatorBadgesTransactionComment();
   }
 
-
-  public function getTitle() {
-    $author_phid = $this->getAuthorPHID();
-    $object_phid = $this->getObjectPHID();
-
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-
-    $type = $this->getTransactionType();
-    switch ($type) {
-      case PhabricatorTransactions::TYPE_CREATE:
-          return pht(
-            '%s created this badge.',
-            $this->renderHandleLink($author_phid));
-      case self::TYPE_NAME:
-        if ($old === null) {
-          return pht(
-            '%s created this badge.',
-            $this->renderHandleLink($author_phid));
-        } else {
-          return pht(
-            '%s renamed this badge from "%s" to "%s".',
-            $this->renderHandleLink($author_phid),
-            $old,
-            $new);
-        }
-        break;
-      case self::TYPE_FLAVOR:
-        if ($old === null) {
-          return pht(
-            '%s set the flavor text for this badge.',
-            $this->renderHandleLink($author_phid));
-        } else {
-          return pht(
-            '%s updated the flavor text for this badge.',
-            $this->renderHandleLink($author_phid));
-        }
-        break;
-      case self::TYPE_DESCRIPTION:
-        if ($old === null) {
-          return pht(
-            '%s set the description for this badge.',
-            $this->renderHandleLink($author_phid));
-        } else {
-          return pht(
-            '%s updated the description for this badge.',
-            $this->renderHandleLink($author_phid));
-        }
-        break;
-      case self::TYPE_STATUS:
-        switch ($new) {
-          case PhabricatorBadgesBadge::STATUS_ACTIVE:
-            return pht(
-              '%s activated this badge.',
-              $this->renderHandleLink($author_phid));
-          case PhabricatorBadgesBadge::STATUS_ARCHIVED:
-            return pht(
-              '%s archived this badge.',
-              $this->renderHandleLink($author_phid));
-        }
-        break;
-      case self::TYPE_ICON:
-        if ($old === null) {
-          return pht(
-            '%s set the icon for this badge as "%s".',
-            $this->renderHandleLink($author_phid),
-            $new);
-        } else {
-          $set = new PhabricatorBadgesIconSet();
-
-          $icon_old = $set->getIconLabel($old);
-          $icon_new = $set->getIconLabel($new);
-
-          return pht(
-            '%s updated the icon for this badge from "%s" to "%s".',
-            $this->renderHandleLink($author_phid),
-            $icon_old,
-            $icon_new);
-        }
-        break;
-      case self::TYPE_QUALITY:
-        $qual_new = PhabricatorBadgesQuality::getQualityName($new);
-        $qual_old = PhabricatorBadgesQuality::getQualityName($old);
-        if ($old === null) {
-          return pht(
-            '%s set the quality for this badge as "%s".',
-            $this->renderHandleLink($author_phid),
-            $qual_new);
-        } else {
-          return pht(
-            '%s updated the quality for this badge from "%s" to "%s".',
-            $this->renderHandleLink($author_phid),
-            $qual_old,
-            $qual_new);
-        }
-        break;
-      case self::TYPE_AWARD:
-        if (!is_array($new)) {
-          $new = array();
-        }
-        $handles = $this->renderHandleList($new);
-        return pht(
-          '%s awarded this badge to %s recipient(s): %s.',
-          $this->renderHandleLink($author_phid),
-          new PhutilNumber(count($new)),
-          $handles);
-      case self::TYPE_REVOKE:
-        if (!is_array($new)) {
-          $new = array();
-        }
-        $handles = $this->renderHandleList($new);
-        return pht(
-          '%s revoked this badge from %s recipient(s): %s.',
-          $this->renderHandleLink($author_phid),
-          new PhutilNumber(count($new)),
-          $handles);
-    }
-
-    return parent::getTitle();
-  }
-
-  public function getTitleForFeed() {
-    $author_phid = $this->getAuthorPHID();
-    $object_phid = $this->getObjectPHID();
-
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-
-    $type = $this->getTransactionType();
-    switch ($type) {
-      case self::TYPE_NAME:
-        if ($old === null) {
-          return pht(
-            '%s created %s.',
-            $this->renderHandleLink($author_phid),
-            $this->renderHandleLink($object_phid));
-
-        } else {
-          return pht(
-            '%s renamed %s.',
-            $this->renderHandleLink($author_phid),
-            $this->renderHandleLink($object_phid));
-        }
-      break;
-      case self::TYPE_FLAVOR:
-        return pht(
-          '%s updated the flavor text for %s.',
-          $this->renderHandleLink($author_phid),
-          $this->renderHandleLink($object_phid));
-      case self::TYPE_ICON:
-        return pht(
-          '%s updated the icon for %s.',
-          $this->renderHandleLink($author_phid),
-          $this->renderHandleLink($object_phid));
-      case self::TYPE_QUALITY:
-        return pht(
-          '%s updated the quality level for %s.',
-          $this->renderHandleLink($author_phid),
-          $this->renderHandleLink($object_phid));
-      case self::TYPE_DESCRIPTION:
-        return pht(
-          '%s updated the description for %s.',
-          $this->renderHandleLink($author_phid),
-          $this->renderHandleLink($object_phid));
-      case self::TYPE_STATUS:
-        switch ($new) {
-          case PhabricatorBadgesBadge::STATUS_ACTIVE:
-            return pht(
-              '%s activated %s.',
-              $this->renderHandleLink($author_phid),
-              $this->renderHandleLink($object_phid));
-          case PhabricatorBadgesBadge::STATUS_ARCHIVED:
-            return pht(
-              '%s archived %s.',
-              $this->renderHandleLink($author_phid),
-              $this->renderHandleLink($object_phid));
-        }
-        break;
-      case self::TYPE_AWARD:
-        if (!is_array($new)) {
-          $new = array();
-        }
-        $handles = $this->renderHandleList($new);
-        return pht(
-          '%s awarded %s to %s recipient(s): %s.',
-          $this->renderHandleLink($author_phid),
-          $this->renderHandleLink($object_phid),
-          new PhutilNumber(count($new)),
-          $handles);
-      case self::TYPE_REVOKE:
-        if (!is_array($new)) {
-          $new = array();
-        }
-        $handles = $this->renderHandleList($new);
-        return pht(
-          '%s revoked %s from %s recipient(s): %s.',
-          $this->renderHandleLink($author_phid),
-          $this->renderHandleLink($object_phid),
-          new PhutilNumber(count($new)),
-          $handles);
-    }
-
-    return parent::getTitleForFeed();
+  public function getBaseTransactionClass() {
+    return 'PhabricatorBadgesBadgeTransactionType';
   }
 
   public function getMailTags() {
@@ -240,14 +30,16 @@ final class PhabricatorBadgesTransaction
       case PhabricatorTransactions::TYPE_COMMENT:
         $tags[] = self::MAILTAG_COMMENT;
         break;
-      case self::TYPE_NAME:
-      case self::TYPE_DESCRIPTION:
-      case self::TYPE_FLAVOR:
-      case self::TYPE_ICON:
-      case self::TYPE_STATUS:
-      case self::TYPE_QUALITY:
+      case PhabricatorBadgesBadgeNameTransaction::TRANSACTIONTYPE:
+      case PhabricatorBadgesBadgeDescriptionTransaction::TRANSACTIONTYPE:
+      case PhabricatorBadgesBadgeFlavorTransaction::TRANSACTIONTYPE:
+      case PhabricatorBadgesBadgeIconTransaction::TRANSACTIONTYPE:
+      case PhabricatorBadgesBadgeStatusTransaction::TRANSACTIONTYPE:
+      case PhabricatorBadgesBadgeQualityTransaction::TRANSACTIONTYPE:
         $tags[] = self::MAILTAG_DETAILS;
         break;
+      case PhabricatorBadgesBadgeAwardTransaction::TRANSACTIONTYPE:
+      case PhabricatorBadgesBadgeRevokeTransaction::TRANSACTIONTYPE:
       default:
         $tags[] = self::MAILTAG_OTHER;
         break;
@@ -255,49 +47,4 @@ final class PhabricatorBadgesTransaction
     return $tags;
   }
 
-
-  public function shouldHide() {
-    $old = $this->getOldValue();
-    switch ($this->getTransactionType()) {
-      case self::TYPE_DESCRIPTION:
-        return ($old === null);
-    }
-    return parent::shouldHide();
-  }
-
-  public function hasChangeDetails() {
-    switch ($this->getTransactionType()) {
-      case self::TYPE_DESCRIPTION:
-        return ($this->getOldValue() !== null);
-    }
-
-    return parent::hasChangeDetails();
-  }
-
-  public function renderChangeDetails(PhabricatorUser $viewer) {
-    return $this->renderTextCorpusChangeDetails(
-      $viewer,
-      $this->getOldValue(),
-      $this->getNewValue());
-  }
-
-  public function getRequiredHandlePHIDs() {
-    $phids = parent::getRequiredHandlePHIDs();
-
-    $type = $this->getTransactionType();
-    switch ($type) {
-       case self::TYPE_AWARD:
-       case self::TYPE_REVOKE:
-        $new = $this->getNewValue();
-        if (!is_array($new)) {
-          $new = array();
-        }
-        foreach ($new as $phid) {
-          $phids[] = $phid;
-        }
-        break;
-    }
-
-    return $phids;
-  }
 }
diff --git a/src/applications/badges/xaction/PhabricatorBadgesBadgeAwardTransaction.php b/src/applications/badges/xaction/PhabricatorBadgesBadgeAwardTransaction.php
new file mode 100644
index 0000000000..5f339fa368
--- /dev/null
+++ b/src/applications/badges/xaction/PhabricatorBadgesBadgeAwardTransaction.php
@@ -0,0 +1,62 @@
+<?php
+
+final class PhabricatorBadgesBadgeAwardTransaction
+  extends PhabricatorBadgesBadgeTransactionType {
+
+  const TRANSACTIONTYPE = 'badge.award';
+
+  public function generateOldValue($object) {
+    return mpull($object->getAwards(), 'getRecipientPHID');
+  }
+
+  public function applyExternalEffects($object, $value) {
+    $awards = $object->getAwards();
+    $awards = mpull($awards, null, 'getRecipientPHID');
+
+    foreach ($value as $phid) {
+      $award = idx($awards, $phid);
+      if (!$award) {
+        $award = PhabricatorBadgesAward::initializeNewBadgesAward(
+          $this->getActor(),
+          $object,
+          $phid);
+        $award->save();
+        $awards[] = $award;
+      }
+    }
+    $object->attachAwards($awards);
+    return;
+  }
+
+  public function getTitle() {
+    $new = $this->getNewValue();
+    if (!is_array($new)) {
+      $new = array();
+    }
+    $handles = $this->renderHandleList($new);
+    return pht(
+      '%s awarded this badge to %s recipient(s): %s.',
+      $this->renderAuthor(),
+      new PhutilNumber(count($new)),
+      $handles);
+  }
+
+  public function getTitleForFeed() {
+    $new = $this->getNewValue();
+    if (!is_array($new)) {
+      $new = array();
+    }
+    $handles = $this->renderHandleList($new);
+    return pht(
+      '%s awarded %s to %s recipient(s): %s.',
+      $this->renderAuthor(),
+      $this->renderObject(),
+      new PhutilNumber(count($new)),
+      $handles);
+  }
+
+  public function getIcon() {
+    return 'fa-user-plus';
+  }
+
+}
diff --git a/src/applications/badges/xaction/PhabricatorBadgesBadgeDescriptionTransaction.php b/src/applications/badges/xaction/PhabricatorBadgesBadgeDescriptionTransaction.php
new file mode 100644
index 0000000000..0abaafc9c6
--- /dev/null
+++ b/src/applications/badges/xaction/PhabricatorBadgesBadgeDescriptionTransaction.php
@@ -0,0 +1,57 @@
+<?php
+
+final class PhabricatorBadgesBadgeDescriptionTransaction
+  extends PhabricatorBadgesBadgeTransactionType {
+
+  const TRANSACTIONTYPE = 'badge.description';
+
+  public function generateOldValue($object) {
+    return $object->getDescription();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setDescription($value);
+  }
+
+  public function getTitle() {
+    return pht(
+      '%s updated the badge description.',
+      $this->renderAuthor());
+  }
+
+  public function getTitleForFeed() {
+    return pht(
+      '%s updated the badge description for %s.',
+      $this->renderAuthor(),
+      $this->renderObject());
+  }
+
+  public function hasChangeDetailView() {
+    return true;
+  }
+
+  public function getMailDiffSectionHeader() {
+    return pht('CHANGES TO BADGE DESCRIPTION');
+  }
+
+  public function newChangeDetailView() {
+    $viewer = $this->getViewer();
+
+    return id(new PhabricatorApplicationTransactionTextDiffDetailView())
+      ->setViewer($viewer)
+      ->setOldText($this->getOldValue())
+      ->setNewText($this->getNewValue());
+  }
+
+  public function newRemarkupChanges() {
+    $changes = array();
+
+    $changes[] = $this->newRemarkupChange()
+      ->setOldValue($this->getOldValue())
+      ->setNewValue($this->getNewValue());
+
+    return $changes;
+  }
+
+
+}
diff --git a/src/applications/badges/xaction/PhabricatorBadgesBadgeFlavorTransaction.php b/src/applications/badges/xaction/PhabricatorBadgesBadgeFlavorTransaction.php
new file mode 100644
index 0000000000..069ec09603
--- /dev/null
+++ b/src/applications/badges/xaction/PhabricatorBadgesBadgeFlavorTransaction.php
@@ -0,0 +1,33 @@
+<?php
+
+final class PhabricatorBadgesBadgeFlavorTransaction
+  extends PhabricatorBadgesBadgeTransactionType {
+
+  const TRANSACTIONTYPE = 'badge.flavor';
+
+  public function generateOldValue($object) {
+    return $object->getFlavor();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setFlavor($value);
+  }
+
+  public function getTitle() {
+    return pht(
+      '%s updated the flavor from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderOldValue(),
+      $this->renderNewValue());
+  }
+
+  public function getTitleForFeed() {
+    return pht(
+      '%s updated %s flavor text from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderObject(),
+      $this->renderOldValue(),
+      $this->renderNewValue());
+  }
+
+}
diff --git a/src/applications/badges/xaction/PhabricatorBadgesBadgeIconTransaction.php b/src/applications/badges/xaction/PhabricatorBadgesBadgeIconTransaction.php
new file mode 100644
index 0000000000..7c0f535192
--- /dev/null
+++ b/src/applications/badges/xaction/PhabricatorBadgesBadgeIconTransaction.php
@@ -0,0 +1,56 @@
+<?php
+
+final class PhabricatorBadgesBadgeIconTransaction
+  extends PhabricatorBadgesBadgeTransactionType {
+
+  const TRANSACTIONTYPE = 'badge.icon';
+
+  public function generateOldValue($object) {
+    return $object->getIcon();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setIcon($value);
+  }
+
+  public function shouldHide() {
+    if ($this->isCreateTransaction()) {
+      return true;
+    }
+
+    return false;
+  }
+
+  public function getTitle() {
+    $old = $this->getIconLabel($this->getOldValue());
+    $new = $this->getIconLabel($this->getNewValue());
+
+    return pht(
+      '%s changed the badge icon from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderValue($old),
+      $this->renderValue($new));
+  }
+
+  public function getTitleForFeed() {
+    $old = $this->getIconLabel($this->getOldValue());
+    $new = $this->getIconLabel($this->getNewValue());
+
+    return pht(
+      '%s changed the badge icon for %s from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderObject(),
+      $this->renderValue($old),
+      $this->renderValue($new));
+  }
+
+  private function getIconLabel($icon) {
+    $set = new PhabricatorBadgesIconSet();
+    return $set->getIconLabel($icon);
+  }
+
+  public function getIcon() {
+    return $this->getNewValue();
+  }
+
+}
diff --git a/src/applications/badges/xaction/PhabricatorBadgesBadgeNameTransaction.php b/src/applications/badges/xaction/PhabricatorBadgesBadgeNameTransaction.php
new file mode 100644
index 0000000000..4565c95fb7
--- /dev/null
+++ b/src/applications/badges/xaction/PhabricatorBadgesBadgeNameTransaction.php
@@ -0,0 +1,44 @@
+<?php
+
+final class PhabricatorBadgesBadgeNameTransaction
+  extends PhabricatorBadgesBadgeTransactionType {
+
+  const TRANSACTIONTYPE = 'badge.name';
+
+  public function generateOldValue($object) {
+    return $object->getName();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setName($value);
+  }
+
+  public function getTitle() {
+    return pht(
+      '%s renamed this badge from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderOldValue(),
+      $this->renderNewValue());
+  }
+
+  public function getTitleForFeed() {
+    return pht(
+      '%s renamed %s badge %s to %s.',
+      $this->renderAuthor(),
+      $this->renderObject(),
+      $this->renderOldValue(),
+      $this->renderNewValue());
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    if ($this->isEmptyTextTransaction($object->getName(), $xactions)) {
+      $errors[] = $this->newRequiredError(
+        pht('Badges must have a name.'));
+    }
+
+    return $errors;
+  }
+
+}
diff --git a/src/applications/badges/xaction/PhabricatorBadgesBadgeQualityTransaction.php b/src/applications/badges/xaction/PhabricatorBadgesBadgeQualityTransaction.php
new file mode 100644
index 0000000000..de060c17e7
--- /dev/null
+++ b/src/applications/badges/xaction/PhabricatorBadgesBadgeQualityTransaction.php
@@ -0,0 +1,70 @@
+<?php
+
+final class PhabricatorBadgesBadgeQualityTransaction
+  extends PhabricatorBadgesBadgeTransactionType {
+
+  const TRANSACTIONTYPE = 'badge.quality';
+
+  public function generateOldValue($object) {
+    return $object->getQuality();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setQuality($value);
+  }
+
+  public function shouldHide() {
+    if ($this->isCreateTransaction()) {
+      return true;
+    }
+
+    return false;
+  }
+
+  public function getTitle() {
+    $old = $this->getQualityLabel($this->getOldValue());
+    $new = $this->getQualityLabel($this->getNewValue());
+
+    return pht(
+      '%s updated the quality from %s to %s.',
+      $this->renderAuthor(),
+      $old,
+      $new);
+  }
+
+  public function getTitleForFeed() {
+    $old = $this->getQualityLabel($this->getOldValue());
+    $new = $this->getQualityLabel($this->getNewValue());
+
+    return pht(
+      '%s updated %s quality from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderObject(),
+      $new,
+      $old);
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    if ($this->isEmptyTextTransaction($object->getQuality(), $xactions)) {
+      $errors[] = $this->newRequiredError(
+        pht('Badge quality must be set.'));
+    }
+
+    $map = PhabricatorBadgesQuality::getQualityMap();
+    if (!$map[$object->getQuality()]) {
+      $errors[] = $this->newRequiredError(
+        pht('Badge quality is not valid.'));
+    }
+
+    return $errors;
+  }
+
+  private function getQualityLabel($quality) {
+    $map = PhabricatorBadgesQuality::getQualityMap();
+    $name = $map[$quality]['name'];
+    return $this->renderValue($name);
+  }
+
+}
diff --git a/src/applications/badges/xaction/PhabricatorBadgesBadgeRevokeTransaction.php b/src/applications/badges/xaction/PhabricatorBadgesBadgeRevokeTransaction.php
new file mode 100644
index 0000000000..704a518371
--- /dev/null
+++ b/src/applications/badges/xaction/PhabricatorBadgesBadgeRevokeTransaction.php
@@ -0,0 +1,54 @@
+<?php
+
+final class PhabricatorBadgesBadgeRevokeTransaction
+  extends PhabricatorBadgesBadgeTransactionType {
+
+  const TRANSACTIONTYPE = 'badge.revoke';
+
+  public function generateOldValue($object) {
+    return null;
+  }
+
+  public function applyExternalEffects($object, $value) {
+    $awards = $object->getAwards();
+    $awards = mpull($awards, null, 'getRecipientPHID');
+
+    foreach ($value as $phid) {
+      $awards[$phid]->delete();
+    }
+    $object->attachAwards($awards);
+    return;
+  }
+
+  public function getTitle() {
+    $new = $this->getNewValue();
+    if (!is_array($new)) {
+      $new = array();
+    }
+    $handles = $this->renderHandleList($new);
+    return pht(
+      '%s revoked this badge from %s recipient(s): %s.',
+      $this->renderAuthor(),
+      new PhutilNumber(count($new)),
+      $handles);
+  }
+
+  public function getTitleForFeed() {
+    $new = $this->getNewValue();
+    if (!is_array($new)) {
+      $new = array();
+    }
+    $handles = $this->renderHandleList($new);
+    return pht(
+      '%s revoked %s from %s recipient(s): %s.',
+      $this->renderAuthor(),
+      $this->renderObject(),
+      new PhutilNumber(count($new)),
+      $handles);
+  }
+
+  public function getIcon() {
+    return 'fa-user-times';
+  }
+
+}
diff --git a/src/applications/badges/xaction/PhabricatorBadgesBadgeStatusTransaction.php b/src/applications/badges/xaction/PhabricatorBadgesBadgeStatusTransaction.php
new file mode 100644
index 0000000000..ba5ad4a5c6
--- /dev/null
+++ b/src/applications/badges/xaction/PhabricatorBadgesBadgeStatusTransaction.php
@@ -0,0 +1,50 @@
+<?php
+
+final class PhabricatorBadgesBadgeStatusTransaction
+  extends PhabricatorBadgesBadgeTransactionType {
+
+  const TRANSACTIONTYPE = 'badges.status';
+
+  public function generateOldValue($object) {
+    return $object->getStatus();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setStatus($value);
+  }
+
+  public function getTitle() {
+    if ($this->getNewValue() == PhabricatorBadgesBadge::STATUS_ARCHIVED) {
+      return pht(
+        '%s disabled this badge.',
+        $this->renderAuthor());
+    } else {
+      return pht(
+        '%s enabled this badge.',
+        $this->renderAuthor());
+    }
+  }
+
+  public function getTitleForFeed() {
+    if ($this->getNewValue() == PhabricatorBadgesBadge::STATUS_ARCHIVED) {
+      return pht(
+        '%s disabled the badge %s.',
+        $this->renderAuthor(),
+        $this->renderObject());
+    } else {
+      return pht(
+        '%s enabled the badge %s.',
+        $this->renderAuthor(),
+        $this->renderObject());
+    }
+  }
+
+  public function getIcon() {
+    if ($this->getNewValue() == PhabricatorBadgesBadge::STATUS_ARCHIVED) {
+      return 'fa-ban';
+    } else {
+      return 'fa-check';
+    }
+  }
+
+}
diff --git a/src/applications/badges/xaction/PhabricatorBadgesBadgeTransactionType.php b/src/applications/badges/xaction/PhabricatorBadgesBadgeTransactionType.php
new file mode 100644
index 0000000000..6d47301eeb
--- /dev/null
+++ b/src/applications/badges/xaction/PhabricatorBadgesBadgeTransactionType.php
@@ -0,0 +1,4 @@
+<?php
+
+abstract class PhabricatorBadgesBadgeTransactionType
+  extends PhabricatorModularTransactionType {}

From 3b6a651b69fe96ce782c24b47308a8d98b781acf Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 23 Feb 2017 14:42:40 -0800
Subject: [PATCH 17/25] Merge multiple Auditors transactions from Herald

Summary:
Fixes T12302. Currently, we aren't merging multiple "AddAuditors" transactions correctly.

This can occur when Herald triggers multiple auditor rules.

Instead, merge them.

Test Plan:
  - Wrote two different Herald rules that add auditors.
  - Pushed a commit which triggered them.
  - After the change, saw all the auditors get added correctly.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12302

Differential Revision: https://secure.phabricator.com/D17403
---
 .../DiffusionCommitAuditorsTransaction.php    | 25 +++++++++++++++++++
 ...habricatorApplicationTransactionEditor.php |  6 +++++
 .../PhabricatorModularTransactionType.php     |  7 ++++++
 3 files changed, 38 insertions(+)

diff --git a/src/applications/diffusion/xaction/DiffusionCommitAuditorsTransaction.php b/src/applications/diffusion/xaction/DiffusionCommitAuditorsTransaction.php
index a4b1d817b4..a02a34590d 100644
--- a/src/applications/diffusion/xaction/DiffusionCommitAuditorsTransaction.php
+++ b/src/applications/diffusion/xaction/DiffusionCommitAuditorsTransaction.php
@@ -58,6 +58,31 @@ final class DiffusionCommitAuditorsTransaction
     return ($old !== $new);
   }
 
+  public function mergeTransactions(
+    $object,
+    PhabricatorApplicationTransaction $u,
+    PhabricatorApplicationTransaction $v) {
+
+    $u_new = $u->getNewValue();
+    $v_new = $v->getNewValue();
+
+    $result = $v_new;
+    foreach (array('-', '+') as $key) {
+      $u_value = idx($u_new, $key, array());
+      $v_value = idx($v_new, $key, array());
+
+      $merged = $u_value + $v_value;
+
+      if ($merged) {
+        $result[$key] = $merged;
+      }
+    }
+
+    $u->setNewValue($result);
+
+    return $u;
+  }
+
   public function applyExternalEffects($object, $value) {
     $src_phid = $object->getPHID();
 
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
index 5851e0186c..60dcc1d513 100644
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -1462,6 +1462,12 @@ abstract class PhabricatorApplicationTransactionEditor
 
     $type = $u->getTransactionType();
 
+    $xtype = $this->getModularTransactionType($type);
+    if ($xtype) {
+      $object = $this->object;
+      return $xtype->mergeTransactions($object, $u, $v);
+    }
+
     switch ($type) {
       case PhabricatorTransactions::TYPE_SUBSCRIBERS:
         return $this->mergePHIDOrEdgeTransactions($u, $v);
diff --git a/src/applications/transactions/storage/PhabricatorModularTransactionType.php b/src/applications/transactions/storage/PhabricatorModularTransactionType.php
index f57d91b08f..55ad678539 100644
--- a/src/applications/transactions/storage/PhabricatorModularTransactionType.php
+++ b/src/applications/transactions/storage/PhabricatorModularTransactionType.php
@@ -87,6 +87,13 @@ abstract class PhabricatorModularTransactionType
     return array();
   }
 
+  public function mergeTransactions(
+    $object,
+    PhabricatorApplicationTransaction $u,
+    PhabricatorApplicationTransaction $v) {
+    return null;
+  }
+
   final public function setStorage(
     PhabricatorApplicationTransaction $xaction) {
     $this->storage = $xaction;

From 89d1403fe89d7ec09fcaaa07d7f661c11a72a74c Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 23 Feb 2017 14:42:42 -0800
Subject: [PATCH 18/25] Explicitly decline to add commit authors as auditors
 from Herald

Summary:
Fixes T12304. If you have a Herald rule which tries to add a commit author as an auditor, it fails validation when trying to apply.

Stop trying to apply these transactions, and explicitly tell the user why. Differential already uses a similar ruleset around reviewers, but Audit was using older code.

Test Plan:
  - Wrote a Herald rule to add A, B and C as auditors.
  - Committed as A.
  - After change, saw B and C added with transacript guidance that A was the author.

{F3235660}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12304

Differential Revision: https://secure.phabricator.com/D17404
---
 .../herald/DiffusionAuditorsHeraldAction.php  | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/src/applications/diffusion/herald/DiffusionAuditorsHeraldAction.php b/src/applications/diffusion/herald/DiffusionAuditorsHeraldAction.php
index 2ca2ee2b6b..4ec23b1faa 100644
--- a/src/applications/diffusion/herald/DiffusionAuditorsHeraldAction.php
+++ b/src/applications/diffusion/herald/DiffusionAuditorsHeraldAction.php
@@ -3,6 +3,7 @@
 abstract class DiffusionAuditorsHeraldAction
   extends HeraldAction {
 
+  const DO_AUTHORS = 'do.authors';
   const DO_ADD_AUDITORS = 'do.add-auditors';
 
   public function getActionGroupKey() {
@@ -19,6 +20,22 @@ abstract class DiffusionAuditorsHeraldAction
 
     $auditors = $object->getAudits();
 
+    // Don't try to add commit authors as auditors.
+    $authors = array();
+    foreach ($phids as $key => $phid) {
+      if ($phid == $object->getAuthorPHID()) {
+        $authors[] = $phid;
+        unset($phids[$key]);
+      }
+    }
+
+    if ($authors) {
+      $this->logEffect(self::DO_AUTHORS, $authors);
+      if (!$phids) {
+        return;
+      }
+    }
+
     $current = array();
     foreach ($auditors as $auditor) {
       if ($auditor->isInteresting()) {
@@ -53,6 +70,11 @@ abstract class DiffusionAuditorsHeraldAction
 
   protected function getActionEffectMap() {
     return array(
+      self::DO_AUTHORS => array(
+        'icon' => 'fa-user',
+        'color' => 'grey',
+        'name' => pht('Commit Author'),
+      ),
       self::DO_ADD_AUDITORS => array(
         'icon' => 'fa-user',
         'color' => 'green',
@@ -63,6 +85,10 @@ abstract class DiffusionAuditorsHeraldAction
 
   protected function renderActionEffectDescription($type, $data) {
     switch ($type) {
+      case self::DO_AUTHORS:
+        return pht(
+          'Declined to add commit author as auditor: %s.',
+          $this->renderHandleList($data));
       case self::DO_ADD_AUDITORS:
         return pht(
           'Added %s auditor(s): %s.',

From d38ee2d79aa6a3a4c6ebdc62868fcd4b7c746045 Mon Sep 17 00:00:00 2001
From: Chad Little <chad@phacility.com>
Date: Fri, 24 Feb 2017 08:27:35 -0800
Subject: [PATCH 19/25] Update Phurl for modular transactions

Summary: Ref T6049. This moves Phurl to modular transactions.

Test Plan: Everything works here, add phurl, edit phurl, use phurl. Test various error states. Left a TODO on the validate dupe keys, not sure how to implement that in modular-land.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T6049

Differential Revision: https://secure.phabricator.com/D17405
---
 src/__phutil_library_map__.php                |  12 +-
 .../editor/PhabricatorPhurlURLEditEngine.php  |  18 +-
 .../editor/PhabricatorPhurlURLEditor.php      | 199 ++--------------
 .../PhabricatorPhurlURLTransaction.php        | 217 ++----------------
 .../PhabricatorPhurlURLAliasTransaction.php   |  77 +++++++
 ...bricatorPhurlURLDescriptionTransaction.php |  60 +++++
 .../PhabricatorPhurlURLLongURLTransaction.php |  59 +++++
 .../PhabricatorPhurlURLNameTransaction.php    |  44 ++++
 .../PhabricatorPhurlURLTransactionType.php    |   4 +
 ...habricatorApplicationTransactionEditor.php |  45 ----
 10 files changed, 304 insertions(+), 431 deletions(-)
 create mode 100644 src/applications/phurl/xaction/PhabricatorPhurlURLAliasTransaction.php
 create mode 100644 src/applications/phurl/xaction/PhabricatorPhurlURLDescriptionTransaction.php
 create mode 100644 src/applications/phurl/xaction/PhabricatorPhurlURLLongURLTransaction.php
 create mode 100644 src/applications/phurl/xaction/PhabricatorPhurlURLNameTransaction.php
 create mode 100644 src/applications/phurl/xaction/PhabricatorPhurlURLTransactionType.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index e6e48571ff..bdb8cb120f 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -3380,16 +3380,20 @@ phutil_register_library_map(array(
     'PhabricatorPhurlShortURLDefaultController' => 'applications/phurl/controller/PhabricatorPhurlShortURLDefaultController.php',
     'PhabricatorPhurlURL' => 'applications/phurl/storage/PhabricatorPhurlURL.php',
     'PhabricatorPhurlURLAccessController' => 'applications/phurl/controller/PhabricatorPhurlURLAccessController.php',
+    'PhabricatorPhurlURLAliasTransaction' => 'applications/phurl/xaction/PhabricatorPhurlURLAliasTransaction.php',
     'PhabricatorPhurlURLCommentController' => 'applications/phurl/controller/PhabricatorPhurlURLCommentController.php',
     'PhabricatorPhurlURLCreateCapability' => 'applications/phurl/capability/PhabricatorPhurlURLCreateCapability.php',
     'PhabricatorPhurlURLDatasource' => 'applications/phurl/typeahead/PhabricatorPhurlURLDatasource.php',
+    'PhabricatorPhurlURLDescriptionTransaction' => 'applications/phurl/xaction/PhabricatorPhurlURLDescriptionTransaction.php',
     'PhabricatorPhurlURLEditConduitAPIMethod' => 'applications/phurl/conduit/PhabricatorPhurlURLEditConduitAPIMethod.php',
     'PhabricatorPhurlURLEditController' => 'applications/phurl/controller/PhabricatorPhurlURLEditController.php',
     'PhabricatorPhurlURLEditEngine' => 'applications/phurl/editor/PhabricatorPhurlURLEditEngine.php',
     'PhabricatorPhurlURLEditor' => 'applications/phurl/editor/PhabricatorPhurlURLEditor.php',
     'PhabricatorPhurlURLListController' => 'applications/phurl/controller/PhabricatorPhurlURLListController.php',
+    'PhabricatorPhurlURLLongURLTransaction' => 'applications/phurl/xaction/PhabricatorPhurlURLLongURLTransaction.php',
     'PhabricatorPhurlURLMailReceiver' => 'applications/phurl/mail/PhabricatorPhurlURLMailReceiver.php',
     'PhabricatorPhurlURLNameNgrams' => 'applications/phurl/storage/PhabricatorPhurlURLNameNgrams.php',
+    'PhabricatorPhurlURLNameTransaction' => 'applications/phurl/xaction/PhabricatorPhurlURLNameTransaction.php',
     'PhabricatorPhurlURLPHIDType' => 'applications/phurl/phid/PhabricatorPhurlURLPHIDType.php',
     'PhabricatorPhurlURLQuery' => 'applications/phurl/query/PhabricatorPhurlURLQuery.php',
     'PhabricatorPhurlURLReplyHandler' => 'applications/phurl/mail/PhabricatorPhurlURLReplyHandler.php',
@@ -3398,6 +3402,7 @@ phutil_register_library_map(array(
     'PhabricatorPhurlURLTransaction' => 'applications/phurl/storage/PhabricatorPhurlURLTransaction.php',
     'PhabricatorPhurlURLTransactionComment' => 'applications/phurl/storage/PhabricatorPhurlURLTransactionComment.php',
     'PhabricatorPhurlURLTransactionQuery' => 'applications/phurl/query/PhabricatorPhurlURLTransactionQuery.php',
+    'PhabricatorPhurlURLTransactionType' => 'applications/phurl/xaction/PhabricatorPhurlURLTransactionType.php',
     'PhabricatorPhurlURLViewController' => 'applications/phurl/controller/PhabricatorPhurlURLViewController.php',
     'PhabricatorPinnedApplicationsSetting' => 'applications/settings/setting/PhabricatorPinnedApplicationsSetting.php',
     'PhabricatorPirateEnglishTranslation' => 'infrastructure/internationalization/translation/PhabricatorPirateEnglishTranslation.php',
@@ -8545,24 +8550,29 @@ phutil_register_library_map(array(
       'PhabricatorNgramsInterface',
     ),
     'PhabricatorPhurlURLAccessController' => 'PhabricatorPhurlController',
+    'PhabricatorPhurlURLAliasTransaction' => 'PhabricatorPhurlURLTransactionType',
     'PhabricatorPhurlURLCommentController' => 'PhabricatorPhurlController',
     'PhabricatorPhurlURLCreateCapability' => 'PhabricatorPolicyCapability',
     'PhabricatorPhurlURLDatasource' => 'PhabricatorTypeaheadDatasource',
+    'PhabricatorPhurlURLDescriptionTransaction' => 'PhabricatorPhurlURLTransactionType',
     'PhabricatorPhurlURLEditConduitAPIMethod' => 'PhabricatorEditEngineAPIMethod',
     'PhabricatorPhurlURLEditController' => 'PhabricatorPhurlController',
     'PhabricatorPhurlURLEditEngine' => 'PhabricatorEditEngine',
     'PhabricatorPhurlURLEditor' => 'PhabricatorApplicationTransactionEditor',
     'PhabricatorPhurlURLListController' => 'PhabricatorPhurlController',
+    'PhabricatorPhurlURLLongURLTransaction' => 'PhabricatorPhurlURLTransactionType',
     'PhabricatorPhurlURLMailReceiver' => 'PhabricatorObjectMailReceiver',
     'PhabricatorPhurlURLNameNgrams' => 'PhabricatorSearchNgrams',
+    'PhabricatorPhurlURLNameTransaction' => 'PhabricatorPhurlURLTransactionType',
     'PhabricatorPhurlURLPHIDType' => 'PhabricatorPHIDType',
     'PhabricatorPhurlURLQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhabricatorPhurlURLReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler',
     'PhabricatorPhurlURLSearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod',
     'PhabricatorPhurlURLSearchEngine' => 'PhabricatorApplicationSearchEngine',
-    'PhabricatorPhurlURLTransaction' => 'PhabricatorApplicationTransaction',
+    'PhabricatorPhurlURLTransaction' => 'PhabricatorModularTransaction',
     'PhabricatorPhurlURLTransactionComment' => 'PhabricatorApplicationTransactionComment',
     'PhabricatorPhurlURLTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
+    'PhabricatorPhurlURLTransactionType' => 'PhabricatorModularTransactionType',
     'PhabricatorPhurlURLViewController' => 'PhabricatorPhurlController',
     'PhabricatorPinnedApplicationsSetting' => 'PhabricatorInternalSetting',
     'PhabricatorPirateEnglishTranslation' => 'PhutilTranslation',
diff --git a/src/applications/phurl/editor/PhabricatorPhurlURLEditEngine.php b/src/applications/phurl/editor/PhabricatorPhurlURLEditEngine.php
index 3efabcceee..12a4d2672f 100644
--- a/src/applications/phurl/editor/PhabricatorPhurlURLEditEngine.php
+++ b/src/applications/phurl/editor/PhabricatorPhurlURLEditEngine.php
@@ -21,6 +21,10 @@ final class PhabricatorPhurlURLEditEngine
     return pht('Configure creation and editing forms in Phurl.');
   }
 
+  public function isEngineConfigurable() {
+    return false;
+  }
+
   protected function newEditableObject() {
     return PhabricatorPhurlURL::initializeNewPhurlURL($this->getViewer());
   }
@@ -73,8 +77,10 @@ final class PhabricatorPhurlURLEditEngine
         ->setKey('name')
         ->setLabel(pht('Name'))
         ->setDescription(pht('URL name.'))
+        ->setIsRequired(true)
         ->setConduitTypeDescription(pht('New URL name.'))
-        ->setTransactionType(PhabricatorPhurlURLTransaction::TYPE_NAME)
+        ->setTransactionType(
+          PhabricatorPhurlURLNameTransaction::TRANSACTIONTYPE)
         ->setValue($object->getName()),
       id(new PhabricatorTextEditField())
         ->setKey('url')
@@ -83,11 +89,14 @@ final class PhabricatorPhurlURLEditEngine
         ->setConduitTypeDescription(pht('New URL.'))
         ->setValue($object->getLongURL())
         ->setIsRequired(true)
-        ->setTransactionType(PhabricatorPhurlURLTransaction::TYPE_URL),
+        ->setTransactionType(
+          PhabricatorPhurlURLLongURLTransaction::TRANSACTIONTYPE),
       id(new PhabricatorTextEditField())
         ->setKey('alias')
         ->setLabel(pht('Alias'))
-        ->setTransactionType(PhabricatorPhurlURLTransaction::TYPE_ALIAS)
+        ->setIsRequired(true)
+        ->setTransactionType(
+          PhabricatorPhurlURLAliasTransaction::TRANSACTIONTYPE)
         ->setDescription(pht('The alias to give the URL.'))
         ->setConduitTypeDescription(pht('New alias.'))
         ->setValue($object->getAlias()),
@@ -96,7 +105,8 @@ final class PhabricatorPhurlURLEditEngine
         ->setLabel(pht('Description'))
         ->setDescription(pht('URL long description.'))
         ->setConduitTypeDescription(pht('New URL description.'))
-        ->setTransactionType(PhabricatorPhurlURLTransaction::TYPE_DESCRIPTION)
+        ->setTransactionType(
+          PhabricatorPhurlURLDescriptionTransaction::TRANSACTIONTYPE)
         ->setValue($object->getDescription()),
     );
   }
diff --git a/src/applications/phurl/editor/PhabricatorPhurlURLEditor.php b/src/applications/phurl/editor/PhabricatorPhurlURLEditor.php
index 2b6290851b..439d62f84a 100644
--- a/src/applications/phurl/editor/PhabricatorPhurlURLEditor.php
+++ b/src/applications/phurl/editor/PhabricatorPhurlURLEditor.php
@@ -11,14 +11,20 @@ final class PhabricatorPhurlURLEditor
     return pht('Phurl');
   }
 
+  public function getCreateObjectTitle($author, $object) {
+    return pht('%s created this URL.', $author);
+  }
+
+  public function getCreateObjectTitleForFeed($author, $object) {
+    return pht('%s created %s.', $author, $object);
+  }
+
+  protected function supportsSearch() {
+    return true;
+  }
+
   public function getTransactionTypes() {
     $types = parent::getTransactionTypes();
-
-    $types[] = PhabricatorPhurlURLTransaction::TYPE_NAME;
-    $types[] = PhabricatorPhurlURLTransaction::TYPE_URL;
-    $types[] = PhabricatorPhurlURLTransaction::TYPE_ALIAS;
-    $types[] = PhabricatorPhurlURLTransaction::TYPE_DESCRIPTION;
-
     $types[] = PhabricatorTransactions::TYPE_COMMENT;
     $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
     $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
@@ -26,160 +32,18 @@ final class PhabricatorPhurlURLEditor
     return $types;
   }
 
-  protected function getCustomTransactionOldValue(
+  protected function shouldSendMail(
     PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-    switch ($xaction->getTransactionType()) {
-      case PhabricatorPhurlURLTransaction::TYPE_NAME:
-        return $object->getName();
-      case PhabricatorPhurlURLTransaction::TYPE_URL:
-        return $object->getLongURL();
-      case PhabricatorPhurlURLTransaction::TYPE_ALIAS:
-        return $object->getAlias();
-      case PhabricatorPhurlURLTransaction::TYPE_DESCRIPTION:
-        return $object->getDescription();
-    }
-
-    return parent::getCustomTransactionOldValue($object, $xaction);
-  }
-
-  protected function getCustomTransactionNewValue(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-    switch ($xaction->getTransactionType()) {
-      case PhabricatorPhurlURLTransaction::TYPE_NAME:
-      case PhabricatorPhurlURLTransaction::TYPE_URL:
-      case PhabricatorPhurlURLTransaction::TYPE_DESCRIPTION:
-        return $xaction->getNewValue();
-      case PhabricatorPhurlURLTransaction::TYPE_ALIAS:
-        if (!strlen($xaction->getNewValue())) {
-          return null;
-        }
-        return $xaction->getNewValue();
-    }
-
-    return parent::getCustomTransactionNewValue($object, $xaction);
-  }
-
-  protected function applyCustomInternalTransaction(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-
-    switch ($xaction->getTransactionType()) {
-      case PhabricatorPhurlURLTransaction::TYPE_NAME:
-        $object->setName($xaction->getNewValue());
-        return;
-      case PhabricatorPhurlURLTransaction::TYPE_URL:
-        $object->setLongURL($xaction->getNewValue());
-        return;
-      case PhabricatorPhurlURLTransaction::TYPE_ALIAS:
-        $object->setAlias($xaction->getNewValue());
-        return;
-      case PhabricatorPhurlURLTransaction::TYPE_DESCRIPTION:
-        $object->setDescription($xaction->getNewValue());
-        return;
-    }
-
-    return parent::applyCustomInternalTransaction($object, $xaction);
-  }
-
-  protected function applyCustomExternalTransaction(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-
-    switch ($xaction->getTransactionType()) {
-      case PhabricatorPhurlURLTransaction::TYPE_NAME:
-      case PhabricatorPhurlURLTransaction::TYPE_URL:
-      case PhabricatorPhurlURLTransaction::TYPE_ALIAS:
-      case PhabricatorPhurlURLTransaction::TYPE_DESCRIPTION:
-        return;
-    }
-
-    return parent::applyCustomExternalTransaction($object, $xaction);
-  }
-
-  protected function validateTransaction(
-    PhabricatorLiskDAO $object,
-    $type,
     array $xactions) {
+    return true;
+  }
 
-    $errors = parent::validateTransaction($object, $type, $xactions);
-
-    switch ($type) {
-      case PhabricatorPhurlURLTransaction::TYPE_ALIAS:
-        $overdrawn = $this->validateIsTextFieldTooLong(
-          $object->getName(),
-          $xactions,
-          64);
-
-        if ($overdrawn) {
-          $errors[] = new PhabricatorApplicationTransactionValidationError(
-            $type,
-            pht('Alias Too Long'),
-            pht('The alias can be no longer than 64 characters.'),
-            nonempty(last($xactions), null));
-        }
-
-        foreach ($xactions as $xaction) {
-          if ($xaction->getOldValue() != $xaction->getNewValue()) {
-            $new_alias = $xaction->getNewValue();
-            $debug_alias = new PHUIInvisibleCharacterView($new_alias);
-            if (!preg_match('/[a-zA-Z]/', $new_alias)) {
-              $errors[] = new PhabricatorApplicationTransactionValidationError(
-                $type,
-                pht('Invalid Alias'),
-                pht('The alias you provided (%s) must contain at least one '.
-                  'letter.',
-                  $debug_alias),
-                $xaction);
-            }
-            if (preg_match('/[^a-z0-9]/i', $new_alias)) {
-              $errors[] = new PhabricatorApplicationTransactionValidationError(
-                $type,
-                pht('Invalid Alias'),
-                pht('The alias you provided (%s) may only contain letters and '.
-                  'numbers.',
-                  $debug_alias),
-                $xaction);
-            }
-          }
-        }
-
-        break;
-      case PhabricatorPhurlURLTransaction::TYPE_URL:
-        $missing = $this->validateIsEmptyTextField(
-          $object->getLongURL(),
-          $xactions);
-
-        if ($missing) {
-          $error = new PhabricatorApplicationTransactionValidationError(
-            $type,
-            pht('Required'),
-            pht('URL path is required.'),
-            nonempty(last($xactions), null));
-
-          $error->setIsMissingFieldError(true);
-          $errors[] = $error;
-        }
-
-        foreach ($xactions as $xaction) {
-          if ($xaction->getOldValue() != $xaction->getNewValue()) {
-            $protocols = PhabricatorEnv::getEnvConfig('uri.allowed-protocols');
-            $uri = new PhutilURI($xaction->getNewValue());
-            if (!isset($protocols[$uri->getProtocol()])) {
-              $errors[] = new PhabricatorApplicationTransactionValidationError(
-                $type,
-                pht('Invalid URL'),
-                pht('The protocol of the URL is invalid.'),
-                null);
-            }
-          }
-        }
-
-        break;
-    }
-
-    return $errors;
+  public function getMailTagsMap() {
+    return array(
+      PhabricatorPhurlURLTransaction::MAILTAG_DETAILS =>
+        pht(
+          "A URL's details change."),
+    );
   }
 
   protected function shouldPublishFeedStory(
@@ -188,36 +52,17 @@ final class PhabricatorPhurlURLEditor
     return true;
   }
 
-  protected function supportsSearch() {
-    return true;
-  }
-
-  protected function shouldSendMail(
-    PhabricatorLiskDAO $object,
-    array $xactions) {
-    return true;
-  }
-
   protected function getMailSubjectPrefix() {
     return pht('[Phurl]');
   }
 
   protected function getMailTo(PhabricatorLiskDAO $object) {
     $phids = array();
-
     $phids[] = $this->getActingAsPHID();
 
     return $phids;
   }
 
-  public function getMailTagsMap() {
-    return array(
-      PhabricatorPhurlURLTransaction::MAILTAG_DETAILS =>
-        pht(
-          "A URL's details change."),
-    );
-  }
-
   protected function buildMailTemplate(PhabricatorLiskDAO $object) {
     $id = $object->getID();
     $name = $object->getName();
@@ -255,7 +100,7 @@ final class PhabricatorPhurlURLEditor
 
     $errors = array();
     $errors[] = new PhabricatorApplicationTransactionValidationError(
-      PhabricatorPhurlURLTransaction::TYPE_ALIAS,
+      PhabricatorPhurlURLAliasTransaction::TRANSACTIONTYPE,
       pht('Duplicate'),
       pht('This alias is already in use.'),
       null);
diff --git a/src/applications/phurl/storage/PhabricatorPhurlURLTransaction.php b/src/applications/phurl/storage/PhabricatorPhurlURLTransaction.php
index d520b30518..cc2f2a8672 100644
--- a/src/applications/phurl/storage/PhabricatorPhurlURLTransaction.php
+++ b/src/applications/phurl/storage/PhabricatorPhurlURLTransaction.php
@@ -1,12 +1,7 @@
 <?php
 
 final class PhabricatorPhurlURLTransaction
-  extends PhabricatorApplicationTransaction {
-
-  const TYPE_NAME = 'phurl.name';
-  const TYPE_URL = 'phurl.longurl';
-  const TYPE_ALIAS = 'phurl.alias';
-  const TYPE_DESCRIPTION = 'phurl.description';
+  extends PhabricatorModularTransaction {
 
   const MAILTAG_DETAILS = 'phurl-details';
 
@@ -22,14 +17,18 @@ final class PhabricatorPhurlURLTransaction
     return new PhabricatorPhurlURLTransactionComment();
   }
 
+  public function getBaseTransactionClass() {
+    return 'PhabricatorPhurlURLTransactionType';
+  }
+
   public function getRequiredHandlePHIDs() {
     $phids = parent::getRequiredHandlePHIDs();
 
     switch ($this->getTransactionType()) {
-      case self::TYPE_NAME:
-      case self::TYPE_URL:
-      case self::TYPE_ALIAS:
-      case self::TYPE_DESCRIPTION:
+      case PhabricatorPhurlURLNameTransaction::TRANSACTIONTYPE:
+      case PhabricatorPhurlURLLongURLTransaction::TRANSACTIONTYPE:
+      case PhabricatorPhurlURLAliasTransaction::TRANSACTIONTYPE:
+      case PhabricatorPhurlURLDescriptionTransaction::TRANSACTIONTYPE:
         $phids[] = $this->getObjectPHID();
         break;
     }
@@ -37,203 +36,13 @@ final class PhabricatorPhurlURLTransaction
     return $phids;
   }
 
-  public function shouldHide() {
-    $old = $this->getOldValue();
-    switch ($this->getTransactionType()) {
-      case self::TYPE_DESCRIPTION:
-        return ($old === null);
-    }
-    return parent::shouldHide();
-  }
-
-  public function getIcon() {
-    switch ($this->getTransactionType()) {
-      case self::TYPE_NAME:
-      case self::TYPE_URL:
-      case self::TYPE_ALIAS:
-      case self::TYPE_DESCRIPTION:
-        return 'fa-pencil';
-        break;
-    }
-    return parent::getIcon();
-  }
-
-  public function getTitle() {
-    $author_phid = $this->getAuthorPHID();
-    $object_phid = $this->getObjectPHID();
-
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-
-    $type = $this->getTransactionType();
-    switch ($type) {
-      case self::TYPE_NAME:
-        if ($old === null) {
-          return pht(
-            '%s created this URL.',
-            $this->renderHandleLink($author_phid));
-        } else {
-          return pht(
-            '%s changed the name of the URL from %s to %s.',
-            $this->renderHandleLink($author_phid),
-            $old,
-            $new);
-        }
-      case self::TYPE_URL:
-        if ($old === null) {
-          return pht(
-            '%s set the destination of the URL to %s.',
-            $this->renderHandleLink($author_phid),
-            $new);
-        } else {
-          return pht(
-            '%s changed the destination of the URL from %s to %s.',
-            $this->renderHandleLink($author_phid),
-            $old,
-            $new);
-        }
-      case self::TYPE_ALIAS:
-        if ($old === null) {
-          return pht(
-            '%s set the alias of the URL to %s.',
-            $this->renderHandleLink($author_phid),
-            $new);
-        } else if ($new === null) {
-          return pht(
-            '%s removed the alias of the URL.',
-            $this->renderHandleLink($author_phid));
-        } else {
-          return pht(
-            '%s changed the alias of the URL from %s to %s.',
-            $this->renderHandleLink($author_phid),
-            $old,
-            $new);
-        }
-      case self::TYPE_DESCRIPTION:
-        return pht(
-          "%s updated the URL's description.",
-          $this->renderHandleLink($author_phid));
-    }
-    return parent::getTitle();
-  }
-
-  public function getTitleForFeed() {
-    $author_phid = $this->getAuthorPHID();
-    $object_phid = $this->getObjectPHID();
-
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-
-    $viewer = $this->getViewer();
-
-    $type = $this->getTransactionType();
-    switch ($type) {
-      case self::TYPE_NAME:
-        if ($old === null) {
-          return pht(
-            '%s created %s.',
-            $this->renderHandleLink($author_phid),
-            $this->renderHandleLink($object_phid));
-        } else {
-          return pht(
-            '%s changed the name of %s from %s to %s.',
-            $this->renderHandleLink($author_phid),
-            $this->renderHandleLink($object_phid),
-            $old,
-            $new);
-        }
-      case self::TYPE_URL:
-        if ($old === null) {
-          return pht(
-            '%s set the destination of %s to %s.',
-            $this->renderHandleLink($author_phid),
-            $this->renderHandleLink($object_phid),
-            $new);
-        } else {
-          return pht(
-            '%s changed the destination of %s from %s to %s.',
-            $this->renderHandleLink($author_phid),
-            $this->renderHandleLink($object_phid),
-            $old,
-            $new);
-        }
-      case self::TYPE_ALIAS:
-        if ($old === null) {
-          return pht(
-            '%s set the alias of %s to %s.',
-            $this->renderHandleLink($author_phid),
-            $this->renderHandleLink($object_phid),
-            $new);
-        } else if ($new === null) {
-          return pht(
-            '%s removed the alias of %s.',
-            $this->renderHandleLink($author_phid),
-            $this->renderHandleLink($object_phid));
-        } else {
-          return pht(
-            '%s changed the alias of %s from %s to %s.',
-            $this->renderHandleLink($author_phid),
-            $this->renderHandleLink($object_phid),
-            $old,
-            $new);
-        }
-      case self::TYPE_DESCRIPTION:
-        return pht(
-          '%s updated the description of %s.',
-          $this->renderHandleLink($author_phid),
-          $this->renderHandleLink($object_phid));
-    }
-
-    return parent::getTitleForFeed();
-  }
-
-  public function getColor() {
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-
-    switch ($this->getTransactionType()) {
-      case self::TYPE_NAME:
-      case self::TYPE_URL:
-      case self::TYPE_ALIAS:
-      case self::TYPE_DESCRIPTION:
-        return PhabricatorTransactions::COLOR_GREEN;
-    }
-
-    return parent::getColor();
-  }
-
-
-  public function hasChangeDetails() {
-    switch ($this->getTransactionType()) {
-      case self::TYPE_DESCRIPTION:
-        return ($this->getOldValue() !== null);
-    }
-
-    return parent::hasChangeDetails();
-  }
-
-  public function renderChangeDetails(PhabricatorUser $viewer) {
-    switch ($this->getTransactionType()) {
-      case self::TYPE_DESCRIPTION:
-        $old = $this->getOldValue();
-        $new = $this->getNewValue();
-
-        return $this->renderTextCorpusChangeDetails(
-          $viewer,
-          $old,
-          $new);
-    }
-
-    return parent::renderChangeDetails($viewer);
-  }
-
   public function getMailTags() {
     $tags = array();
     switch ($this->getTransactionType()) {
-      case self::TYPE_NAME:
-      case self::TYPE_DESCRIPTION:
-      case self::TYPE_URL:
-      case self::TYPE_ALIAS:
+      case PhabricatorPhurlURLNameTransaction::TRANSACTIONTYPE:
+      case PhabricatorPhurlURLLongURLTransaction::TRANSACTIONTYPE:
+      case PhabricatorPhurlURLAliasTransaction::TRANSACTIONTYPE:
+      case PhabricatorPhurlURLDescriptionTransaction::TRANSACTIONTYPE:
         $tags[] = self::MAILTAG_DETAILS;
         break;
     }
diff --git a/src/applications/phurl/xaction/PhabricatorPhurlURLAliasTransaction.php b/src/applications/phurl/xaction/PhabricatorPhurlURLAliasTransaction.php
new file mode 100644
index 0000000000..2a1f203ba2
--- /dev/null
+++ b/src/applications/phurl/xaction/PhabricatorPhurlURLAliasTransaction.php
@@ -0,0 +1,77 @@
+<?php
+
+final class PhabricatorPhurlURLAliasTransaction
+  extends PhabricatorPhurlURLTransactionType {
+
+  const TRANSACTIONTYPE = 'phurl.alias';
+
+  public function generateOldValue($object) {
+    return $object->getAlias();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setAlias($value);
+  }
+
+  public function getTitle() {
+    return pht(
+      '%s changed the alias from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderOldValue(),
+      $this->renderNewValue());
+  }
+
+  public function getTitleForFeed() {
+    return pht(
+      '%s changed the alias of %s from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderObject(),
+      $this->renderOldValue(),
+      $this->renderNewValue());
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    if ($this->isEmptyTextTransaction($object->getAlias(), $xactions)) {
+      $errors[] = $this->newRequiredError(
+        pht('Phurls must have an alias.'));
+    }
+
+    $max_length = $object->getColumnMaximumByteLength('alias');
+    foreach ($xactions as $xaction) {
+      $new_alias = $xaction->getNewValue();
+
+      // Check length
+      $new_length = strlen($new_alias);
+      if ($new_length > $max_length) {
+        $errors[] = $this->newRequiredError(
+          pht('The alias can be no longer than %d characters.', $max_length));
+      }
+
+      // Check characters
+      if ($xaction->getOldValue() != $xaction->getNewValue()) {
+        $debug_alias = new PHUIInvisibleCharacterView($new_alias);
+        if (!preg_match('/[a-zA-Z]/', $new_alias)) {
+          $errors[] = $this->newRequiredError(
+            pht('The alias you provided (%s) must contain at least one '.
+              'letter.',
+              $debug_alias));
+        }
+        if (preg_match('/[^a-z0-9]/i', $new_alias)) {
+          $errors[] = $this->newRequiredError(
+            pht('The alias you provided (%s) may only contain letters and '.
+              'numbers.',
+              $debug_alias));
+        }
+      }
+    }
+
+    return $errors;
+  }
+
+  public function getIcon() {
+    return 'fa-compress';
+  }
+
+}
diff --git a/src/applications/phurl/xaction/PhabricatorPhurlURLDescriptionTransaction.php b/src/applications/phurl/xaction/PhabricatorPhurlURLDescriptionTransaction.php
new file mode 100644
index 0000000000..8b6734e1dd
--- /dev/null
+++ b/src/applications/phurl/xaction/PhabricatorPhurlURLDescriptionTransaction.php
@@ -0,0 +1,60 @@
+<?php
+
+final class PhabricatorPhurlURLDescriptionTransaction
+  extends PhabricatorPhurlURLTransactionType {
+
+  const TRANSACTIONTYPE = 'phurl.description';
+
+  public function generateOldValue($object) {
+    return $object->getDescription();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setDescription($value);
+  }
+
+  public function getTitle() {
+    return pht(
+      '%s updated the description.',
+      $this->renderAuthor());
+  }
+
+  public function getTitleForFeed() {
+    return pht(
+      '%s updated the description for %s.',
+      $this->renderAuthor(),
+      $this->renderObject());
+  }
+
+  public function hasChangeDetailView() {
+    return true;
+  }
+
+  public function getMailDiffSectionHeader() {
+    return pht('CHANGES TO PHURL DESCRIPTION');
+  }
+
+  public function newChangeDetailView() {
+    $viewer = $this->getViewer();
+
+    return id(new PhabricatorApplicationTransactionTextDiffDetailView())
+      ->setViewer($viewer)
+      ->setOldText($this->getOldValue())
+      ->setNewText($this->getNewValue());
+  }
+
+  public function newRemarkupChanges() {
+    $changes = array();
+
+    $changes[] = $this->newRemarkupChange()
+      ->setOldValue($this->getOldValue())
+      ->setNewValue($this->getNewValue());
+
+    return $changes;
+  }
+
+  public function getIcon() {
+    return 'fa-file-text-o';
+  }
+
+}
diff --git a/src/applications/phurl/xaction/PhabricatorPhurlURLLongURLTransaction.php b/src/applications/phurl/xaction/PhabricatorPhurlURLLongURLTransaction.php
new file mode 100644
index 0000000000..7a4e34516e
--- /dev/null
+++ b/src/applications/phurl/xaction/PhabricatorPhurlURLLongURLTransaction.php
@@ -0,0 +1,59 @@
+<?php
+
+final class PhabricatorPhurlURLLongURLTransaction
+  extends PhabricatorPhurlURLTransactionType {
+
+  const TRANSACTIONTYPE = 'phurl.longurl';
+
+  public function generateOldValue($object) {
+    return $object->getLongURL();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setLongURL($value);
+  }
+
+  public function getTitle() {
+    return pht(
+      '%s changed the destination URL from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderOldValue(),
+      $this->renderNewValue());
+  }
+
+  public function getTitleForFeed() {
+    return pht(
+      '%s changed the destination URL %s from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderObject(),
+      $this->renderOldValue(),
+      $this->renderNewValue());
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    if ($this->isEmptyTextTransaction($object->getLongURL(), $xactions)) {
+      $errors[] = $this->newRequiredError(
+        pht('URL path is required'));
+    }
+
+    foreach ($xactions as $xaction) {
+      if ($xaction->getOldValue() != $xaction->getNewValue()) {
+        $protocols = PhabricatorEnv::getEnvConfig('uri.allowed-protocols');
+        $uri = new PhutilURI($xaction->getNewValue());
+        if (!isset($protocols[$uri->getProtocol()])) {
+          $errors[] = $this->newRequiredError(
+            pht('The protocol of the URL is invalid.'));
+        }
+      }
+    }
+
+    return $errors;
+  }
+
+  public function getIcon() {
+    return 'fa-external-link-square';
+  }
+
+}
diff --git a/src/applications/phurl/xaction/PhabricatorPhurlURLNameTransaction.php b/src/applications/phurl/xaction/PhabricatorPhurlURLNameTransaction.php
new file mode 100644
index 0000000000..331f6aa389
--- /dev/null
+++ b/src/applications/phurl/xaction/PhabricatorPhurlURLNameTransaction.php
@@ -0,0 +1,44 @@
+<?php
+
+final class PhabricatorPhurlURLNameTransaction
+  extends PhabricatorPhurlURLTransactionType {
+
+  const TRANSACTIONTYPE = 'phurl.name';
+
+  public function generateOldValue($object) {
+    return $object->getName();
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setName($value);
+  }
+
+  public function getTitle() {
+    return pht(
+      '%s changed the name of the URL from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderOldValue(),
+      $this->renderNewValue());
+  }
+
+  public function getTitleForFeed() {
+    return pht(
+      '%s changed the name of %s from %s to %s.',
+      $this->renderAuthor(),
+      $this->renderObject(),
+      $this->renderOldValue(),
+      $this->renderNewValue());
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    if ($this->isEmptyTextTransaction($object->getName(), $xactions)) {
+      $errors[] = $this->newRequiredError(
+        pht('Phurls must have a name.'));
+    }
+
+    return $errors;
+  }
+
+}
diff --git a/src/applications/phurl/xaction/PhabricatorPhurlURLTransactionType.php b/src/applications/phurl/xaction/PhabricatorPhurlURLTransactionType.php
new file mode 100644
index 0000000000..a2fa4fcfd2
--- /dev/null
+++ b/src/applications/phurl/xaction/PhabricatorPhurlURLTransactionType.php
@@ -0,0 +1,4 @@
+<?php
+
+abstract class PhabricatorPhurlURLTransactionType
+  extends PhabricatorModularTransactionType {}
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
index 60dcc1d513..dc615f0f8e 100644
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -2324,51 +2324,6 @@ abstract class PhabricatorApplicationTransactionEditor
     return true;
   }
 
-  /**
-   * Check that text field input isn't longer than a specified length.
-   *
-   * A text field input is invalid if the length of the input is longer than a
-   * specified length. This length can be determined by the space allotted in
-   * the database, or given arbitrarily.
-   * This method is intended to make implementing @{method:validateTransaction}
-   * more convenient:
-   *
-   *   $overdrawn = $this->validateIsTextFieldTooLong(
-   *     $object->getName(),
-   *     $xactions,
-   *     $field_length);
-   *
-   * This will return `true` if the net effect of the object and transactions
-   * is a field that is too long.
-   *
-   * @param wild Current field value.
-   * @param list<PhabricatorApplicationTransaction> Transactions editing the
-   *          field.
-   * @param integer for maximum field length.
-   * @return bool True if the field will be too long after edits.
-   */
-  protected function validateIsTextFieldTooLong(
-    $field_value,
-    array $xactions,
-    $length) {
-
-    if ($xactions) {
-      $new_value_length = phutil_utf8_strlen(last($xactions)->getNewValue());
-      if ($new_value_length <= $length) {
-        return false;
-      } else {
-        return true;
-      }
-    }
-
-    $old_value_length = phutil_utf8_strlen($field_value);
-    if ($old_value_length <= $length) {
-      return false;
-    }
-
-    return true;
-  }
-
 
 /* -(  Implicit CCs  )------------------------------------------------------- */
 

From 40cc403d2385ed50314c04a717adeb28f1230e28 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Fri, 24 Feb 2017 09:07:15 -0800
Subject: [PATCH 20/25] Allow the Trigger daemon to hibernate, reducing
 processes to 0

Summary:
Ref T12298. The trigger daemon already has routine long-term sleep, and few external events can impact when it should ideally wake up. The relevant events are:

  - Someone creates a new Nuance source (ideally, we should wake up right away and start polling it).
  - Someone creates a Calendar event about 16 minutes in the future (ideally, we should send them a reminder in about a minute).
  - Someone changes GC config to be extremely aggressive (ideally, we should immediately respect the change).

None of these cases are very important. We don't hibernate for more than 3 minutes, so the worst case is that your Nuance source takes 3 minutes to start importing or your Calendar notification comes two minutes too late (13 minutes before the event instead of 15).

This change makes GC sightly more CPU-expensive on average: currently, we do a GC sweep every 4 hours. After this change, we'll end up doing one every 3 minutes, because we lose the fact that we did a sweep recently when the daemon restarts.

We could fix this by keeping track of when the last GC sweep was in the database, instead of in the Daemon process, but the cost of a sweep is normally very small so I don't plan to do this anytime soon.

Test Plan:
  - Ran `bin/phd debug trigger`, saw daemon go through 3-minute hibernate + restart cycles.
  - Ran `bin/phd debug task`, saw daemon run normally.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12298

Differential Revision: https://secure.phabricator.com/D17408
---
 .../daemon/workers/PhabricatorTriggerDaemon.php              | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/infrastructure/daemon/workers/PhabricatorTriggerDaemon.php b/src/infrastructure/daemon/workers/PhabricatorTriggerDaemon.php
index cec2fbf80e..02ac55a160 100644
--- a/src/infrastructure/daemon/workers/PhabricatorTriggerDaemon.php
+++ b/src/infrastructure/daemon/workers/PhabricatorTriggerDaemon.php
@@ -108,6 +108,11 @@ final class PhabricatorTriggerDaemon
       $sleep_duration = $this->runNuanceImportCursors($sleep_duration);
       $sleep_duration = $this->runGarbageCollection($sleep_duration);
       $sleep_duration = $this->runCalendarNotifier($sleep_duration);
+
+      if ($this->shouldHibernate($sleep_duration)) {
+        break;
+      }
+
       $this->sleep($sleep_duration);
     } while (!$this->shouldExit());
   }

From 4270649abe1dac93dd06be86afc5287d2356534a Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Fri, 24 Feb 2017 10:01:11 -0800
Subject: [PATCH 21/25] Increase the size of the Diffusion commit cache

Summary:
Ref T12296. This cache is used to cache Git ref heads (branches, tags, etc). Reasonable repositories may have more than 2048 of these.

When we miss the cache, we need to single-get refs to check them, which is relatively expensive.

Increasing the size of the cache to 65535 should only require about 7.5MB of RAM.

Additionally, fill only as much of the cache as actually fits. The FIFO nature of the cache can get us into trouble otherwise.

If we insert "A, B, C, D" and then lookup A, B, C, D, but the cache has maximum size 3, we get this:

  - Insert A, B, C, D: cache is now "B, C, D".
  - Lookup A: miss, single get, insert, purge, cache is now "C, D, A".
  - Lookup B: miss, singel get, insert, purge, cache is now "D, A, B".

Test Plan:
  - Reduced cache size to 5, observed reasonable behavior on the `array_slice()` locally with `bin/repository update` + `var_dump()`.
  - Used this script to estimate the size of 65535 cache entries as 7.5MB:

```
epriestley@orbital ~ $ cat size.php
<?php

$cache = array();

$mem_start = memory_get_usage();
for ($ii = 0; $ii < 65535; $ii++) {
  $cache[sha1($ii)] = true;
}

echo number_format(memory_get_usage() - $mem_start)." bytes\n";
epriestley@orbital ~ $ php -f size.php
7,602,176 bytes
```

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12296

Differential Revision: https://secure.phabricator.com/D17409
---
 .../engine/PhabricatorRepositoryDiscoveryEngine.php | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
index 5b33a9a097..d0b84a6b31 100644
--- a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
+++ b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
@@ -14,7 +14,7 @@ final class PhabricatorRepositoryDiscoveryEngine
   private $commitCache = array();
   private $workingSet = array();
 
-  const MAX_COMMIT_CACHE_SIZE = 2048;
+  const MAX_COMMIT_CACHE_SIZE = 65535;
 
 
 /* -(  Discovering Repositories  )------------------------------------------- */
@@ -476,6 +476,15 @@ final class PhabricatorRepositoryDiscoveryEngine
       return;
     }
 
+    $max_size = self::MAX_COMMIT_CACHE_SIZE;
+
+    // If we're filling more identifiers than would fit in the cache, ignore
+    // the ones that don't fit. Because the cache is FIFO, overfilling it can
+    // cause the entire cache to miss. See T12296.
+    if (count($identifiers) > $max_size) {
+      $identifiers = array_slice($identifiers, 0, $max_size);
+    }
+
     // When filling the cache we ignore commits which have been marked as
     // unreachable, treating them as though they do not exist. When recording
     // commits later we'll revive commits that exist but are unreachable.
@@ -492,7 +501,7 @@ final class PhabricatorRepositoryDiscoveryEngine
       $this->commitCache[$commit->getCommitIdentifier()] = true;
     }
 
-    while (count($this->commitCache) > self::MAX_COMMIT_CACHE_SIZE) {
+    while (count($this->commitCache) > $max_size) {
       array_shift($this->commitCache);
     }
   }

From 80cccebca292899f2188370afa4f889a1602c554 Mon Sep 17 00:00:00 2001
From: Chad Little <chad@phacility.com>
Date: Fri, 24 Feb 2017 13:15:30 -0800
Subject: [PATCH 22/25] Build a Badges page for Profiles

Summary: Ref T12270. Moves badges into their own page and menu item. Capable of displaying hundreds of useful tokens of appreciation and dedication.

Test Plan:
Test blank state, mobile, awards badges.

{F3284139}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12270

Differential Revision: https://secure.phabricator.com/D17410
---
 resources/celerity/map.php                    |   8 +-
 src/__phutil_library_map__.php                |   4 +
 .../PhabricatorBadgesAwardController.php      |   2 +-
 .../PhabricatorPeopleApplication.php          |   2 +
 ...abricatorPeopleProfileBadgesController.php | 137 ++++++++++++++++++
 .../PhabricatorPeopleProfileController.php    |   7 +-
 ...abricatorPeopleProfileManageController.php |   2 +-
 ...bricatorPeopleProfilePictureController.php |   6 +-
 ...PhabricatorPeopleProfileViewController.php | 104 -------------
 .../PhabricatorPeopleProfileMenuEngine.php    |  10 ++
 ...PhabricatorPeopleBadgesProfileMenuItem.php |  59 ++++++++
 .../css/application/project/project-view.css  |   4 -
 webroot/rsrc/css/phui/phui-badge.css          |   5 +-
 13 files changed, 228 insertions(+), 122 deletions(-)
 create mode 100644 src/applications/people/controller/PhabricatorPeopleProfileBadgesController.php
 create mode 100644 src/applications/people/menuitem/PhabricatorPeopleBadgesProfileMenuItem.php

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index 7122e68f39..f2726a4568 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -97,7 +97,7 @@ return array(
     'rsrc/css/application/policy/policy.css' => '957ea14c',
     'rsrc/css/application/ponder/ponder-view.css' => 'fbd45f96',
     'rsrc/css/application/project/project-card-view.css' => '77219296',
-    'rsrc/css/application/project/project-view.css' => '9f6ce0e1',
+    'rsrc/css/application/project/project-view.css' => '792c9057',
     'rsrc/css/application/releeph/releeph-core.css' => '9b3c5733',
     'rsrc/css/application/releeph/releeph-preview-branch.css' => 'b7a6f4a5',
     'rsrc/css/application/releeph/releeph-request-differential-create-dialog.css' => '8d8b92cd',
@@ -130,7 +130,7 @@ return array(
     'rsrc/css/phui/object-item/phui-oi-simple-ui.css' => 'a8beebea',
     'rsrc/css/phui/phui-action-list.css' => 'f980c059',
     'rsrc/css/phui/phui-action-panel.css' => '91c7b835',
-    'rsrc/css/phui/phui-badge.css' => '22fe77f8',
+    'rsrc/css/phui/phui-badge.css' => '22c0cf4f',
     'rsrc/css/phui/phui-basic-nav-view.css' => 'a0705f53',
     'rsrc/css/phui/phui-big-info-view.css' => 'bd903741',
     'rsrc/css/phui/phui-box.css' => '269cbc99',
@@ -837,7 +837,7 @@ return array(
     'phrequent-css' => 'ffc185ad',
     'phriction-document-css' => '4282e4ad',
     'phui-action-panel-css' => '91c7b835',
-    'phui-badge-view-css' => '22fe77f8',
+    'phui-badge-view-css' => '22c0cf4f',
     'phui-basic-nav-view-css' => 'a0705f53',
     'phui-big-info-view-css' => 'bd903741',
     'phui-box-css' => '269cbc99',
@@ -906,7 +906,7 @@ return array(
     'policy-transaction-detail-css' => '82100a43',
     'ponder-view-css' => 'fbd45f96',
     'project-card-view-css' => '77219296',
-    'project-view-css' => '9f6ce0e1',
+    'project-view-css' => '792c9057',
     'releeph-core' => '9b3c5733',
     'releeph-preview-branch' => 'b7a6f4a5',
     'releeph-request-differential-create-dialog' => '8d8b92cd',
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index bdb8cb120f..0fd7dd8418 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -3316,6 +3316,7 @@ phutil_register_library_map(array(
     'PhabricatorPeopleAnyOwnerDatasource' => 'applications/people/typeahead/PhabricatorPeopleAnyOwnerDatasource.php',
     'PhabricatorPeopleApplication' => 'applications/people/application/PhabricatorPeopleApplication.php',
     'PhabricatorPeopleApproveController' => 'applications/people/controller/PhabricatorPeopleApproveController.php',
+    'PhabricatorPeopleBadgesProfileMenuItem' => 'applications/people/menuitem/PhabricatorPeopleBadgesProfileMenuItem.php',
     'PhabricatorPeopleController' => 'applications/people/controller/PhabricatorPeopleController.php',
     'PhabricatorPeopleCreateController' => 'applications/people/controller/PhabricatorPeopleCreateController.php',
     'PhabricatorPeopleCreateGuidanceContext' => 'applications/people/guidance/PhabricatorPeopleCreateGuidanceContext.php',
@@ -3339,6 +3340,7 @@ phutil_register_library_map(array(
     'PhabricatorPeopleNoOwnerDatasource' => 'applications/people/typeahead/PhabricatorPeopleNoOwnerDatasource.php',
     'PhabricatorPeopleOwnerDatasource' => 'applications/people/typeahead/PhabricatorPeopleOwnerDatasource.php',
     'PhabricatorPeoplePictureProfileMenuItem' => 'applications/people/menuitem/PhabricatorPeoplePictureProfileMenuItem.php',
+    'PhabricatorPeopleProfileBadgesController' => 'applications/people/controller/PhabricatorPeopleProfileBadgesController.php',
     'PhabricatorPeopleProfileController' => 'applications/people/controller/PhabricatorPeopleProfileController.php',
     'PhabricatorPeopleProfileEditController' => 'applications/people/controller/PhabricatorPeopleProfileEditController.php',
     'PhabricatorPeopleProfileManageController' => 'applications/people/controller/PhabricatorPeopleProfileManageController.php',
@@ -8473,6 +8475,7 @@ phutil_register_library_map(array(
     'PhabricatorPeopleAnyOwnerDatasource' => 'PhabricatorTypeaheadDatasource',
     'PhabricatorPeopleApplication' => 'PhabricatorApplication',
     'PhabricatorPeopleApproveController' => 'PhabricatorPeopleController',
+    'PhabricatorPeopleBadgesProfileMenuItem' => 'PhabricatorProfileMenuItem',
     'PhabricatorPeopleController' => 'PhabricatorController',
     'PhabricatorPeopleCreateController' => 'PhabricatorPeopleController',
     'PhabricatorPeopleCreateGuidanceContext' => 'PhabricatorGuidanceContext',
@@ -8496,6 +8499,7 @@ phutil_register_library_map(array(
     'PhabricatorPeopleNoOwnerDatasource' => 'PhabricatorTypeaheadDatasource',
     'PhabricatorPeopleOwnerDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
     'PhabricatorPeoplePictureProfileMenuItem' => 'PhabricatorProfileMenuItem',
+    'PhabricatorPeopleProfileBadgesController' => 'PhabricatorPeopleProfileController',
     'PhabricatorPeopleProfileController' => 'PhabricatorPeopleController',
     'PhabricatorPeopleProfileEditController' => 'PhabricatorPeopleProfileController',
     'PhabricatorPeopleProfileManageController' => 'PhabricatorPeopleProfileController',
diff --git a/src/applications/badges/controller/PhabricatorBadgesAwardController.php b/src/applications/badges/controller/PhabricatorBadgesAwardController.php
index ad409f8297..3fa474018e 100644
--- a/src/applications/badges/controller/PhabricatorBadgesAwardController.php
+++ b/src/applications/badges/controller/PhabricatorBadgesAwardController.php
@@ -15,7 +15,7 @@ final class PhabricatorBadgesAwardController
       return new Aphront404Response();
     }
 
-    $view_uri = '/p/'.$user->getUsername();
+    $view_uri = '/people/badges/'.$user->getID().'/';
 
     if ($request->isFormPost()) {
       $badge_phids = $request->getArr('badgePHIDs');
diff --git a/src/applications/people/application/PhabricatorPeopleApplication.php b/src/applications/people/application/PhabricatorPeopleApplication.php
index 5278c2e801..d295319b71 100644
--- a/src/applications/people/application/PhabricatorPeopleApplication.php
+++ b/src/applications/people/application/PhabricatorPeopleApplication.php
@@ -64,6 +64,8 @@ final class PhabricatorPeopleApplication extends PhabricatorApplication {
         'ldap/' => 'PhabricatorPeopleLdapController',
         'editprofile/(?P<id>[1-9]\d*)/' =>
           'PhabricatorPeopleProfileEditController',
+        'badges/(?P<id>[1-9]\d*)/' =>
+          'PhabricatorPeopleProfileBadgesController',
         'picture/(?P<id>[1-9]\d*)/' =>
           'PhabricatorPeopleProfilePictureController',
         'manage/(?P<id>[1-9]\d*)/' =>
diff --git a/src/applications/people/controller/PhabricatorPeopleProfileBadgesController.php b/src/applications/people/controller/PhabricatorPeopleProfileBadgesController.php
new file mode 100644
index 0000000000..1b22452430
--- /dev/null
+++ b/src/applications/people/controller/PhabricatorPeopleProfileBadgesController.php
@@ -0,0 +1,137 @@
+<?php
+
+final class PhabricatorPeopleProfileBadgesController
+  extends PhabricatorPeopleProfileController {
+
+  public function handleRequest(AphrontRequest $request) {
+    $viewer = $this->getViewer();
+    $id = $request->getURIData('id');
+
+    $user = id(new PhabricatorPeopleQuery())
+      ->setViewer($viewer)
+      ->withIDs(array($id))
+      ->needProfile(true)
+      ->needProfileImage(true)
+      ->needAvailability(true)
+      ->needBadges(true)
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+        ))
+      ->executeOne();
+    if (!$user) {
+      return new Aphront404Response();
+    }
+
+    $class = 'PhabricatorBadgesApplication';
+    if (!PhabricatorApplication::isClassInstalledForViewer($class, $viewer)) {
+      return new Aphront404Response();
+    }
+
+    $this->setUser($user);
+    $title = array(pht('Badges'), $user->getUsername());
+    $header = $this->buildProfileHeader();
+    $badges = $this->buildBadgesView($user);
+
+    $crumbs = $this->buildApplicationCrumbs();
+    $crumbs->addTextCrumb(pht('Badges'));
+    $crumbs->setBorder(true);
+
+    $nav = $this->getProfileMenu();
+    $nav->selectFilter(PhabricatorPeopleProfileMenuEngine::ITEM_BADGES);
+
+    // Best option?
+    $badges = id(new PhabricatorBadgesQuery())
+      ->setViewer($viewer)
+      ->withStatuses(array(
+        PhabricatorBadgesBadge::STATUS_ACTIVE,
+      ))
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->execute();
+
+    $button = id(new PHUIButtonView())
+      ->setTag('a')
+      ->setIcon('fa-plus')
+      ->setText(pht('Award Badge'))
+      ->setWorkflow(true)
+      ->setHref('/badges/award/'.$user->getID().'/');
+
+    if (count($badges)) {
+      $header->addActionLink($button);
+    }
+
+    $view = id(new PHUITwoColumnView())
+      ->setHeader($header)
+      ->addClass('project-view-home')
+      ->addClass('project-view-people-home')
+      ->setFooter(array(
+        $this->buildBadgesView($user)
+      ));
+
+    return $this->newPage()
+      ->setTitle($title)
+      ->setCrumbs($crumbs)
+      ->setNavigation($nav)
+      ->appendChild($view);
+  }
+
+  private function buildBadgesView(PhabricatorUser $user) {
+    $viewer = $this->getViewer();
+
+    $awards = array();
+    $badges = array();
+    if ($user->getBadgePHIDs()) {
+      $awards = id(new PhabricatorBadgesAwardQuery())
+        ->setViewer($viewer)
+        ->withRecipientPHIDs(array($user->getPHID()))
+        ->execute();
+      $awards = mpull($awards, null, 'getBadgePHID');
+
+      $badges = array();
+      foreach ($awards as $award) {
+        $badge = $award->getBadge();
+        if ($badge->getStatus() == PhabricatorBadgesBadge::STATUS_ACTIVE) {
+          $badges[$award->getBadgePHID()] = $badge;
+        }
+      }
+    }
+
+    if (count($badges)) {
+      $flex = new PHUIBadgeBoxView();
+
+      foreach ($badges as $badge) {
+        if ($badge) {
+          $awarder_info = array();
+
+          $award = idx($awards, $badge->getPHID(), null);
+          $awarder_phid = $award->getAwarderPHID();
+          $awarder_handle = $viewer->renderHandle($awarder_phid);
+
+          $awarder_info = pht(
+            'Awarded by %s',
+            $awarder_handle->render());
+
+          $item = id(new PHUIBadgeView())
+            ->setIcon($badge->getIcon())
+            ->setHeader($badge->getName())
+            ->setSubhead($badge->getFlavor())
+            ->setQuality($badge->getQuality())
+            ->setHref($badge->getViewURI())
+            ->addByLine($awarder_info);
+
+          $flex->addItem($item);
+        }
+      }
+    } else {
+      $flex = id(new PHUIInfoView())
+        ->setSeverity(PHUIInfoView::SEVERITY_NOTICE)
+        ->appendChild(pht('User has not been awarded any badges.'));
+    }
+
+    return $flex;
+  }
+}
diff --git a/src/applications/people/controller/PhabricatorPeopleProfileController.php b/src/applications/people/controller/PhabricatorPeopleProfileController.php
index 7393df8455..902b21efcc 100644
--- a/src/applications/people/controller/PhabricatorPeopleProfileController.php
+++ b/src/applications/people/controller/PhabricatorPeopleProfileController.php
@@ -86,6 +86,9 @@ abstract class PhabricatorPeopleProfileController
     if ($user->getIsMailingList()) {
       $roles[] = pht('Mailing List');
     }
+    if (!$user->getIsEmailVerified()) {
+      $roles[] = pht('Email Not Verified');
+    }
 
     $tag = null;
     if ($roles) {
@@ -101,10 +104,10 @@ abstract class PhabricatorPeopleProfileController
       ->setProfileHeader(true)
       ->addClass('people-profile-header');
 
+    require_celerity_resource('project-view-css');
+
     if ($user->getIsDisabled()) {
       $header->setStatus('fa-ban', 'red', pht('Disabled'));
-    } else if (!$user->getIsEmailVerified()) {
-      $header->setStatus('fa-envelope', 'red', pht('Email Not Verified'));
     } else {
       $header->setStatus($profile_icon, 'bluegrey', $profile_title);
     }
diff --git a/src/applications/people/controller/PhabricatorPeopleProfileManageController.php b/src/applications/people/controller/PhabricatorPeopleProfileManageController.php
index 84acfe1af9..2ac3e6de89 100644
--- a/src/applications/people/controller/PhabricatorPeopleProfileManageController.php
+++ b/src/applications/people/controller/PhabricatorPeopleProfileManageController.php
@@ -39,9 +39,9 @@ final class PhabricatorPeopleProfileManageController
     $manage = id(new PHUITwoColumnView())
       ->setHeader($header)
       ->addClass('project-view-home')
+      ->addClass('project-view-people-home')
       ->setCurtain($curtain)
       ->addPropertySection(pht('Details'), $properties);
-    require_celerity_resource('project-view-css');
 
     return $this->newPage()
       ->setTitle(
diff --git a/src/applications/people/controller/PhabricatorPeopleProfilePictureController.php b/src/applications/people/controller/PhabricatorPeopleProfilePictureController.php
index 226646bc70..2c9a4bf10c 100644
--- a/src/applications/people/controller/PhabricatorPeopleProfilePictureController.php
+++ b/src/applications/people/controller/PhabricatorPeopleProfilePictureController.php
@@ -258,12 +258,12 @@ final class PhabricatorPeopleProfilePictureController
     $nav = $this->getProfileMenu();
     $nav->selectFilter(PhabricatorPeopleProfileMenuEngine::ITEM_MANAGE);
 
-    $header = id(new PHUIHeaderView())
-      ->setHeader(pht('Edit Profile Picture'))
-      ->setHeaderIcon('fa-camera');
+    $header = $this->buildProfileHeader();
 
     $view = id(new PHUITwoColumnView())
       ->setHeader($header)
+      ->addClass('project-view-home')
+      ->addClass('project-view-people-home')
       ->setFooter(array(
         $form_box,
         $upload_box,
diff --git a/src/applications/people/controller/PhabricatorPeopleProfileViewController.php b/src/applications/people/controller/PhabricatorPeopleProfileViewController.php
index 889ecd2969..9db106081d 100644
--- a/src/applications/people/controller/PhabricatorPeopleProfileViewController.php
+++ b/src/applications/people/controller/PhabricatorPeopleProfileViewController.php
@@ -14,7 +14,6 @@ final class PhabricatorPeopleProfileViewController
     $user = id(new PhabricatorPeopleQuery())
       ->setViewer($viewer)
       ->withUsernames(array($username))
-      ->needBadges(true)
       ->needProfileImage(true)
       ->needAvailability(true)
       ->executeOne();
@@ -36,8 +35,6 @@ final class PhabricatorPeopleProfileViewController
 
     $projects = $this->buildProjectsView($user);
     $calendar = $this->buildCalendarDayView($user);
-    $badges = $this->buildBadgesView($user);
-    require_celerity_resource('project-view-css');
 
     $home = id(new PHUITwoColumnView())
       ->setHeader($header)
@@ -52,7 +49,6 @@ final class PhabricatorPeopleProfileViewController
         array(
           $projects,
           $calendar,
-          $badges,
         ));
 
     $nav = $this->getProfileMenu();
@@ -228,106 +224,6 @@ final class PhabricatorPeopleProfileViewController
     return $box;
   }
 
-  private function buildBadgesView(PhabricatorUser $user) {
-
-    $viewer = $this->getViewer();
-    $class = 'PhabricatorBadgesApplication';
-
-    if (!PhabricatorApplication::isClassInstalledForViewer($class, $viewer)) {
-      return null;
-    }
-
-    $awards = array();
-    $badges = array();
-    if ($user->getBadgePHIDs()) {
-      $awards = id(new PhabricatorBadgesAwardQuery())
-        ->setViewer($viewer)
-        ->withRecipientPHIDs(array($user->getPHID()))
-        ->execute();
-      $awards = mpull($awards, null, 'getBadgePHID');
-
-      $badges = array();
-      foreach ($awards as $award) {
-        $badge = $award->getBadge();
-        if ($badge->getStatus() == PhabricatorBadgesBadge::STATUS_ACTIVE) {
-          $badges[$award->getBadgePHID()] = $badge;
-        }
-      }
-    }
-
-    if (count($badges)) {
-      $flex = new PHUIBadgeBoxView();
-
-      foreach ($badges as $badge) {
-        if ($badge) {
-          $awarder_info = array();
-
-          $award = idx($awards, $badge->getPHID(), null);
-          $awarder_phid = $award->getAwarderPHID();
-          $awarder_handle = $viewer->renderHandle($awarder_phid);
-
-          $awarder_info = pht(
-            'Awarded by %s',
-            $awarder_handle->render());
-
-          $item = id(new PHUIBadgeView())
-            ->setIcon($badge->getIcon())
-            ->setHeader($badge->getName())
-            ->setSubhead($badge->getFlavor())
-            ->setQuality($badge->getQuality())
-            ->setHref($badge->getViewURI())
-            ->addByLine($awarder_info);
-
-          $flex->addItem($item);
-        }
-      }
-    } else {
-      $flex = id(new PHUIInfoView())
-        ->setSeverity(PHUIInfoView::SEVERITY_NODATA)
-        ->appendChild(pht('User does not have any badges.'));
-    }
-
-    // Best option?
-    $badges = id(new PhabricatorBadgesQuery())
-      ->setViewer($viewer)
-      ->withStatuses(array(
-        PhabricatorBadgesBadge::STATUS_ACTIVE,
-      ))
-      ->requireCapabilities(
-        array(
-          PhabricatorPolicyCapability::CAN_VIEW,
-          PhabricatorPolicyCapability::CAN_EDIT,
-        ))
-      ->execute();
-
-    $button = id(new PHUIButtonView())
-      ->setTag('a')
-      ->setIcon('fa-plus')
-      ->setText(pht('Award'))
-      ->setWorkflow(true)
-      ->setHref('/badges/award/'.$user->getID().'/');
-
-    $can_award = false;
-    if (count($badges)) {
-      $can_award = true;
-    }
-
-    $header = id(new PHUIHeaderView())
-      ->setHeader(pht('Badges'));
-
-    if (count($badges)) {
-      $header->addActionLink($button);
-    }
-
-    $box = id(new PHUIObjectBoxView())
-      ->setHeader($header)
-      ->addClass('project-view-badges')
-      ->appendChild($flex)
-      ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY);
-
-    return $box;
-  }
-
   private function buildPeopleFeed(
     PhabricatorUser $user,
     $viewer) {
diff --git a/src/applications/people/engine/PhabricatorPeopleProfileMenuEngine.php b/src/applications/people/engine/PhabricatorPeopleProfileMenuEngine.php
index 2fd2ec3bd2..9e4bbba874 100644
--- a/src/applications/people/engine/PhabricatorPeopleProfileMenuEngine.php
+++ b/src/applications/people/engine/PhabricatorPeopleProfileMenuEngine.php
@@ -6,6 +6,7 @@ final class PhabricatorPeopleProfileMenuEngine
   const ITEM_PROFILE = 'people.profile';
   const ITEM_MANAGE = 'people.manage';
   const ITEM_PICTURE = 'people.picture';
+  const ITEM_BADGES = 'people.badges';
 
   protected function isMenuEngineConfigurable() {
     return false;
@@ -31,6 +32,15 @@ final class PhabricatorPeopleProfileMenuEngine
       ->setBuiltinKey(self::ITEM_PROFILE)
       ->setMenuItemKey(PhabricatorPeopleDetailsProfileMenuItem::MENUITEMKEY);
 
+    $have_badges = PhabricatorApplication::isClassInstalledForViewer(
+      'PhabricatorBadgesApplication',
+      $viewer);
+    if ($have_badges) {
+      $items[] = $this->newItem()
+        ->setBuiltinKey(self::ITEM_BADGES)
+        ->setMenuItemKey(PhabricatorPeopleBadgesProfileMenuItem::MENUITEMKEY);
+    }
+
     $have_maniphest = PhabricatorApplication::isClassInstalledForViewer(
       'PhabricatorManiphestApplication',
       $viewer);
diff --git a/src/applications/people/menuitem/PhabricatorPeopleBadgesProfileMenuItem.php b/src/applications/people/menuitem/PhabricatorPeopleBadgesProfileMenuItem.php
new file mode 100644
index 0000000000..0e4da29b61
--- /dev/null
+++ b/src/applications/people/menuitem/PhabricatorPeopleBadgesProfileMenuItem.php
@@ -0,0 +1,59 @@
+<?php
+
+final class PhabricatorPeopleBadgesProfileMenuItem
+  extends PhabricatorProfileMenuItem {
+
+  const MENUITEMKEY = 'people.badges';
+
+  public function getMenuItemTypeName() {
+    return pht('Badges');
+  }
+
+  private function getDefaultName() {
+    return pht('Badges');
+  }
+
+  public function canHideMenuItem(
+    PhabricatorProfileMenuItemConfiguration $config) {
+    return true;
+  }
+
+  public function getDisplayName(
+    PhabricatorProfileMenuItemConfiguration $config) {
+    $name = $config->getMenuItemProperty('name');
+
+    if (strlen($name)) {
+      return $name;
+    }
+
+    return $this->getDefaultName();
+  }
+
+  public function buildEditEngineFields(
+    PhabricatorProfileMenuItemConfiguration $config) {
+    return array(
+      id(new PhabricatorTextEditField())
+        ->setKey('name')
+        ->setLabel(pht('Name'))
+        ->setPlaceholder($this->getDefaultName())
+        ->setValue($config->getMenuItemProperty('name')),
+    );
+  }
+
+  protected function newNavigationMenuItems(
+    PhabricatorProfileMenuItemConfiguration $config) {
+
+    $user = $config->getProfileObject();
+    $id = $user->getID();
+
+    $item = $this->newItem()
+      ->setHref("/people/badges/{$id}/")
+      ->setName($this->getDisplayName($config))
+      ->setIcon('fa-trophy');
+
+    return array(
+      $item,
+    );
+  }
+
+}
diff --git a/webroot/rsrc/css/application/project/project-view.css b/webroot/rsrc/css/application/project/project-view.css
index cc8d484426..82c5a2e13b 100644
--- a/webroot/rsrc/css/application/project/project-view.css
+++ b/webroot/rsrc/css/application/project/project-view.css
@@ -67,10 +67,6 @@
     text-align: center;
 }
 
-.project-view-badges .phui-badge-flex-view {
-  background-color: #fff;
-}
-
 .project-view-home .phui-box-grey .phui-oi-attribute .phui-icon-view {
   color: {$lightgreytext};
 }
diff --git a/webroot/rsrc/css/phui/phui-badge.css b/webroot/rsrc/css/phui/phui-badge.css
index a38dff9154..1538e5de4d 100644
--- a/webroot/rsrc/css/phui/phui-badge.css
+++ b/webroot/rsrc/css/phui/phui-badge.css
@@ -3,14 +3,13 @@
  */
 
 .phui-badge-flex-view {
-  padding: 12px 4px 8px;
+  padding: 0;
   overflow: hidden;
-  text-align: center;
 }
 
 .phui-badge-flex-item {
   display: inline-block;
-  padding: 4px 8px;
+  padding: 4px 12px 4px 0;
 }
 
 .phui-badge-flex-view.flex-view-collapsed {

From 730d88c414d03cd9d9fe52fc3a929940dbf3b765 Mon Sep 17 00:00:00 2001
From: Chad Little <chad@phacility.com>
Date: Fri, 24 Feb 2017 13:49:42 -0800
Subject: [PATCH 23/25] Re-center timeline badges

Summary: This layout inadvertantly changed, so make timeline badges always center.

Test Plan: Review in sandbox.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12270

Differential Revision: https://secure.phabricator.com/D17411
---
 resources/celerity/map.php                   | 6 +++---
 webroot/rsrc/css/phui/phui-timeline-view.css | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index f2726a4568..d14b06d712 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -9,7 +9,7 @@ return array(
   'names' => array(
     'conpherence.pkg.css' => '6875302f',
     'conpherence.pkg.js' => '6249a1cf',
-    'core.pkg.css' => '12c56bd9',
+    'core.pkg.css' => 'c0340df0',
     'core.pkg.js' => '1fa7c0c5',
     'darkconsole.pkg.js' => 'e7393ebb',
     'differential.pkg.css' => '90b30783',
@@ -168,7 +168,7 @@ return array(
     'rsrc/css/phui/phui-spacing.css' => '042804d6',
     'rsrc/css/phui/phui-status.css' => 'd5263e49',
     'rsrc/css/phui/phui-tag-view.css' => '84d65f26',
-    'rsrc/css/phui/phui-timeline-view.css' => 'bc523970',
+    'rsrc/css/phui/phui-timeline-view.css' => 'bf45789e',
     'rsrc/css/phui/phui-two-column-view.css' => '8a1074c7',
     'rsrc/css/phui/workboards/phui-workboard-color.css' => '783cdff5',
     'rsrc/css/phui/workboards/phui-workboard.css' => '3bc85455',
@@ -889,7 +889,7 @@ return array(
     'phui-status-list-view-css' => 'd5263e49',
     'phui-tag-view-css' => '84d65f26',
     'phui-theme-css' => '9f261c6b',
-    'phui-timeline-view-css' => 'bc523970',
+    'phui-timeline-view-css' => 'bf45789e',
     'phui-two-column-view-css' => '8a1074c7',
     'phui-workboard-color-css' => '783cdff5',
     'phui-workboard-view-css' => '3bc85455',
diff --git a/webroot/rsrc/css/phui/phui-timeline-view.css b/webroot/rsrc/css/phui/phui-timeline-view.css
index 9460fbac2f..fb1ddbd768 100644
--- a/webroot/rsrc/css/phui/phui-timeline-view.css
+++ b/webroot/rsrc/css/phui/phui-timeline-view.css
@@ -411,6 +411,7 @@ a.phui-timeline-menu .phui-icon-view {
   left: -64px;
   top: 52px;
   width: 54px;
+  text-align: center;
 }
 
 .phui-timeline-badges .phui-badge-mini {

From eec6cd865c854fd00c8de41b8ca8ed0da78760e8 Mon Sep 17 00:00:00 2001
From: Chad Little <chad@phacility.com>
Date: Fri, 24 Feb 2017 15:21:23 -0800
Subject: [PATCH 24/25] Miscellanous badge fixes

Summary: Ref T12270. Add transaction validation for name, alias, award, revoke. Change auto subscribe for authors. Fix some typos.

Test Plan: Add badge, award badge, revoke badge, edit badge.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12270

Differential Revision: https://secure.phabricator.com/D17412
---
 .../PhabricatorBadgesAwardController.php      |  2 +-
 ...icatorBadgesRemoveRecipientsController.php |  2 +-
 .../PhabricatorBadgesViewController.php       |  9 ------
 .../editor/PhabricatorBadgesEditEngine.php    |  2 +-
 .../badges/editor/PhabricatorBadgesEditor.php | 24 +++++++++++++++
 .../query/PhabricatorBadgesSearchEngine.php   |  2 +-
 .../badges/storage/PhabricatorBadgesBadge.php |  2 +-
 ...PhabricatorBadgesBadgeAwardTransaction.php | 28 ++++++++++++++++++
 ...habricatorBadgesBadgeFlavorTransaction.php | 17 +++++++++++
 .../PhabricatorBadgesBadgeNameTransaction.php | 11 +++++++
 ...habricatorBadgesBadgeRevokeTransaction.php | 29 +++++++++++++++++++
 11 files changed, 114 insertions(+), 14 deletions(-)

diff --git a/src/applications/badges/controller/PhabricatorBadgesAwardController.php b/src/applications/badges/controller/PhabricatorBadgesAwardController.php
index 3fa474018e..e934e4f2a1 100644
--- a/src/applications/badges/controller/PhabricatorBadgesAwardController.php
+++ b/src/applications/badges/controller/PhabricatorBadgesAwardController.php
@@ -67,7 +67,7 @@ final class PhabricatorBadgesAwardController
                   ))));
 
     $dialog = $this->newDialog()
-      ->setTitle(pht('Grant Badge'))
+      ->setTitle(pht('Award Badge'))
       ->appendForm($form)
       ->addCancelButton($view_uri)
       ->addSubmitButton(pht('Award'));
diff --git a/src/applications/badges/controller/PhabricatorBadgesRemoveRecipientsController.php b/src/applications/badges/controller/PhabricatorBadgesRemoveRecipientsController.php
index b26a2c2159..e6638b745c 100644
--- a/src/applications/badges/controller/PhabricatorBadgesRemoveRecipientsController.php
+++ b/src/applications/badges/controller/PhabricatorBadgesRemoveRecipientsController.php
@@ -29,7 +29,7 @@ final class PhabricatorBadgesRemoveRecipientsController
       return new Aphront404Response();
     }
 
-    $view_uri = $this->getApplicationURI('view/'.$badge->getID().'/');
+    $view_uri = $this->getApplicationURI('recipients/'.$badge->getID().'/');
 
     if ($request->isFormPost()) {
       $xactions = array();
diff --git a/src/applications/badges/controller/PhabricatorBadgesViewController.php b/src/applications/badges/controller/PhabricatorBadgesViewController.php
index 3c434ff1d5..a5390ea5f4 100644
--- a/src/applications/badges/controller/PhabricatorBadgesViewController.php
+++ b/src/applications/badges/controller/PhabricatorBadgesViewController.php
@@ -90,7 +90,6 @@ final class PhabricatorBadgesViewController
     $id = $badge->getID();
     $edit_uri = $this->getApplicationURI("/edit/{$id}/");
     $archive_uri = $this->getApplicationURI("/archive/{$id}/");
-    $award_uri = $this->getApplicationURI("/recipients/{$id}/add/");
 
     $curtain = $this->newCurtainView($badge);
 
@@ -119,14 +118,6 @@ final class PhabricatorBadgesViewController
           ->setHref($archive_uri));
     }
 
-    $curtain->addAction(
-      id(new PhabricatorActionView())
-        ->setName('Add Recipients')
-        ->setIcon('fa-users')
-        ->setDisabled(!$can_edit)
-        ->setWorkflow(true)
-        ->setHref($award_uri));
-
     return $curtain;
   }
 
diff --git a/src/applications/badges/editor/PhabricatorBadgesEditEngine.php b/src/applications/badges/editor/PhabricatorBadgesEditEngine.php
index 597cf470f6..596e70cd13 100644
--- a/src/applications/badges/editor/PhabricatorBadgesEditEngine.php
+++ b/src/applications/badges/editor/PhabricatorBadgesEditEngine.php
@@ -92,7 +92,7 @@ final class PhabricatorBadgesEditEngine
         ->setIsRequired(true),
       id(new PhabricatorTextEditField())
         ->setKey('flavor')
-        ->setLabel(pht('Flavor text'))
+        ->setLabel(pht('Flavor Text'))
         ->setDescription(pht('Short description of the badge.'))
         ->setConduitTypeDescription(pht('New badge flavor.'))
         ->setValue($object->getFlavor())
diff --git a/src/applications/badges/editor/PhabricatorBadgesEditor.php b/src/applications/badges/editor/PhabricatorBadgesEditor.php
index 65b2ca6cdf..48057f66e6 100644
--- a/src/applications/badges/editor/PhabricatorBadgesEditor.php
+++ b/src/applications/badges/editor/PhabricatorBadgesEditor.php
@@ -55,6 +55,30 @@ final class PhabricatorBadgesEditor
     return true;
   }
 
+  protected function expandTransactions(
+    PhabricatorLiskDAO $object,
+    array $xactions) {
+
+    $actor = $this->getActor();
+    $actor_phid = $actor->getPHID();
+
+    $results = parent::expandTransactions($object, $xactions);
+
+    // Automatically subscribe the author when they create a badge.
+    if ($this->getIsNewObject()) {
+      if ($actor_phid) {
+        $results[] = id(new PhabricatorBadgesTransaction())
+          ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
+          ->setNewValue(
+            array(
+              '+' => array($actor_phid => $actor_phid),
+            ));
+      }
+    }
+
+    return $results;
+  }
+
   protected function buildReplyHandler(PhabricatorLiskDAO $object) {
     return id(new PhabricatorBadgesReplyHandler())
       ->setMailReceiver($object);
diff --git a/src/applications/badges/query/PhabricatorBadgesSearchEngine.php b/src/applications/badges/query/PhabricatorBadgesSearchEngine.php
index b57eae5910..4db0cdd979 100644
--- a/src/applications/badges/query/PhabricatorBadgesSearchEngine.php
+++ b/src/applications/badges/query/PhabricatorBadgesSearchEngine.php
@@ -147,7 +147,7 @@ final class PhabricatorBadgesSearchEngine
       ->setTitle(pht('Welcome to %s', $app_name))
       ->setDescription(
         pht('Badges let you award and distinguish special users '.
-          'throughout your instance.'))
+          'throughout your install.'))
       ->addAction($create_button);
 
       return $view;
diff --git a/src/applications/badges/storage/PhabricatorBadgesBadge.php b/src/applications/badges/storage/PhabricatorBadgesBadge.php
index 4cdc4edf61..cf02f705ce 100644
--- a/src/applications/badges/storage/PhabricatorBadgesBadge.php
+++ b/src/applications/badges/storage/PhabricatorBadgesBadge.php
@@ -156,7 +156,7 @@ final class PhabricatorBadgesBadge extends PhabricatorBadgesDAO
 
 
   public function isAutomaticallySubscribed($phid) {
-    return ($this->creatorPHID == $phid);
+    return false;
   }
 
 
diff --git a/src/applications/badges/xaction/PhabricatorBadgesBadgeAwardTransaction.php b/src/applications/badges/xaction/PhabricatorBadgesBadgeAwardTransaction.php
index 5f339fa368..5ecacdd441 100644
--- a/src/applications/badges/xaction/PhabricatorBadgesBadgeAwardTransaction.php
+++ b/src/applications/badges/xaction/PhabricatorBadgesBadgeAwardTransaction.php
@@ -59,4 +59,32 @@ final class PhabricatorBadgesBadgeAwardTransaction
     return 'fa-user-plus';
   }
 
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    foreach ($xactions as $xaction) {
+      $user_phids = $xaction->getNewValue();
+      if (!$user_phids) {
+        $errors[] = $this->newRequiredError(
+          pht('Recipient is required.'));
+        continue;
+      }
+
+      foreach ($user_phids as $user_phid) {
+        $user = id(new PhabricatorPeopleQuery())
+          ->setViewer($this->getActor())
+          ->withPHIDs(array($user_phid))
+          ->executeOne();
+        if (!$user) {
+          $errors[] = $this->newInvalidError(
+            pht(
+              'Recipient PHID "%s" is not a valid user PHID.',
+              $user_phid));
+        }
+      }
+    }
+
+    return $errors;
+  }
+
 }
diff --git a/src/applications/badges/xaction/PhabricatorBadgesBadgeFlavorTransaction.php b/src/applications/badges/xaction/PhabricatorBadgesBadgeFlavorTransaction.php
index 069ec09603..2635aab1a6 100644
--- a/src/applications/badges/xaction/PhabricatorBadgesBadgeFlavorTransaction.php
+++ b/src/applications/badges/xaction/PhabricatorBadgesBadgeFlavorTransaction.php
@@ -30,4 +30,21 @@ final class PhabricatorBadgesBadgeFlavorTransaction
       $this->renderNewValue());
   }
 
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    $max_length = $object->getColumnMaximumByteLength('flavor');
+    foreach ($xactions as $xaction) {
+      $new_value = $xaction->getNewValue();
+      $new_length = strlen($new_value);
+      if ($new_length > $max_length) {
+        $errors[] = $this->newRequiredError(
+          pht('The flavor text can be no longer than %s characters.',
+          new PhutilNumber($max_length)));
+      }
+    }
+
+    return $errors;
+  }
+
 }
diff --git a/src/applications/badges/xaction/PhabricatorBadgesBadgeNameTransaction.php b/src/applications/badges/xaction/PhabricatorBadgesBadgeNameTransaction.php
index 4565c95fb7..1df7d89a70 100644
--- a/src/applications/badges/xaction/PhabricatorBadgesBadgeNameTransaction.php
+++ b/src/applications/badges/xaction/PhabricatorBadgesBadgeNameTransaction.php
@@ -38,6 +38,17 @@ final class PhabricatorBadgesBadgeNameTransaction
         pht('Badges must have a name.'));
     }
 
+    $max_length = $object->getColumnMaximumByteLength('name');
+    foreach ($xactions as $xaction) {
+      $new_value = $xaction->getNewValue();
+      $new_length = strlen($new_value);
+      if ($new_length > $max_length) {
+        $errors[] = $this->newRequiredError(
+          pht('The name can be no longer than %s characters.',
+          new PhutilNumber($max_length)));
+      }
+    }
+
     return $errors;
   }
 
diff --git a/src/applications/badges/xaction/PhabricatorBadgesBadgeRevokeTransaction.php b/src/applications/badges/xaction/PhabricatorBadgesBadgeRevokeTransaction.php
index 704a518371..a3fa58a060 100644
--- a/src/applications/badges/xaction/PhabricatorBadgesBadgeRevokeTransaction.php
+++ b/src/applications/badges/xaction/PhabricatorBadgesBadgeRevokeTransaction.php
@@ -51,4 +51,33 @@ final class PhabricatorBadgesBadgeRevokeTransaction
     return 'fa-user-times';
   }
 
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    foreach ($xactions as $xaction) {
+      $award_phids = $xaction->getNewValue();
+      if (!$award_phids) {
+        $errors[] = $this->newRequiredError(
+          pht('Recipient is required.'));
+        continue;
+      }
+
+      foreach ($award_phids as $award_phid) {
+        $award = id(new PhabricatorBadgesAwardQuery())
+          ->setViewer($this->getActor())
+          ->withRecipientPHIDs(array($award_phid))
+          ->withBadgePHIDs(array($object->getPHID()))
+          ->executeOne();
+        if (!$award) {
+          $errors[] = $this->newInvalidError(
+            pht(
+              'Recipient PHID "%s" has not been awarded.',
+              $award_phid));
+        }
+      }
+    }
+
+    return $errors;
+  }
+
 }

From c15501fc9b06f46a699bcded6d2e14c5c48af2db Mon Sep 17 00:00:00 2001
From: Chad Little <chad@phacility.com>
Date: Fri, 24 Feb 2017 16:34:32 -0800
Subject: [PATCH 25/25] Add clearer language and options to Arcanist install
 guides

Summary: Fixes T12315. Reworks the copy placement a little, adds more links.

Test Plan: Read in sandbox

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12315

Differential Revision: https://secure.phabricator.com/D17413
---
 src/docs/user/userguide/arcanist_mac_os_x.diviner    |  1 +
 src/docs/user/userguide/arcanist_quick_start.diviner | 11 +++++------
 src/docs/user/userguide/arcanist_windows.diviner     |  1 +
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/docs/user/userguide/arcanist_mac_os_x.diviner b/src/docs/user/userguide/arcanist_mac_os_x.diviner
index a34adfbf5d..d75df8b33b 100644
--- a/src/docs/user/userguide/arcanist_mac_os_x.diviner
+++ b/src/docs/user/userguide/arcanist_mac_os_x.diviner
@@ -9,6 +9,7 @@ You need to install:
 
   - PHP, which should be installed by default.
   - Arcanist itself, see @{article:Arcanist User Guide}.
+  or @{article:Arcanist Quick Start}
   - SVN, Git, or Mercurial.
 
 Then, configure:
diff --git a/src/docs/user/userguide/arcanist_quick_start.diviner b/src/docs/user/userguide/arcanist_quick_start.diviner
index 98633bfaef..743afe4a11 100644
--- a/src/docs/user/userguide/arcanist_quick_start.diviner
+++ b/src/docs/user/userguide/arcanist_quick_start.diviner
@@ -4,16 +4,15 @@
 Quick guide to getting Arcanist working for a new project.
 
 This is a summary of steps to install Arcanist, configure a project for use with
-it, and run `arc` to send changes for review.
-
-= Install Arcanist =
-
-For detailed instructions on installing Arcanist, see
-@{article:Arcanist User Guide}.
+it, and run `arc` to send changes for review. For detailed instructions on
+installing Arcanist, see @{article:Arcanist User Guide}. OS specific guides
+are also available.
 
   - For Mac OS X, see @{article:Arcanist User Guide: Mac OS X}.
   - For Windows, see @{article:Arcanist User Guide: Windows}.
 
+= Installing Arcanist =
+
 First, install dependencies:
 
   - Install PHP.
diff --git a/src/docs/user/userguide/arcanist_windows.diviner b/src/docs/user/userguide/arcanist_windows.diviner
index 150e4f3372..56fbfea318 100644
--- a/src/docs/user/userguide/arcanist_windows.diviner
+++ b/src/docs/user/userguide/arcanist_windows.diviner
@@ -17,6 +17,7 @@ into issues.
 You need to install:
 
   - Arcanist itself, see @{article:Arcanist User Guide}.
+  or @{article:Arcanist Quick Start}.
   - PHP (see "Detailed PHP Install Instructions" below).
   - SVN, Git, or Mercurial.