From ad4bd3d0013bb95eac9db20af0d62c9d607ba05a Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Sat, 27 Jan 2018 09:57:16 +1030 Subject: [PATCH] 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) --- src/Api/Controller/CreatePostController.php | 2 +- .../Controller/ShowDiscussionController.php | 4 +- .../Controller/UpdateDiscussionController.php | 2 +- src/Database/ScopeVisibilityTrait.php | 2 +- src/Discussion/Discussion.php | 35 ++++---- src/Discussion/DiscussionPolicy.php | 54 ++++++++----- src/Event/GetModelIsPrivate.php | 33 ++++++++ src/Event/ScopeHiddenDiscussionVisibility.php | 48 ----------- src/Event/ScopeModelVisibility.php | 17 ++-- src/Event/ScopePostVisibility.php | 49 ------------ .../ScopePrivateDiscussionVisibility.php | 41 ---------- src/Event/ScopePrivatePostVisibility.php | 49 ------------ src/Post/Post.php | 43 +++++++--- src/Post/PostPolicy.php | 61 +++++++------- src/Post/PostRepository.php | 79 ++++++------------- src/User/AbstractPolicy.php | 12 ++- 16 files changed, 197 insertions(+), 334 deletions(-) create mode 100644 src/Event/GetModelIsPrivate.php delete mode 100644 src/Event/ScopeHiddenDiscussionVisibility.php delete mode 100644 src/Event/ScopePostVisibility.php delete mode 100644 src/Event/ScopePrivateDiscussionVisibility.php delete mode 100644 src/Event/ScopePrivatePostVisibility.php diff --git a/src/Api/Controller/CreatePostController.php b/src/Api/Controller/CreatePostController.php index 050314838..b4c881137 100644 --- a/src/Api/Controller/CreatePostController.php +++ b/src/Api/Controller/CreatePostController.php @@ -84,7 +84,7 @@ class CreatePostController extends AbstractCreateController } $discussion = $post->discussion; - $discussion->posts = $discussion->postsVisibleTo($actor)->orderBy('time')->pluck('id'); + $discussion->posts = $discussion->posts()->whereVisibleTo($actor)->orderBy('time')->pluck('id'); return $post; } diff --git a/src/Api/Controller/ShowDiscussionController.php b/src/Api/Controller/ShowDiscussionController.php index 6321820f0..ba1c43684 100644 --- a/src/Api/Controller/ShowDiscussionController.php +++ b/src/Api/Controller/ShowDiscussionController.php @@ -118,7 +118,7 @@ class ShowDiscussionController extends AbstractShowController */ private function loadPostIds(Discussion $discussion, User $actor) { - return $discussion->postsVisibleTo($actor)->orderBy('time')->pluck('id')->all(); + return $discussion->posts()->whereVisibleTo($actor)->orderBy('time')->pluck('id')->all(); } /** @@ -170,7 +170,7 @@ class ShowDiscussionController extends AbstractShowController */ private function loadPosts($discussion, $actor, $offset, $limit, array $include) { - $query = $discussion->postsVisibleTo($actor); + $query = $discussion->posts()->whereVisibleTo($actor); $query->orderBy('time')->skip($offset)->take($limit)->with($include); diff --git a/src/Api/Controller/UpdateDiscussionController.php b/src/Api/Controller/UpdateDiscussionController.php index f8c8cd0b4..86b0f8152 100644 --- a/src/Api/Controller/UpdateDiscussionController.php +++ b/src/Api/Controller/UpdateDiscussionController.php @@ -64,7 +64,7 @@ class UpdateDiscussionController extends AbstractShowController if ($posts = $discussion->getModifiedPosts()) { $posts = (new Collection($posts))->load('discussion', 'user'); - $discussionPosts = $discussion->postsVisibleTo($actor)->orderBy('time')->pluck('id')->all(); + $discussionPosts = $discussion->posts()->whereVisibleTo($actor)->orderBy('time')->pluck('id')->all(); foreach ($discussionPosts as &$id) { foreach ($posts as $post) { diff --git a/src/Database/ScopeVisibilityTrait.php b/src/Database/ScopeVisibilityTrait.php index 03c18e341..62d73c028 100644 --- a/src/Database/ScopeVisibilityTrait.php +++ b/src/Database/ScopeVisibilityTrait.php @@ -26,7 +26,7 @@ trait ScopeVisibilityTrait public function scopeWhereVisibleTo(Builder $query, User $actor) { static::$dispatcher->fire( - new ScopeModelVisibility($query->getModel(), $query, $actor) + new ScopeModelVisibility($query, $actor, 'view') ); } } diff --git a/src/Discussion/Discussion.php b/src/Discussion/Discussion.php index c80404e3e..a693940b5 100644 --- a/src/Discussion/Discussion.php +++ b/src/Discussion/Discussion.php @@ -18,7 +18,7 @@ use Flarum\Discussion\Event\Hidden; use Flarum\Discussion\Event\Renamed; use Flarum\Discussion\Event\Restored; use Flarum\Discussion\Event\Started; -use Flarum\Event\ScopePostVisibility; +use Flarum\Event\GetModelIsPrivate; use Flarum\Foundation\EventGeneratorTrait; use Flarum\Post\Event\Deleted as PostDeleted; use Flarum\Post\MergeableInterface; @@ -118,6 +118,12 @@ class Discussion extends AbstractModel // read the discussion. $discussion->readers()->detach(); }); + + static::saving(function (Discussion $discussion) { + $event = new GetModelIsPrivate($discussion); + + $discussion->is_private = static::$dispatcher->until($event) === true; + }); } /** @@ -304,25 +310,7 @@ class Discussion extends AbstractModel */ public function posts() { - return $this->hasMany('Flarum\Post\Post'); - } - - /** - * Define the relationship with the discussion's posts, but only ones which - * are visible to the given user. - * - * @param User $user - * @return \Illuminate\Database\Eloquent\Relations\HasMany - */ - public function postsVisibleTo(User $user) - { - $relation = $this->posts(); - - static::$dispatcher->fire( - new ScopePostVisibility($this, $relation->getQuery(), $user) - ); - - return $relation; + return $this->hasMany(Post::class); } /** @@ -332,7 +320,10 @@ class Discussion extends AbstractModel */ public function comments() { - return $this->postsVisibleTo(new Guest)->where('type', 'comment'); + return $this->posts() + ->where('is_private', false) + ->whereNull('hide_time') + ->where('type', 'comment'); } /** @@ -345,6 +336,8 @@ class Discussion extends AbstractModel { return User::join('posts', 'posts.user_id', '=', 'users.id') ->where('posts.discussion_id', $this->id) + ->where('posts.is_private', false) + ->where('posts.type', 'comment') ->select('users.*') ->distinct(); } diff --git a/src/Discussion/DiscussionPolicy.php b/src/Discussion/DiscussionPolicy.php index 2040bf09a..fca8f6c70 100644 --- a/src/Discussion/DiscussionPolicy.php +++ b/src/Discussion/DiscussionPolicy.php @@ -12,8 +12,7 @@ namespace Flarum\Discussion; use Carbon\Carbon; -use Flarum\Event\ScopeHiddenDiscussionVisibility; -use Flarum\Event\ScopePrivateDiscussionVisibility; +use Flarum\Event\ScopeModelVisibility; use Flarum\Settings\SettingsRepositoryInterface; use Flarum\User\AbstractPolicy; use Flarum\User\Gate; @@ -73,28 +72,47 @@ class DiscussionPolicy extends AbstractPolicy */ public function find(User $actor, Builder $query) { - // Hide private discussions per default. - $query->where(function ($query) use ($actor) { - $query->where('discussions.is_private', false); - - $this->events->fire( - new ScopePrivateDiscussionVisibility($query, $actor) - ); - }); - if ($actor->cannot('viewDiscussions')) { $query->whereRaw('FALSE'); - } elseif (! $actor->hasPermission('discussion.hide')) { + + return; + } + + // Hide private discussions by default. + $query->where(function ($query) use ($actor) { + $query->where('discussions.is_private', false) + ->orWhere(function ($query) use ($actor) { + $this->events->fire( + new ScopeModelVisibility($query, $actor, 'viewPrivate') + ); + }); + }); + + // Hide hidden discussions, unless they are authored by the current + // user, or the current user has permission to view hidden discussions. + if (! $actor->hasPermission('discussion.hide')) { $query->where(function ($query) use ($actor) { $query->whereNull('discussions.hide_time') - ->where('comments_count', '>', 0) - ->orWhere('start_user_id', $actor->id); - - $this->events->fire( - new ScopeHiddenDiscussionVisibility($query, $actor, 'discussion.hide') - ); + ->orWhere('start_user_id', $actor->id) + ->orWhere(function ($query) use ($actor) { + $this->events->fire( + new ScopeModelVisibility($query, $actor, 'hide') + ); + }); }); } + + // Hide discussions with no comments, unless they are authored by the + // current user. + $query->where(function ($query) use ($actor) { + $query->where('comments_count', '>', 0) + ->orWhere('start_user_id', $actor->id) + ->orWhere(function ($query) use ($actor) { + $this->events->fire( + new ScopeModelVisibility($query, $actor, 'viewEmpty') + ); + }); + }); } /** diff --git a/src/Event/GetModelIsPrivate.php b/src/Event/GetModelIsPrivate.php new file mode 100644 index 000000000..30308b439 --- /dev/null +++ b/src/Event/GetModelIsPrivate.php @@ -0,0 +1,33 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Flarum\Event; + +use Flarum\Database\AbstractModel; + +/** + * Determine whether or not a model should be marked as `is_private`. + */ +class GetModelIsPrivate +{ + /** + * @var AbstractModel + */ + public $model; + + /** + * @param AbstractModel $model + */ + public function __construct(AbstractModel $model) + { + $this->model = $model; + } +} diff --git a/src/Event/ScopeHiddenDiscussionVisibility.php b/src/Event/ScopeHiddenDiscussionVisibility.php deleted file mode 100644 index 411b89be6..000000000 --- a/src/Event/ScopeHiddenDiscussionVisibility.php +++ /dev/null @@ -1,48 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Flarum\Event; - -use Flarum\User\User; -use Illuminate\Database\Eloquent\Builder; - -/** - * The `ScopeHiddenDiscussionVisibility` event. - */ -class ScopeHiddenDiscussionVisibility -{ - /** - * @var Builder - */ - public $query; - - /** - * @var User - */ - public $actor; - - /** - * @var string - */ - public $permission; - - /** - * @param Builder $query - * @param User $actor - * @param string $permission - */ - public function __construct(Builder $query, User $actor, $permission) - { - $this->query = $query; - $this->actor = $actor; - $this->permission = $permission; - } -} diff --git a/src/Event/ScopeModelVisibility.php b/src/Event/ScopeModelVisibility.php index 9eb5de654..f4de0380b 100644 --- a/src/Event/ScopeModelVisibility.php +++ b/src/Event/ScopeModelVisibility.php @@ -13,7 +13,6 @@ namespace Flarum\Event; use Flarum\User\User; use Illuminate\Database\Eloquent\Builder; -use Illuminate\Database\Eloquent\Model; /** * The `ScopeModelVisibility` event allows constraints to be applied in a query @@ -21,11 +20,6 @@ use Illuminate\Database\Eloquent\Model; */ class ScopeModelVisibility { - /** - * @var Model - */ - public $model; - /** * @var Builder */ @@ -37,14 +31,19 @@ class ScopeModelVisibility public $actor; /** - * @param Model $model + * @var string + */ + public $ability; + + /** * @param Builder $query * @param User $actor + * @param string $ability */ - public function __construct(Model $model, Builder $query, User $actor) + public function __construct(Builder $query, User $actor, $ability) { - $this->model = $model; $this->query = $query; $this->actor = $actor; + $this->ability = $ability; } } diff --git a/src/Event/ScopePostVisibility.php b/src/Event/ScopePostVisibility.php deleted file mode 100644 index d8d2304e2..000000000 --- a/src/Event/ScopePostVisibility.php +++ /dev/null @@ -1,49 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Flarum\Event; - -use Flarum\Discussion\Discussion; -use Flarum\User\User; -use Illuminate\Database\Eloquent\Builder; - -/** - * The `ScopePostVisibility` event. - */ -class ScopePostVisibility -{ - /** - * @var \Flarum\Discussion\Discussion - */ - public $discussion; - - /** - * @var Builder - */ - public $query; - - /** - * @var User - */ - public $actor; - - /** - * @param Discussion $discussion - * @param Builder $query - * @param User $actor - */ - public function __construct(Discussion $discussion, Builder $query, User $actor) - { - $this->discussion = $discussion; - $this->query = $query; - $this->actor = $actor; - } -} diff --git a/src/Event/ScopePrivateDiscussionVisibility.php b/src/Event/ScopePrivateDiscussionVisibility.php deleted file mode 100644 index c0b2758fc..000000000 --- a/src/Event/ScopePrivateDiscussionVisibility.php +++ /dev/null @@ -1,41 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Flarum\Event; - -use Flarum\User\User; -use Illuminate\Database\Eloquent\Builder; - -/** - * The `ScopePrivateDiscussionVisibility` event. - */ -class ScopePrivateDiscussionVisibility -{ - /** - * @var Builder - */ - public $query; - - /** - * @var User - */ - public $actor; - - /** - * @param Builder $query - * @param User $actor - */ - public function __construct(Builder $query, User $actor) - { - $this->query = $query; - $this->actor = $actor; - } -} diff --git a/src/Event/ScopePrivatePostVisibility.php b/src/Event/ScopePrivatePostVisibility.php deleted file mode 100644 index bb84cda39..000000000 --- a/src/Event/ScopePrivatePostVisibility.php +++ /dev/null @@ -1,49 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Flarum\Event; - -use Flarum\Discussion\Discussion; -use Flarum\User\User; -use Illuminate\Database\Eloquent\Builder; - -/** - * The `ScopePrivatePostVisibility` event. - */ -class ScopePrivatePostVisibility -{ - /** - * @var \Flarum\Discussion\Discussion - */ - public $discussion; - - /** - * @var Builder - */ - public $query; - - /** - * @var User - */ - public $actor; - - /** - * @param \Flarum\Discussion\Discussion $discussion - * @param Builder $query - * @param User $actor - */ - public function __construct(Discussion $discussion, Builder $query, User $actor) - { - $this->discussion = $discussion; - $this->query = $query; - $this->actor = $actor; - } -} diff --git a/src/Post/Post.php b/src/Post/Post.php index 1dba8616a..2d46283ed 100644 --- a/src/Post/Post.php +++ b/src/Post/Post.php @@ -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); } /** diff --git a/src/Post/PostPolicy.php b/src/Post/PostPolicy.php index a4a550daf..e1dc8ac75 100644 --- a/src/Post/PostPolicy.php +++ b/src/Post/PostPolicy.php @@ -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') + ); + }); + }); }); } } diff --git a/src/Post/PostRepository.php b/src/Post/PostRepository.php index d85fe4aa5..8213be5e8 100644 --- a/src/Post/PostRepository.php +++ b/src/Post/PostRepository.php @@ -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); } } diff --git a/src/User/AbstractPolicy.php b/src/User/AbstractPolicy.php index 4d559e6c1..3608d144a 100644 --- a/src/User/AbstractPolicy.php +++ b/src/User/AbstractPolicy.php @@ -59,8 +59,16 @@ abstract class AbstractPolicy */ public function scopeModelVisibility(ScopeModelVisibility $event) { - if ($event->model instanceof $this->model && method_exists($this, 'find')) { - call_user_func_array([$this, 'find'], [$event->actor, $event->query]); + if ($event->query->getModel() instanceof $this->model) { + if (substr($event->ability, 0, 4) === 'view') { + $method = 'find'.substr($event->ability, 4); + + if (method_exists($this, $method)) { + call_user_func_array([$this, $method], [$event->actor, $event->query]); + } + } elseif (method_exists($this, 'findWithPermission')) { + call_user_func_array([$this, 'findWithPermission'], [$event->actor, $event->query, $event->ability]); + } } } }