1
0
mirror of https://github.com/flarum/core.git synced 2025-10-21 03:36:05 +02:00

Overhaul model visibility scoping (#1342)

* Overhaul the way model visibility scoping works

- Previously post visibility scoping required concrete knowledge of the
  parent discussion, ie. you needed a Discussion model on which you
  would call `postsVisibleTo($actor)`. This meant that to fetch posts
  from different discussions (eg. when listing user posts), it was a
  convoluted process, ultimately causing #1333.

  Now posts behave like any other model in terms of visibility scoping,
  and you simply call `whereVisibleTo($actor)` on a Post query. This
  scope will automatically apply a WHERE EXISTS clause that scopes the
  query to only include posts whose discussions are visible too. Thus,
  fetching posts from multiple discussions can now be done in a single
  query, simplifying things greatly and fixing #1333.

- As such, the ScopePostVisibility event has been removed. Also, the
  rest of the "Scope" events have been consolidated into a single event,
  ScopeModelVisibility. This event is called whenever a user must have
  a certain $ability in order to see a set of discussions. Typically
  this ability is just "view". But in the case of discussions which have
  been marked as `is_private`, it is "viewPrivate". And in the case of
  discussions which have been hidden, it is "hide". etc.

  The relevant API on AbstractPolicy has been refined, now providing
  `find`, `findPrivate`, `findEmpty`, and `findWithPermission` methods.
  This could probably do with further refinement and we can re-address
  it once we get around to implementing more Extenders.

- An additional change is that Discussion::comments() (the relation
  used to calculate the cached number of replies) now yields "comments
  that are not private", where before it meant "comments that are
  visible to Guests". This was flawed because eg. comments in non-public
  tags are technically not visible to Guests.

  Consequently, the Approval extension must adopt usage of `is_private`,
  so that posts which are not approved are not included in the replies
  count. Fundamentally, `is_private` now indicates that a discussion/
  post should be hidden by default and should only be visible if it
  meets certain criteria. This is in comparison to non-is_private
  entities, which are visible by default and may be hidden if they don't
  meet certain criteria.

Note that these changes have not been extensively tested, but I have
been over the logic multiple times and it seems to check out.

* Add event to determine whether a discussion `is_private`

See https://github.com/flarum/core/pull/1153#issuecomment-292693624

* Don't include hidden posts in the comments count

* Apply fixes from StyleCI (#1350)
This commit is contained in:
Toby Zerner
2018-01-27 09:57:16 +10:30
committed by GitHub
parent 4b1a299b3c
commit ad4bd3d001
16 changed files with 197 additions and 334 deletions

View File

@@ -12,7 +12,9 @@
namespace Flarum\Post;
use Flarum\Database\AbstractModel;
use Flarum\Database\ScopeVisibilityTrait;
use Flarum\Discussion\Discussion;
use Flarum\Event\GetModelIsPrivate;
use Flarum\Event\ScopeModelVisibility;
use Flarum\Foundation\EventGeneratorTrait;
use Flarum\Post\Event\Deleted;
use Flarum\User\User;
@@ -40,7 +42,6 @@ use Illuminate\Database\Eloquent\Builder;
class Post extends AbstractModel
{
use EventGeneratorTrait;
use ScopeVisibilityTrait;
/**
* {@inheritdoc}
@@ -96,6 +97,12 @@ class Post extends AbstractModel
$post->discussion->save();
});
static::saving(function (Post $post) {
$event = new GetModelIsPrivate($post);
$post->is_private = static::$dispatcher->until($event) === true;
});
static::deleted(function (Post $post) {
$post->raise(new Deleted($post));
});
@@ -103,6 +110,28 @@ class Post extends AbstractModel
static::addGlobalScope(new RegisteredTypesScope);
}
/**
* @param Builder $query
* @param User $actor
*/
public function scopeWhereVisibleTo(Builder $query, User $actor)
{
static::$dispatcher->dispatch(
new ScopeModelVisibility($query, $actor, 'view')
);
// Make sure the post's discussion is visible as well
$query->whereExists(function ($query) use ($actor) {
$query->selectRaw('1')
->from('discussions')
->whereRaw('discussions.id = posts.discussion_id');
static::$dispatcher->dispatch(
new ScopeModelVisibility(Discussion::query()->setQuery($query), $actor, 'view')
);
});
}
/**
* Determine whether or not this post is visible to the given user.
*
@@ -111,15 +140,7 @@ class Post extends AbstractModel
*/
public function isVisibleTo(User $user)
{
$discussion = $this->discussion()->whereVisibleTo($user)->first();
if ($discussion) {
$this->setRelation('discussion', $discussion);
return (bool) $discussion->postsVisibleTo($user)->where('id', $this->id)->count();
}
return false;
return (bool) $this->newQuery()->whereVisibleTo($user)->find($this->id);
}
/**

View File

@@ -12,12 +12,13 @@
namespace Flarum\Post;
use Carbon\Carbon;
use Flarum\Event\ScopePostVisibility;
use Flarum\Event\ScopePrivatePostVisibility;
use Flarum\Discussion\Discussion;
use Flarum\Event\ScopeModelVisibility;
use Flarum\Settings\SettingsRepositoryInterface;
use Flarum\User\AbstractPolicy;
use Flarum\User\User;
use Illuminate\Contracts\Events\Dispatcher;
use Illuminate\Database\Eloquent\Builder;
class PostPolicy extends AbstractPolicy
{
@@ -30,6 +31,7 @@ class PostPolicy extends AbstractPolicy
* @var SettingsRepositoryInterface
*/
protected $settings;
/**
* @var Dispatcher
*/
@@ -37,6 +39,7 @@ class PostPolicy extends AbstractPolicy
/**
* @param SettingsRepositoryInterface $settings
* @param Dispatcher $events
*/
public function __construct(SettingsRepositoryInterface $settings, Dispatcher $events)
{
@@ -44,16 +47,6 @@ class PostPolicy extends AbstractPolicy
$this->events = $events;
}
/**
* {@inheritdoc}
*/
public function subscribe(Dispatcher $events)
{
parent::subscribe($events);
$events->listen(ScopePostVisibility::class, [$this, 'scopePostVisibility']);
}
/**
* @param User $actor
* @param string $ability
@@ -68,26 +61,38 @@ class PostPolicy extends AbstractPolicy
}
/**
* @param ScopePostVisibility $event
* @param User $actor
* @param Builder $query
*/
public function scopePostVisibility(ScopePostVisibility $event)
public function find(User $actor, $query)
{
// Hide private posts per default.
$event->query->where(function ($query) use ($event) {
$query->where('posts.is_private', false);
$this->events->fire(
new ScopePrivatePostVisibility($event->discussion, $query, $event->actor)
);
// Hide private posts by default.
$query->where(function ($query) use ($actor) {
$query->where('posts.is_private', false)
->orWhere(function ($query) use ($actor) {
$this->events->fire(
new ScopeModelVisibility($query, $actor, 'viewPrivate')
);
});
});
// When fetching a discussion's posts: if the user doesn't have permission
// to moderate the discussion, then they can't see posts that have been
// hidden by someone other than themself.
if ($event->actor->cannot('editPosts', $event->discussion)) {
$event->query->where(function ($query) use ($event) {
$query->whereNull('hide_time')
->orWhere('user_id', $event->actor->id);
// Hide hidden posts, unless they are authored by the current user, or
// the current user has permission to view hidden posts in the
// discussion.
if (! $actor->hasPermission('discussion.editPosts')) {
$query->where(function ($query) use ($actor) {
$query->whereNull('posts.hide_time')
->orWhere('user_id', $actor->id)
->orWhereExists(function ($query) use ($actor) {
$query->selectRaw('1')
->from('discussions')
->whereRaw('discussions.id = posts.discussion_id')
->where(function ($query) use ($actor) {
$this->events->dispatch(
new ScopeModelVisibility(Discussion::query()->setQuery($query), $actor, 'editPosts')
);
});
});
});
}
}

View File

@@ -12,22 +12,36 @@
namespace Flarum\Post;
use Flarum\Discussion\Discussion;
use Flarum\Event\ScopePostVisibility;
use Flarum\User\User;
use Illuminate\Database\Eloquent\ModelNotFoundException;
use Illuminate\Database\Eloquent\Builder;
class PostRepository
{
/**
* Get a new query builder for the posts table.
*
* @return \Illuminate\Database\Eloquent\Builder
* @return Builder
*/
public function query()
{
return Post::query();
}
/**
* @param User|null $user
* @return Builder
*/
protected function queryVisibleTo(User $user = null)
{
$query = $this->query();
if ($user !== null) {
$query->whereVisibleTo($user);
}
return $query;
}
/**
* Find a post by ID, optionally making sure it is visible to a certain
* user, or throw an exception.
@@ -40,13 +54,7 @@ class PostRepository
*/
public function findOrFail($id, User $actor = null)
{
$posts = $this->findByIds([$id], $actor);
if (! count($posts)) {
throw new ModelNotFoundException;
}
return $posts->first();
return $this->queryVisibleTo($actor)->findOrFail($id);
}
/**
@@ -62,7 +70,8 @@ class PostRepository
*/
public function findWhere(array $where = [], User $actor = null, $sort = [], $count = null, $start = 0)
{
$query = Post::where($where)
$query = $this->queryVisibleTo($actor)
->where($where)
->skip($start)
->take($count);
@@ -70,9 +79,7 @@ class PostRepository
$query->orderBy($field, $order);
}
$ids = $query->pluck('id')->all();
return $this->findByIds($ids, $actor);
return $query->get();
}
/**
@@ -111,7 +118,7 @@ class PostRepository
*/
public function filterVisibleIds(array $ids, User $actor)
{
return $this->queryIds($ids, $actor)->pluck('id')->all();
return $this->queryIds($ids, $actor)->pluck('posts.id')->all();
}
/**
@@ -127,7 +134,8 @@ class PostRepository
public function getIndexForNumber($discussionId, $number, User $actor = null)
{
$query = Discussion::find($discussionId)
->postsVisibleTo($actor)
->posts()
->whereVisibleTo($actor)
->where('time', '<', function ($query) use ($discussionId, $number) {
$query->select('time')
->from('posts')
@@ -146,45 +154,10 @@ class PostRepository
/**
* @param array $ids
* @param User|null $actor
* @return mixed
* @return Builder
*/
protected function queryIds(array $ids, User $actor = null)
{
$discussions = $this->getDiscussionsForPosts($ids, $actor);
$posts = Post::whereIn('id', $ids)
->where(function ($query) use ($discussions, $actor) {
foreach ($discussions as $discussion) {
$query->orWhere(function ($query) use ($discussion, $actor) {
$query->where('discussion_id', $discussion->id);
event(new ScopePostVisibility($discussion, $query, $actor));
});
}
$query->orWhereRaw('FALSE');
});
foreach ($posts as $post) {
$post->discussion = $discussions->find($post->discussion_id);
}
return $posts;
}
/**
* @param $postIds
* @param User $actor
* @return mixed
*/
protected function getDiscussionsForPosts($postIds, User $actor)
{
return Discussion::query()
->select('discussions.*')
->join('posts', 'posts.discussion_id', '=', 'discussions.id')
->whereIn('posts.id', $postIds)
->groupBy('discussions.id')
->whereVisibleTo($actor)
->get();
return $this->queryVisibleTo($actor)->whereIn('posts.id', $ids);
}
}