From 13649e146c602708e4699e96f3b66212959666ce Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Sun, 22 Sep 2024 17:26:02 +0200 Subject: [PATCH] Improve pagination support in API (#5073) --- migrations/Version20240920105524.php | 4 +++ src/API/BaseApiController.php | 41 ++++++++++++++++++++++++++ src/API/InvoiceController.php | 24 ++------------- tests/API/InvoiceControllerTest.php | 19 ++++++++++++ tests/DataFixtures/InvoiceFixtures.php | 5 ++-- 5 files changed, 70 insertions(+), 23 deletions(-) diff --git a/migrations/Version20240920105524.php b/migrations/Version20240920105524.php index 6bf7f7f9..b813ab3d 100644 --- a/migrations/Version20240920105524.php +++ b/migrations/Version20240920105524.php @@ -43,6 +43,10 @@ WHERE kp.value > 0 'value' => 'day', ]); } + + if (\count($ids) === 0) { + $this->preventEmptyMigrationWarning(); + } } public function down(Schema $schema): void diff --git a/src/API/BaseApiController.php b/src/API/BaseApiController.php index 490904cc..ab57f2b0 100644 --- a/src/API/BaseApiController.php +++ b/src/API/BaseApiController.php @@ -13,6 +13,7 @@ use App\Entity\User; use App\Repository\Query\BaseQuery; use App\Timesheet\DateTimeFactory; use App\Utils\Pagination; +use FOS\RestBundle\Request\ParamFetcherInterface; use FOS\RestBundle\View\View; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\Form\Extension\Core\Type\DateTimeType; @@ -52,6 +53,46 @@ abstract class BaseApiController extends AbstractController return DateTimeFactory::createByUser($user); } + protected function prepareQuery(BaseQuery $query, ParamFetcherInterface $paramFetcher): void + { + $query->setIsApiCall(true); + $query->setCurrentUser($this->getUser()); + + // there is no function has() in ParamFetcherInterface, so we need to use all() and check for the key + $all = $paramFetcher->all(true); + + if (\array_key_exists('page', $all)) { + $page = $all['page']; + if (is_numeric($page)) { + $query->setPage((int) $page); + } + } + + if (\array_key_exists('size', $all)) { + $size = $all['size']; + if (is_numeric($size)) { + $query->setPageSize((int) $size); + } + } + + if (\array_key_exists('pageSize', $all)) { + $size = $all['pageSize']; + if (is_numeric($size)) { + $query->setPageSize((int) $size); + } + } + } + + protected function createPaginatedView(Pagination $pagination): View + { + $results = (array) $pagination->getCurrentPageResults(); + + $view = new View($results, 200); + $this->addPagination($view, $pagination); + + return $view; + } + protected function addPagination(View $view, Pagination $pagination): void { $view->setHeader('X-Page', (string) $pagination->getCurrentPage()); diff --git a/src/API/InvoiceController.php b/src/API/InvoiceController.php index 541b4a75..2f1beedc 100644 --- a/src/API/InvoiceController.php +++ b/src/API/InvoiceController.php @@ -10,7 +10,6 @@ namespace App\API; use App\Entity\Invoice; -use App\Entity\User; use App\Repository\CustomerRepository; use App\Repository\InvoiceRepository; use App\Repository\Query\InvoiceArchiveQuery; @@ -39,7 +38,7 @@ final class InvoiceController extends BaseApiController } /** - * Returns a collection of invoices (which are visible to the user) + * Returns a paginated collection of invoices. * * Needs permission: view_invoice */ @@ -54,11 +53,8 @@ final class InvoiceController extends BaseApiController #[Rest\QueryParam(name: 'size', requirements: '\d+', strict: true, nullable: true, description: 'The amount of entries for each page (default: 50)')] public function cgetAction(ParamFetcherInterface $paramFetcher, CustomerRepository $customerRepository): Response { - /** @var User $user */ - $user = $this->getUser(); - $query = new InvoiceArchiveQuery(); - $query->setCurrentUser($user); + $this->prepareQuery($query, $paramFetcher); $factory = $this->getDateTimeFactory(); $begin = $paramFetcher->get('begin'); @@ -79,29 +75,15 @@ final class InvoiceController extends BaseApiController } } - $page = $paramFetcher->get('page'); - if (\is_string($page) && $page !== '') { - $query->setPage((int) $page); - } - - $size = $paramFetcher->get('size'); - if (is_numeric($size)) { - $query->setPageSize((int) $size); - } - /** @var array $customers */ $customers = $paramFetcher->get('customers'); foreach ($customerRepository->findByIds(array_unique($customers)) as $customer) { $query->addCustomer($customer); } - $query->setIsApiCall(true); $data = $this->repository->getPagerfantaForQuery($query); - $results = (array) $data->getCurrentPageResults(); - - $view = new View($results, 200); + $view = $this->createPaginatedView($data); $view->getContext()->setGroups(self::GROUPS_COLLECTION); - $this->addPagination($view, $data); return $this->viewHandler->handle($view); } diff --git a/tests/API/InvoiceControllerTest.php b/tests/API/InvoiceControllerTest.php index 3cd2d414..b8977b38 100644 --- a/tests/API/InvoiceControllerTest.php +++ b/tests/API/InvoiceControllerTest.php @@ -76,6 +76,25 @@ class InvoiceControllerTest extends APIControllerBaseTest self::assertApiResponseTypeStructure('InvoiceCollection', $result[0]); } + public function testGetCollectionWithPagination(): void + { + $client = $this->getClientForAuthenticatedUser(User::ROLE_TEAMLEAD); + $this->importInvoiceFixtures(20); + + $query = ['page' => 2, 'size' => 4]; + $this->assertAccessIsGranted($client, '/api/invoices', 'GET', $query); + + $content = $client->getResponse()->getContent(); + $this->assertIsString($content); + $result = json_decode($content, true); + + $this->assertIsArray($result); + $this->assertNotEmpty($result); + $this->assertEquals(4, \count($result)); + $this->assertPagination($client->getResponse(), 2, 4, 5, 20); + self::assertApiResponseTypeStructure('InvoiceCollection', $result[0]); + } + public function testGetEntityIsSecure(): void { $client = $this->getClientForAuthenticatedUser(); diff --git a/tests/DataFixtures/InvoiceFixtures.php b/tests/DataFixtures/InvoiceFixtures.php index be070759..e910b12f 100644 --- a/tests/DataFixtures/InvoiceFixtures.php +++ b/tests/DataFixtures/InvoiceFixtures.php @@ -57,11 +57,12 @@ class InvoiceFixtures implements TestFixture $invoice->setTax($tax); $invoice->setCustomer($customers[array_rand($customers)]); - $invoice->setInvoiceNumber($i . '_ ' . $faker->text(10)); + $prefix = uniqid($i . '_') . '_'; + $invoice->setInvoiceNumber($prefix . $faker->randomNumber(3)); + $invoice->setFilename($prefix . $faker->randomNumber(3)); $invoice->setCreatedAt($faker->dateTimeBetween('-1 year', 'now')); $invoice->setUser($users[array_rand($users)]); $invoice->setDueDays($faker->randomNumber(2)); - $invoice->setFilename($faker->text(30)); $invoice->setComment($faker->text(300)); $invoice->setCurrency($faker->currencyCode());