From a36b1e8f64abf187f0007600c369fc95c8d68d2b Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sun, 12 Mar 2017 13:45:52 -0700
Subject: [PATCH 01/15] Fix two typos ("Adminstrator", "Recipents")

Summary: Fixes T12387.

Test Plan: Consulted a dictionary.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12387

Differential Revision: https://secure.phabricator.com/D17493
---
 .../badges/view/PhabricatorBadgesRecipientsListView.php         | 2 +-
 src/applications/uiexample/examples/PHUIBadgeExample.php        | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/applications/badges/view/PhabricatorBadgesRecipientsListView.php b/src/applications/badges/view/PhabricatorBadgesRecipientsListView.php
index f566cd7b7a..ce84971d41 100644
--- a/src/applications/badges/view/PhabricatorBadgesRecipientsListView.php
+++ b/src/applications/badges/view/PhabricatorBadgesRecipientsListView.php
@@ -36,7 +36,7 @@ final class PhabricatorBadgesRecipientsListView extends AphrontView {
     $award_button = id(new PHUIButtonView())
       ->setTag('a')
       ->setIcon('fa-plus')
-      ->setText(pht('Add Recipents'))
+      ->setText(pht('Add Recipients'))
       ->setWorkflow(true)
       ->setDisabled(!$can_edit)
       ->setHref('/badges/recipients/'.$badge->getID().'/add/');
diff --git a/src/applications/uiexample/examples/PHUIBadgeExample.php b/src/applications/uiexample/examples/PHUIBadgeExample.php
index 5207db753c..14eb3469d0 100644
--- a/src/applications/uiexample/examples/PHUIBadgeExample.php
+++ b/src/applications/uiexample/examples/PHUIBadgeExample.php
@@ -97,7 +97,7 @@ final class PHUIBadgeExample extends PhabricatorUIExample {
 
     $badges2[] = id(new PHUIBadgeView())
       ->setIcon('fa-user')
-      ->setHeader(pht('Adminstrator'))
+      ->setHeader(pht('Administrator'))
       ->setSubhead(pht('Drew the short stick'))
       ->setQuality(PhabricatorBadgesQuality::LEGENDARY)
       ->setSource(pht('People (automatic)'))

From 81675e3302a2f2464764b24d8cc94238e4a1e541 Mon Sep 17 00:00:00 2001
From: Chad Little <chad@phacility.com>
Date: Mon, 13 Mar 2017 14:39:28 -0700
Subject: [PATCH 02/15] Stronger scoping for Conpherence chat forms

Summary: Fixes T12391. Adds better scoping to these rules to contain changes to just Conpherence.

Test Plan: Test Conpherence, Task comment, persistent chat on mobile / desktop.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12391

Differential Revision: https://secure.phabricator.com/D17496
---
 resources/celerity/map.php                            |  6 +++---
 .../rsrc/css/application/conpherence/message-pane.css | 11 ++++++-----
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index 53295dbae9..c6039fa138 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -7,7 +7,7 @@
  */
 return array(
   'names' => array(
-    'conpherence.pkg.css' => '6875302f',
+    'conpherence.pkg.css' => '32f2c040',
     'conpherence.pkg.js' => '6249a1cf',
     'core.pkg.css' => '35645dec',
     'core.pkg.js' => '1fa7c0c5',
@@ -48,7 +48,7 @@ return array(
     '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',
+    'rsrc/css/application/conpherence/message-pane.css' => 'd1fc13e1',
     'rsrc/css/application/conpherence/notification.css' => '965db05b',
     'rsrc/css/application/conpherence/participant-pane.css' => '604a8b02',
     'rsrc/css/application/conpherence/transaction.css' => '85129c68',
@@ -566,7 +566,7 @@ return array(
     'conpherence-durable-column-view' => '292c71f0',
     'conpherence-header-pane-css' => 'db93ebc6',
     'conpherence-menu-css' => '3d8e5c9c',
-    'conpherence-message-pane-css' => 'b085d40d',
+    'conpherence-message-pane-css' => 'd1fc13e1',
     'conpherence-notification-css' => '965db05b',
     'conpherence-participant-pane-css' => '604a8b02',
     'conpherence-thread-manager' => 'c8b5ee6f',
diff --git a/webroot/rsrc/css/application/conpherence/message-pane.css b/webroot/rsrc/css/application/conpherence/message-pane.css
index 9c8856fe0d..4f83a19948 100644
--- a/webroot/rsrc/css/application/conpherence/message-pane.css
+++ b/webroot/rsrc/css/application/conpherence/message-pane.css
@@ -181,13 +181,14 @@
   border: none;
 }
 
-.device .remarkup-assist-button,
-.device .remarkup-assist-separator {
+.device .conpherence-message-pane .remarkup-assist-button,
+.device .conpherence-message-pane .remarkup-assist-separator {
   display: none;
 }
 
-.device .remarkup-assist-button.remarkup-assist-upload {
-  display: block;
+.device .conpherence-message-pane
+  .remarkup-assist-button.remarkup-assist-upload {
+    display: block;
 }
 
 .device .conpherence-message-pane .phui-form-view {
@@ -343,7 +344,7 @@
   padding: 2px 0 8px 0;
 }
 
-.conpherence-message-pane .aphront-form-control {
+body .conpherence-message-pane .aphront-form-control {
   padding: 0;
 }
 

From 251ee9b660b8987dfd10cbfae7ea9d9417e22466 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 13 Mar 2017 11:31:57 -0700
Subject: [PATCH 03/15] Add dedicated "reviewers" storage to Differential and
 do double writes

Summary:
Ref T10967. This is an incremental step toward removing "reviewers" back to a dedicated storage table so we can handle changes like T11050.

This adds the storage table, and starts doing double writes to it (so new or updated reviewers write to both the old edge table and the new "reviewers" table).

Then we can do a migration, swap readers over one at a time, and eventually remove the old write and old storage and then implement new features.

This change has no user-facing impact, it just causes us to write new data to two places instead of one.

This is not completely exhaustive: the Herald "Add Reviewers" action is still doing a manual EDGE transaction. I'll clean that up next and do another pass to look for anything else I missed.

This is also a bit copy/pastey for now but the logic around "RESIGN" is a little different in the two cases until T11050. I'll unify it in future changes.

Test Plan:
  - Did a no-op edit.
  - Did a no-op comment.
  - Added reviewers.
  - Removed reviewers.
  - Accepted and rejected revisions.

After all of these edits, did a `SELECT * FROM differential_reviewer` manually and saw consistent-looking rows in the database.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10967

Differential Revision: https://secure.phabricator.com/D17495
---
 .../sql/autopatches/20170313.reviewers.01.sql |  9 ++++
 src/__phutil_library_map__.php                |  2 +
 .../storage/DifferentialReviewer.php          | 24 ++++++++++
 .../DifferentialRevisionReviewTransaction.php | 39 +++++++++++++++
 ...fferentialRevisionReviewersTransaction.php | 48 +++++++++++++++++++
 5 files changed, 122 insertions(+)
 create mode 100644 resources/sql/autopatches/20170313.reviewers.01.sql
 create mode 100644 src/applications/differential/storage/DifferentialReviewer.php

diff --git a/resources/sql/autopatches/20170313.reviewers.01.sql b/resources/sql/autopatches/20170313.reviewers.01.sql
new file mode 100644
index 0000000000..4b243b6f6f
--- /dev/null
+++ b/resources/sql/autopatches/20170313.reviewers.01.sql
@@ -0,0 +1,9 @@
+CREATE TABLE {$NAMESPACE}_differential.differential_reviewer (
+  id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
+  revisionPHID VARBINARY(64) NOT NULL,
+  reviewerPHID VARBINARY(64) NOT NULL,
+  reviewerStatus VARCHAR(64) NOT NULL COLLATE {$COLLATE_TEXT},
+  dateCreated INT UNSIGNED NOT NULL,
+  dateModified INT UNSIGNED NOT NULL,
+  UNIQUE KEY `key_revision` (revisionPHID, reviewerPHID)
+) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT};
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 544b4f2b39..1279ae4077 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -491,6 +491,7 @@ phutil_register_library_map(array(
     'DifferentialRevertPlanCommitMessageField' => 'applications/differential/field/DifferentialRevertPlanCommitMessageField.php',
     'DifferentialRevertPlanField' => 'applications/differential/customfield/DifferentialRevertPlanField.php',
     'DifferentialReviewedByCommitMessageField' => 'applications/differential/field/DifferentialReviewedByCommitMessageField.php',
+    'DifferentialReviewer' => 'applications/differential/storage/DifferentialReviewer.php',
     'DifferentialReviewerDatasource' => 'applications/differential/typeahead/DifferentialReviewerDatasource.php',
     'DifferentialReviewerForRevisionEdgeType' => 'applications/differential/edge/DifferentialReviewerForRevisionEdgeType.php',
     'DifferentialReviewerProxy' => 'applications/differential/storage/DifferentialReviewerProxy.php',
@@ -5247,6 +5248,7 @@ phutil_register_library_map(array(
     'DifferentialRevertPlanCommitMessageField' => 'DifferentialCommitMessageCustomField',
     'DifferentialRevertPlanField' => 'DifferentialStoredCustomField',
     'DifferentialReviewedByCommitMessageField' => 'DifferentialCommitMessageField',
+    'DifferentialReviewer' => 'DifferentialDAO',
     'DifferentialReviewerDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
     'DifferentialReviewerForRevisionEdgeType' => 'PhabricatorEdgeType',
     'DifferentialReviewerProxy' => 'Phobject',
diff --git a/src/applications/differential/storage/DifferentialReviewer.php b/src/applications/differential/storage/DifferentialReviewer.php
new file mode 100644
index 0000000000..72317a0a4c
--- /dev/null
+++ b/src/applications/differential/storage/DifferentialReviewer.php
@@ -0,0 +1,24 @@
+<?php
+
+final class DifferentialReviewer
+  extends DifferentialDAO {
+
+  protected $revisionPHID;
+  protected $reviewerPHID;
+  protected $reviewerStatus;
+
+  protected function getConfiguration() {
+    return array(
+      self::CONFIG_COLUMN_SCHEMA => array(
+        'reviewerStatus' => 'text64',
+      ),
+      self::CONFIG_KEY_SCHEMA => array(
+        'key_revision' => array(
+          'columns' => array('revisionPHID', 'reviewerPHID'),
+          'unique' => true,
+        ),
+      ),
+    ) + parent::getConfiguration();
+  }
+
+}
diff --git a/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php b/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
index 7e0c60a7b1..9ac731fea9 100644
--- a/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
@@ -116,6 +116,9 @@ abstract class DifferentialRevisionReviewTransaction
       );
     }
 
+    // This is currently double-writing: to the old (edge) store and the new
+    // (reviewer) store. Do the old edge write first.
+
     $src_phid = $revision->getPHID();
     $edge_type = DifferentialRevisionHasReviewerEdgeType::EDGECONST;
 
@@ -131,6 +134,42 @@ abstract class DifferentialRevisionReviewTransaction
     }
 
     $editor->save();
+
+    // Now, do the new write.
+
+    if ($map) {
+      $table = new DifferentialReviewer();
+
+      $reviewers = $table->loadAllWhere(
+        'revisionPHID = %s AND reviewerPHID IN (%Ls)',
+        $src_phid,
+        array_keys($map));
+      $reviewers = mpull($reviewers, null, 'getReviewerPHID');
+
+      foreach ($map as $dst_phid => $edge_data) {
+        $reviewer = idx($reviewers, $dst_phid);
+        if (!$reviewer) {
+          $reviewer = id(new DifferentialReviewer())
+            ->setRevisionPHID($src_phid)
+            ->setReviewerPHID($dst_phid);
+        }
+
+        $reviewer->setReviewerStatus($status);
+
+        if ($status == DifferentialReviewerStatus::STATUS_RESIGNED) {
+          if ($reviewer->getID()) {
+            $reviewer->delete();
+          }
+        } else {
+          try {
+            $reviewer->save();
+          } catch (AphrontDuplicateKeyQueryException $ex) {
+            // At least for now, just ignore it if we lost a race.
+          }
+        }
+      }
+    }
+
   }
 
 }
diff --git a/src/applications/differential/xaction/DifferentialRevisionReviewersTransaction.php b/src/applications/differential/xaction/DifferentialRevisionReviewersTransaction.php
index 1e1b35c1c3..c4112a4516 100644
--- a/src/applications/differential/xaction/DifferentialRevisionReviewersTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionReviewersTransaction.php
@@ -106,6 +106,9 @@ final class DifferentialRevisionReviewersTransaction
   public function applyExternalEffects($object, $value) {
     $src_phid = $object->getPHID();
 
+    // This is currently double-writing: to the old (edge) store and the new
+    // (reviewer) store. Do the old edge write first.
+
     $old = $this->generateOldValue($object);
     $new = $value;
     $edge_type = DifferentialRevisionHasReviewerEdgeType::EDGECONST;
@@ -138,6 +141,51 @@ final class DifferentialRevisionReviewersTransaction
     }
 
     $editor->save();
+
+    // Now, do the new write.
+
+    $table = new DifferentialReviewer();
+    $table_name = $table->getTableName();
+    $conn = $table->establishConnection('w');
+
+    if ($rem) {
+      queryfx(
+        $conn,
+        'DELETE FROM %T WHERE revisionPHID = %s AND reviewerPHID IN (%Ls)',
+        $table_name,
+        $src_phid,
+        array_keys($rem));
+    }
+
+    if ($new) {
+      $reviewers = $table->loadAllWhere(
+        'revisionPHID = %s AND reviewerPHID IN (%Ls)',
+        $src_phid,
+        array_keys($new));
+      $reviewers = mpull($reviewers, null, 'getReviewerPHID');
+
+      foreach ($new as $dst_phid => $status) {
+        $old_status = idx($old, $dst_phid);
+        if ($old_status === $status) {
+          continue;
+        }
+
+        $reviewer = idx($reviewers, $dst_phid);
+        if (!$reviewer) {
+          $reviewer = id(new DifferentialReviewer())
+            ->setRevisionPHID($src_phid)
+            ->setReviewerPHID($dst_phid);
+        }
+
+        $reviewer->setReviewerStatus($status);
+
+        try {
+          $reviewer->save();
+        } catch (AphrontDuplicateKeyQueryException $ex) {
+          // At least for now, just ignore it if we lost a race.
+        }
+      }
+    }
   }
 
   public function getTitle() {

From a72d18765f85cf116331fcc9bad0ff05471378bb Mon Sep 17 00:00:00 2001
From: Chad Little <chad@phacility.com>
Date: Tue, 14 Mar 2017 14:21:43 -0700
Subject: [PATCH 04/15] Basic "Install Dashboard" workflow

Summary: Ref T12264. This allows users to install a dashboard they are viewing to their personal home menu or as a global home menu item. Has some basic ability to be extended later for maybe projects.

Test Plan:
Build a dashboard, click "Install Dashboard".

 - As user only get personal option
 - As HomeApp edit person, see both options
 - Try installation as either, with and without label set
 - Fake "global" form as user, get error
 - Don't set anything, get error

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12264

Differential Revision: https://secure.phabricator.com/D17492
---
 resources/celerity/map.php                    |  10 +-
 src/__phutil_library_map__.php                |   2 +
 .../PhabricatorDashboardApplication.php       |   1 +
 .../PhabricatorDashboardArrangeController.php |   8 +
 .../PhabricatorDashboardInstallController.php | 141 ++++++++++++++++++
 .../PhabricatorDashboardViewController.php    |   8 +
 webroot/rsrc/css/aphront/dialog-view.css      |   4 +-
 webroot/rsrc/css/phui/phui-form-view.css      |   5 +-
 8 files changed, 169 insertions(+), 10 deletions(-)
 create mode 100644 src/applications/dashboard/controller/PhabricatorDashboardInstallController.php

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index c6039fa138..46bb152704 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -9,7 +9,7 @@ return array(
   'names' => array(
     'conpherence.pkg.css' => '32f2c040',
     'conpherence.pkg.js' => '6249a1cf',
-    'core.pkg.css' => '35645dec',
+    'core.pkg.css' => 'c0c87dac',
     'core.pkg.js' => '1fa7c0c5',
     'darkconsole.pkg.js' => 'e7393ebb',
     'differential.pkg.css' => '90b30783',
@@ -21,7 +21,7 @@ return array(
     'maniphest.pkg.js' => '5ab2753f',
     'rsrc/css/aphront/aphront-bars.css' => '231ac33c',
     'rsrc/css/aphront/dark-console.css' => 'f54bf286',
-    'rsrc/css/aphront/dialog-view.css' => '5e5aa60b',
+    'rsrc/css/aphront/dialog-view.css' => '685c7e2d',
     'rsrc/css/aphront/list-filter-view.css' => '5d6f0526',
     'rsrc/css/aphront/multi-column.css' => '84cc6640',
     'rsrc/css/aphront/notification.css' => '3f6c89c9',
@@ -146,7 +146,7 @@ return array(
     'rsrc/css/phui/phui-document.css' => 'c32e8dec',
     'rsrc/css/phui/phui-feed-story.css' => '44a9c8e9',
     'rsrc/css/phui/phui-fontkit.css' => 'b78a0059',
-    'rsrc/css/phui/phui-form-view.css' => 'adca31ce',
+    'rsrc/css/phui/phui-form-view.css' => 'cf198e10',
     'rsrc/css/phui/phui-form.css' => 'b62c01d8',
     'rsrc/css/phui/phui-head-thing.css' => 'fd311e5f',
     'rsrc/css/phui/phui-header-view.css' => 'fef6a54e',
@@ -548,7 +548,7 @@ return array(
     'almanac-css' => 'dbb9b3af',
     'aphront-bars' => '231ac33c',
     'aphront-dark-console-css' => 'f54bf286',
-    'aphront-dialog-view-css' => '5e5aa60b',
+    'aphront-dialog-view-css' => '685c7e2d',
     'aphront-list-filter-view-css' => '5d6f0526',
     'aphront-multi-column-view-css' => '84cc6640',
     'aphront-panel-view-css' => '8427b78d',
@@ -859,7 +859,7 @@ return array(
     'phui-font-icon-base-css' => '870a7360',
     'phui-fontkit-css' => 'b78a0059',
     'phui-form-css' => 'b62c01d8',
-    'phui-form-view-css' => 'adca31ce',
+    'phui-form-view-css' => 'cf198e10',
     'phui-head-thing-view-css' => 'fd311e5f',
     'phui-header-view-css' => 'fef6a54e',
     'phui-hovercard' => '1bd28176',
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 1279ae4077..59e2feb7d0 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -2496,6 +2496,7 @@ phutil_register_library_map(array(
     'PhabricatorDashboardEditController' => 'applications/dashboard/controller/PhabricatorDashboardEditController.php',
     'PhabricatorDashboardIconSet' => 'applications/dashboard/icon/PhabricatorDashboardIconSet.php',
     'PhabricatorDashboardInstall' => 'applications/dashboard/storage/PhabricatorDashboardInstall.php',
+    'PhabricatorDashboardInstallController' => 'applications/dashboard/controller/PhabricatorDashboardInstallController.php',
     'PhabricatorDashboardLayoutConfig' => 'applications/dashboard/layoutconfig/PhabricatorDashboardLayoutConfig.php',
     'PhabricatorDashboardListController' => 'applications/dashboard/controller/PhabricatorDashboardListController.php',
     'PhabricatorDashboardManageController' => 'applications/dashboard/controller/PhabricatorDashboardManageController.php',
@@ -7562,6 +7563,7 @@ phutil_register_library_map(array(
     'PhabricatorDashboardEditController' => 'PhabricatorDashboardController',
     'PhabricatorDashboardIconSet' => 'PhabricatorIconSet',
     'PhabricatorDashboardInstall' => 'PhabricatorDashboardDAO',
+    'PhabricatorDashboardInstallController' => 'PhabricatorDashboardController',
     'PhabricatorDashboardLayoutConfig' => 'Phobject',
     'PhabricatorDashboardListController' => 'PhabricatorDashboardController',
     'PhabricatorDashboardManageController' => 'PhabricatorDashboardProfileController',
diff --git a/src/applications/dashboard/application/PhabricatorDashboardApplication.php b/src/applications/dashboard/application/PhabricatorDashboardApplication.php
index 123dcc9f6e..c0ce2ff09e 100644
--- a/src/applications/dashboard/application/PhabricatorDashboardApplication.php
+++ b/src/applications/dashboard/application/PhabricatorDashboardApplication.php
@@ -30,6 +30,7 @@ final class PhabricatorDashboardApplication extends PhabricatorApplication {
         'arrange/(?P<id>\d+)/' => 'PhabricatorDashboardArrangeController',
         'create/' => 'PhabricatorDashboardEditController',
         'edit/(?:(?P<id>\d+)/)?' => 'PhabricatorDashboardEditController',
+        'install/(?:(?P<id>\d+)/)?' => 'PhabricatorDashboardInstallController',
         'addpanel/(?P<id>\d+)/' => 'PhabricatorDashboardAddPanelController',
         'movepanel/(?P<id>\d+)/' => 'PhabricatorDashboardMovePanelController',
         'removepanel/(?P<id>\d+)/'
diff --git a/src/applications/dashboard/controller/PhabricatorDashboardArrangeController.php b/src/applications/dashboard/controller/PhabricatorDashboardArrangeController.php
index ed8d24d737..1ef482b7f4 100644
--- a/src/applications/dashboard/controller/PhabricatorDashboardArrangeController.php
+++ b/src/applications/dashboard/controller/PhabricatorDashboardArrangeController.php
@@ -51,6 +51,14 @@ final class PhabricatorDashboardArrangeController
       ->addClass('dashboard-preview-box')
       ->appendChild($rendered_dashboard);
 
+    $install_button = id(new PHUIButtonView())
+      ->setTag('a')
+      ->setText('Install Dashboard')
+      ->setIcon('fa-plus')
+      ->setWorkflow(true)
+      ->setHref($this->getApplicationURI("/install/{$id}/"));
+    $header->addActionLink($install_button);
+
     $view = id(new PHUITwoColumnView())
       ->setHeader($header)
       ->setFooter(array(
diff --git a/src/applications/dashboard/controller/PhabricatorDashboardInstallController.php b/src/applications/dashboard/controller/PhabricatorDashboardInstallController.php
new file mode 100644
index 0000000000..145c6f530e
--- /dev/null
+++ b/src/applications/dashboard/controller/PhabricatorDashboardInstallController.php
@@ -0,0 +1,141 @@
+<?php
+
+final class PhabricatorDashboardInstallController
+  extends PhabricatorDashboardController {
+
+  public function handleRequest(AphrontRequest $request) {
+    $viewer = $request->getViewer();
+    $id = $request->getURIData('id');
+
+    $dashboard = id(new PhabricatorDashboardQuery())
+      ->setViewer($viewer)
+      ->withIDs(array($id))
+      ->executeOne();
+    if (!$dashboard) {
+      return new Aphront404Response();
+    }
+
+    $cancel_uri = $this->getApplicationURI(
+      'view/'.$dashboard->getID().'/');
+
+    $home_app = new PhabricatorHomeApplication();
+
+    $options = array();
+    $options['home'] = array(
+      'personal' =>
+        array(
+          'capability' => PhabricatorPolicyCapability::CAN_VIEW,
+          'application' => $home_app,
+          'name' => pht('Personal Dashboard'),
+          'value' => 'personal',
+          'description' => pht('Places this dashboard as a menu item on home '.
+            'as a personal menu item. It will only be on your personal '.
+            'home.'),
+        ),
+      'global' =>
+        array(
+          'capability' => PhabricatorPolicyCapability::CAN_EDIT,
+          'application' => $home_app,
+          'name' => pht('Global Dashboard'),
+          'value' => 'global',
+          'description' => pht('Places this dashboard as a menu item on home '.
+            'as a global menu item. It will be available to all users.'),
+        ),
+    );
+
+
+    $errors = array();
+    $v_name = null;
+    if ($request->isFormPost()) {
+      $menuitem = new PhabricatorDashboardProfileMenuItem();
+      $dashboard_phid = $dashboard->getPHID();
+      $home = new PhabricatorHomeApplication();
+      $v_name = $request->getStr('name');
+      $v_home = $request->getStr('home');
+
+      if ($v_home) {
+        $application = $options['home'][$v_home]['application'];
+        $capability = $options['home'][$v_home]['capability'];
+
+        $can_edit_home = PhabricatorPolicyFilter::hasCapability(
+          $viewer,
+          $application,
+          $capability);
+
+        if (!$can_edit_home) {
+          $errors[] = pht(
+            'You do not have permission to install a dashboard on home.');
+        }
+      } else {
+          $errors[] = pht(
+            'You must select a destination to install this dashboard.');
+      }
+
+      $v_phid = $viewer->getPHID();
+      if ($v_home == 'global') {
+        $v_phid = null;
+      }
+
+      if (!$errors) {
+        $install = PhabricatorProfileMenuItemConfiguration::initializeNewItem(
+          $home,
+          $menuitem,
+          $v_phid);
+
+        $install->setMenuItemProperty('dashboardPHID', $dashboard_phid);
+        $install->setMenuItemProperty('name', $v_name);
+        $install->setMenuItemOrder(1);
+
+        $xactions = array();
+
+        $editor = id(new PhabricatorProfileMenuEditor())
+          ->setActor($viewer)
+          ->setContinueOnNoEffect(true)
+          ->setContinueOnMissingFields(true)
+          ->setContentSourceFromRequest($request);
+
+        $editor->applyTransactions($install, $xactions);
+
+        $view_uri = '/home/menu/view/'.$install->getID().'/';
+
+        return id(new AphrontRedirectResponse())->setURI($view_uri);
+      }
+    }
+
+    $form = id(new AphrontFormView())
+      ->setUser($viewer)
+      ->appendChild(
+        id(new AphrontFormTextControl())
+          ->setLabel(pht('Menu Label'))
+          ->setName('name')
+          ->setValue($v_name));
+
+    $radio = id(new AphrontFormRadioButtonControl())
+      ->setLabel(pht('Home Menu'))
+      ->setName('home');
+
+    foreach ($options['home'] as $type => $option) {
+      $can_edit = PhabricatorPolicyFilter::hasCapability(
+        $viewer,
+        $option['application'],
+        $option['capability']);
+      if ($can_edit) {
+        $radio->addButton(
+          $option['value'],
+          $option['name'],
+          $option['description']);
+      }
+    }
+
+    $form->appendChild($radio);
+
+    return $this->newDialog()
+      ->setTitle(pht('Install Dashboard'))
+      ->setErrors($errors)
+      ->setWidth(AphrontDialogView::WIDTH_FORM)
+      ->appendChild($form->buildLayoutView())
+      ->addCancelButton($cancel_uri)
+      ->addSubmitButton(pht('Install Dashboard'));
+  }
+
+}
diff --git a/src/applications/dashboard/controller/PhabricatorDashboardViewController.php b/src/applications/dashboard/controller/PhabricatorDashboardViewController.php
index 547398f3fd..07faee8f05 100644
--- a/src/applications/dashboard/controller/PhabricatorDashboardViewController.php
+++ b/src/applications/dashboard/controller/PhabricatorDashboardViewController.php
@@ -43,6 +43,14 @@ final class PhabricatorDashboardViewController
     $navigation = $this->buildSideNavView('view');
     $header = $this->buildHeaderView();
 
+    $install_button = id(new PHUIButtonView())
+      ->setTag('a')
+      ->setText('Install Dashboard')
+      ->setIcon('fa-plus')
+      ->setWorkflow(true)
+      ->setHref($this->getApplicationURI("/install/{$id}/"));
+    $header->addActionLink($install_button);
+
     $view = id(new PHUITwoColumnView())
       ->setHeader($header)
       ->setFooter(array(
diff --git a/webroot/rsrc/css/aphront/dialog-view.css b/webroot/rsrc/css/aphront/dialog-view.css
index 3b2de1cee6..0e6f6d4cfc 100644
--- a/webroot/rsrc/css/aphront/dialog-view.css
+++ b/webroot/rsrc/css/aphront/dialog-view.css
@@ -3,7 +3,7 @@
  */
 
 .aphront-dialog-view {
-  width: 540px;
+  width: 560px;
   margin: 32px auto 16px;
   border: 1px solid {$lightblueborder};
   border-radius: 3px;
@@ -32,7 +32,7 @@
 }
 
 .aphront-dialog-view-width-form {
-  width: 600px;
+  width: 640px;
 }
 
 .aphront-dialog-view-width-full {
diff --git a/webroot/rsrc/css/phui/phui-form-view.css b/webroot/rsrc/css/phui/phui-form-view.css
index cc33808c5a..9dce0a4476 100644
--- a/webroot/rsrc/css/phui/phui-form-view.css
+++ b/webroot/rsrc/css/phui/phui-form-view.css
@@ -208,14 +208,13 @@
 
 table.aphront-form-control-radio-layout,
 table.aphront-form-control-checkbox-layout {
-  margin-top: 3px;
+  margin-top: 4px !important;
   font-size: {$normalfontsize};
 }
 
 table.aphront-form-control-radio-layout th {
-  padding-top: 3px;
   padding-left: 8px;
-  padding-bottom: 4px;
+  padding-bottom: 8px;
   font-weight: bold;
   color: {$darkgreytext};
 }

From fd69dfaa9a14cf73fe83233716ed2332c88b46c3 Mon Sep 17 00:00:00 2001
From: Chad Little <chad@phacility.com>
Date: Wed, 15 Mar 2017 12:43:54 -0700
Subject: [PATCH 05/15] Allow searching for Badge Awards by Badge status

Summary: Fixes T12398. This adds `withBadgeStatuses` as a query parameter when searching for Awards to show. In most (all?) cases we currently only show active badges.

Test Plan: Assign myself a badge, archive it and verify it does not appear on profile, comment form, or timeline.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12398

Differential Revision: https://secure.phabricator.com/D17499
---
 .../query/PhabricatorBadgesAwardQuery.php     | 48 +++++++++++++++++--
 ...abricatorPeopleProfileBadgesController.php |  5 +-
 ...catorApplicationTransactionCommentView.php |  6 ++-
 src/view/phui/PHUITimelineView.php            |  6 +--
 4 files changed, 52 insertions(+), 13 deletions(-)

diff --git a/src/applications/badges/query/PhabricatorBadgesAwardQuery.php b/src/applications/badges/query/PhabricatorBadgesAwardQuery.php
index a147af0b75..347462de4d 100644
--- a/src/applications/badges/query/PhabricatorBadgesAwardQuery.php
+++ b/src/applications/badges/query/PhabricatorBadgesAwardQuery.php
@@ -6,7 +6,7 @@ final class PhabricatorBadgesAwardQuery
   private $badgePHIDs;
   private $recipientPHIDs;
   private $awarderPHIDs;
-
+  private $badgeStatuses = null;
 
   protected function willFilterPage(array $awards) {
     $badge_phids = array();
@@ -22,6 +22,11 @@ final class PhabricatorBadgesAwardQuery
     $badges = mpull($badges, null, 'getPHID');
     foreach ($awards as $key => $award) {
       $award_badge = idx($badges, $award->getBadgePHID());
+      if (!$award_badge) {
+        unset($awards[$key]);
+        $this->didRejectResult($award);
+        continue;
+      }
       $award->attachBadge($award_badge);
     }
 
@@ -43,6 +48,15 @@ final class PhabricatorBadgesAwardQuery
     return $this;
   }
 
+  public function withBadgeStatuses(array $statuses) {
+    $this->badgeStatuses = $statuses;
+    return $this;
+  }
+
+  private function shouldJoinBadge() {
+    return (bool)$this->badgeStatuses;
+  }
+
   protected function loadPage() {
     return $this->loadStandardPage($this->newResultObject());
   }
@@ -51,33 +65,59 @@ final class PhabricatorBadgesAwardQuery
     return new PhabricatorBadgesAward();
   }
 
+  protected function getPrimaryTableAlias() {
+    return 'badges_award';
+  }
+
   protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
     $where = parent::buildWhereClauseParts($conn);
 
     if ($this->badgePHIDs !== null) {
       $where[] = qsprintf(
         $conn,
-        'badgePHID IN (%Ls)',
+        'badges_award.badgePHID IN (%Ls)',
         $this->badgePHIDs);
     }
 
     if ($this->recipientPHIDs !== null) {
       $where[] = qsprintf(
         $conn,
-        'recipientPHID IN (%Ls)',
+        'badges_award.recipientPHID IN (%Ls)',
         $this->recipientPHIDs);
     }
 
     if ($this->awarderPHIDs !== null) {
       $where[] = qsprintf(
         $conn,
-        'awarderPHID IN (%Ls)',
+        'badges_award.awarderPHID IN (%Ls)',
         $this->awarderPHIDs);
     }
 
+    if ($this->badgeStatuses !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'badges_badge.status IN (%Ls)',
+        $this->badgeStatuses);
+    }
+
+
     return $where;
   }
 
+  protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) {
+    $join = parent::buildJoinClauseParts($conn);
+    $badges = new PhabricatorBadgesBadge();
+
+    if ($this->shouldJoinBadge()) {
+      $join[] = qsprintf(
+        $conn,
+        'JOIN %T badges_badge ON badges_award.badgePHID = badges_badge.phid',
+        $badges->getTableName());
+    }
+
+    return $join;
+  }
+
   public function getQueryApplicationClass() {
     return 'PhabricatorBadgesApplication';
   }
diff --git a/src/applications/people/controller/PhabricatorPeopleProfileBadgesController.php b/src/applications/people/controller/PhabricatorPeopleProfileBadgesController.php
index 93b1f91ea8..e96ed4a89e 100644
--- a/src/applications/people/controller/PhabricatorPeopleProfileBadgesController.php
+++ b/src/applications/people/controller/PhabricatorPeopleProfileBadgesController.php
@@ -84,15 +84,14 @@ final class PhabricatorPeopleProfileBadgesController
     $awards = id(new PhabricatorBadgesAwardQuery())
       ->setViewer($viewer)
       ->withRecipientPHIDs(array($user->getPHID()))
+      ->withBadgeStatuses(array(PhabricatorBadgesBadge::STATUS_ACTIVE))
       ->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;
-      }
+      $badges[$award->getBadgePHID()] = $badge;
     }
 
     if (count($badges)) {
diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php
index 0847edd666..b91155183c 100644
--- a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php
+++ b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php
@@ -528,12 +528,14 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView {
     $awards = id(new PhabricatorBadgesAwardQuery())
       ->setViewer($this->getUser())
       ->withRecipientPHIDs(array($user->getPHID()))
+      ->withBadgeStatuses(array(PhabricatorBadgesBadge::STATUS_ACTIVE))
       ->setLimit(2)
       ->execute();
 
+    $badges = mpull($awards, 'getBadge');
+
     $badge_view = null;
-    if ($awards) {
-      $badges = mpull($awards, 'getBadge');
+    if ($badges) {
       $badge_list = array();
       foreach ($badges as $badge) {
         $badge_view = id(new PHUIBadgeMiniView())
diff --git a/src/view/phui/PHUITimelineView.php b/src/view/phui/PHUITimelineView.php
index 20ce5e9e65..57b07a94bf 100644
--- a/src/view/phui/PHUITimelineView.php
+++ b/src/view/phui/PHUITimelineView.php
@@ -220,7 +220,6 @@ final class PHUITimelineView extends AphrontView {
     }
 
     $user_phid_type = PhabricatorPeopleUserPHIDType::TYPECONST;
-    $badge_edge_type = PhabricatorRecipientHasBadgeEdgeType::EDGECONST;
 
     $user_phids = array();
     foreach ($events as $key => $event) {
@@ -248,6 +247,7 @@ final class PHUITimelineView extends AphrontView {
     $awards = id(new PhabricatorBadgesAwardQuery())
       ->setViewer($this->getViewer())
       ->withRecipientPHIDs($user_phids)
+      ->withBadgeStatuses(array(PhabricatorBadgesBadge::STATUS_ACTIVE))
       ->execute();
 
     $awards = mgroup($awards, 'getRecipientPHID');
@@ -259,9 +259,7 @@ final class PHUITimelineView extends AphrontView {
       $badges = array();
       foreach ($author_awards as $award) {
         $badge = $award->getBadge();
-        if ($badge->getStatus() == PhabricatorBadgesBadge::STATUS_ACTIVE) {
-          $badges[$award->getBadgePHID()] = $badge;
-        }
+        $badges[$award->getBadgePHID()] = $badge;
       }
 
       // TODO: Pick the "best" badges in some smart way. For now, just pick

From d6d3ad6f80d90d68355e97f80896c94f709865c4 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 15 Mar 2017 11:44:27 -0700
Subject: [PATCH 06/15] Allow administrators to get a list of users who don't
 have MFA configured

Summary:
Fixes T12400. Adds a "Has MFA" filter to People so you can figure out who you need to harass before turning on "require MFA".

When you run this as a non-admin, you don't currently actually hit the exception: the query just doesn't work. I think this is probably okay, but if we add more of these it might be better to make the "this didn't work" more explicit since it could be confusing in some weird edge cases (like, an administrator sending a non-administrator a link which they expect will show the non-administrator some interesting query results, but they actually just get no constraint). The exception is more of a fail-safe in case we make application changes in the future and don't remember this weird special case.

Test Plan:
  - As an administrator and non-administrator, used People and Conduit to query MFA, no-MFA, and don't-care-about-MFA. These queries worked for an admin and didn't work for a non-admin.
  - Viewed the list as an administrator, saw MFA users annotated.
  - Viewed config help, clicked link as an admin, ended up in the right place.

{F4093033}

{F4093034}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12400

Differential Revision: https://secure.phabricator.com/D17500
---
 src/__phutil_library_map__.php                |  2 +
 .../PhabricatorSecurityConfigOptions.php      | 23 ++++---
 .../people/query/PhabricatorPeopleQuery.php   | 13 ++++
 .../query/PhabricatorPeopleSearchEngine.php   | 61 +++++++++++++++----
 ...PhabricatorApplicationSearchController.php |  2 +
 .../PhabricatorSearchConstraintException.php  |  4 ++
 6 files changed, 86 insertions(+), 19 deletions(-)
 create mode 100644 src/applications/search/exception/PhabricatorSearchConstraintException.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 59e2feb7d0..cfd11d630f 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -3763,6 +3763,7 @@ phutil_register_library_map(array(
     'PhabricatorSearchBaseController' => 'applications/search/controller/PhabricatorSearchBaseController.php',
     'PhabricatorSearchCheckboxesField' => 'applications/search/field/PhabricatorSearchCheckboxesField.php',
     'PhabricatorSearchConfigOptions' => 'applications/search/config/PhabricatorSearchConfigOptions.php',
+    'PhabricatorSearchConstraintException' => 'applications/search/exception/PhabricatorSearchConstraintException.php',
     'PhabricatorSearchController' => 'applications/search/controller/PhabricatorSearchController.php',
     'PhabricatorSearchCustomFieldProxyField' => 'applications/search/field/PhabricatorSearchCustomFieldProxyField.php',
     'PhabricatorSearchDAO' => 'applications/search/storage/PhabricatorSearchDAO.php',
@@ -9072,6 +9073,7 @@ phutil_register_library_map(array(
     'PhabricatorSearchBaseController' => 'PhabricatorController',
     'PhabricatorSearchCheckboxesField' => 'PhabricatorSearchField',
     'PhabricatorSearchConfigOptions' => 'PhabricatorApplicationConfigOptions',
+    'PhabricatorSearchConstraintException' => 'Exception',
     'PhabricatorSearchController' => 'PhabricatorSearchBaseController',
     'PhabricatorSearchCustomFieldProxyField' => 'PhabricatorSearchField',
     'PhabricatorSearchDAO' => 'PhabricatorLiskDAO',
diff --git a/src/applications/config/option/PhabricatorSecurityConfigOptions.php b/src/applications/config/option/PhabricatorSecurityConfigOptions.php
index 4d4da4bd02..47fd1bb6f2 100644
--- a/src/applications/config/option/PhabricatorSecurityConfigOptions.php
+++ b/src/applications/config/option/PhabricatorSecurityConfigOptions.php
@@ -66,6 +66,21 @@ EOTEXT
       ,
       PhabricatorEnv::getDoclink('Configuring Encryption')));
 
+    $require_mfa_description = $this->deformat(pht(<<<EOTEXT
+By default, Phabricator allows users to add multi-factor authentication to
+their accounts, but does not require it. By enabling this option, you can
+force all users to add at least one authentication factor before they can use
+their accounts.
+
+Administrators can query a list of users who do not have MFA configured in
+{nav People}:
+
+  - **[[ %s | %s ]]**
+EOTEXT
+      ,
+      '/people/?mfa=false',
+      pht('List of Users Without MFA')));
+
     return array(
       $this->newOption('security.alternate-file-domain', 'string', null)
         ->setLocked(true)
@@ -132,13 +147,7 @@ EOTEXT
         ->setLocked(true)
         ->setSummary(
           pht('Require all users to configure multi-factor authentication.'))
-        ->setDescription(
-          pht(
-            'By default, Phabricator allows users to add multi-factor '.
-            'authentication to their accounts, but does not require it. '.
-            'By enabling this option, you can force all users to add '.
-            'at least one authentication factor before they can use their '.
-            'accounts.'))
+        ->setDescription($require_mfa_description)
         ->setBoolOptions(
           array(
             pht('Multi-Factor Required'),
diff --git a/src/applications/people/query/PhabricatorPeopleQuery.php b/src/applications/people/query/PhabricatorPeopleQuery.php
index b6791a7be2..6cb2922083 100644
--- a/src/applications/people/query/PhabricatorPeopleQuery.php
+++ b/src/applications/people/query/PhabricatorPeopleQuery.php
@@ -18,6 +18,7 @@ final class PhabricatorPeopleQuery
   private $nameLike;
   private $nameTokens;
   private $namePrefixes;
+  private $isEnrolledInMultiFactor;
 
   private $needPrimaryEmail;
   private $needProfile;
@@ -100,6 +101,11 @@ final class PhabricatorPeopleQuery
     return $this;
   }
 
+  public function withIsEnrolledInMultiFactor($enrolled) {
+    $this->isEnrolledInMultiFactor = $enrolled;
+    return $this;
+  }
+
   public function needPrimaryEmail($need) {
     $this->needPrimaryEmail = $need;
     return $this;
@@ -337,6 +343,13 @@ final class PhabricatorPeopleQuery
         $this->nameLike);
     }
 
+    if ($this->isEnrolledInMultiFactor !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'user.isEnrolledInMultiFactor = %d',
+        (int)$this->isEnrolledInMultiFactor);
+    }
+
     return $where;
   }
 
diff --git a/src/applications/people/query/PhabricatorPeopleSearchEngine.php b/src/applications/people/query/PhabricatorPeopleSearchEngine.php
index ce448b0bc9..e800a8ee1a 100644
--- a/src/applications/people/query/PhabricatorPeopleSearchEngine.php
+++ b/src/applications/people/query/PhabricatorPeopleSearchEngine.php
@@ -18,7 +18,7 @@ final class PhabricatorPeopleSearchEngine
   }
 
   protected function buildCustomSearchFields() {
-    return array(
+    $fields = array(
       id(new PhabricatorSearchStringListField())
         ->setLabel(pht('Usernames'))
         ->setKey('usernames')
@@ -84,18 +84,36 @@ final class PhabricatorPeopleSearchEngine
           pht(
             'Pass true to find only users awaiting administrative approval, '.
             'or false to omit these users.')),
-      id(new PhabricatorSearchDateField())
-        ->setKey('createdStart')
-        ->setLabel(pht('Joined After'))
-        ->setDescription(
-          pht('Find user accounts created after a given time.')),
-      id(new PhabricatorSearchDateField())
-        ->setKey('createdEnd')
-        ->setLabel(pht('Joined Before'))
-        ->setDescription(
-          pht('Find user accounts created before a given time.')),
-
     );
+
+    $viewer = $this->requireViewer();
+    if ($viewer->getIsAdmin()) {
+      $fields[] = id(new PhabricatorSearchThreeStateField())
+        ->setLabel(pht('Has MFA'))
+        ->setKey('mfa')
+        ->setOptions(
+          pht('(Show All)'),
+          pht('Show Only Users With MFA'),
+          pht('Hide Users With MFA'))
+        ->setDescription(
+          pht(
+            'Pass true to find only users who are enrolled in MFA, or false '.
+            'to omit these users.'));
+    }
+
+    $fields[] = id(new PhabricatorSearchDateField())
+      ->setKey('createdStart')
+      ->setLabel(pht('Joined After'))
+      ->setDescription(
+        pht('Find user accounts created after a given time.'));
+
+    $fields[] = id(new PhabricatorSearchDateField())
+      ->setKey('createdEnd')
+      ->setLabel(pht('Joined Before'))
+      ->setDescription(
+        pht('Find user accounts created before a given time.'));
+
+    return $fields;
   }
 
   protected function getDefaultFieldOrder() {
@@ -151,6 +169,19 @@ final class PhabricatorPeopleSearchEngine
       $query->withIsApproved(!$map['needsApproval']);
     }
 
+    if (idx($map, 'mfa') !== null) {
+      $viewer = $this->requireViewer();
+      if (!$viewer->getIsAdmin()) {
+        throw new PhabricatorSearchConstraintException(
+          pht(
+            'The "Has MFA" query constraint may only be used by '.
+            'administrators, to prevent attackers from using it to target '.
+            'weak accounts.'));
+      }
+
+      $query->withIsEnrolledInMultiFactor($map['mfa']);
+    }
+
     if ($map['createdStart']) {
       $query->withDateCreatedAfter($map['createdStart']);
     }
@@ -254,6 +285,12 @@ final class PhabricatorPeopleSearchEngine
         $item->addIcon('fa-envelope-o', pht('Mailing List'));
       }
 
+      if ($viewer->getIsAdmin()) {
+        if ($user->getIsEnrolledInMultiFactor()) {
+          $item->addIcon('fa-lock', pht('Has MFA'));
+        }
+      }
+
       if ($viewer->getIsAdmin()) {
         $user_id = $user->getID();
         if ($is_approval) {
diff --git a/src/applications/search/controller/PhabricatorApplicationSearchController.php b/src/applications/search/controller/PhabricatorApplicationSearchController.php
index c7c2b432b4..faca9991ea 100644
--- a/src/applications/search/controller/PhabricatorApplicationSearchController.php
+++ b/src/applications/search/controller/PhabricatorApplicationSearchController.php
@@ -331,6 +331,8 @@ final class PhabricatorApplicationSearchController
           'query parameters and correct errors.');
       } catch (PhutilSearchQueryCompilerSyntaxException $ex) {
         $exec_errors[] = $ex->getMessage();
+      } catch (PhabricatorSearchConstraintException $ex) {
+        $exec_errors[] = $ex->getMessage();
       }
 
       // The engine may have encountered additional errors during rendering;
diff --git a/src/applications/search/exception/PhabricatorSearchConstraintException.php b/src/applications/search/exception/PhabricatorSearchConstraintException.php
new file mode 100644
index 0000000000..7d674518e1
--- /dev/null
+++ b/src/applications/search/exception/PhabricatorSearchConstraintException.php
@@ -0,0 +1,4 @@
+<?php
+
+final class PhabricatorSearchConstraintException
+  extends Exception {}

From 7626ec0ce10e5a9665d84622259e073744c9872d Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 16 Mar 2017 09:06:22 -0700
Subject: [PATCH 07/15] Correct an issue where "View Raw File" in Differential
 generated a file with overbroad permissions

Summary:
Via HackerOne. When you view a raw file in Differential, we currently generate a permanent file with default permissions. This may be incorrect: default permissions may be broader than the diff's permissions.

The other three methods of downloading/viewing raw files ("Download" in Diffusion and Differential, "View Raw" in Diffusion and Differential) already apply policies correctly and generate temporary files. However, this workflow was missed when other workflows were updated.

Beyond updating the workflow, delete any files we've generated in the past. This wipes the slate clean on any security issues and frees up a little disk space.

Test Plan:
  - Ran migration script, saw existing files get purged.
  - Did "View Raw File", got a new file.
  - Verified that the file was temporary and properly attached to the diff, with "NO ONE" permissions.
  - Double-checked that Diffusion already runs policy logic correctly and applies appropriate policies.
  - Double-checked that "Download Raw Diff" in Differential already runs policy logic correctly.
  - Double-chekced that "Download Raw Diff" in Diffusion already runs policy logic correctly.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17504
---
 .../sql/autopatches/20170316.rawfiles.01.php  | 53 +++++++++++++++++++
 .../DifferentialChangesetViewController.php   |  8 ++-
 2 files changed, 60 insertions(+), 1 deletion(-)
 create mode 100644 resources/sql/autopatches/20170316.rawfiles.01.php

diff --git a/resources/sql/autopatches/20170316.rawfiles.01.php b/resources/sql/autopatches/20170316.rawfiles.01.php
new file mode 100644
index 0000000000..df2fa93c9a
--- /dev/null
+++ b/resources/sql/autopatches/20170316.rawfiles.01.php
@@ -0,0 +1,53 @@
+<?php
+
+// Previously, files generated by "View Raw File" were written as permanent
+// files with excessively wide view permissions. This destroys these files
+// so they are recreated as temporary files with correct permissions when
+// next accessed.
+
+$table = new DifferentialChangeset();
+$conn = $table->establishConnection('w');
+$viewer = PhabricatorUser::getOmnipotentUser();
+
+$iterator = new LiskRawMigrationIterator(
+  $conn,
+  $table->getTableName());
+
+echo tsprintf(
+  "%s\n",
+  pht('Purging old raw changeset file caches...'));
+
+$keys = array(
+  'raw:new:phid',
+  'raw:old:phid',
+);
+
+foreach ($iterator as $changeset) {
+  try {
+    $metadata = phutil_json_decode($changeset['metadata']);
+
+    $phids = array();
+    foreach ($keys as $key) {
+      if (isset($metadata[$key])) {
+        $phids[] = $metadata[$key];
+      }
+    }
+
+    foreach ($phids as $phid) {
+      $file = id(new PhabricatorFileQuery())
+        ->setViewer($viewer)
+        ->withPHIDs(array($phid))
+        ->executeOne();
+      if ($file) {
+        id(new PhabricatorDestructionEngine())
+          ->destroyObject($file);
+      }
+    }
+
+    // NOTE: We don't bother updating the changeset record itself: we already
+    // regenerate the cache properly if the stored value isn't valid.
+
+  } catch (Exception $ex) {
+    // Just move on if we run into trouble.
+  }
+}
diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php
index ff0da2f821..b5bfe7464d 100644
--- a/src/applications/differential/controller/DifferentialChangesetViewController.php
+++ b/src/applications/differential/controller/DifferentialChangesetViewController.php
@@ -350,13 +350,19 @@ final class DifferentialChangesetViewController extends DifferentialController {
         $data = $changeset->makeOldFile();
       }
 
+      $diff = $changeset->getDiff();
+
       $file = PhabricatorFile::newFromFileData(
         $data,
         array(
-          'name'      => $changeset->getFilename(),
+          'name' => $changeset->getFilename(),
           'mime-type' => 'text/plain',
+          'ttl' => phutil_units('24 hours in seconds'),
+          'viewPolicy' => PhabricatorPolicies::POLICY_NOONE,
         ));
 
+      $file->attachToObject($diff->getPHID());
+
       $metadata[$key] = $file->getPHID();
       $changeset->setMetadata($metadata);
       $changeset->save();

From 19af10df37063230e219a95b4d773c70f2ea3ce0 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 15 Mar 2017 18:09:27 -0700
Subject: [PATCH 08/15] Apply the wordwrap() hack for "To" to PHPMailerLite

Summary:
Fixes T12372. Long-term fix is T12404, this is a bandaid in the interim.

See T12372 for additional discussion.

Test Plan: Confirmed functional by a user in T12372.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12372

Differential Revision: https://secure.phabricator.com/D17501
---
 externals/phpmailer/class.phpmailer-lite.php | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/externals/phpmailer/class.phpmailer-lite.php b/externals/phpmailer/class.phpmailer-lite.php
index c064d6cddd..610de99438 100644
--- a/externals/phpmailer/class.phpmailer-lite.php
+++ b/externals/phpmailer/class.phpmailer-lite.php
@@ -656,6 +656,10 @@ class PHPMailerLite {
     $addr_str .= implode(', ', $addresses);
     $addr_str .= $this->LE;
 
+    // NOTE: This is a narrow hack to fix an issue with 1000+ characters of
+    // recipients, described in T12372.
+    $addr_str = wordwrap($addr_str, 75, "\n ");
+
     return $addr_str;
   }
 

From de4e8728b2fa8e1611cac54c7e41cf7fe3a863e9 Mon Sep 17 00:00:00 2001
From: Chad Little <chad@phacility.com>
Date: Thu, 16 Mar 2017 11:32:01 -0700
Subject: [PATCH 09/15] Add ActionIcon to PHUIListItemView, use in Dashboards

Summary: Extends PHUIListItemView to take an icon, link as an "Action Item" that displays on the right side of the menu link. Does not display on Favorites. This allows for adding edit, external, or other links (documentation?) to any menu item. Right now the secondary link is only visible when the item is selected. This feels right, but if we offer it in other ways, users may always want it visible. We could look at making it onhover.

Test Plan:
Add a bunch of random global and personal dashboards to my menu. Add a menu to Favorites, see no link. Test mobile, link works.

{F4136699}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17505
---
 resources/celerity/map.php                    |  6 +--
 .../PhabricatorDashboardProfileMenuItem.php   |  4 +-
 src/view/phui/PHUIListItemView.php            | 30 +++++++++++++-
 webroot/rsrc/css/phui/phui-list.css           | 40 +++++++++++++++++++
 4 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index 46bb152704..2eab0ac0c3 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -9,7 +9,7 @@ return array(
   'names' => array(
     'conpherence.pkg.css' => '32f2c040',
     'conpherence.pkg.js' => '6249a1cf',
-    'core.pkg.css' => 'c0c87dac',
+    'core.pkg.css' => '491d7018',
     'core.pkg.js' => '1fa7c0c5',
     'darkconsole.pkg.js' => 'e7393ebb',
     'differential.pkg.css' => '90b30783',
@@ -158,7 +158,7 @@ return array(
     'rsrc/css/phui/phui-info-view.css' => 'ec92802a',
     'rsrc/css/phui/phui-invisible-character-view.css' => '6993d9f0',
     'rsrc/css/phui/phui-lightbox.css' => '0a035e40',
-    'rsrc/css/phui/phui-list.css' => '9da2aa00',
+    'rsrc/css/phui/phui-list.css' => 'a3ec3cf1',
     'rsrc/css/phui/phui-object-box.css' => '8b289e3d',
     'rsrc/css/phui/phui-pager.css' => '77d8a794',
     'rsrc/css/phui/phui-pinboard-view.css' => '2495140e',
@@ -872,7 +872,7 @@ return array(
     'phui-inline-comment-view-css' => 'be663c95',
     'phui-invisible-character-view-css' => '6993d9f0',
     'phui-lightbox-css' => '0a035e40',
-    'phui-list-view-css' => '9da2aa00',
+    'phui-list-view-css' => 'a3ec3cf1',
     'phui-object-box-css' => '8b289e3d',
     'phui-oi-big-ui-css' => '19f9369b',
     'phui-oi-color-css' => 'cd2b9b77',
diff --git a/src/applications/search/menuitem/PhabricatorDashboardProfileMenuItem.php b/src/applications/search/menuitem/PhabricatorDashboardProfileMenuItem.php
index c442a2cc46..7d4f319b61 100644
--- a/src/applications/search/menuitem/PhabricatorDashboardProfileMenuItem.php
+++ b/src/applications/search/menuitem/PhabricatorDashboardProfileMenuItem.php
@@ -133,11 +133,13 @@ final class PhabricatorDashboardProfileMenuItem
     $icon = $dashboard->getIcon();
     $name = $this->getDisplayName($config);
     $href = $this->getItemViewURI($config);
+    $action_href = '/dashboard/arrange/'.$dashboard->getID().'/';
 
     $item = $this->newItem()
       ->setHref($href)
       ->setName($name)
-      ->setIcon($icon);
+      ->setIcon($icon)
+      ->setActionIcon('fa-pencil', $action_href);
 
     return array(
       $item,
diff --git a/src/view/phui/PHUIListItemView.php b/src/view/phui/PHUIListItemView.php
index 7b6e55a697..1a6d26d917 100644
--- a/src/view/phui/PHUIListItemView.php
+++ b/src/view/phui/PHUIListItemView.php
@@ -31,6 +31,8 @@ final class PHUIListItemView extends AphrontTagView {
   private $icons = array();
   private $openInNewWindow = false;
   private $tooltip;
+  private $actionIcon;
+  private $actionIconHref;
 
   public function setOpenInNewWindow($open_in_new_window) {
     $this->openInNewWindow = $open_in_new_window;
@@ -154,6 +156,12 @@ final class PHUIListItemView extends AphrontTagView {
     return $this->name;
   }
 
+  public function setActionIcon($icon, $href) {
+    $this->actionIcon = $icon;
+    $this->actionIconHref = $href;
+    return $this;
+  }
+
   public function setIsExternal($is_external) {
     $this->isExternal = $is_external;
     return $this;
@@ -207,6 +215,10 @@ final class PHUIListItemView extends AphrontTagView {
       $classes[] = $this->statusColor;
     }
 
+    if ($this->actionIcon) {
+      $classes[] = 'phui-list-item-has-action-icon';
+    }
+
     return array(
       'class' => implode(' ', $classes),
     );
@@ -311,9 +323,23 @@ final class PHUIListItemView extends AphrontTagView {
       $classes[] = 'phui-list-item-indented';
     }
 
+    $action_link = null;
+    if ($this->actionIcon) {
+      $action_icon = id(new PHUIIconView())
+        ->setIcon($this->actionIcon)
+        ->addClass('phui-list-item-action-icon');
+      $action_link = phutil_tag(
+        'a',
+        array(
+          'href' => $this->actionIconHref,
+          'class' => 'phui-list-item-action-href',
+        ),
+        $action_icon);
+    }
+
     $icons = $this->getIcons();
 
-    return javelin_tag(
+    $list_item = javelin_tag(
       $this->href ? 'a' : 'div',
       array(
         'href' => $this->href,
@@ -329,6 +355,8 @@ final class PHUIListItemView extends AphrontTagView {
         $this->renderChildren(),
         $name,
       ));
+
+    return array($list_item, $action_link);
   }
 
 }
diff --git a/webroot/rsrc/css/phui/phui-list.css b/webroot/rsrc/css/phui/phui-list.css
index 25504a611d..eadd1987ee 100644
--- a/webroot/rsrc/css/phui/phui-list.css
+++ b/webroot/rsrc/css/phui/phui-list.css
@@ -2,6 +2,10 @@
  * @provides phui-list-view-css
  */
 
+.phui-list-item-view {
+  position: relative;
+}
+
 .phui-list-item-header,
 .phui-list-item-header a {
   color: {$bluetext};
@@ -188,3 +192,39 @@
   margin-top: 16px;
   border-top: 1px solid {$thinblueborder};
 }
+
+/* - Action Icon ----------------------------------------------------------- */
+
+.phui-list-item-has-action-icon .phui-list-item-action-href {
+  position: absolute;
+  width: 28px;
+  top: 0;
+  right: 0;
+  bottom: 0;
+  text-align: center;
+  line-height: 28px;
+  background-color: transparent;
+  display: none;
+}
+
+.phui-list-item-has-action-icon.phui-list-item-selected .phui-list-item-href {
+  padding-right: 32px;
+}
+
+.phui-list-item-has-action-icon.phui-list-item-selected
+  .phui-list-item-action-href {
+    display: block;
+}
+
+.phui-list-item-has-action-icon .phui-list-item-action-href:hover {
+  background-color: rgba({$alphablack},.05);
+}
+
+.phui-list-item-has-action-icon .phui-list-item-action-icon {
+  opacity: 0.5;
+}
+
+.phui-list-item-has-action-icon .phui-list-item-action-href:hover
+  .phui-list-item-action-icon {
+    opacity: 1;
+}

From ba2ee3a66e893055a76f1bae0cda36cdb0bb2029 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 16 Mar 2017 11:55:16 -0700
Subject: [PATCH 10/15] Make "bin/config set --database ..." resurrect deleted
 values

Summary:
Fixes T12409. Config entries may be marked as "deleted", and `bin/config set --database` doesn't un-delete them, so the edit doesn't do anything.

The "most correct" fix here is to swap to transactions so we run the same code, but just fix this narrowly for now since it's one line of code.

Test Plan:
  - Set `maniphest.default-priority` to `123`.
  - Deleted `maniphest.default-priority` from the web UI by deleting all the text in the box.
  - Before patch: `bin/config set --database maniphest.default-priority 789` had no effect.
  - After patch: `bin/config set --database maniphest.default-priority 789` worked.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12409

Differential Revision: https://secure.phabricator.com/D17506
---
 .../management/PhabricatorConfigManagementSetWorkflow.php     | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/applications/config/management/PhabricatorConfigManagementSetWorkflow.php b/src/applications/config/management/PhabricatorConfigManagementSetWorkflow.php
index f6d40a5126..4511865785 100644
--- a/src/applications/config/management/PhabricatorConfigManagementSetWorkflow.php
+++ b/src/applications/config/management/PhabricatorConfigManagementSetWorkflow.php
@@ -158,6 +158,10 @@ final class PhabricatorConfigManagementSetWorkflow
       $config_type = 'database';
       $config_entry = PhabricatorConfigEntry::loadConfigEntry($key);
       $config_entry->setValue($value);
+
+      // If the entry has been deleted, resurrect it.
+      $config_entry->setIsDeleted(0);
+
       $config_entry->save();
     } else {
       $config_type = 'local';

From 65de9e9f5e7f2ecc018ff300e13227d77ea342da Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 16 Mar 2017 12:22:26 -0700
Subject: [PATCH 11/15] Ignore "Auditors: author" when inferring auditors from
 commit messages

Summary:
Fixes T12406. When importing commits, we automatically add auditors if the message lists "Auditors: username".

If the list of auditors includes the commit author, this edit fails because you can't audit your own commits (previously, you sometimes could and/or we didn't validate).

Instead, just ignore "Auditors: author".

Test Plan:
  - Made a commit with "Auditors: epriestley".
  - Pushed it.
  - Saw the HeraldWorker get stuck with the error in T12406.
  - Applied the change; worker now succeeded.

Reviewers: chad

Reviewed By: chad

Subscribers: alexmv

Maniphest Tasks: T12406

Differential Revision: https://secure.phabricator.com/D17507
---
 .../audit/editor/PhabricatorAuditEditor.php          | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php
index 1c43b08905..0cf6339239 100644
--- a/src/applications/audit/editor/PhabricatorAuditEditor.php
+++ b/src/applications/audit/editor/PhabricatorAuditEditor.php
@@ -306,6 +306,18 @@ final class PhabricatorAuditEditor
 
     $field_key = DifferentialAuditorsCommitMessageField::FIELDKEY;
     $phids = idx($result, $field_key, null);
+
+    if (!$phids) {
+      return array();
+    }
+
+    // If a commit lists its author as an auditor, just pretend it does not.
+    foreach ($phids as $key => $phid) {
+      if ($phid == $commit->getAuthorPHID()) {
+        unset($phids[$key]);
+      }
+    }
+
     if (!$phids) {
       return array();
     }

From aef2a39a81b0c32e776b1e58cc63c4043e58ffe4 Mon Sep 17 00:00:00 2001
From: Chad Little <chad@phacility.com>
Date: Fri, 17 Mar 2017 10:32:07 -0700
Subject: [PATCH 12/15] Add Badges to UserCache

Summary: Ref T12270. Builds out a BadgeCache for PhabricatorUser, primarily for Timeline, potentially feed? This should still work if we later let people pick which two, just switch query in BadgeCache.

Test Plan: Give out badges, test timeline for displaying badges from handles and without queries. Revoke a badge, see cache change.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12270

Differential Revision: https://secure.phabricator.com/D17503
---
 src/__phutil_library_map__.php                |  2 +
 .../badges/editor/PhabricatorBadgesEditor.php | 41 +++++++++++++
 .../cache/PhabricatorUserBadgesCacheType.php  | 61 +++++++++++++++++++
 .../people/query/PhabricatorPeopleQuery.php   | 13 ++++
 .../people/storage/PhabricatorUser.php        |  5 ++
 ...catorApplicationTransactionCommentView.php | 19 ++----
 src/view/phui/PHUITimelineView.php            | 37 ++++-------
 7 files changed, 141 insertions(+), 37 deletions(-)
 create mode 100644 src/applications/people/cache/PhabricatorUserBadgesCacheType.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index cfd11d630f..29bf2e418f 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -4067,6 +4067,7 @@ phutil_register_library_map(array(
     'PhabricatorUnknownContentSource' => 'infrastructure/contentsource/PhabricatorUnknownContentSource.php',
     'PhabricatorUnsubscribedFromObjectEdgeType' => 'applications/transactions/edges/PhabricatorUnsubscribedFromObjectEdgeType.php',
     'PhabricatorUser' => 'applications/people/storage/PhabricatorUser.php',
+    'PhabricatorUserBadgesCacheType' => 'applications/people/cache/PhabricatorUserBadgesCacheType.php',
     'PhabricatorUserBlurbField' => 'applications/people/customfield/PhabricatorUserBlurbField.php',
     'PhabricatorUserCache' => 'applications/people/storage/PhabricatorUserCache.php',
     'PhabricatorUserCacheType' => 'applications/people/cache/PhabricatorUserCacheType.php',
@@ -9415,6 +9416,7 @@ phutil_register_library_map(array(
       'PhabricatorFulltextInterface',
       'PhabricatorConduitResultInterface',
     ),
+    'PhabricatorUserBadgesCacheType' => 'PhabricatorUserCacheType',
     'PhabricatorUserBlurbField' => 'PhabricatorUserCustomField',
     'PhabricatorUserCache' => 'PhabricatorUserDAO',
     'PhabricatorUserCacheType' => 'Phobject',
diff --git a/src/applications/badges/editor/PhabricatorBadgesEditor.php b/src/applications/badges/editor/PhabricatorBadgesEditor.php
index a077f4c3ba..fddc55747c 100644
--- a/src/applications/badges/editor/PhabricatorBadgesEditor.php
+++ b/src/applications/badges/editor/PhabricatorBadgesEditor.php
@@ -118,4 +118,45 @@ final class PhabricatorBadgesEditor
     return pht('[Badge]');
   }
 
+  protected function applyFinalEffects(
+    PhabricatorLiskDAO $object,
+    array $xactions) {
+
+    $badge_phid = $object->getPHID();
+    $user_phids = array();
+    $clear_everything = false;
+
+    foreach ($xactions as $xaction) {
+      switch ($xaction->getTransactionType()) {
+        case PhabricatorBadgesBadgeAwardTransaction::TRANSACTIONTYPE:
+        case PhabricatorBadgesBadgeRevokeTransaction::TRANSACTIONTYPE:
+          foreach ($xaction->getNewValue() as $user_phid) {
+            $user_phids[] = $user_phid;
+          }
+          break;
+        default:
+          $clear_everything = true;
+          break;
+      }
+    }
+
+    if ($clear_everything) {
+      $awards = id(new PhabricatorBadgesAwardQuery())
+        ->setViewer($this->getActor())
+        ->withBadgePHIDs(array($badge_phid))
+        ->execute();
+      foreach ($awards as $award) {
+        $user_phids[] = $award->getRecipientPHID();
+      }
+    }
+
+    if ($user_phids) {
+      PhabricatorUserCache::clearCaches(
+        PhabricatorUserBadgesCacheType::KEY_BADGES,
+        $user_phids);
+    }
+
+    return $xactions;
+  }
+
 }
diff --git a/src/applications/people/cache/PhabricatorUserBadgesCacheType.php b/src/applications/people/cache/PhabricatorUserBadgesCacheType.php
new file mode 100644
index 0000000000..28eef59777
--- /dev/null
+++ b/src/applications/people/cache/PhabricatorUserBadgesCacheType.php
@@ -0,0 +1,61 @@
+<?php
+
+final class PhabricatorUserBadgesCacheType
+  extends PhabricatorUserCacheType {
+
+  const CACHETYPE = 'badges.award';
+
+  const KEY_BADGES = 'user.badge.award.v1';
+
+  const BADGE_COUNT = 2;
+
+  public function getAutoloadKeys() {
+    return array(
+      self::KEY_BADGES,
+    );
+  }
+
+  public function canManageKey($key) {
+    return ($key === self::KEY_BADGES);
+  }
+
+  public function getValueFromStorage($value) {
+    return phutil_json_decode($value);
+  }
+
+  public function newValueForUsers($key, array $users) {
+    if (!$users) {
+      return array();
+    }
+
+    $user_phids = mpull($users, 'getPHID');
+
+    $results = array();
+    foreach ($user_phids as $user_phid) {
+      $awards = id(new PhabricatorBadgesAwardQuery())
+        ->setViewer($this->getViewer())
+        ->withRecipientPHIDs(array($user_phid))
+        ->withBadgeStatuses(array(PhabricatorBadgesBadge::STATUS_ACTIVE))
+        ->setLimit(self::BADGE_COUNT)
+        ->execute();
+
+      $award_data = array();
+      if ($awards) {
+        foreach ($awards as $award) {
+          $badge = $award->getBadge();
+          $award_data[] = array(
+            'icon' => $badge->getIcon(),
+            'name' => $badge->getName(),
+            'quality' => $badge->getQuality(),
+            'id' => $badge->getID(),
+          );
+        }
+      }
+      $results[$user_phid] = phutil_json_encode($award_data);
+
+    }
+
+    return $results;
+  }
+
+}
diff --git a/src/applications/people/query/PhabricatorPeopleQuery.php b/src/applications/people/query/PhabricatorPeopleQuery.php
index 6cb2922083..7367382e50 100644
--- a/src/applications/people/query/PhabricatorPeopleQuery.php
+++ b/src/applications/people/query/PhabricatorPeopleQuery.php
@@ -24,6 +24,7 @@ final class PhabricatorPeopleQuery
   private $needProfile;
   private $needProfileImage;
   private $needAvailability;
+  private $needBadgeAwards;
   private $cacheKeys = array();
 
   public function withIDs(array $ids) {
@@ -145,6 +146,18 @@ final class PhabricatorPeopleQuery
     return $this;
   }
 
+  public function needBadgeAwards($need) {
+    $cache_key = PhabricatorUserBadgesCacheType::KEY_BADGES;
+
+    if ($need) {
+      $this->cacheKeys[$cache_key] = true;
+    } else {
+      unset($this->cacheKeys[$cache_key]);
+    }
+
+    return $this;
+  }
+
   public function newResultObject() {
     return new PhabricatorUser();
   }
diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php
index 215f8b9eed..7edbcd4e52 100644
--- a/src/applications/people/storage/PhabricatorUser.php
+++ b/src/applications/people/storage/PhabricatorUser.php
@@ -848,6 +848,11 @@ final class PhabricatorUser
     return $this->requireCacheData($message_key);
   }
 
+  public function getRecentBadgeAwards() {
+    $badges_key = PhabricatorUserBadgesCacheType::KEY_BADGES;
+    return $this->requireCacheData($badges_key);
+  }
+
   public function getFullName() {
     if (strlen($this->getRealName())) {
       return $this->getUsername().' ('.$this->getRealName().')';
diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php
index b91155183c..232420d4c2 100644
--- a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php
+++ b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php
@@ -525,25 +525,18 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView {
       return null;
     }
 
-    $awards = id(new PhabricatorBadgesAwardQuery())
-      ->setViewer($this->getUser())
-      ->withRecipientPHIDs(array($user->getPHID()))
-      ->withBadgeStatuses(array(PhabricatorBadgesBadge::STATUS_ACTIVE))
-      ->setLimit(2)
-      ->execute();
-
-    $badges = mpull($awards, 'getBadge');
-
+    // Pull Badges from UserCache
+    $badges = $user->getRecentBadgeAwards();
     $badge_view = null;
     if ($badges) {
       $badge_list = array();
       foreach ($badges as $badge) {
         $badge_view = id(new PHUIBadgeMiniView())
-          ->setIcon($badge->getIcon())
-          ->setQuality($badge->getQuality())
-          ->setHeader($badge->getName())
+          ->setIcon($badge['icon'])
+          ->setQuality($badge['quality'])
+          ->setHeader($badge['name'])
           ->setTipDirection('E')
-          ->setHref('/badges/view/'.$badge->getID());
+          ->setHref('/badges/view/'.$badge['id'].'/');
 
         $badge_list[] = $badge_view;
       }
diff --git a/src/view/phui/PHUITimelineView.php b/src/view/phui/PHUITimelineView.php
index 57b07a94bf..804b060d55 100644
--- a/src/view/phui/PHUITimelineView.php
+++ b/src/view/phui/PHUITimelineView.php
@@ -243,37 +243,26 @@ final class PHUITimelineView extends AphrontView {
       return;
     }
 
-
-    $awards = id(new PhabricatorBadgesAwardQuery())
-      ->setViewer($this->getViewer())
-      ->withRecipientPHIDs($user_phids)
-      ->withBadgeStatuses(array(PhabricatorBadgesBadge::STATUS_ACTIVE))
+    $users = id(new PhabricatorPeopleQuery())
+      ->setViewer($viewer)
+      ->withPHIDs($user_phids)
+      ->needBadgeAwards(true)
       ->execute();
-
-    $awards = mgroup($awards, 'getRecipientPHID');
+    $users = mpull($users, null, 'getPHID');
 
     foreach ($events as $event) {
-
-      $author_awards = idx($awards, $event->getAuthorPHID(), array());
-
-      $badges = array();
-      foreach ($author_awards as $award) {
-        $badge = $award->getBadge();
-        $badges[$award->getBadgePHID()] = $badge;
+      $user_phid = $event->getAuthorPHID();
+      if (!array_key_exists($user_phid, $users)) {
+        continue;
       }
-
-      // TODO: Pick the "best" badges in some smart way. For now, just pick
-      // the first two.
-      $badges = array_slice($badges, 0, 2);
-
+      $badges = $users[$user_phid]->getRecentBadgeAwards();
       foreach ($badges as $badge) {
         $badge_view = id(new PHUIBadgeMiniView())
-          ->setIcon($badge->getIcon())
-          ->setQuality($badge->getQuality())
-          ->setHeader($badge->getName())
+          ->setIcon($badge['icon'])
+          ->setQuality($badge['quality'])
+          ->setHeader($badge['name'])
           ->setTipDirection('E')
-          ->setHref('/badges/view/'.$badge->getID());
-
+          ->setHref('/badges/view/'.$badge['id'].'/');
         $event->addBadge($badge_view);
       }
     }

From 2b0ad243d179f81baad24ccd9748a4ba59017d25 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 14 Mar 2017 14:56:21 -0700
Subject: [PATCH 13/15] Use "git ls-remote" to guess if "git fetch" is a no-op

Summary:
Ref T12296. Ref T12392. Currently, when we're observing a remote repository, we periodically run `git fetch ...`.

Instead, periodically run `git ls-remote` (to list refs in the remote) and `git for-each-ref` (to list local refs) and only continue if the two lists are different.

The motivations for this are:

  - In T12296, it appears that doing this is //faster// than doing a no-op `git fetch`. This effect seems to reproduce locally in a clean environment (900ms for `ls-remote` + 100ms for `for-each-ref` vs about 1.4s for `fetch`). I don't have any explanation for why this is, but there it is. This isn't a huge change, although the time we're saving does appear to mostly be local CPU time, which is good for us.
  - Because we control all writes, we could cache `git for-each-ref` in the future and do fewer disk operations. This doesn't necessarily seem too valuable, though.
  - This allows us to tell if a fetch will do anything or not, and make better decisions around clustering (in particular, simplify how observed repository versioning works). With `git fetch`, we can't easily distinguish between "fetch, but nothing changed" and "legitimate fetch".

If a repository updates very regularly we end up doing slightly more work this way (that is, if `ls-remote` always comes back with changes, we do a little extra work), but this is normally very rare.

This might not get non-bare repositories quite right in some cases (i.e., incorrectly detect them as changed when they are unchanged) but we haven't created non-bare repositories for many years.

Test Plan: Ran `bin/repository update --trace --verbose PHABX`, saw sensible construction of local and remote maps and accurate detection of whether a fetch would do anything or not.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12392, T12296

Differential Revision: https://secure.phabricator.com/D17497
---
 .../PhabricatorRepositoryPullEngine.php       | 88 ++++++++++++++++++-
 1 file changed, 85 insertions(+), 3 deletions(-)

diff --git a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php
index 610bbbfc55..8e6b61d759 100644
--- a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php
+++ b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php
@@ -120,6 +120,7 @@ final class PhabricatorRepositoryPullEngine
           pht(
             'Updating the working copy for repository "%s".',
             $repository->getDisplayName()));
+
         if ($is_git) {
           $this->verifyGitOrigin($repository);
           $this->executeGitUpdate();
@@ -157,7 +158,7 @@ final class PhabricatorRepositoryPullEngine
   }
 
   private function skipPull($message) {
-    $this->log('%s', $message);
+    $this->log($message);
     $this->donePull();
   }
 
@@ -172,7 +173,7 @@ final class PhabricatorRepositoryPullEngine
   }
 
   private function logPull($message) {
-    $this->log('%s', $message);
+    $this->log($message);
   }
 
   private function donePull() {
@@ -190,7 +191,7 @@ final class PhabricatorRepositoryPullEngine
   }
 
   private function installHook($path, array $hook_argv = array()) {
-    $this->log('%s', pht('Installing commit hook to "%s"...', $path));
+    $this->log(pht('Installing commit hook to "%s"...', $path));
 
     $repository = $this->getRepository();
     $identifier = $this->getHookContextIdentifier($repository);
@@ -339,6 +340,18 @@ final class PhabricatorRepositoryPullEngine
       throw new Exception($message);
     }
 
+    $remote_refs = $this->loadGitRemoteRefs($repository);
+    $local_refs = $this->loadGitLocalRefs($repository);
+    if ($remote_refs === $local_refs) {
+      $this->log(
+        pht(
+          'Skipping fetch because local and remote refs are already '.
+          'identical.'));
+      return false;
+    }
+
+    $this->logRefDifferences($remote_refs, $local_refs);
+
     $retry = false;
     do {
       // This is a local command, but needs credentials.
@@ -396,6 +409,75 @@ final class PhabricatorRepositoryPullEngine
     $this->installHook($root.$path);
   }
 
+  private function loadGitRemoteRefs(PhabricatorRepository $repository) {
+    $remote_envelope = $repository->getRemoteURIEnvelope();
+
+    list($stdout) = $repository->execxRemoteCommand(
+      'ls-remote -- %P',
+      $remote_envelope);
+
+    $map = array();
+    $lines = phutil_split_lines($stdout, false);
+    foreach ($lines as $line) {
+      list($hash, $name) = preg_split('/\s+/', $line, 2);
+
+      // If the remote has a HEAD, just ignore it.
+      if ($name == 'HEAD') {
+        continue;
+      }
+
+      // If the remote ref is itself a remote ref, ignore it.
+      if (preg_match('(^refs/remotes/)', $name)) {
+        continue;
+      }
+
+      $map[$name] = $hash;
+    }
+
+    ksort($map);
+
+    return $map;
+  }
+
+  private function loadGitLocalRefs(PhabricatorRepository $repository) {
+    $refs = id(new DiffusionLowLevelGitRefQuery())
+      ->setRepository($repository)
+      ->execute();
+
+    $map = array();
+    foreach ($refs as $ref) {
+      $fields = $ref->getRawFields();
+      $map[idx($fields, 'refname')] = $ref->getCommitIdentifier();
+    }
+
+    ksort($map);
+
+    return $map;
+  }
+
+  private function logRefDifferences(array $remote, array $local) {
+    $all = $local + $remote;
+
+    $differences = array();
+    foreach ($all as $key => $ignored) {
+      $remote_ref = idx($remote, $key, pht('<null>'));
+      $local_ref = idx($local, $key, pht('<null>'));
+      if ($remote_ref !== $local_ref) {
+        $differences[] = pht(
+          '%s (remote: "%s", local: "%s")',
+          $key,
+          $remote_ref,
+          $local_ref);
+      }
+    }
+
+    $this->log(
+      pht(
+        "Updating repository after detecting ref differences:\n%s",
+        implode("\n", $differences)));
+  }
+
+
 
 /* -(  Pulling Mercurial Working Copies  )----------------------------------- */
 

From 20892ae502363c30316689132b02af8d09855359 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 14 Mar 2017 15:32:31 -0700
Subject: [PATCH 14/15] Simplify "git fetch" behavior in the Pull daemon

Summary:
Ref T12392. The logic currently goes like this:

  - Try a fetch.
  - If that fails, try repairing the origin URI.
  - Then try again.

This is pretty complicated, and we can use this simpler logic instead:

  - Set the origin URI to the right value.
  - Try a fetch.

Setting the origin URI is very fast. This can normally only get us in any trouble in very obscure situations which haven't occurred for many years:

  - Pretty much all of this is already covered by `verifyGitOrigin()`, which we run earlier.
  - Origins could be configured to have multiple URIs for some reason, but shouldn't be.
  - Years ago, you could configure Phabricator to point at a local repository it didn't own and that could conceivably have a different "origin" that you might not want us to delete. If you did this, the daemons have been spewing errors for 3-4 years without you fixing it. The cost of fixing the remote URI is very small even if anyone is affected by this (just set it back to the old value) and there's zero reason to do this and the scenario is ridiculous.

Test Plan: Ran `bin/repository update PHABX --trace --verbose`, saw fetches go through cleanly after URI adjustment.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12392

Differential Revision: https://secure.phabricator.com/D17498
---
 .../PhabricatorRepositoryPullEngine.php       | 52 ++++++-------------
 1 file changed, 16 insertions(+), 36 deletions(-)

diff --git a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php
index 8e6b61d759..1219e678d5 100644
--- a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php
+++ b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php
@@ -352,44 +352,24 @@ final class PhabricatorRepositoryPullEngine
 
     $this->logRefDifferences($remote_refs, $local_refs);
 
-    $retry = false;
-    do {
-      // This is a local command, but needs credentials.
-      if ($repository->isWorkingCopyBare()) {
-        // For bare working copies, we need this magic incantation.
-        $future = $repository->getRemoteCommandFuture(
-          'fetch origin %s --prune',
-          '+refs/*:refs/*');
-      } else {
-        $future = $repository->getRemoteCommandFuture(
-          'fetch --all --prune');
-      }
+    // Force the "origin" URI to the configured value.
+    $repository->execxLocalCommand(
+      'remote set-url origin -- %P',
+      $repository->getRemoteURIEnvelope());
 
-      $future->setCWD($path);
-      list($err, $stdout, $stderr) = $future->resolve();
+    if ($repository->isWorkingCopyBare()) {
+      // For bare working copies, we need this magic incantation.
+      $future = $repository->getRemoteCommandFuture(
+        'fetch origin %s --prune',
+        '+refs/*:refs/*');
+    } else {
+      $future = $repository->getRemoteCommandFuture(
+        'fetch --all --prune');
+    }
 
-      if ($err && !$retry && $repository->canDestroyWorkingCopy()) {
-        $retry = true;
-        // Fix remote origin url if it doesn't match our configuration
-        $origin_url = $repository->execLocalCommand(
-          'config --get remote.origin.url');
-        $remote_uri = $repository->getRemoteURIEnvelope();
-        if ($origin_url != $remote_uri->openEnvelope()) {
-          $repository->execLocalCommand(
-            'remote set-url origin %P',
-            $remote_uri);
-        }
-      } else if ($err) {
-        throw new CommandException(
-          pht('Failed to fetch changes!'),
-          $future->getCommand(),
-          $err,
-          $stdout,
-          $stderr);
-      } else {
-        $retry = false;
-      }
-    } while ($retry);
+    $future
+      ->setCWD($path)
+      ->resolvex();
   }
 
 

From 688c120f9f33eaab5ad54647ff4fdde405cf983a Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 15 Mar 2017 18:49:03 -0700
Subject: [PATCH 15/15] Provide PhabricatorEnv::isSelfURI to test if a URI
 points at the current install

Summary:
Ref T5378. This repackages an existing check to see if a URI is a URI for the current install into a more reasonable form.

In an upcoming change, I'll use this new check to test whether `http://example.whatever.com/T123` is a link to a task on the current install or not.

Test Plan: This stuff has good test coverage already; added some more.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T5378

Differential Revision: https://secure.phabricator.com/D17502
---
 ...fferentialRevisionIDCommitMessageField.php | 22 ++--------
 ...DifferentialCommitMessageFieldTestCase.php |  1 +
 src/infrastructure/env/PhabricatorEnv.php     | 43 ++++++++++++++-----
 .../env/__tests__/PhabricatorEnvTestCase.php  | 35 +++++++++++++++
 4 files changed, 73 insertions(+), 28 deletions(-)

diff --git a/src/applications/differential/field/DifferentialRevisionIDCommitMessageField.php b/src/applications/differential/field/DifferentialRevisionIDCommitMessageField.php
index 8c85553e71..ac8ba4ebd4 100644
--- a/src/applications/differential/field/DifferentialRevisionIDCommitMessageField.php
+++ b/src/applications/differential/field/DifferentialRevisionIDCommitMessageField.php
@@ -50,24 +50,10 @@ final class DifferentialRevisionIDCommitMessageField
     $uri = new PhutilURI($uri_string);
     $path = $uri->getPath();
 
-    $matches = null;
-    if (preg_match('#^/D(\d+)$#', $path, $matches)) {
-      $id = (int)$matches[1];
-
-      $prod_uri = new PhutilURI(PhabricatorEnv::getProductionURI('/D'.$id));
-
-      // Make sure the URI is the same as our URI. Basically, we want to ignore
-      // commits from other Phabricator installs.
-      if ($uri->getDomain() == $prod_uri->getDomain()) {
-        return $id;
-      }
-
-      $allowed_uris = PhabricatorEnv::getAllowedURIs('/D'.$id);
-
-      foreach ($allowed_uris as $allowed_uri) {
-        if ($uri_string == $allowed_uri) {
-          return $id;
-        }
+    if (PhabricatorEnv::isSelfURI($uri_string)) {
+      $matches = null;
+      if (preg_match('#^/D(\d+)$#', $path, $matches)) {
+        return (int)$matches[1];
       }
     }
 
diff --git a/src/applications/differential/field/__tests__/DifferentialCommitMessageFieldTestCase.php b/src/applications/differential/field/__tests__/DifferentialCommitMessageFieldTestCase.php
index 295da8ca59..41fd2992f4 100644
--- a/src/applications/differential/field/__tests__/DifferentialCommitMessageFieldTestCase.php
+++ b/src/applications/differential/field/__tests__/DifferentialCommitMessageFieldTestCase.php
@@ -13,6 +13,7 @@ final class DifferentialCommitMessageFieldTestCase
       "D123\nSome-Custom-Field: The End" => 123,
       "{$base_uri}D123" => 123,
       "{$base_uri}D123\nSome-Custom-Field: The End" => 123,
+      'https://www.other.com/D123' => null,
     );
 
     $env = PhabricatorEnv::beginScopedEnv();
diff --git a/src/infrastructure/env/PhabricatorEnv.php b/src/infrastructure/env/PhabricatorEnv.php
index b50c85c0f0..70ddb80630 100644
--- a/src/infrastructure/env/PhabricatorEnv.php
+++ b/src/infrastructure/env/PhabricatorEnv.php
@@ -406,21 +406,44 @@ final class PhabricatorEnv extends Phobject {
     return rtrim($production_domain, '/').$path;
   }
 
-  public static function getAllowedURIs($path) {
-    $uri = new PhutilURI($path);
-    if ($uri->getDomain()) {
-      return $path;
+
+  public static function isSelfURI($raw_uri) {
+    $uri = new PhutilURI($raw_uri);
+
+    $host = $uri->getDomain();
+    if (!strlen($host)) {
+      return false;
     }
 
-    $allowed_uris = self::getEnvConfig('phabricator.allowed-uris');
-    $return = array();
-    foreach ($allowed_uris as $allowed_uri) {
-      $return[] = rtrim($allowed_uri, '/').$path;
-    }
+    $host = phutil_utf8_strtolower($host);
 
-    return $return;
+    $self_map = self::getSelfURIMap();
+    return isset($self_map[$host]);
   }
 
+  private static function getSelfURIMap() {
+    $self_uris = array();
+    $self_uris[] = self::getProductionURI('/');
+    $self_uris[] = self::getURI('/');
+
+    $allowed_uris = self::getEnvConfig('phabricator.allowed-uris');
+    foreach ($allowed_uris as $allowed_uri) {
+      $self_uris[] = $allowed_uri;
+    }
+
+    $self_map = array();
+    foreach ($self_uris as $self_uri) {
+      $host = id(new PhutilURI($self_uri))->getDomain();
+      if (!strlen($host)) {
+        continue;
+      }
+
+      $host = phutil_utf8_strtolower($host);
+      $self_map[$host] = $host;
+    }
+
+    return $self_map;
+  }
 
   /**
    * Get the fully-qualified production URI for a static resource path.
diff --git a/src/infrastructure/env/__tests__/PhabricatorEnvTestCase.php b/src/infrastructure/env/__tests__/PhabricatorEnvTestCase.php
index d47258822b..f73299aa12 100644
--- a/src/infrastructure/env/__tests__/PhabricatorEnvTestCase.php
+++ b/src/infrastructure/env/__tests__/PhabricatorEnvTestCase.php
@@ -218,4 +218,39 @@ final class PhabricatorEnvTestCase extends PhabricatorTestCase {
     $this->assertFalse($caught instanceof Exception);
   }
 
+  public function testSelfURI() {
+    $base_uri = 'https://allowed.example.com/';
+
+    $allowed_uris = array(
+      'https://old.example.com/',
+    );
+
+    $env = PhabricatorEnv::beginScopedEnv();
+    $env->overrideEnvConfig('phabricator.base-uri', $base_uri);
+    $env->overrideEnvConfig('phabricator.allowed-uris', $allowed_uris);
+
+    $map = array(
+      'https://allowed.example.com/' => true,
+      'https://allowed.example.com' => true,
+      'https://allowed.EXAMPLE.com' => true,
+      'http://allowed.example.com/' => true,
+      'https://allowed.example.com/path/to/resource.png' => true,
+
+      'https://old.example.com/' => true,
+      'https://old.example.com' => true,
+      'https://old.EXAMPLE.com' => true,
+      'http://old.example.com/' => true,
+      'https://old.example.com/path/to/resource.png' => true,
+
+      'https://other.example.com/' => false,
+    );
+
+    foreach ($map as $input => $expect) {
+      $this->assertEqual(
+        $expect,
+        PhabricatorEnv::isSelfURI($input),
+        pht('Is self URI? %s', $input));
+    }
+  }
+
 }