De-garbage the horrible garbage project section of the policy selection control

Summary:
Fixes T4136.

When listing projects in the "Visible To" selector control:

  - Instead of showing every project you are a member of, show only a few.
  - Add an option to choose something else which isn't in the menu.
  - If you've used the control before, show the stuff you've selected in the recent past.
  - If you haven't used the control before or haven't used it much, show the stuff you've picked and them some filler.
  - Don't offer milestones.
  - Also don't offer milestones in the custom policy UI.

Test Plan:
{F1091999}

{F1092000}

  - Selected a project.
  - Used "find" to select a different project.
  - Saw reasonable defaults.
  - Saw favorites stick.
  - Tried to typeahead a milestone (nope).
  - Used "Custom Policy", tried to typeahead a milestone (nope).
  - Used "Custom Policy" in general.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4136

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

src/applications/policy/controller/PhabricatorPolicyEditController.php

@@ -6,7 +6,6 @@ final class PhabricatorPolicyEditController
   public function handleRequest(AphrontRequest $request) {
     $viewer = $this->getViewer();
 
-
     $object_phid = $request->getURIData('objectPHID');
     if ($object_phid) {
       $object = id(new PhabricatorObjectQuery())
@@ -32,6 +31,17 @@ final class PhabricatorPolicyEditController
       }
     }
 
+    $phid = $request->getURIData('phid');
+    switch ($phid) {
+      case AphrontFormPolicyControl::getSelectProjectKey():
+        return $this->handleProjectRequest($request);
+      case AphrontFormPolicyControl::getSelectCustomKey():
+        $phid = null;
+        break;
+      default:
+        break;
+    }
+
     $action_options = array(
       PhabricatorPolicy::ACTION_ALLOW => pht('Allow'),
       PhabricatorPolicy::ACTION_DENY => pht('Deny'),
@@ -55,7 +65,6 @@ final class PhabricatorPolicyEditController
       'value' => null,
     );
 
-    $phid = $request->getURIData('phid');
     if ($phid) {
       $policies = id(new PhabricatorPolicyQuery())
         ->setViewer($viewer)
@@ -253,4 +262,79 @@ final class PhabricatorPolicyEditController
     return id(new AphrontDialogResponse())->setDialog($dialog);
   }
 
+  private function handleProjectRequest(AphrontRequest $request) {
+    $viewer = $this->getViewer();
+
+    $errors = array();
+    $e_project = true;
+
+    if ($request->isFormPost()) {
+      $project_phids = $request->getArr('projectPHIDs');
+      $project_phid = head($project_phids);
+
+      $project = id(new PhabricatorObjectQuery())
+        ->setViewer($viewer)
+        ->withPHIDs(array($project_phid))
+        ->executeOne();
+
+      if ($project) {
+        // Save this project as one of the user's most recently used projects,
+        // so we'll show it by default in future menus.
+
+        $pref_key = PhabricatorUserPreferences::PREFERENCE_FAVORITE_POLICIES;
+
+        $preferences = $viewer->loadPreferences();
+        $favorites = $preferences->getPreference($pref_key);
+        if (!is_array($favorites)) {
+          $favorites = array();
+        }
+
+        // Add this, or move it to the end of the list.
+        unset($favorites[$project_phid]);
+        $favorites[$project_phid] = true;
+
+        $preferences->setPreference($pref_key, $favorites);
+        $preferences->save();
+
+        $data = array(
+          'phid' => $project->getPHID(),
+          'info' => array(
+            'name' => $project->getName(),
+            'full' => $project->getName(),
+            'icon' => $project->getDisplayIconIcon(),
+          ),
+        );
+
+        return id(new AphrontAjaxResponse())->setContent($data);
+      } else {
+        $errors[] = pht('You must choose a project.');
+        $e_project = pht('Required');
+      }
+    }
+
+    $project_datasource = id(new PhabricatorProjectDatasource())
+      ->setParameters(
+        array(
+          'policy' => 1,
+        ));
+
+    $form = id(new AphrontFormView())
+      ->setUser($viewer)
+      ->appendControl(
+        id(new AphrontFormTokenizerControl())
+          ->setLabel(pht('Members Of'))
+          ->setName('projectPHIDs')
+          ->setLimit(1)
+          ->setError($e_project)
+          ->setDatasource($project_datasource));
+
+    return $this->newDialog()
+      ->setWidth(AphrontDialogView::WIDTH_FORM)
+      ->setErrors($errors)
+      ->setTitle(pht('Select Project'))
+      ->appendForm($form)
+      ->addSubmitButton(pht('Done'))
+      ->addCancelButton('#');
+  }
+
 }

src/applications/policy/query/PhabricatorPolicyQuery.php

