Merge pull request #8284 from pmattmann/feature/material-items-endpoint-require-filter

MaterialItem-Endpoint requires filter on camp, period, materialList or materialNode

Author: pmattmann

api/config/services.yaml

@@ -85,6 +85,14 @@ services:
         arguments:
             - '@api_platform.doctrine.orm.state.collection_provider'
 
+    App\State\ChecklistItemCollectionProvider:
+        arguments:
+            - '@api_platform.doctrine.orm.state.collection_provider'
+            
+    App\State\MaterialItemCollectionProvider:
+        arguments:
+            - '@api_platform.doctrine.orm.state.collection_provider'
+
     App\Serializer\SerializerContextBuilder:
         decorates: 'api_platform.serializer.context_builder'
 

api/src/Entity/ChecklistItem.php

@@ -15,6 +15,7 @@
 use App\Entity\ContentNode\ChecklistNode;
 use App\InputFilter;
 use App\Repository\ChecklistItemRepository;
+use App\State\ChecklistItemCollectionProvider;
 use App\Util\EntityMap;
 use App\Validator\AssertNoLoop;
 use App\Validator\ChecklistItem\AssertBelongsToSameChecklist;
@@ -49,7 +50,8 @@
             validationContext: ['groups' => ['delete']],
         ),
         new GetCollection(
-            security: 'is_authenticated()'
+            security: 'is_authenticated()',
+            provider: ChecklistItemCollectionProvider::class
         ),
         new Post(
             denormalizationContext: ['groups' => ['write', 'create']],

api/src/Entity/MaterialItem.php

@@ -15,6 +15,7 @@
 use App\Entity\ContentNode\MaterialNode;
 use App\InputFilter;
 use App\Repository\MaterialItemRepository;
+use App\State\MaterialItemCollectionProvider;
 use App\State\MaterialItemCreateProcessor;
 use App\Util\EntityMap;
 use App\Validator\AssertBelongsToSameCamp;
@@ -43,7 +44,8 @@
             security: 'is_granted("CAMP_MEMBER", object) or is_granted("CAMP_MANAGER", object)'
         ),
         new GetCollection(
-            security: 'is_authenticated()'
+            security: 'is_authenticated()',
+            provider: MaterialItemCollectionProvider::class
         ),
         new Post(
             processor: MaterialItemCreateProcessor::class,

api/src/State/ChecklistItemCollectionProvider.php

@@ -0,0 +1,30 @@
+<?php
+
+namespace App\State;
+
+use ApiPlatform\Metadata\Operation;
+use ApiPlatform\State\ProviderInterface;
+use App\Entity\ChecklistItem;
+use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
+
+/**
+ * @template-implements ProviderInterface<ChecklistItem>
+ */
+class ChecklistItemCollectionProvider implements ProviderInterface {
+    public function __construct(
+        private ProviderInterface $decorated,
+        private RequestStack $requestStack
+    ) {}
+
+    public function provide(Operation $operation, array $uriVariables = [], array $context = []): array|object|null {
+        $request = $this->requestStack->getCurrentRequest();
+        $hasFilter = $request?->query->has('checklist') || $request?->query->has('checklist_camp') || $request?->query->has('checklistNodes');
+
+        if (!$hasFilter) {
+            throw new BadRequestHttpException('Filter on checklist, checklist.camp or checklistNodes is required');
+        }
+
+        return $this->decorated->provide($operation, $uriVariables, $context);
+    }
+}

api/src/State/MaterialItemCollectionProvider.php

@@ -0,0 +1,30 @@
+<?php
+
+namespace App\State;
+
+use ApiPlatform\Metadata\Operation;
+use ApiPlatform\State\ProviderInterface;
+use App\Entity\MaterialItem;
+use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
+
+/**
+ * @template-implements ProviderInterface<MaterialItem>
+ */
+class MaterialItemCollectionProvider implements ProviderInterface {
+    public function __construct(
+        private ProviderInterface $decorated,
+        private RequestStack $requestStack
+    ) {}
+
+    public function provide(Operation $operation, array $uriVariables = [], array $context = []): array|object|null {
+        $request = $this->requestStack->getCurrentRequest();
+        $hasFilter = $request?->query->has('camp') || $request?->query->has('period') || $request?->query->has('materialList') || $request?->query->has('materialNode');
+
+        if (!$hasFilter) {
+            throw new BadRequestHttpException('Filter on camp, period, materialList or materialNode is required');
+        }
+
+        return $this->decorated->provide($operation, $uriVariables, $context);
+    }
+}

api/tests/Api/ChecklistItems/ListChecklistItemsTest.php

@@ -17,30 +17,12 @@ public function testListChecklistItemsIsDeniedForAnonymousUser() {
         ]);
     }
 
