diff --git a/module/Core/src/Tag/Model/TagsListFiltering.php b/module/Core/src/Tag/Model/TagsListFiltering.php index 3c4e79d2..8f078788 100644 --- a/module/Core/src/Tag/Model/TagsListFiltering.php +++ b/module/Core/src/Tag/Model/TagsListFiltering.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Tag\Model; +use Shlinkio\Shlink\Core\Model\Ordering; use Shlinkio\Shlink\Rest\Entity\ApiKey; final class TagsListFiltering @@ -12,10 +13,16 @@ final class TagsListFiltering private ?int $limit = null, private ?int $offset = null, private ?string $searchTerm = null, + private ?Ordering $orderBy = null, private ?ApiKey $apiKey = null, ) { } + public static function fromRangeAndParams(int $limit, int $offset, TagsParams $params, ?ApiKey $apiKey): self + { + return new self($limit, $offset, $params->searchTerm(), $params->orderBy(), $apiKey); + } + public function limit(): ?int { return $this->limit; @@ -31,6 +38,11 @@ final class TagsListFiltering return $this->searchTerm; } + public function orderBy(): ?Ordering + { + return $this->orderBy; + } + public function apiKey(): ?ApiKey { return $this->apiKey; diff --git a/module/Core/src/Tag/Model/TagsParams.php b/module/Core/src/Tag/Model/TagsParams.php index 7fd12348..3f40debe 100644 --- a/module/Core/src/Tag/Model/TagsParams.php +++ b/module/Core/src/Tag/Model/TagsParams.php @@ -5,11 +5,19 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Tag\Model; use Shlinkio\Shlink\Core\Model\AbstractInfinitePaginableListParams; +use Shlinkio\Shlink\Core\Model\Ordering; + +use function Shlinkio\Shlink\Common\parseOrderBy; final class TagsParams extends AbstractInfinitePaginableListParams { - private function __construct(private ?string $searchTerm, ?int $page, ?int $itemsPerPage) - { + private function __construct( + private ?string $searchTerm, + private Ordering $orderBy, + private bool $withStats, + ?int $page, + ?int $itemsPerPage, + ) { parent::__construct($page, $itemsPerPage); } @@ -17,6 +25,8 @@ final class TagsParams extends AbstractInfinitePaginableListParams { return new self( $query['searchTerm'] ?? null, + Ordering::fromTuple(isset($query['orderBy']) ? parseOrderBy($query['orderBy']) : [null, null]), + ($query['withStats'] ?? null) === 'true', isset($query['page']) ? (int) $query['page'] : null, isset($query['itemsPerPage']) ? (int) $query['itemsPerPage'] : null, ); @@ -26,4 +36,14 @@ final class TagsParams extends AbstractInfinitePaginableListParams { return $this->searchTerm; } + + public function orderBy(): Ordering + { + return $this->orderBy; + } + + public function withStats(): bool + { + return $this->withStats; + } } diff --git a/module/Core/src/Tag/Paginator/Adapter/TagsInfoPaginatorAdapter.php b/module/Core/src/Tag/Paginator/Adapter/TagsInfoPaginatorAdapter.php index a92a190a..c2917200 100644 --- a/module/Core/src/Tag/Paginator/Adapter/TagsInfoPaginatorAdapter.php +++ b/module/Core/src/Tag/Paginator/Adapter/TagsInfoPaginatorAdapter.php @@ -11,7 +11,7 @@ class TagsInfoPaginatorAdapter extends AbstractTagsPaginatorAdapter public function getSlice(int $offset, int $length): iterable { return $this->repo->findTagsWithInfo( - new TagsListFiltering($length, $offset, $this->params->searchTerm(), $this->apiKey), + TagsListFiltering::fromRangeAndParams($length, $offset, $this->params, $this->apiKey), ); } } diff --git a/module/Core/src/Tag/Paginator/Adapter/TagsPaginatorAdapter.php b/module/Core/src/Tag/Paginator/Adapter/TagsPaginatorAdapter.php index 5b1da6f7..d6bc0b7b 100644 --- a/module/Core/src/Tag/Paginator/Adapter/TagsPaginatorAdapter.php +++ b/module/Core/src/Tag/Paginator/Adapter/TagsPaginatorAdapter.php @@ -13,7 +13,10 @@ class TagsPaginatorAdapter extends AbstractTagsPaginatorAdapter { $conditions = [ new WithApiKeySpecsEnsuringJoin($this->apiKey), - Spec::orderBy('name'), + Spec::orderBy( + 'name', // Ordering by other fields makes no sense here + $this->params->orderBy()->orderDirection(), + ), Spec::limit($length), Spec::offset($offset), ]; diff --git a/module/Core/test-db/Tag/Paginator/Adapter/TagsPaginatorAdapterTest.php b/module/Core/test-db/Tag/Paginator/Adapter/TagsPaginatorAdapterTest.php index db3e444a..f8cd2e8e 100644 --- a/module/Core/test-db/Tag/Paginator/Adapter/TagsPaginatorAdapterTest.php +++ b/module/Core/test-db/Tag/Paginator/Adapter/TagsPaginatorAdapterTest.php @@ -10,6 +10,8 @@ use Shlinkio\Shlink\Core\Tag\Model\TagsParams; use Shlinkio\Shlink\Core\Tag\Paginator\Adapter\TagsPaginatorAdapter; use Shlinkio\Shlink\TestUtils\DbTest\DatabaseTestCase; +use function Functional\map; + class TagsPaginatorAdapterTest extends DatabaseTestCase { private TagRepository $repo; @@ -25,9 +27,10 @@ class TagsPaginatorAdapterTest extends DatabaseTestCase */ public function expectedListOfTagsIsReturned( ?string $searchTerm, + ?string $orderBy, int $offset, int $length, - int $expectedSliceSize, + array $expectedTags, int $expectedTotalCount, ): void { $names = ['foo', 'bar', 'baz', 'another']; @@ -36,22 +39,31 @@ class TagsPaginatorAdapterTest extends DatabaseTestCase } $this->getEntityManager()->flush(); - $adapter = new TagsPaginatorAdapter($this->repo, TagsParams::fromRawData(['searchTerm' => $searchTerm]), null); + $adapter = new TagsPaginatorAdapter($this->repo, TagsParams::fromRawData([ + 'searchTerm' => $searchTerm, + 'orderBy' => $orderBy, + ]), null); - self::assertCount($expectedSliceSize, $adapter->getSlice($offset, $length)); + $tagNames = map($adapter->getSlice($offset, $length), static fn (Tag $tag) => $tag->__toString()); + + self::assertEquals($expectedTags, $tagNames); self::assertEquals($expectedTotalCount, $adapter->getNbResults()); } public function provideFilters(): iterable { - yield [null, 0, 10, 4, 4]; - yield [null, 2, 10, 2, 4]; - yield [null, 1, 3, 3, 4]; - yield [null, 3, 3, 1, 4]; - yield [null, 0, 2, 2, 4]; - yield ['ba', 0, 10, 2, 2]; - yield ['ba', 0, 1, 1, 2]; - yield ['foo', 0, 10, 1, 1]; - yield ['a', 0, 10, 3, 3]; + yield [null, null, 0, 10, ['another', 'bar', 'baz', 'foo'], 4]; + yield [null, null, 2, 10, ['baz', 'foo'], 4]; + yield [null, null, 1, 3, ['bar', 'baz', 'foo'], 4]; + yield [null, null, 3, 3, ['foo'], 4]; + yield [null, null, 0, 2, ['another', 'bar'], 4]; + yield ['ba', null, 0, 10, ['bar', 'baz'], 2]; + yield ['ba', null, 0, 1, ['bar'], 2]; + yield ['foo', null, 0, 10, ['foo'], 1]; + yield ['a', null, 0, 10, ['another', 'bar', 'baz'], 3]; + yield [null, 'name-DESC', 0, 10, ['foo', 'baz', 'bar', 'another'], 4]; + yield [null, 'name-ASC', 0, 10, ['another', 'bar', 'baz', 'foo'], 4]; + yield [null, 'name-DESC', 0, 2, ['foo', 'baz'], 4]; + yield ['ba', 'name-DESC', 0, 1, ['baz'], 2]; } } diff --git a/module/Core/test/Tag/TagServiceTest.php b/module/Core/test/Tag/TagServiceTest.php index 0eb59df0..c3efc4b5 100644 --- a/module/Core/test/Tag/TagServiceTest.php +++ b/module/Core/test/Tag/TagServiceTest.php @@ -83,26 +83,26 @@ class TagServiceTest extends TestCase { yield 'no API key, no filter' => [ null, - TagsParams::fromRawData([]), - new TagsListFiltering(2, 0, null, null), + $params = TagsParams::fromRawData([]), + TagsListFiltering::fromRangeAndParams(2, 0, $params, null), 1, ]; yield 'admin API key, no filter' => [ $apiKey = ApiKey::create(), - TagsParams::fromRawData([]), - new TagsListFiltering(2, 0, null, $apiKey), + $params = TagsParams::fromRawData([]), + TagsListFiltering::fromRangeAndParams(2, 0, $params, $apiKey), 1, ]; yield 'no API key, search term' => [ null, - TagsParams::fromRawData(['searchTerm' => $searchTerm = 'foobar']), - new TagsListFiltering(2, 0, $searchTerm, null), + $params = TagsParams::fromRawData(['searchTerm' => 'foobar']), + TagsListFiltering::fromRangeAndParams(2, 0, $params, null), 1, ]; yield 'admin API key, limits' => [ $apiKey = ApiKey::create(), - TagsParams::fromRawData(['page' => 1, 'itemsPerPage' => 1]), - new TagsListFiltering(1, 0, null, $apiKey), + $params = TagsParams::fromRawData(['page' => 1, 'itemsPerPage' => 1]), + TagsListFiltering::fromRangeAndParams(1, 0, $params, $apiKey), 0, ]; } diff --git a/module/Rest/src/Action/Tag/ListTagsAction.php b/module/Rest/src/Action/Tag/ListTagsAction.php index c4e0e0de..ecf379cb 100644 --- a/module/Rest/src/Action/Tag/ListTagsAction.php +++ b/module/Rest/src/Action/Tag/ListTagsAction.php @@ -30,11 +30,10 @@ class ListTagsAction extends AbstractRestAction public function handle(ServerRequestInterface $request): ResponseInterface { $query = $request->getQueryParams(); - $withStats = ($query['withStats'] ?? null) === 'true'; - $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); $params = TagsParams::fromRawData($query); + $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); - if (! $withStats) { + if (! $params->withStats()) { return new JsonResponse([ 'tags' => $this->serializePaginator($this->tagService->listTags($params, $apiKey)), ]);