Upgrade user account activity logs to modern construction

Summary: Depends on D18965. Ref T13049. Move this Query and SearchEngine to be a little more modern, to prepare for Export support.

Test Plan:
  - Used all the query fields, viewed activity logs via People and Settings.
  - I'm not sure the "Session" query is used/useful and may remove it before I'm done here, but I just left it in place for now.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

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

src/applications/people/query/PhabricatorPeopleLogQuery.php

@@ -40,70 +40,61 @@ final class PhabricatorPeopleLogQuery
     return $this;
   }
 
-  protected function loadPage() {
+  public function newResultObject() {
-    $table  = new PhabricatorUserLog();
+    return new PhabricatorUserLog();
-    $conn_r = $table->establishConnection('r');
-
-    $data = queryfx_all(
-      $conn_r,
-      'SELECT * FROM %T %Q %Q %Q',
-      $table->getTableName(),
-      $this->buildWhereClause($conn_r),
-      $this->buildOrderClause($conn_r),
-      $this->buildLimitClause($conn_r));
-
-    return $table->loadAllFromArray($data);
   }
 
-  protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
+  protected function loadPage() {
-    $where = array();
+    return $this->loadStandardPage($this->newResultObject());
+  }
+
+  protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
+    $where = parent::buildWhereClauseParts($conn);
 
     if ($this->actorPHIDs !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'actorPHID IN (%Ls)',
         $this->actorPHIDs);
     }
 
     if ($this->userPHIDs !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'userPHID IN (%Ls)',
         $this->userPHIDs);
     }
 
     if ($this->relatedPHIDs !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
-        'actorPHID IN (%Ls) OR userPHID IN (%Ls)',
+        '(actorPHID IN (%Ls) OR userPHID IN (%Ls))',
         $this->relatedPHIDs,
         $this->relatedPHIDs);
     }
 
     if ($this->sessionKeys !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'session IN (%Ls)',
         $this->sessionKeys);
     }
 
     if ($this->actions !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'action IN (%Ls)',
         $this->actions);
     }
 
     if ($this->remoteAddressPrefix !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'remoteAddr LIKE %>',
         $this->remoteAddressPrefix);
     }
 
-    $where[] = $this->buildPagingClause($conn_r);
+    return $where;
-
-    return $this->formatWhereClause($where);
   }
 
   public function getQueryApplicationClass() {

src/applications/people/query/PhabricatorPeopleLogSearchEngine.php

@@ -15,34 +15,8 @@ final class PhabricatorPeopleLogSearchEngine
     return 500;
   }
 
-  public function buildSavedQueryFromRequest(AphrontRequest $request) {
+  public function newQuery() {
-    $saved = new PhabricatorSavedQuery();
+    $query = new PhabricatorPeopleLogQuery();
-
-    $saved->setParameter(
-      'userPHIDs',
-      $this->readUsersFromRequest($request, 'users'));
-
-    $saved->setParameter(
-      'actorPHIDs',
-      $this->readUsersFromRequest($request, 'actors'));
-
-    $saved->setParameter(
-      'actions',
-      $this->readListFromRequest($request, 'actions'));
-
-    $saved->setParameter(
-      'ip',
-      $request->getStr('ip'));
-
-    $saved->setParameter(
-      'sessions',
-      $this->readListFromRequest($request, 'sessions'));
-
-    return $saved;
-  }
-
-  public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) {
-    $query = id(new PhabricatorPeopleLogQuery());
 
     // NOTE: If the viewer isn't an administrator, always restrict the query to
     // related records. This echoes the policy logic of these logs. This is
@@ -54,82 +28,61 @@ final class PhabricatorPeopleLogSearchEngine
       $query->withRelatedPHIDs(array($viewer->getPHID()));
     }
 
-    $actor_phids = $saved->getParameter('actorPHIDs', array());
+    return $query;
-    if ($actor_phids) {
-      $query->withActorPHIDs($actor_phids);
   }
 
