Adding some filters and queries to Macro application

Summary:
Fixes T2778

Introduces `PhabricatorMacroQuery`, which should consolidate all queries regarding macros

Adds `PolicyInterface` to `PhabricatorImageMacro`, as else the query would fail (we should consider adding it to the ApplicationTransaction instead, if that was ever planned)

Adds `Active Macros` filter, making it the default

Adds `My Macros` filter. You may ask why it overwrites `$authors`. Well, I did not want the page jump to the conclusion that it is a search result. It //is// one more or less, but the filter would jump to `seach` instead of `my`. If you want `My Macros` removed, tell me. It is useful only to heavy-macro-uploaders-and-users though. Five or six people in `http://secure.phabricator.(org|com)`, and an estimated dozen and a half at bigger installs.

Test Plan: created multiple macros from multiple authors, disabled them at will. browsed around, verified that Macros only appeared in the right filters and that nothing else broke.

Reviewers: epriestley, chad, btrahan

CC: aran, Korvin

Maniphest Tasks: T2778

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

src/__phutil_library_map__.php

@@ -1032,6 +1032,7 @@ phutil_register_library_map(array(
     'PhabricatorMacroListController' => 'applications/macro/controller/PhabricatorMacroListController.php',
     'PhabricatorMacroMemeController' => 'applications/macro/controller/PhabricatorMacroMemeController.php',
     'PhabricatorMacroMemeDialogController' => 'applications/macro/controller/PhabricatorMacroMemeDialogController.php',
+    'PhabricatorMacroQuery' => 'applications/macro/query/PhabricatorMacroQuery.php',
     'PhabricatorMacroReplyHandler' => 'applications/macro/mail/PhabricatorMacroReplyHandler.php',
     'PhabricatorMacroTransaction' => 'applications/macro/storage/PhabricatorMacroTransaction.php',
     'PhabricatorMacroTransactionComment' => 'applications/macro/storage/PhabricatorMacroTransactionComment.php',
@@ -2588,6 +2589,7 @@ phutil_register_library_map(array(
       0 => 'PhabricatorFileDAO',
       1 => 'PhabricatorSubscribableInterface',
       2 => 'PhabricatorApplicationTransactionInterface',
+      3 => 'PhabricatorPolicyInterface',
     ),
     'PhabricatorFileInfoController' => 'PhabricatorFileController',
     'PhabricatorFileLinkListView' => 'AphrontView',
@@ -2658,6 +2660,7 @@ phutil_register_library_map(array(
     'PhabricatorMacroListController' => 'PhabricatorMacroController',
     'PhabricatorMacroMemeController' => 'PhabricatorMacroController',
     'PhabricatorMacroMemeDialogController' => 'PhabricatorMacroController',
+    'PhabricatorMacroQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhabricatorMacroReplyHandler' => 'PhabricatorMailReplyHandler',
     'PhabricatorMacroTransaction' => 'PhabricatorApplicationTransaction',
     'PhabricatorMacroTransactionComment' => 'PhabricatorApplicationTransactionComment',

src/applications/macro/application/PhabricatorApplicationMacro.php

@@ -25,7 +25,7 @@ final class PhabricatorApplicationMacro extends PhabricatorApplication {
   public function getRoutes() {
     return array(
       '/macro/' => array(
-        '' => 'PhabricatorMacroListController',
+        '((?P<filter>all|active|my)/)?' => 'PhabricatorMacroListController',
         'create/' => 'PhabricatorMacroEditController',
         'view/(?P<id>[1-9]\d*)/' => 'PhabricatorMacroViewController',
         'comment/(?P<id>[1-9]\d*)/' => 'PhabricatorMacroCommentController',

src/applications/macro/controller/PhabricatorMacroController.php

@@ -15,7 +15,9 @@ abstract class PhabricatorMacroController
     }
 
     $nav->addLabel(pht('Macros'));
-    $nav->addFilter('/', pht('All Macros'));
+    $nav->addFilter('active', pht('Active Macros'));
+    $nav->addFilter('all', pht('All Macros'));
+    $nav->addFilter('my', pht('My Macros'));
     if ($has_search) {
       $nav->addFilter('search',
         pht('Search'),

src/applications/macro/controller/PhabricatorMacroListController.php

@@ -3,6 +3,12 @@
 final class PhabricatorMacroListController
   extends PhabricatorMacroController {
 
+  private $filter;
+
+  public function willProcessRequest(array $data) {
+    $this->filter = idx($data, 'filter', 'active');
+  }
+
   public function processRequest() {
 
     $request = $this->getRequest();
@@ -12,55 +18,39 @@ final class PhabricatorMacroListController
     $file_table = new PhabricatorFile();
     $conn = $macro_table->establishConnection('r');
 
-    $where = array();
+    $pager = id(new AphrontCursorPagerView())
+      ->readFromRequest($request);
 
-    $join = array();
+    $query = new PhabricatorMacroQuery();
-    $join[] = qsprintf($conn, '%T m', $macro_table->getTableName());
+    $query->setViewer($viewer);
 
     $filter = $request->getStr('name');
     if (strlen($filter)) {
-      $where[] = qsprintf($conn, 'm.name LIKE %~', $filter);
+      $query->withNameLike($filter);
     }
 
     $authors = $request->getArr('authors');
+
     if ($authors) {
-      $join[] = qsprintf(
+      $query->withAuthorPHIDs($authors);
-        $conn,
-        '%T f ON m.filePHID = f.phid',
-        $file_table->getTableName());
-      $where[] = qsprintf($conn, 'f.authorPHID IN (%Ls)', $authors);
     }
 
-    $has_search = $where;
+    $has_search = $filter || $authors;
 
+    if ($this->filter == 'my') {
+      $query->withAuthorPHIDs(array($viewer->getPHID()));
+      // For pre-filling the tokenizer
+      $authors = array($viewer->getPHID());
+    }
+
+    if ($this->filter == 'active') {
+      $query->withStatus(PhabricatorMacroQuery::STATUS_ACTIVE);
+    }
+
+    $macros = $query->executeWithCursorPager($pager);
     if ($has_search) {
-      $macros = queryfx_all(
-        $conn,
-        'SELECT m.* FROM  %Q WHERE %Q',
-        implode(' JOIN ', $join),
-        implode(' AND ', $where));
-      $macros = $macro_table->loadAllFromArray($macros);
       $nodata = pht('There are no macros matching the filter.');
     } else {
-      $pager = new AphrontPagerView();
-      $pager->setOffset($request->getInt('page'));
-
-      $macros = $macro_table->loadAllWhere(
-        '1 = 1 ORDER BY id DESC LIMIT %d, %d',
-        $pager->getOffset(),
-        $pager->getPageSize());
-
-      // Get an exact count since the size here is reasonably going to be a few
-      // thousand at most in any reasonable case.
-      $count = queryfx_one(
-        $conn,
-        'SELECT COUNT(*) N FROM %T',
-        $macro_table->getTableName());
-      $count = $count['N'];
-
-      $pager->setCount($count);
-      $pager->setURI($request->getRequestURI(), 'page');
-
       $nodata = pht('There are no image macros yet.');
     }
 
@@ -70,16 +60,10 @@ final class PhabricatorMacroListController
       $author_phids = array();
     }
 
-    $file_phids = mpull($macros, 'getFilePHID');
+    $files = mpull($macros, 'getFile');
-
+    if ($files) {
-    $files = array();
-    if ($file_phids) {
-      $files = $file_table->loadAllWhere(
-        "phid IN (%Ls)",
-        $file_phids);
       $author_phids += mpull($files, 'getAuthorPHID', 'getAuthorPHID');
     }
-    $files_map = mpull($files, null, 'getPHID');
 
     $this->loadHandles($author_phids);
     $author_handles = array_select_keys($this->getLoadedHandles(), $authors);
@@ -108,15 +92,14 @@ final class PhabricatorMacroListController
     $nav = $this->buildSideNavView(
       $for_app = false,
       $has_search);
-    $nav->selectFilter($has_search ? 'search' : '/');
+    $nav->selectFilter($has_search ? 'search' : $this->filter);
 
     $nav->appendChild($filter_view);
 
     $pinboard = new PhabricatorPinboardView();
     $pinboard->setNoDataString($nodata);
     foreach ($macros as $macro) {
-      $file_phid = $macro->getFilePHID();
+      $file = $macro->getFile();
-      $file = idx($files_map, $file_phid);
 
       $item = new PhabricatorPinboardItemView();
       if ($file) {
@@ -143,7 +126,20 @@ final class PhabricatorMacroListController
 
     if (!$has_search) {
       $nav->appendChild($pager);
+      switch ($this->filter) {
+        case 'all':
           $name = pht('All Macros');
+          break;
+        case 'my':
+          $name = pht('My Macros');
+          break;
+        case 'active':
+          $name = pht('Active Macros');
+          break;
+        default:
+          throw new Exception("Unknown filter $this->filter");
+          break;
+      }
     } else {
       $name = pht('Search');
     }

src/applications/macro/query/PhabricatorMacroQuery.php

@@ -0,0 +1,143 @@
+<?php
+
+/**
+ * @group phriction
+ */
+final class PhabricatorMacroQuery
+  extends PhabricatorCursorPagedPolicyAwareQuery {
+
+  private $ids;
+  private $phids;
+  private $authors;
+  private $name;
+
+  private $status = 'status-any';
+  const STATUS_ANY = 'status-any';
+  const STATUS_ACTIVE = 'status-active';
+
+  public function withIDs(array $ids) {
+    $this->ids = $ids;
+    return $this;
+  }
+
+  public function withPHIDs(array $phids) {
+    $this->phids = $phids;
+    return $this;
+  }
+
+  public function withAuthorPHIDs(array $authors) {
+    $this->authors = $authors;
+    return $this;
+  }
+
+  public function withNameLike($name) {
+    $this->name = $name;
+    return $this;
+  }
+
+  public function withStatus($status) {
+    $this->status = $status;
+    return $this;
+  }
+
+  protected function loadPage() {
+    $macro_table = new PhabricatorFileImageMacro();
+    $conn = $macro_table->establishConnection('r');
+
+    $rows = queryfx_all(
+      $conn,
+      'SELECT * FROM %T m %Q %Q %Q %Q',
+      $macro_table->getTableName(),
+      $this->buildJoinClause($conn),
+      $this->buildWhereClause($conn),
+      $this->buildOrderClause($conn),
+      $this->buildLimitClause($conn));
+
+    return $macro_table->loadAllFromArray($rows);
+  }
+
+  protected function buildJoinClause(AphrontDatabaseConnection $conn) {
+    $joins = array();
+
+    if ($this->authors) {
+      $file_table = new PhabricatorFile();
+      $joins[] = qsprintf(
+        $conn,
+        'JOIN %T f ON m.filePHID = f.phid',
+        $file_table->getTableName());
+    }
+
+    return implode(' ', $joins);
+  }
+
+  protected function buildWhereClause(AphrontDatabaseConnection $conn) {
+    $where = array();
+
+    if ($this->ids) {
+      $where[] = qsprintf(
+        $conn,
+        'm.id IN (%Ld)',
+        $this->ids);
+    }
+
+    if ($this->phids) {
+      $where[] = qsprintf(
+        $conn,
+        'm.phid IN (%Ld)',
+        $this->phids);
+    }
+
+    if ($this->authors) {
+      $where[] = qsprintf(
+        $conn,
+        'f.authorPHID IN (%Ls)',
+        $this->authors);
+    }
+
+    if ($this->name) {
+      $where[] = qsprintf(
+        $conn,
+        'm.name LIKE %~',
+        $this->name);
+    }
+
+    if ($this->status == self::STATUS_ACTIVE) {
+      $where[] = qsprintf(
+        $conn,
+        'm.isDisabled = 0');
+    }
+
+    $where[] = $this->buildPagingClause($conn);
+
+    return $this->formatWhereClause($where);
+  }
+
+  protected function willFilterPage(array $macros) {
+    if (!$macros) {
+      return array();
+    }
+
+    $file_phids = mpull($macros, 'getFilePHID');
+    $files = id(new PhabricatorFileQuery())
+      ->setViewer($this->getViewer())
+      ->withPHIDs($file_phids)
+      ->execute();
+    $files = mpull($files, null, 'getPHID');
+
+    foreach ($macros as $key => $macro) {
+      $file = idx($files, $macro->getFilePHID());
+      if (!$file) {
+        unset($macros[$key]);
+        continue;
+      }
+      $macro->attachFile($file);
+    }
+
+    return $macros;
+  }
+
+  protected function getPagingColumn() {
+    return 'm.id';
+  }
+
+}

src/applications/macro/storage/PhabricatorFileImageMacro.php

@@ -3,13 +3,29 @@
 final class PhabricatorFileImageMacro extends PhabricatorFileDAO
   implements
     PhabricatorSubscribableInterface,
-    PhabricatorApplicationTransactionInterface {
+    PhabricatorApplicationTransactionInterface,
+    PhabricatorPolicyInterface {
 
   protected $filePHID;
   protected $phid;
   protected $name;
   protected $isDisabled = 0;
 
+  private $file;
+
+  public function attachFile(PhabricatorFile $file) {
+    $this->file = $file;
+    return $this;
+  }
+
+  public function getFile() {
+    if (!$this->file) {
+      throw new Exception("Attach a file with attachFile() first!");
+    }
+
+    return $this->file;
+  }
+
   public function getConfiguration() {
     return array(
       self::CONFIG_AUX_PHID  => true,
@@ -33,5 +49,20 @@ final class PhabricatorFileImageMacro extends PhabricatorFileDAO
     return new PhabricatorMacroTransaction();
   }
 
+  public function getCapabilities() {
+    return array(
+      PhabricatorPolicyCapability::CAN_VIEW,
+      PhabricatorPolicyCapability::CAN_EDIT,
+    );
+  }
+
+  public function getPolicy($capability) {
+    return PhabricatorPolicies::POLICY_USER;
+  }
+
+  public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
+    return false;
+  }
+
 }