From 5a1bf08d3f97650744b5b6df4c40d7009c31d9bb Mon Sep 17 00:00:00 2001 From: MatusMak Date: Mon, 25 Oct 2021 17:48:25 +0200 Subject: [PATCH] #2492 - Groups filtering & retrieve single endpoint (#3084) Fixes #2492 * Added api/groups/{id} endpoint for retrieving a single group by its id * Fixed GroupRepository incorrectly opening query to User instead of Group model * Added filtering & paging abilities to GET api/groups endpoint * Added test for sorting for GET api/groups endpoint Co-authored-by: Alexander Skvortsov <38059171+askvortsov1@users.noreply.github.com> --- src/Api/Controller/ListGroupsController.php | 57 +++++++- src/Api/Controller/ShowGroupController.php | 51 +++++++ src/Api/routes.php | 7 + src/Filter/FilterServiceProvider.php | 5 + src/Group/Filter/GroupFilterer.php | 40 ++++++ src/Group/Filter/HiddenFilter.php | 26 ++++ src/Group/GroupRepository.php | 2 +- tests/integration/api/groups/ListTest.php | 142 ++++++++++++++++++++ tests/integration/api/groups/ShowTest.php | 126 +++++++++++++++++ 9 files changed, 451 insertions(+), 5 deletions(-) create mode 100644 src/Api/Controller/ShowGroupController.php create mode 100644 src/Group/Filter/GroupFilterer.php create mode 100644 src/Group/Filter/HiddenFilter.php create mode 100644 tests/integration/api/groups/ShowTest.php diff --git a/src/Api/Controller/ListGroupsController.php b/src/Api/Controller/ListGroupsController.php index 93960b702..5db7b661b 100644 --- a/src/Api/Controller/ListGroupsController.php +++ b/src/Api/Controller/ListGroupsController.php @@ -10,8 +10,10 @@ namespace Flarum\Api\Controller; use Flarum\Api\Serializer\GroupSerializer; -use Flarum\Group\Group; +use Flarum\Group\Filter\GroupFilterer; use Flarum\Http\RequestUtil; +use Flarum\Http\UrlGenerator; +use Flarum\Query\QueryCriteria; use Psr\Http\Message\ServerRequestInterface; use Tobscure\JsonApi\Document; @@ -22,6 +24,38 @@ class ListGroupsController extends AbstractListController */ public $serializer = GroupSerializer::class; + /** + * {@inheritdoc} + */ + public $sortFields = ['nameSingular', 'namePlural', 'isHidden']; + + /** + * {@inheritdoc} + * + * @var int + */ + public $limit = -1; + + /** + * @var GroupFilterer + */ + protected $filterer; + + /** + * @var UrlGenerator + */ + protected $url; + + /** + * @param GroupFilterer $filterer + * @param UrlGenerator $url + */ + public function __construct(GroupFilterer $filterer, UrlGenerator $url) + { + $this->filterer = $filterer; + $this->url = $url; + } + /** * {@inheritdoc} */ @@ -29,10 +63,25 @@ class ListGroupsController extends AbstractListController { $actor = RequestUtil::getActor($request); - $results = Group::whereVisibleTo($actor)->get(); + $filters = $this->extractFilter($request); + $sort = $this->extractSort($request); + $sortIsDefault = $this->sortIsDefault($request); - $this->loadRelations($results, []); + $limit = $this->extractLimit($request); + $offset = $this->extractOffset($request); - return $results; + $criteria = new QueryCriteria($actor, $filters, $sort, $sortIsDefault); + + $queryResults = $this->filterer->filter($criteria, $limit, $offset); + + $document->addPaginationLinks( + $this->url->to('api')->route('groups.index'), + $request->getQueryParams(), + $offset, + $limit, + $queryResults->areMoreResults() ? null : 0 + ); + + return $queryResults->getResults(); } } diff --git a/src/Api/Controller/ShowGroupController.php b/src/Api/Controller/ShowGroupController.php new file mode 100644 index 000000000..9aee28f6d --- /dev/null +++ b/src/Api/Controller/ShowGroupController.php @@ -0,0 +1,51 @@ +groups = $groups; + } + + /** + * {@inheritdoc} + */ + protected function data(ServerRequestInterface $request, Document $document) + { + $id = Arr::get($request->getQueryParams(), 'id'); + $actor = RequestUtil::getActor($request); + + $group = $this->groups->findOrFail($id, $actor); + + return $group; + } +} diff --git a/src/Api/routes.php b/src/Api/routes.php index 778da8bfd..503a8a0db 100644 --- a/src/Api/routes.php +++ b/src/Api/routes.php @@ -224,6 +224,13 @@ return function (RouteCollection $map, RouteHandlerFactory $route) { $route->toController(Controller\CreateGroupController::class) ); + // Show a single group + $map->get( + '/groups/{id}', + 'groups.show', + $route->toController(Controller\ShowGroupController::class) + ); + // Edit a group $map->patch( '/groups/{id}', diff --git a/src/Filter/FilterServiceProvider.php b/src/Filter/FilterServiceProvider.php index 43b447f66..19a222361 100644 --- a/src/Filter/FilterServiceProvider.php +++ b/src/Filter/FilterServiceProvider.php @@ -13,6 +13,8 @@ use Flarum\Discussion\Filter\DiscussionFilterer; use Flarum\Discussion\Query as DiscussionQuery; use Flarum\Foundation\AbstractServiceProvider; use Flarum\Foundation\ContainerUtil; +use Flarum\Group\Filter as GroupFilter; +use Flarum\Group\Filter\GroupFilterer; use Flarum\Post\Filter as PostFilter; use Flarum\Post\Filter\PostFilterer; use Flarum\User\Filter\UserFilterer; @@ -41,6 +43,9 @@ class FilterServiceProvider extends AbstractServiceProvider UserQuery\EmailFilterGambit::class, UserQuery\GroupFilterGambit::class, ], + GroupFilterer::class => [ + GroupFilter\HiddenFilter::class, + ], PostFilterer::class => [ PostFilter\AuthorFilter::class, PostFilter\DiscussionFilter::class, diff --git a/src/Group/Filter/GroupFilterer.php b/src/Group/Filter/GroupFilterer.php new file mode 100644 index 000000000..008202d12 --- /dev/null +++ b/src/Group/Filter/GroupFilterer.php @@ -0,0 +1,40 @@ +groups = $groups; + } + + protected function getQuery(User $actor): Builder + { + return $this->groups->query()->whereVisibleTo($actor); + } +} diff --git a/src/Group/Filter/HiddenFilter.php b/src/Group/Filter/HiddenFilter.php new file mode 100644 index 000000000..653c50655 --- /dev/null +++ b/src/Group/Filter/HiddenFilter.php @@ -0,0 +1,26 @@ +getQuery()->where('is_hidden', $negate ? '!=' : '=', $filterValue); + } +} diff --git a/src/Group/GroupRepository.php b/src/Group/GroupRepository.php index 93745b1e2..769e0d606 100644 --- a/src/Group/GroupRepository.php +++ b/src/Group/GroupRepository.php @@ -21,7 +21,7 @@ class GroupRepository */ public function query() { - return User::query(); + return Group::query(); } /** diff --git a/tests/integration/api/groups/ListTest.php b/tests/integration/api/groups/ListTest.php index 7773222d9..b59af92d1 100644 --- a/tests/integration/api/groups/ListTest.php +++ b/tests/integration/api/groups/ListTest.php @@ -65,6 +65,148 @@ class ListTest extends TestCase $this->assertEquals(['1', '2', '3', '4', '10'], Arr::pluck($data['data'], 'id')); } + /** + * @test + */ + public function filters_only_public_groups_for_admin() + { + $response = $this->send( + $this->request('GET', '/api/groups', [ + 'authenticatedAs' => 1, + ]) + ->withQueryParams([ + 'filter' => ['hidden' => 0], + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true); + + // The four default groups created by the installer without our hidden group + $this->assertEquals(['1', '2', '3', '4'], Arr::pluck($data['data'], 'id')); + } + + /** + * @test + */ + public function filters_only_hidden_groups_for_admin() + { + $response = $this->send( + $this->request('GET', '/api/groups', [ + 'authenticatedAs' => 1, + ]) + ->withQueryParams([ + 'filter' => ['hidden' => 1], + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true); + + // Only our hidden group + $this->assertEquals(['10'], Arr::pluck($data['data'], 'id')); + } + + /** + * @test + */ + public function filters_only_public_groups_for_guest() + { + $response = $this->send( + $this->request('GET', '/api/groups') + ->withQueryParams([ + 'filter' => ['hidden' => 0], + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true); + + // The four default groups created by the installer without our hidden group + $this->assertEquals(['1', '2', '3', '4'], Arr::pluck($data['data'], 'id')); + } + + /** + * @test + */ + public function hides_hidden_groups_when_filtering_for_guest() + { + $response = $this->send( + $this->request('GET', '/api/groups') + ->withQueryParams([ + 'filter' => ['hidden' => 1], + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true); + + // When guest attempts to filter for hidden groups, system should + // still apply scoping and exclude those groups from results + $this->assertEquals([], Arr::pluck($data['data'], 'id')); + } + + /** + * @test + */ + public function paginates_groups_without_filter() + { + $response = $this->send( + $this->request('GET', '/api/groups') + ->withQueryParams([ + 'page' => ['limit' => '2', 'offset' => '2'], + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true); + + // Show second page of groups + $this->assertEquals(['3', '4'], Arr::pluck($data['data'], 'id')); + } + + /** + * @test + */ + public function paginates_groups_with_filter() + { + $response = $this->send( + $this->request('GET', '/api/groups') + ->withQueryParams([ + 'filter' => ['hidden' => 1], + 'page' => ['limit' => '1', 'offset' => '1'], + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true); + + // Show second page of groups. Because there is only one hidden group, + // second page should be empty. + $this->assertEmpty($data['data']); + } + + /** + * @test + */ + public function sorts_groups_by_name() + { + $response = $this->send( + $this->request('GET', '/api/groups', [ + 'authenticatedAs' => 1, + ]) + ->withQueryParams([ + 'sort' => 'nameSingular', + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true); + + // Ascending alphabetical order is: Admin - Guest - Hidden - Member - Mod + $this->assertEquals(['1', '2', '10', '3', '4'], Arr::pluck($data['data'], 'id')); + } + protected function hiddenGroup(): array { return [ diff --git a/tests/integration/api/groups/ShowTest.php b/tests/integration/api/groups/ShowTest.php new file mode 100644 index 000000000..2a7585a8a --- /dev/null +++ b/tests/integration/api/groups/ShowTest.php @@ -0,0 +1,126 @@ +prepareDatabase([ + 'groups' => [ + $this->hiddenGroup(), + ], + ]); + } + + /** + * @test + */ + public function shows_public_group_for_guest() + { + $response = $this->send( + $this->request('GET', '/api/groups/1') + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true); + + // Default group created by the installer should be returned + $this->assertEquals('1', Arr::get($data, 'data.id')); + } + + /** + * @test + */ + public function shows_public_group_for_admin() + { + $response = $this->send( + $this->request('GET', '/api/groups/1', [ + 'authenticatedAs' => 1, + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true); + + // Default group created by the installer should be returned + $this->assertEquals('1', Arr::get($data, 'data.id')); + } + + /** + * @test + */ + public function hides_hidden_group_for_guest() + { + $response = $this->send( + $this->request('GET', '/api/groups/10') + ); + + // Hidden group should not be returned for guest + $this->assertEquals(404, $response->getStatusCode()); + } + + /** + * @test + */ + public function shows_hidden_group_for_admin() + { + $response = $this->send( + $this->request('GET', '/api/groups/10', [ + 'authenticatedAs' => 1, + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + $data = json_decode($response->getBody()->getContents(), true); + + // Hidden group should be returned for admin + $this->assertEquals('10', Arr::get($data, 'data.id')); + } + + /** + * @test + */ + public function rejects_request_for_non_existing_group() + { + $response = $this->send( + $this->request('GET', '/api/groups/999', [ + 'authenticatedAs' => 1, + ]) + ); + + // If group does not exist in database, controller + // should reject the request with 404 Not found + $this->assertEquals(404, $response->getStatusCode()); + } + + protected function hiddenGroup(): array + { + return [ + 'id' => 10, + 'name_singular' => 'Hidden', + 'name_plural' => 'Ninjas', + 'color' => null, + 'icon' => 'fas fa-wrench', + 'is_hidden' => 1 + ]; + } +}