-    public function testListChecklistItemsIsAllowedForLoggedInUserButFiltered() {
+    public function testListChecklistItemsWithoutFilterIsNotAllowedForLoggedInUser() {
         // precondition: There is a checklist-item that the user doesn't have access to
         $this->assertNotEmpty(static::$fixtures['checklistItemUnrelated_1_1']);
 
         $response = static::createClientWithCredentials()->request('GET', '/checklist_items');
-        $this->assertResponseStatusCodeSame(200);
-        $this->assertJsonContains([
-            'totalItems' => 7,
-            '_links' => [
-                'items' => [],
-            ],
-            '_embedded' => [
-                'items' => [],
-            ],
-        ]);
-        $this->assertEqualsCanonicalizing([
-            ['href' => $this->getIriFor('checklistItem1_1_1')],
-            ['href' => $this->getIriFor('checklistItem1_1_2')],
-            ['href' => $this->getIriFor('checklistItem1_1_2_3')],
-            ['href' => $this->getIriFor('checklistItem1_1_2_3_4')],
-            ['href' => $this->getIriFor('checklistItem2_1_1')],
-            ['href' => $this->getIriFor('checklistItemCampPrototype_1_1')],
-            ['href' => $this->getIriFor('checklistItemCampShared_1_1')],
-        ], $response->toArray()['_links']['items']);
+        $this->assertResponseStatusCodeSame(400);
     }
 
     public function testListChecklistItemsFilteredByChecklistIsAllowedForCollaborator() {

api/tests/Api/MaterialItems/ListMaterialItemsTest.php

@@ -17,28 +17,12 @@ public function testListMaterialItemsIsDeniedForAnonymousUser() {
         ]);
     }
 