-    $user_phids = $saved->getParameter('userPHIDs', array());
+  protected function buildQueryFromParameters(array $map) {
-    if ($user_phids) {
+    $query = $this->newQuery();
-      $query->withUserPHIDs($user_phids);
+
+    if ($map['userPHIDs']) {
+      $query->withUserPHIDs($map['userPHIDs']);
     }
 
-    $actions = $saved->getParameter('actions', array());
+    if ($map['actorPHIDs']) {
-    if ($actions) {
+      $query->withActorPHIDs($map['actorPHIDs']);
-      $query->withActions($actions);
     }
 
-    $remote_prefix = $saved->getParameter('ip');
+    if ($map['actions']) {
-    if (strlen($remote_prefix)) {
+      $query->withActions($map['actions']);
-      $query->withRemoteAddressprefix($remote_prefix);
     }
 
-    $sessions = $saved->getParameter('sessions', array());
+    if (strlen($map['ip'])) {
-    if ($sessions) {
+      $query->withRemoteAddressPrefix($map['ip']);
-      $query->withSessionKeys($sessions);
+    }
+
+    if ($map['sessions']) {
+      $query->withSessionKeys($map['sessions']);
     }
 
     return $query;
   }
 
-  public function buildSearchForm(
+  protected function buildCustomSearchFields() {
-    AphrontFormView $form,
+    return array(
-    PhabricatorSavedQuery $saved) {
+      id(new PhabricatorUsersSearchField())
-
+        ->setKey('userPHIDs')
-    $actor_phids = $saved->getParameter('actorPHIDs', array());
+        ->setAliases(array('users', 'user', 'userPHID'))
-    $user_phids = $saved->getParameter('userPHIDs', array());
-
-    $actions = $saved->getParameter('actions', array());
-    $remote_prefix = $saved->getParameter('ip');
-    $sessions = $saved->getParameter('sessions', array());
-
-    $actions = array_fuse($actions);
-    $action_control = id(new AphrontFormCheckboxControl())
-      ->setLabel(pht('Actions'));
-    $action_types = PhabricatorUserLog::getActionTypeMap();
-    foreach ($action_types as $type => $label) {
-      $action_control->addCheckbox(
-        'actions[]',
-        $type,
-        $label,
-        isset($actions[$label]));
-    }
-
-    $form
-      ->appendControl(
-        id(new AphrontFormTokenizerControl())
-          ->setDatasource(new PhabricatorPeopleDatasource())
-          ->setName('actors')
-          ->setLabel(pht('Actors'))
-          ->setValue($actor_phids))
-      ->appendControl(
-        id(new AphrontFormTokenizerControl())
-          ->setDatasource(new PhabricatorPeopleDatasource())
-          ->setName('users')
         ->setLabel(pht('Users'))
-          ->setValue($user_phids))
+        ->setDescription(pht('Search for activity affecting specific users.')),
-      ->appendChild($action_control)
+      id(new PhabricatorUsersSearchField())
-      ->appendChild(
+        ->setKey('actorPHIDs')
-        id(new AphrontFormTextControl())
+        ->setAliases(array('actors', 'actor', 'actorPHID'))
+        ->setLabel(pht('Actors'))
+        ->setDescription(pht('Search for activity by specific users.')),
+      id(new PhabricatorSearchCheckboxesField())
+        ->setKey('actions')
+        ->setLabel(pht('Actions'))
+        ->setDescription(pht('Search for particular types of activity.'))
+        ->setOptions(PhabricatorUserLog::getActionTypeMap()),
+      id(new PhabricatorSearchTextField())
+        ->setKey('ip')
         ->setLabel(pht('Filter IP'))
-          ->setName('ip')
+        ->setDescription(pht('Search for actions by remote address.')),
-          ->setValue($remote_prefix))
+      id(new PhabricatorSearchStringListField())
-      ->appendChild(
+        ->setKey('sessions')
-        id(new AphrontFormTextControl())
         ->setLabel(pht('Sessions'))
-          ->setName('sessions')
+        ->setDescription(pht('Search for activity in particular sessions.')),
-          ->setValue(implode(', ', $sessions)));
+    );
-
   }
 
   protected function getURI($path) {
@@ -156,19 +109,6 @@ final class PhabricatorPeopleLogSearchEngine
     return parent::buildSavedQueryFromBuiltin($query_key);
   }
 
-  protected function getRequiredHandlePHIDsForResultList(
-    array $logs,
-    PhabricatorSavedQuery $query) {
-
-    $phids = array();
-    foreach ($logs as $log) {
-      $phids[$log->getActorPHID()] = true;
-      $phids[$log->getUserPHID()] = true;
-    }
-
-    return array_keys($phids);
-  }
-
   protected function renderResultList(
     array $logs,
     PhabricatorSavedQuery $query,
@@ -179,16 +119,13 @@ final class PhabricatorPeopleLogSearchEngine
 
     $table = id(new PhabricatorUserLogView())
       ->setUser($viewer)
-      ->setLogs($logs)
+      ->setLogs($logs);
-      ->setHandles($handles);
 
     if ($viewer->getIsAdmin()) {
       $table->setSearchBaseURI($this->getApplicationURI('logs/'));
     }
 
-    $result = new PhabricatorApplicationSearchResultView();
+    return id(new PhabricatorApplicationSearchResultView())
-    $result->setTable($table);
+      ->setTable($table);
-
-    return $result;
   }
 }

src/applications/people/view/PhabricatorUserLogView.php

@@ -3,7 +3,6 @@
 final class PhabricatorUserLogView extends AphrontView {
 
   private $logs;
-  private $handles;
   private $searchBaseURI;
 
   public function setSearchBaseURI($search_base_uri) {
@@ -17,17 +16,17 @@ final class PhabricatorUserLogView extends AphrontView {
     return $this;
   }
 
-  public function setHandles(array $handles) {
-    assert_instances_of($handles, 'PhabricatorObjectHandle');
-    $this->handles = $handles;
-    return $this;
-  }
-
   public function render() {
     $logs = $this->logs;
-    $handles = $this->handles;
     $viewer = $this->getUser();
 
+    $phids = array();
+    foreach ($logs as $log) {
+      $phids[] = $log->getActorPHID();
+      $phids[] = $log->getUserPHID();
+    }
+    $handles = $viewer->loadHandles($phids);
+
     $action_map = PhabricatorUserLog::getActionTypeMap();
     $base_uri = $this->searchBaseURI;
 

src/applications/settings/panel/PhabricatorActivitySettingsPanel.php

@@ -43,8 +43,7 @@ final class PhabricatorActivitySettingsPanel extends PhabricatorSettingsPanel {
 
     $table = id(new PhabricatorUserLogView())
       ->setUser($viewer)
-      ->setLogs($logs)
+      ->setLogs($logs);
-      ->setHandles($handles);
 
     $panel = $this->newBox(pht('Account Activity Logs'), $table);