Make posts 1:1 with blogs and implement policy controls

Summary: This leaves the UI in a pretty rough state, but implements blog policy controls and queries, and 1:1 relationships between posts and blogs. Needs a bunch more cleanup but seemed like an okayish breaking point in terms of cohesiveness. Posts have these rules: - Drafts are visible only to the author. - Published posts are visible to anyone who can see the blog they appear on. - Posts are only editable by the author. ...so we don't need any special policy UI or state to accommodate these rules. Posts may have no blog if they're grandfathered in or you write a post to a blog and then lose the ability to see the blog. This is the messiest edge case -- specifically: - You write a post to blog A. - You publish the post. - I edit the "Visible To:" for blog A and set it to exclude you. What we do in this case is let you see the post in "My Posts", but you can no longer see the blog and you'll see the post as not being part of a blog. We can maybe give you some UI to let you move it later or something. Test Plan: Hit all (I think?) of the interfaces without issues. Definitely some UI problems still right now. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T1373 Differential Revision: https://secure.phabricator.com/D3694
2024-11-22 06:42:42 +01:00 · 2012-10-15 14:50:04 -07:00 · 2012-10-15 14:50:04 -07:00 · a50b8e39b1
commit a50b8e39b1
parent dbcf2e44e8
14 changed files with 181 additions and 197 deletions
--- a/resources/sql/patches/phameoneblog.sql
+++ b/resources/sql/patches/phameoneblog.sql
@ -0,0 +1,2 @@
+ALTER TABLE `{$NAMESPACE}_phame`.`phame_post`
+  ADD `blogPHID` varchar(64) COLLATE utf8_bin;
--- a/src/applications/phame/controller/blog/PhameBlogViewController.php
+++ b/src/applications/phame/controller/blog/PhameBlogViewController.php
@ -19,28 +19,9 @@
 /**
 * @group phame
 */
