Make Flags policy aware

Summary: Ref T1809. Ref T603. Ref T3599. Makes flags policy aware. This change reduces the utility of flag search/browse; the next change will switch it to ApplicationSearch to restore utility. Representing all that ordering in terms of cursor paging is also a giant pain. Test Plan: Viewed Differential, Flags, etc. Grepped for all PhabricatorFlagQuery callsites. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T603, T1809, T3599 Differential Revision: https://secure.phabricator.com/D6746
2024-11-26 16:52:41 +01:00 · 2013-08-13 16:17:42 -07:00 · 2013-08-13 16:17:42 -07:00 · 275f67294c
commit 275f67294c
parent 3f9a4d42f7
5 changed files with 83 additions and 206 deletions
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@ -3251,7 +3251,11 @@ phutil_register_library_map(array(
    'PhabricatorFilesManagementPurgeWorkflow' => 'PhabricatorFilesManagementWorkflow',
    'PhabricatorFilesManagementRebuildWorkflow' => 'PhabricatorFilesManagementWorkflow',
    'PhabricatorFilesManagementWorkflow' => 'PhutilArgumentWorkflow',
-    'PhabricatorFlag' => 'PhabricatorFlagDAO',
+    'PhabricatorFlag' =>
+    array(
+      0 => 'PhabricatorFlagDAO',
+      1 => 'PhabricatorPolicyInterface',
+    ),
    'PhabricatorFlagColor' => 'PhabricatorFlagConstants',
    'PhabricatorFlagController' => 'PhabricatorController',
    'PhabricatorFlagDAO' => 'PhabricatorLiskDAO',
@ -3259,6 +3263,7 @@ phutil_register_library_map(array(
    'PhabricatorFlagEditController' => 'PhabricatorFlagController',
    'PhabricatorFlagListController' => 'PhabricatorFlagController',
    'PhabricatorFlagListView' => 'AphrontView',
+    'PhabricatorFlagQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
    'PhabricatorFlagsUIEventListener' => 'PhutilEventListener',
    'PhabricatorFormExample' => 'PhabricatorUIExample',
    'PhabricatorGarbageCollectorConfigOptions' => 'PhabricatorApplicationConfigOptions',
--- a/src/applications/flag/controller/PhabricatorFlagListController.php
+++ b/src/applications/flag/controller/PhabricatorFlagListController.php
@ -19,94 +19,21 @@ final class PhabricatorFlagListController extends PhabricatorFlagController {
      ->setHref($request->getRequestURI()));
    $nav->setCrumbs($crumbs);

-    $filter_form = new AphrontFormView();
-    $filter_form->setNoShading(true);
-    $filter_form->setUser($user);
-    $filter_form->appendChild(
-      id(new AphrontFormToggleButtonsControl())
-        ->setName('o')
-        ->setLabel(pht('Sort Order'))
-        ->setBaseURI($request->getRequestURI(), 'o')
-        ->setValue($flag_order)
-        ->setButtons(
-          array(
-            'n'   => pht('Date'),
-            'c'   => pht('Color'),
-            'o'   => pht('Object Type'),
-          )));
-
-    $filter = new AphrontListFilterView();
-    $filter->appendChild($filter_form);
-    $nav->appendChild($filter);
-
    $query = new PhabricatorFlagQuery();
    $query->withOwnerPHIDs(array($user->getPHID()));
    $query->setViewer($user);
    $query->needHandles(true);

-    switch ($flag_order) {
-      //   'r'
-      //   'a'
-      case 'n':
-        $order = PhabricatorFlagQuery::ORDER_ID;
-        break;
-      case 'c':
-        $order = PhabricatorFlagQuery::ORDER_COLOR;
-        break;
-      case 'o':
-        $order = PhabricatorFlagQuery::ORDER_OBJECT;
-        break;
-      default:
-        throw new Exception("Unknown order!");
-    }
-    $query->withOrder($order);
-
    $flags = $query->execute();

    $views = array();
-    if ($flag_order == 'n') {
-      $view = new PhabricatorFlagListView();
-      $view->setFlags($flags);
-      $view->setUser($user);
-      $view->setFlush(true);
-      $views[] = array(
-        'view'  => $view,
-      );
-    } else {
-      switch ($flag_order) {
-        case 'c':
-          $flags_tmp = mgroup($flags, 'getColor');
-          $flags = array();
-          foreach ($flags_tmp as $color => $flag_group) {
-            $title = pht('%s Flags',
-              PhabricatorFlagColor::getColorName($color));
-            $flags[$title] = $flag_group;
-          }
-          break;
-        case 'o':
-          $flags_tmp = mgroup($flags, 'getType');
-          $flags = array();
-          foreach ($flags_tmp as $color => $flag_group) {
-            // Appending an 's' to fake plurals
-            $title = head($flag_group)->getHandle()->getTypeName() . 's';
-            $flags[$title] = $flag_group;
-          }
-          break;
-        default:
-          throw new Exception("Unknown order!");
-      }
-
-      foreach ($flags as $group_title => $flag_group) {
-        $view = new PhabricatorFlagListView();
-        $view->setFlags($flag_group);
-        $view->setUser($user);
-        $view->setFlush(true);
-        $views[] = array(
-          'title' => pht('%s (%d)', $group_title, count($flag_group)),
-          'view'  => $view,
-        );
-      }
-    }
+    $view = new PhabricatorFlagListView();
+    $view->setFlags($flags);
+    $view->setUser($user);
+    $view->setFlush(true);
+    $views[] = array(
+      'view'  => $view,
+    );

    foreach ($views as $view) {
      $panel = new AphrontPanelView();
--- a/src/applications/flag/query/PhabricatorFlagQuery.php
+++ b/src/applications/flag/query/PhabricatorFlagQuery.php
@ -1,29 +1,15 @@
 <?php

-final class PhabricatorFlagQuery {
+final class PhabricatorFlagQuery
+  extends PhabricatorCursorPagedPolicyAwareQuery {

  private $ownerPHIDs;
  private $types;
  private $objectPHIDs;
-  private $color;
-
-  private $limit;
-  private $offset;
+  private $colors;

  private $needHandles;
  private $needObjects;
-  private $viewer;
-
-  private $order     = 'order-id';
-  const ORDER_ID     = 'order-id';
-  const ORDER_COLOR  = 'order-color';
-  const ORDER_OBJECT = 'order-object';
-  const ORDER_REASON = 'order-reason';
-
-  public function setViewer($viewer) {
-    $this->viewer = $viewer;
-    return $this;
-  }

  public function withOwnerPHIDs(array $owner_phids) {
    $this->ownerPHIDs = $owner_phids;
@ -40,13 +26,8 @@ final class PhabricatorFlagQuery {
    return $this;
  }

-  public function withColor($color) {
-    $this->color = $color;
-    return $this;
-  }
-
-  public function withOrder($order) {
-    $this->order = $order;
+  public function withColors(array $colors) {
+    $this->colors = $colors;
    return $this;
  }

@ -60,67 +41,58 @@ final class PhabricatorFlagQuery {
    return $this;
  }

-  public function setLimit($limit) {
-    $this->limit = $limit;
-    return $this;
-  }
-
-  public function setOffset($offset) {
-    $this->offset = $offset;
-    return $this;
-  }
-
  public static function loadUserFlag(PhabricatorUser $user, $object_phid) {
    // Specifying the type in the query allows us to use a key.
-    return id(new PhabricatorFlag())->loadOneWhere(
-      'ownerPHID = %s AND type = %s AND objectPHID = %s',
-      $user->getPHID(),
-      phid_get_type($object_phid),
-      $object_phid);
+    return id(new PhabricatorFlagQuery())
+      ->setViewer($user)
+      ->withOwnerPHIDs(array($user->getPHID()))
+      ->withTypes(array(phid_get_type($object_phid)))
+      ->withObjectPHIDs(array($object_phid))
+      ->executeOne();
  }


-  public function execute() {
+  public function loadPage() {
    $table = new PhabricatorFlag();
    $conn_r = $table->establishConnection('r');

-    $where = $this->buildWhereClause($conn_r);
-    $limit = $this->buildLimitClause($conn_r);
-    $order = $this->buildOrderClause($conn_r);
-
    $data = queryfx_all(
      $conn_r,
      'SELECT * FROM %T flag %Q %Q %Q',
      $table->getTableName(),
-      $where,
-      $order,
-      $limit);
+      $this->buildWhereClause($conn_r),
+      $this->buildOrderClause($conn_r),
+      $this->buildLimitClause($conn_r));

-    $flags = $table->loadAllFromArray($data);
+    return $table->loadAllFromArray($data);
+  }

-    if ($this->needHandles || $this->needObjects) {
-      $phids = ipull($data, 'objectPHID');
-      $query = new PhabricatorObjectHandleData($phids);
-      $query->setViewer($this->viewer);
+  public function willFilterPage(array $flags) {

-      if ($this->needHandles) {
-        $handles = $query->loadHandles();
-        foreach ($flags as $flag) {
-          $handle = idx($handles, $flag->getObjectPHID());
-          if ($handle) {
-            $flag->attachHandle($handle);
-          }
+    if ($this->needObjects) {
+      $objects = id(new PhabricatorObjectQuery())
+        ->setViewer($this->getViewer())
+        ->withPHIDs(mpull($flags, 'getObjectPHID'))
+        ->execute();
+      $objects = mpull($objects, null, 'getPHID');
+      foreach ($flags as $key => $flag) {
+        $object = idx($objects, $flag->getObjectPHID());
+        if ($object) {
+          $flags[$key]->attachObject($object);
+        } else {
+          unset($flags[$key]);
        }
      }
+    }

-      if ($this->needObjects) {
-        $objects = $query->loadObjects();
-        foreach ($flags as $flag) {
-          $object = idx($objects, $flag->getObjectPHID());
-          if ($object) {
-            $flag->attachObject($object);
-          }
-        }
+    if ($this->needHandles) {
+      $handles = id(new PhabricatorHandleQuery())
+        ->setViewer($this->getViewer())
+        ->withPHIDs(mpull($flags, 'getObjectPHID'))
+        ->execute();
+
+      foreach ($flags as $flag) {
+        $flag->attachHandle($handles[$flag->getObjectPHID()]);
      }
    }

@ -128,7 +100,6 @@ final class PhabricatorFlagQuery {
  }

  private function buildWhereClause($conn_r) {
-
    $where = array();

    if ($this->ownerPHIDs) {
@ -152,56 +123,16 @@ final class PhabricatorFlagQuery {
        $this->objectPHIDs);
    }

-    if (strlen($this->color)) {
+    if ($this->colors) {
      $where[] = qsprintf(
        $conn_r,
-        'flag.color = %d',
-        $this->color);
+        'flag.color IN (%Ld)',
+        $this->colors);
    }

-    if ($where) {
-      return 'WHERE ('.implode(') AND (', $where).')';
-    } else {
-      return '';
-    }
-  }
+    $where[] = $this->buildPagingClause($conn_r);

-  private function buildOrderClause($conn_r) {
-    return qsprintf($conn_r,
-      'ORDER BY %Q',
-      $this->getOrderColumn($conn_r));
-  }
-
-  private function getOrderColumn($conn_r) {
-    switch ($this->order) {
-      case self::ORDER_ID:
-        return 'id DESC';
-        break;
-      case self::ORDER_COLOR:
-        return 'color ASC';
-        break;
-      case self::ORDER_OBJECT:
-        return 'type DESC';
-        break;
-      case self::ORDER_REASON:
-        return 'reasonPHID DESC';
-        break;
-      default:
-        throw new Exception("Unknown order {$this->order}!");
-        break;
-    }
-  }
-
-  private function buildLimitClause($conn_r) {
-    if ($this->limit && $this->offset) {
-      return qsprintf($conn_r, 'LIMIT %d, %d', $this->offset, $this->limit);
-    } else if ($this->limit) {
-      return qsprintf($conn_r, 'LIMIT %d', $this->limit);
-    } else if ($this->offset) {
-      return qsprintf($conn_r, 'LIMIT %d, %d', $this->offset, PHP_INT_MAX);
-    } else {
-      return '';
-    }
+    return $this->formatWhereClause($where);
  }

 }
--- a/src/applications/flag/storage/PhabricatorFlag.php
+++ b/src/applications/flag/storage/PhabricatorFlag.php
@ -1,6 +1,7 @@
 <?php

-final class PhabricatorFlag extends PhabricatorFlagDAO {
+final class PhabricatorFlag extends PhabricatorFlagDAO
+  implements PhabricatorPolicyInterface {

  protected $ownerPHID;
  protected $type;
@ -9,14 +10,11 @@ final class PhabricatorFlag extends PhabricatorFlagDAO {
  protected $color = PhabricatorFlagColor::COLOR_BLUE;
  protected $note;

-  private $handle = false;
-  private $object = false;
+  private $handle = self::ATTACHABLE;
+  private $object = self::ATTACHABLE;

  public function getObject() {
-    if ($this->object === false) {
-      throw new Exception('Call attachObject() before getObject()!');
-    }
-    return $this->object;
+    return $this->assertAttached($this->object);
  }

  public function attachObject($object) {
@ -25,10 +23,7 @@ final class PhabricatorFlag extends PhabricatorFlagDAO {
  }

  public function getHandle() {
-    if ($this->handle === false) {
-      throw new Exception('Call attachHandle() before getHandle()!');
-    }
-    return $this->handle;
+    return $this->assertAttached($this->handle);
  }

  public function attachHandle(PhabricatorObjectHandle $handle) {
@ -36,4 +31,23 @@ final class PhabricatorFlag extends PhabricatorFlagDAO {
    return $this;
  }

+
+/* -(  PhabricatorPolicyInterface  )----------------------------------------- */
+
+
+  public function getCapabilities() {
+    return array(
+      PhabricatorPolicyCapability::CAN_VIEW,
+      PhabricatorPolicyCapability::CAN_EDIT,
+    );
+  }
+
+  public function getPolicy($capability) {
+    return PhabricatorPolicies::POLICY_NOONE;
+  }
+
+  public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
+    return ($viewer->getPHID() == $this->getOwnerPHID());
+  }
+
 }
--- a/src/applications/macro/query/PhabricatorMacroQuery.php
+++ b/src/applications/macro/query/PhabricatorMacroQuery.php
@ -172,7 +172,7 @@ final class PhabricatorMacroQuery
      $flags = id(new PhabricatorFlagQuery())
        ->withOwnerPHIDs(array($this->getViewer()->getPHID()))
        ->withTypes(array(PhabricatorMacroPHIDTypeMacro::TYPECONST))
-        ->withColor($this->flagColor)
+        ->withColors(array($this->flagColor))
        ->setViewer($this->getViewer())
        ->execute();