IBX-9266: Review remarks

Author: barw4

src/bundle/Controller/ContentTypeFieldsByExpressionController.php

@@ -8,10 +8,13 @@
 
 namespace Ibexa\Bundle\AdminUi\Controller;
 
+use Ibexa\AdminUi\Exception\FieldTypeExpressionParserException;
 use Ibexa\AdminUi\REST\Value\ContentType\FieldDefinitionInfoList;
 use Ibexa\Contracts\AdminUi\ContentType\ContentTypeFieldsByExpressionServiceInterface;
 use Ibexa\Rest\Message;
 use Ibexa\Rest\Server\Controller as RestController;
+use Ibexa\Rest\Server\Exceptions\BadRequestException;
+use LogicException;
 use Symfony\Component\HttpFoundation\Request;
 
 final class ContentTypeFieldsByExpressionController extends RestController
@@ -36,8 +39,12 @@ public function loadFieldDefinitionsFromExpression(Request $request): FieldDefin
             )
         );
 
-        return new FieldDefinitionInfoList(
-            $this->fieldsByExpressionService->getFieldsFromExpression($input->expression),
-        );
+        try {
+            $fieldDefinitions = $this->fieldsByExpressionService->getFieldsFromExpression($input->expression);
+        } catch (FieldTypeExpressionParserException | LogicException $e) {
+            throw new BadRequestException($e->getMessage());
+        }
+
+        return new FieldDefinitionInfoList($fieldDefinitions);
     }
 }

src/bundle/Resources/config/services/controllers.yaml

@@ -261,7 +261,6 @@ services:
 
     Ibexa\Bundle\AdminUi\Controller\ContentTypeFieldsByExpressionController:
         parent: Ibexa\Rest\Server\Controller
-        autowire: true
         arguments:
             $fieldsByExpressionService: '@Ibexa\Contracts\AdminUi\ContentType\ContentTypeFieldsByExpressionServiceInterface'
         tags:

src/contracts/ContentType/ContentTypeFieldsByExpressionServiceInterface.php

@@ -16,6 +16,8 @@ interface ContentTypeFieldsByExpressionServiceInterface
      * @return list<\Ibexa\Contracts\Core\Repository\Values\ContentType\FieldDefinition>
      *
      * @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException
+     * @throws \Ibexa\AdminUi\Exception\FieldTypeExpressionParserException
+     * @throws \LogicException
      */
     public function getFieldsFromExpression(string $expression): array;
 

src/lib/REST/Output/ValueObjectVisitor/ContentType/FieldDefinitionInfoList.php

@@ -28,14 +28,9 @@ public function visit(Visitor $visitor, Generator $generator, $data): void
         foreach ($fieldDefinitionList->fieldDefinitions as $fieldDefinition) {
             $generator->startObjectElement('FieldDefinitionInfo');
 
-            $generator->startValueElement('id', $fieldDefinition->id);
-            $generator->endValueElement('id');
-
-            $generator->startValueElement('identifier', $fieldDefinition->identifier);
-            $generator->endValueElement('identifier');
-
-            $generator->startValueElement('position', $fieldDefinition->position);
-            $generator->endValueElement('position');
+            $generator->valueElement('id', $fieldDefinition->id);
+            $generator->valueElement('identifier', $fieldDefinition->identifier);
+            $generator->valueElement('position', $fieldDefinition->position);
 
             $this->visitNamesList($generator, $fieldDefinition->getNames());
 

src/lib/Util/ContentTypeFieldsExtractorInterface.php

@@ -14,6 +14,8 @@ interface ContentTypeFieldsExtractorInterface
      * @return list<int>
      *
      * @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException
+     * @throws \Ibexa\AdminUi\Exception\FieldTypeExpressionParserException
+     * @throws \LogicException
      */
     public function extractFieldsFromExpression(string $expression): array;
 

tests/integration/REST/PostLoadFieldDefinitionsFromExpression.php renamed to tests/integration/REST/PostLoadFieldDefinitionsFromExpressionTest.php

@@ -14,7 +14,7 @@
 /**
  * Coverage for /content-type/load-field-definitions-from-expression REST endpoint.
  */
-final class PostLoadFieldDefinitionsFromExpression extends BaseAdminUiRestWebTestCase
+final class PostLoadFieldDefinitionsFromExpressionTest extends BaseAdminUiRestWebTestCase
 {
     private const INPUT_MEDIA_TYPE = 'FieldDefinitionExpression';