@@ -195,10 +195,48 @@ final class PhabricatorPolicyQuery
     $viewer = $this->getViewer();
 
     if ($viewer->getPHID()) {
+      $pref_key = PhabricatorUserPreferences::PREFERENCE_FAVORITE_POLICIES;
+
+      $favorite_limit = 10;
+      $default_limit = 5;
+
+      // If possible, show the user's 10 most recently used projects.
+      $preferences = $viewer->loadPreferences();
+      $favorites = $preferences->getPreference($pref_key);
+      if (!is_array($favorites)) {
+        $favorites = array();
+      }
+      $favorite_phids = array_keys($favorites);
+      $favorite_phids = array_slice($favorite_phids, -$favorite_limit);
+
+      if ($favorite_phids) {
         $projects = id(new PhabricatorProjectQuery())
           ->setViewer($viewer)
-        ->withMemberPHIDs(array($viewer->getPHID()))
+          ->withPHIDs($favorite_phids)
+          ->withIsMilestone(false)
+          ->setLimit($favorite_limit)
           ->execute();
+        $projects = mpull($projects, null, 'getPHID');
+      } else {
+        $projects = array();
+      }
+
+      // If we didn't find enough favorites, add some default projects. These
+      // are just arbitrary projects that the viewer is a member of, but may
+      // be useful on smaller installs and for new users until they can use
+      // the control enough time to establish useful favorites.
+      if (count($projects) < $default_limit) {
+        $default_projects = id(new PhabricatorProjectQuery())
+          ->setViewer($viewer)
+          ->withMemberPHIDs(array($viewer->getPHID()))
+          ->withIsMilestone(false)
+          ->setLimit($default_limit)
+          ->execute();
+        $default_projects = mpull($default_projects, null, 'getPHID');
+        $projects = $projects + $default_projects;
+        $projects = array_slice($projects, 0, $default_limit);
+      }
+
       foreach ($projects as $project) {
         $phids[] = $project->getPHID();
       }

src/applications/project/policyrule/PhabricatorProjectsPolicyRule.php

@@ -48,7 +48,13 @@ final class PhabricatorProjectsPolicyRule
   }
 
   public function getValueControlTemplate() {
-    return $this->getDatasourceTemplate(new PhabricatorProjectDatasource());
+    $datasource = id(new PhabricatorProjectDatasource())
+      ->setParameters(
+        array(
+          'policy' => 1,
+        ));
+
+    return $this->getDatasourceTemplate($datasource);
   }
 
   public function getRuleOrder() {

src/applications/project/typeahead/PhabricatorProjectDatasource.php

@@ -32,6 +32,12 @@ final class PhabricatorProjectDatasource
       $query->withNameTokens($tokens);
     }
 
+    // If this is for policy selection, prevent users from using milestones.
+    $for_policy = $this->getParameter('policy');
+    if ($for_policy) {
+      $query->withIsMilestone(false);
+    }
+
     $projs = $this->executeQuery($query);
 
     $projs = mpull($projs, null, 'getPHID');

src/applications/settings/storage/PhabricatorUserPreferences.php