-    public function testListMaterialItemsIsAllowedForLoggedInUserButFiltered() {
+    public function testListMaterialItemsWithoutFilterIsNotAllowedForLoggedInUser() {
         // precondition: There is a material item that the user doesn't have access to
         $this->assertNotEmpty(static::$fixtures['materialItem1period1campUnrelated']);
 
         $response = static::createClientWithCredentials()->request('GET', '/material_items');
-        $this->assertResponseStatusCodeSame(200);
-        $this->assertJsonContains([
-            'totalItems' => 5,
-            '_links' => [
-                'items' => [],
-            ],
-            '_embedded' => [
-                'items' => [],
-            ],
-        ]);
-        $this->assertEqualsCanonicalizing([
-            ['href' => $this->getIriFor('materialItem1')],
-            ['href' => $this->getIriFor('materialItem1period1')],
-            ['href' => $this->getIriFor('materialItem1period1camp2')],
-            ['href' => $this->getIriFor('materialItem1period1campPrototype')],
-            ['href' => $this->getIriFor('materialItem1period1campShared')],
-        ], $response->toArray()['_links']['items']);
+        $this->assertResponseStatusCodeSame(400);
     }
 
     public function testListMaterialItemsFilteredByMaterialListIsAllowedForCollaborator() {

api/tests/Api/SnapshotTests/EndpointPerformanceTest.php

@@ -243,7 +243,9 @@ private static function getCollectionEndpoints() {
                 '/auth/reset_password' => false,
                 '/auth/resend_activation' => false,
                 '/content_nodes' => false,
+                '/checklist_items' => false,
                 '/invitations' => false,
+                '/material_items' => false,
                 '/personal_invitations' => false,
                 '/token/refresh' => false,
                 default => true

api/tests/Api/SnapshotTests/ResponseSnapshotTest.php

@@ -81,6 +81,7 @@ public function testOpenApiSpecMatchesSnapshot() {
      * @throws TransportExceptionInterface
      */
     #[DataProvider('getCollectionEndpoints')]
+    #[DataProvider('getCollectionEndpointsFiltered')]
     public function testGetCollectionMatchesStructure(Client $client, string $endpoint) {
         $response = $client->request('GET', $endpoint);
 
@@ -116,7 +117,9 @@ public static function getCollectionEndpoints() {
                 '/auth/reset_password' => false,
                 '/auth/resend_activation' => false,
                 '/content_nodes' => false,
+                '/checklist_items' => false,
                 '/invitations' => false,
+                '/material_items' => false,
                 '/personal_invitations' => false,
                 '/token/refresh' => false,
                 '/users' => false,
@@ -135,6 +138,26 @@ public static function getCollectionEndpoints() {
         return $withUrlAsKey;
     }
 
+    /**
+     * @throws RedirectionExceptionInterface
+     * @throws DecodingExceptionInterface
+     * @throws ClientExceptionInterface
+     * @throws TransportExceptionInterface
+     * @throws ServerExceptionInterface
+     */
+    public static function getCollectionEndpointsFiltered() {
+        static::bootKernel();
+        $client = static::createClientWithCredentials();
+        $client->disableReboot();
+        $response = $client->request('GET', '/');
+
+        return [
+            [$client, '/content_nodes?camp=/camps/'.self::getFixtureFor('/camps')->getId()],
+            [$client, '/checklist_items?checklist=/checklists/'.self::getFixtureFor('/checklists')->getId()],
+            [$client, '/material_items?camp=/camps/'.self::getFixtureFor('/camps')->getId()],
+        ];
+    }
+
     /**
      * @throws ClientExceptionInterface
      * @throws DecodingExceptionInterface
@@ -145,7 +168,7 @@ public static function getCollectionEndpoints() {
     #[DataProvider('getItemEndpoints')]
     public function testGetItemMatchesStructure(Client $client, string $endpoint) {
         /** @var BaseEntity $fixtureFor */
-        $fixtureFor = $this->getFixtureFor($endpoint);
+        $fixtureFor = self::getFixtureFor($endpoint);
 
         $itemResponse = $client->request('GET', "{$endpoint}/{$fixtureFor->getId()}");
 
@@ -163,7 +186,6 @@ public function testGetItemMatchesStructure(Client $client, string $endpoint) {
     public static function getItemEndpoints() {
         return array_filter(self::getCollectionEndpoints(), function (array $endpoint) {
             return match ($endpoint[1]) {
-                '/content_nodes' => false,
                 default => true,
             };
         });
@@ -179,7 +201,7 @@ public static function getItemEndpoints() {
     #[DataProvider('getPatchEndpoints')]
     public function testPatchResponseMatchesGetItemResponse(Client $client, string $endpoint) {
         /** @var BaseEntity $fixtureFor */
-        $fixtureFor = $this->getFixtureFor($endpoint);
+        $fixtureFor = self::getFixtureFor($endpoint);
 
         $itemResponse = $client->request('GET', "{$endpoint}/{$fixtureFor->getId()}");
         assertThat($itemResponse->getStatusCode(), equalTo(200));
@@ -238,7 +260,7 @@ public static function getPatchEndpoints() {
     #[TestWith(['/periods', '/days'], '/periods_{campId}_days')]
     #[TestWith(['/periods', '/schedule_entries'], '/periods_{campId}_schedule_entries')]
     public function testSubResourceUrlMatchesSnapshot(string $endpoint, string $subresource) {
-        $fixture = $this->getFixtureFor($endpoint);
+        $fixture = self::getFixtureFor($endpoint);
         $uri = "{$endpoint}/{$fixture->getId()}{$subresource}";
 
         $response = static::createClientWithCredentials()->request('GET', $uri);
@@ -247,7 +269,7 @@ public function testSubResourceUrlMatchesSnapshot(string $endpoint, string $subr
         $this->assertMatchesEscapedResponseSnapshot($response);
     }
 
-    private function getFixtureFor(string $collectionEndpoint) {
+    private static function getFixtureFor(string $collectionEndpoint) {
         $fixtures = FixtureStore::getFixtures();
 
         return ReadItemFixtureMap::get($collectionEndpoint, $fixtures);