-final class PhameBlogViewController
-  extends PhameController {
+final class PhameBlogViewController extends PhameController {

  private $blogPHID;
-  private $bloggerPHIDs;
-  private $postPHIDs;
-
-  private function setPostPHIDs($post_phids) {
-    $this->postPHIDs = $post_phids;
-    return $this;
-  }
-  private function getPostPHIDs() {
-    return $this->postPHIDs;
-  }
-
-  private function setBloggerPHIDs($blogger_phids) {
-    $this->bloggerPHIDs = $blogger_phids;
-    return $this;
-  }
-  private function getBloggerPHIDs() {
-    return $this->bloggerPHIDs;
-  }

  private function setBlogPHID($blog_phid) {
    $this->blogPHID = $blog_phid;
@ -54,6 +35,7 @@ final class PhameBlogViewController
    $filter = 'blog/view/'.$this->getBlogPHID();
    return $filter;
  }
+
  protected function getSideNavExtraBlogFilters() {
      $filters =  array(
        array('key'  => $this->getSideNavFilter(),
@ -67,57 +49,26 @@ final class PhameBlogViewController
  }

  public function processRequest() {
-    $request   = $this->getRequest();
-    $user      = $request->getUser();
-    $blog_phid = $this->getBlogPHID();
+    $request = $this->getRequest();
+    $user = $request->getUser();

    $blog = id(new PhameBlogQuery())
      ->setViewer($user)
-      ->withPHIDs(array($blog_phid))
+      ->withPHIDs(array($this->getBlogPHID()))
      ->executeOne();
    if (!$blog) {
      return new Aphront404Response();
    }

-    $this->loadEdges();
-
-    $blogger_phids = $this->getBloggerPHIDs();
-    if ($blogger_phids) {
-      $bloggers = $this->loadViewerHandles($blogger_phids);
-    } else {
-      $bloggers = array();
-    }
-
-    $post_phids = $this->getPostPHIDs();
-    if ($post_phids) {
-      $posts = id(new PhamePostQuery())
-        ->withPHIDs($post_phids)
-        ->withVisibility(PhamePost::VISIBILITY_PUBLISHED)
-        ->execute();
-    } else {
-      $posts = array();
-    }
-
-    $notice = array();
-    if ($request->getExists('new')) {
-      $notice =
-        array(
-          'title' => 'Successfully created your blog.',
-          'body'  => 'Time to write some posts.'
-        );
-    } else if ($request->getExists('edit')) {
-      $notice =
-        array(
-          'title' => 'Successfully edited your blog.',
-          'body'  => 'Time to write some posts.'
-        );
-    }
+    $posts = id(new PhamePostQuery())
+      ->setViewer($user)
+      ->withBlogPHIDs(array($blog->getPHID()))
+      ->execute();

    $skin = $blog->getSkinRenderer();
    $skin
      ->setUser($this->getRequest()->getUser())
-      ->setNotice($notice)
-      ->setBloggers($bloggers)
+      ->setBloggers($this->loadViewerHandles(mpull($posts, 'getBloggerPHID')))
      ->setPosts($posts)
      ->setBlog($blog)
      ->setRequestURI($this->getRequest()->getRequestURI());
@ -135,30 +86,4 @@ final class PhameBlogViewController
        'title' => $blog->getName(),
      ));
  }
-
-  private function loadEdges() {
-
-    $edge_types = array(
-      PhabricatorEdgeConfig::TYPE_BLOG_HAS_BLOGGER,
-      PhabricatorEdgeConfig::TYPE_BLOG_HAS_POST,
-    );
-    $blog_phid = $this->getBlogPHID();
-    $phids = array($blog_phid);
-
-    $edges = id(new PhabricatorEdgeQuery())
-      ->withSourcePHIDs($phids)
-      ->withEdgeTypes($edge_types)
-      ->execute();
-
-    $blogger_phids = array_keys(
-      $edges[$blog_phid][PhabricatorEdgeConfig::TYPE_BLOG_HAS_BLOGGER]
-    );
-    $this->setBloggerPHIDs($blogger_phids);
-
-    $post_phids = array_keys(
-      $edges[$blog_phid][PhabricatorEdgeConfig::TYPE_BLOG_HAS_POST]
-    );
-    $this->setPostPHIDs($post_phids);
-
-  }
 }
--- a/src/applications/phame/controller/post/PhamePostDeleteController.php
+++ b/src/applications/phame/controller/post/PhamePostDeleteController.php
@ -40,32 +40,33 @@ extends PhameController {
  public function processRequest() {
    $request   = $this->getRequest();
    $user      = $request->getUser();
-    $post_phid = $this->getPostPHID();
-    $posts     = id(new PhamePostQuery())
-      ->withPHIDs(array($post_phid))
-      ->execute();
-    $post      = reset($posts);
-    if (empty($post)) {
+
+    $post = id(new PhamePostQuery())
+      ->setViewer($user)
+      ->withPHIDs(array($this->getPostPHID()))
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->executeOne();
+    if (!$post) {
      return new Aphront404Response();
    }
-    if ($post->getBloggerPHID() != $user->getPHID()) {
-      return new Aphront403Response();
-    }
    $post_noun = $post->getHumanName();

    if ($request->isFormPost()) {
      $edge_type = PhabricatorEdgeConfig::TYPE_POST_HAS_BLOG;
      $edges     = id(new PhabricatorEdgeQuery())
-        ->withSourcePHIDs(array($post_phid))
+        ->withSourcePHIDs(array($post->getPHID()))
        ->withEdgeTypes(array($edge_type))
        ->execute();

-      $blog_edges = $edges[$post_phid][$edge_type];
+      $blog_edges = $edges[$post->getPHID()][$edge_type];
      $blog_phids = array_keys($blog_edges);
      $editor     = id(new PhabricatorEdgeEditor())
        ->setActor($user);
      foreach ($blog_phids as $phid) {
-        $editor->removeEdge($post_phid, $edge_type, $phid);
+        $editor->removeEdge($post->getPHID(), $edge_type, $phid);
      }
      $editor->save();

--- a/src/applications/phame/controller/post/PhamePostEditController.php
+++ b/src/applications/phame/controller/post/PhamePostEditController.php
@ -115,16 +115,18 @@ final class PhamePostEditController
    $errors        = array();

    if ($this->isPostEdit()) {
-      $posts = id(new PhamePostQuery())
+      $post = id(new PhamePostQuery())
+        ->setViewer($user)
        ->withPHIDs(array($this->getPostPHID()))
-        ->execute();
-      $post  = reset($posts);
-      if (empty($post)) {
+        ->requireCapabilities(
+          array(
+            PhabricatorPolicyCapability::CAN_EDIT,
+          ))
+        ->executeOne();
+      if (!$post) {
        return new Aphront404Response();
      }
-      if ($post->getBloggerPHID() != $user->getPHID()) {
-        return new Aphront403Response();
-      }
+
      $post_noun     = ucfirst($post->getHumanName());
      $cancel_uri    = $post->getViewURI($user->getUsername());
      $submit_button = 'Save Changes';
--- a/src/applications/phame/controller/post/PhamePostViewController.php
+++ b/src/applications/phame/controller/post/PhamePostViewController.php
@ -19,8 +19,7 @@
 /**
 * @group phame
 */
-final class PhamePostViewController
-extends PhameController {
+final class PhamePostViewController extends PhameController {

  private $postPHID;
  private $phameTitle;
@ -75,29 +74,21 @@ extends PhameController {
  public function processRequest() {
    $request   = $this->getRequest();
    $user      = $request->getUser();
-    $post_phid = null;

    if ($this->getPostPHID()) {
-      $post_phid = $this->getPostPHID();
-      if (!$post_phid) {
+      $post = id(new PhamePostQuery())
+        ->setViewer($user)
+        ->withPHIDs(array($this->getPostPHID()))
+        ->executeOne();
+
+      if (!$post) {
        return new Aphront404Response();
      }

-      $posts = id(new PhamePostQuery())
-        ->withPHIDs(array($post_phid))
-        ->execute();
-      $post = reset($posts);
-
-      if ($post) {
-        $this->setPhameTitle($post->getPhameTitle());
-        $blogger = PhabricatorObjectHandleData::loadOneHandle(
-          $post->getBloggerPHID(),
-          $user);
-        if (!$blogger) {
-          return new Aphront404Response();
-        }
-      }
-
+      $this->setPhameTitle($post->getPhameTitle());
+      $blogger = PhabricatorObjectHandleData::loadOneHandle(
+        $post->getBloggerPHID(),
+        $user);
    } else if ($this->getBloggerName() && $this->getPhameTitle()) {
      $phame_title = $this->getPhameTitle();
      $phame_title = PhabricatorSlug::normalize($phame_title);
@ -114,8 +105,9 @@ extends PhameController {
        return new Aphront404Response();
      }
      $posts = id(new PhamePostQuery())
-        ->withBloggerPHID($blogger->getPHID())
-        ->withPhameTitle($phame_title)
+        ->setViewer($user)
+        ->withBloggerPHIDs(array($blogger->getPHID()))
+        ->withPhameTitles(array($phame_title))
        ->execute();
      $post = reset($posts);

--- a/src/applications/phame/controller/post/list/PhameAllPostListController.php
+++ b/src/applications/phame/controller/post/list/PhameAllPostListController.php
@ -76,8 +76,8 @@ final class PhameAllPostListController
  public function processRequest() {
    $user = $this->getRequest()->getUser();

-    $query = new PhamePostQuery();
-    $query->withVisibility(PhamePost::VISIBILITY_PUBLISHED);
+    $query = id(new PhamePostQuery())
+      ->setViewer($user);
    $this->setPhamePostQuery($query);

    $this->setActions(array());
--- a/src/applications/phame/controller/post/list/PhameBloggerPostListController.php
+++ b/src/applications/phame/controller/post/list/PhameBloggerPostListController.php
@ -59,9 +59,10 @@ final class PhameBloggerPostListController
    }
    $this->setActions($actions);

-    $query = new PhamePostQuery();
-    $query->withBloggerPHID($blogger_phid);
-    $query->withVisibility(PhamePost::VISIBILITY_PUBLISHED);
+    $query = id(new PhamePostQuery())
+      ->setViewer($user)
+      ->withBloggerPHIDs(array($blogger_phid));
+
    $this->setPhamePostQuery($query);

    $page_title = 'Posts by '.$this->getBloggerName();
--- a/src/applications/phame/controller/post/list/PhameDraftListController.php
+++ b/src/applications/phame/controller/post/list/PhameDraftListController.php
@ -51,9 +51,11 @@ final class PhameDraftListController
    $user = $this->getRequest()->getUser();
    $phid = $user->getPHID();

-    $query = new PhamePostQuery();
-    $query->withBloggerPHID($phid);
-    $query->withVisibility(PhamePost::VISIBILITY_DRAFT);
+    $query = id(new PhamePostQuery())
+      ->setViewer($user)
+      ->withBloggerPHIDs(array($phid))
+      ->withVisibility(PhamePost::VISIBILITY_DRAFT);
+
    $this->setPhamePostQuery($query);

    $actions = array('view', 'edit');
--- a/src/applications/phame/controller/post/list/PhameUserPostListController.php
+++ b/src/applications/phame/controller/post/list/PhameUserPostListController.php
@ -47,9 +47,10 @@ final class PhameUserPostListController
    $user = $this->getRequest()->getUser();
    $phid = $user->getPHID();

-    $query = new PhamePostQuery();
-    $query->withBloggerPHID($phid);
-    $query->withVisibility(PhamePost::VISIBILITY_PUBLISHED);
+    $query = id(new PhamePostQuery())
+      ->setViewer($user)
+      ->withBloggerPHIDs(array($phid));
+
    $this->setPhamePostQuery($query);

    $actions = array('view', 'edit');
--- a/src/applications/phame/query/PhamePostQuery.php
+++ b/src/applications/phame/query/PhamePostQuery.php
@ -19,31 +19,31 @@
 /**
 * @group phame
 */
-final class PhamePostQuery extends PhabricatorOffsetPagedQuery {
+final class PhamePostQuery extends PhabricatorCursorPagedPolicyAwareQuery {

-  private $bloggerPHID;
-  private $withoutBloggerPHID;
-  private $phameTitle;
+  private $blogPHIDs;
+  private $bloggerPHIDs;
+  private $phameTitles;
  private $visibility;
  private $phids;

-  /**
-   * Mutually exlusive with @{method:withoutBloggerPHID}.
-   *
-   * @{method:withBloggerPHID} wins because being positive and inclusive is
-   * cool.
-   */
-  public function withBloggerPHID($blogger_phid) {
-    $this->bloggerPHID = $blogger_phid;
-    return $this;
-  }
-  public function withoutBloggerPHID($blogger_phid) {
-    $this->withoutBloggerPHID = $blogger_phid;
+  public function withPHIDs(array $phids) {
+    $this->phids = $phids;
    return $this;
  }

-  public function withPhameTitle($phame_title) {
-    $this->phameTitle = $phame_title;
+  public function withBloggerPHIDs(array $blogger_phids) {
+    $this->bloggerPHIDs = $blogger_phids;
+    return $this;
+  }
+
+  public function withBlogPHIDs(array $blog_phids) {
+    $this->blogPHIDs = $blog_phids;
+    return $this;
+  }
+
+  public function withPhameTitles(array $phame_titles) {
+    $this->phameTitles = $phame_titles;
    return $this;
  }

@ -52,12 +52,7 @@ final class PhamePostQuery extends PhabricatorOffsetPagedQuery {
    return $this;
  }

-  public function withPHIDs($phids) {
-    $this->phids = $phids;
-    return $this;
-  }
-
-  public function execute() {
+  protected function loadPage() {
    $table  = new PhamePost();
    $conn_r = $table->establishConnection('r');

@ -75,54 +70,65 @@ final class PhamePostQuery extends PhabricatorOffsetPagedQuery {

    $posts = $table->loadAllFromArray($data);

+    if ($posts) {
+      // We require these to do visibility checks, so load them unconditionally.
+      $blog_phids = mpull($posts, 'getBlogPHID');
+      $blogs = id(new PhameBlogQuery())
+        ->setViewer($this->getViewer())
+        ->withPHIDs($blog_phids)
+        ->execute();
+      $blogs = mpull($blogs, 'getPHID');
+      foreach ($posts as $post) {
+        if (isset($blogs[$post->getBlogPHID()])) {
+          $post->setBlog($blogs[$post->getBlogPHID()]);
+        }
+      }
+    }
+
    return $posts;
  }

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

    if ($this->phids) {
      $where[] = qsprintf(
        $conn_r,
-        'phid IN (%Ls)',
-        $this->phids
-      );
+        'p.phid IN (%Ls)',
+        $this->phids);
    }

-    if ($this->bloggerPHID) {
+    if ($this->bloggerPHIDs) {
      $where[] = qsprintf(
        $conn_r,
-        'bloggerPHID = %s',
-        $this->bloggerPHID
-      );
-    } else if ($this->withoutBloggerPHID) {
-      $where[] = qsprintf(
-        $conn_r,
-        'bloggerPHID != %s',
-        $this->withoutBloggerPHID
-      );
+        'p.bloggerPHID IN (%Ls)',
+        $this->bloggerPHIDs);
    }

-    if ($this->phameTitle) {
+    if ($this->phameTitles) {
      $where[] = qsprintf(
        $conn_r,
-        'phameTitle = %s',
-        $this->phameTitle
-      );
+        'p.phameTitle IN (%Ls)',
+        $this->phameTitles);
    }

    if ($this->visibility !== null) {
      $where[] = qsprintf(
        $conn_r,
-        'visibility = %d',
-        $this->visibility
-      );
+        'p.visibility = %d',
+        $this->visibility);
    }

+    if ($this->blogPHIDs) {
+      $where[] = qsprintf(
+        $conn_r,
+        'p.blogPHID in (%Ls)',
+        $this->blogPHIDs);
+    }
+
+    $where[] = $this->buildPagingClause($conn_r);
+
    return $this->formatWhereClause($where);
  }

-  private function buildOrderClause($conn_r) {
-    return 'ORDER BY datePublished DESC, id DESC';
-  }
 }
--- a/src/applications/phame/storage/PhamePost.php
+++ b/src/applications/phame/storage/PhamePost.php
@ -19,7 +19,7 @@
 /**
 * @group phame
 */
-final class PhamePost extends PhameDAO {
+final class PhamePost extends PhameDAO implements PhabricatorPolicyInterface {

  const VISIBILITY_DRAFT     = 0;
  const VISIBILITY_PUBLISHED = 1;
@ -33,6 +33,18 @@ final class PhamePost extends PhameDAO {
  protected $visibility;
  protected $configData;
  protected $datePublished;
+  protected $blogPHID;
+
+  private $blog;
+
+  public function setBlog(PhameBlog $blog) {
+    $this->blog = $blog;
+    return $this;
+  }
+
+  public function getBlog() {
+    return $this->blog;
+  }

  public function getViewURI($blogger_name = '') {
    // go for the pretty uri if we can
@ -44,15 +56,19 @@ final class PhamePost extends PhameDAO {
    }
    return $uri;
  }
+
  public function getEditURI() {
    return $this->getActionURI('edit');
  }
+
  public function getDeleteURI() {
    return $this->getActionURI('delete');
  }
+
  public function getChangeVisibilityURI() {
    return $this->getActionURI('changevisibility');
  }
+
  private function getActionURI($action) {
    return '/phame/post/'.$action.'/'.$this->getPHID().'/';
  }
@ -78,6 +94,7 @@ final class PhamePost extends PhameDAO {
    }
    return idx($config_data, 'comments_widget', 'none');
  }
+
  public function getConfiguration() {
    return array(
      self::CONFIG_AUX_PHID   => true,
@ -116,4 +133,45 @@ final class PhamePost extends PhameDAO {
    return $options;
  }

+
+/* -(  PhabricatorPolicyInterface Implementation  )-------------------------- */
+
+
+  public function getCapabilities() {
+    return array(
+      PhabricatorPolicyCapability::CAN_VIEW,
+      PhabricatorPolicyCapability::CAN_EDIT,
+    );
+  }
+
+
+  public function getPolicy($capability) {
+    // Draft posts are visible only to the author. Published posts are visible
+    // to whoever the blog is visible to.
+
+    switch ($capability) {
+      case PhabricatorPolicyCapability::CAN_VIEW:
+        if (!$this->isDraft() && $this->getBlog()) {
+          return $this->getBlog()->getViewPolicy();
+        } else {
+          return PhabricatorPolicies::POLICY_NOONE;
+        }
+        break;
+      case PhabricatorPolicyCapability::CAN_EDIT:
+        return PhabricatorPolicies::POLICY_NOONE;
+    }
+  }
+
+
+  public function hasAutomaticCapability($capability, PhabricatorUser $user) {
+    // A blog post's author can always view it, and is the only user allowed
+    // to edit it.
+
+    switch ($capability) {
+      case PhabricatorPolicyCapability::CAN_VIEW:
+      case PhabricatorPolicyCapability::CAN_EDIT:
+        return ($user->getPHID() == $this->getBloggerPHID());
+    }
+  }
+
 }
--- a/src/applications/phame/view/PhameBlogDetailView.php
+++ b/src/applications/phame/view/PhameBlogDetailView.php
@ -81,13 +81,6 @@ final class PhameBlogDetailView extends AphrontView {
          'class' => 'description'
        ),
        $description
-      ).
-      phutil_render_tag(
-        'div',
-        array(
-          'class' => 'bloggers'
-        ),
-        'Current bloggers: '.$this->getBloggersHTML($bloggers)
      )
    );

--- a/src/applications/phame/view/PhameBlogListView.php
+++ b/src/applications/phame/view/PhameBlogListView.php
@ -71,12 +71,9 @@ final class PhameBlogListView extends AphrontView {
      $item = id(new PhabricatorObjectItemView())
        ->setHeader($blog->getName())
        ->setHref($blog->getViewURI())
-        ->addDetail(
-          'Bloggers',
-          implode(', ', mpull($bloggers, 'renderLink')))
        ->addDetail(
          'Custom Domain',
-          $blog->getDomain());
+          phutil_escape_html($blog->getDomain()));

      $can_edit = PhabricatorPolicyFilter::hasCapability(
        $user,
--- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php
+++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php
@ -1008,6 +1008,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList {
        'type'    => 'sql',
        'name'    => $this->getPatchPath('phamepolicy.sql'),
      ),
+      'phameoneblog.sql' => array(
+        'type'    => 'sql',
+        'name'    => $this->getPatchPath('phameoneblog.sql'),
+      ),
    );
  }