@@ -42,6 +42,7 @@ final class PhabricatorUserPreferences extends PhabricatorUserDAO {
   const PREFERENCE_DESKTOP_NOTIFICATIONS = 'desktop-notifications';
 
   const PREFERENCE_PROFILE_MENU_COLLAPSED = 'profile-menu.collapsed';
+  const PREFERENCE_FAVORITE_POLICIES = 'policy.favorites';
 
   // These are in an unusual order for historic reasons.
   const MAILTAG_PREFERENCE_NOTIFY       = 0;

src/view/form/control/AphrontFormPolicyControl.php

@@ -103,6 +103,24 @@ final class AphrontFormPolicyControl extends AphrontFormControl {
   protected function getOptions() {
     $capability = $this->capability;
     $policies = $this->policies;
+    $viewer = $this->getUser();
+
+    // Check if we're missing the policy for the current control value. This
+    // is unusual, but can occur if the user is submitting a form and selected
+    // an unusual project as a policy but the change has not been saved yet.
+    $policy_map = mpull($policies, null, 'getPHID');
+    $value = $this->getValue();
+    if ($value && empty($policy_map[$value])) {
+      $handle = id(new PhabricatorHandleQuery())
+        ->setViewer($viewer)
+        ->withPHIDs(array($value))
+        ->executeOne();
+      if ($handle->isComplete()) {
+        $policies[] = PhabricatorPolicy::newFromPolicyAndHandle(
+          $value,
+          $handle);
+      }
+    }
 
     // Exclude object policies which don't make sense here. This primarily
     // filters object policies associated from template capabilities (like
@@ -143,12 +161,27 @@ final class AphrontFormPolicyControl extends AphrontFormControl {
         'name' => $policy_short_name,
         'full' => $policy->getName(),
         'icon' => $policy->getIcon(),
+        'sort' => phutil_utf8_strtolower($policy->getName()),
       );
     }
 
+    $type_project = PhabricatorPolicyType::TYPE_PROJECT;
+
+    $placeholder = id(new PhabricatorPolicy())
+      ->setName(pht('Other Project...'))
+      ->setIcon('fa-search');
+
+    $options[$type_project] = isort($options[$type_project], 'sort');
+
+    $options[$type_project][$this->getSelectProjectKey()] = array(
+      'name' => $placeholder->getName(),
+      'full' => $placeholder->getName(),
+      'icon' => $placeholder->getIcon(),
+    );
+
     // If we were passed several custom policy options, throw away the ones
     // which aren't the value for this capability. For example, an object might
-    // have a custom view pollicy and a custom edit policy. When we render
+    // have a custom view policy and a custom edit policy. When we render
     // the selector for "Can View", we don't want to show the "Can Edit"
     // custom policy -- if we did, the menu would look like this:
     //
@@ -172,7 +205,7 @@ final class AphrontFormPolicyControl extends AphrontFormControl {
     if (empty($options[$type_custom])) {
       $placeholder = new PhabricatorPolicy();
       $placeholder->setName(pht('Custom Policy...'));
-      $options[$type_custom][$this->getCustomPolicyPlaceholder()] = array(
+      $options[$type_custom][$this->getSelectCustomKey()] = array(
         'name' => $placeholder->getName(),
         'full' => $placeholder->getName(),
         'icon' => $placeholder->getIcon(),
@@ -266,12 +299,12 @@ final class AphrontFormPolicyControl extends AphrontFormControl {
         'options' => $flat_options,
         'groups' => array_keys($options),
         'order' => $order,
-        'icons' => $icons,
         'labels' => $labels,
         'value' => $this->getValue(),
         'capability' => $this->capability,
         'editURI' => '/policy/edit/'.$context_path,
-        'customPlaceholder' => $this->getCustomPolicyPlaceholder(),
+        'customKey' => $this->getSelectCustomKey(),
+        'projectKey' => $this->getSelectProjectKey(),
         'disabled' => $this->getDisabled(),
       ));
 
@@ -322,8 +355,12 @@ final class AphrontFormPolicyControl extends AphrontFormControl {
       ));
   }
 
-  private function getCustomPolicyPlaceholder() {
+  public static function getSelectCustomKey() {
-    return 'custom:placeholder';
+    return 'select:custom';
+  }
+
+  public static function getSelectProjectKey() {
+    return 'select:project';
   }
 
   private function buildSpacesControl() {

webroot/rsrc/js/application/policy/behavior-policy-control.js

@@ -7,6 +7,7 @@
  *           phuix-action-list-view
  *           phuix-action-view
  *           javelin-workflow
+ *           phuix-icon-view
  * @javelin
  */
 JX.behavior('policy-control', function(config) {
@@ -57,6 +58,21 @@ JX.behavior('policy-control', function(config) {
               .start();
 
           }, phid);
+        } else if (phid == config.projectKey) {
+          onselect = JX.bind(null, function(phid) {
+            var uri = get_custom_uri(phid, config.capability);
+
+            new JX.Workflow(uri)
+              .setHandler(function(response) {
+                if (!response.phid) {
+                  return;
+                }
+
+                add_policy(phid, response.phid, response.info);
+                select_policy(response.phid);
+              })
+              .start();
+          }, phid);
         } else {
           onselect = JX.bind(null, select_policy, phid);
         }
@@ -101,20 +117,22 @@ JX.behavior('policy-control', function(config) {
       name = JX.$N('span', {title: option.full}, name);
     }
 
-    return [JX.$H(config.icons[option.icon]), name];
+    return [render_icon(option.icon), name];
   };
 
+  var render_icon = function(icon) {
+    return new JX.PHUIXIconView()
+      .setIcon(icon)
+      .getNode();
+  };
 
   /**
    * Get the workflow URI to create or edit a policy with a given PHID.
    */
   var get_custom_uri = function(phid, capability) {
-    var uri = config.editURI;
+    return JX.$U(config.editURI + phid + '/')
-    if (phid != config.customPlaceholder) {
+      .setQueryParam('capability', capability)
-      uri += phid + '/';
+      .toString();
-    }
-    uri += '?capability=' + capability;
-    return uri;
   };
 
 
@@ -123,16 +141,28 @@ JX.behavior('policy-control', function(config) {
    * policies after the user edits them.
    */
   var replace_policy = function(old_phid, new_phid, info) {
+    return add_policy(old_phid, new_phid, info, true);
+  };
+
+
+  /**
+   * Add a new policy above an existing one, optionally replacing it.
+   */
+  var add_policy = function(old_phid, new_phid, info, replace) {
+    if (config.options[new_phid]) {
+      return;
+    }
+
     config.options[new_phid] = info;
+
     for (var k in config.order) {
       for (var ii = 0; ii < config.order[k].length; ii++) {
         if (config.order[k][ii] == old_phid) {
-          config.order[k][ii] = new_phid;
+          config.order[k].splice(ii, (replace ? 1 : 0), new_phid);
           return;
         }
       }
     }
   };
 
-
 });