From 9ba050600be9b8773af8bfcaf3d1e90cb35f011d Mon Sep 17 00:00:00 2001 From: boo-code Date: Sun, 28 Jun 2026 18:54:18 +0200 Subject: [PATCH 1/4] Add Attachment Admin API endpoints Expose CRUD for attachments: GET/POST/PATCH/DELETE /attachments. The create and update operations accept a multipart file upload alongside the localized name and description. Contributes to #39630. --- .../Resources/Attachment/Attachment.php | 133 ++++++++++++++++ .../ApiPlatform/AttachmentEndpointTest.php | 145 ++++++++++++++++++ 2 files changed, 278 insertions(+) create mode 100644 src/ApiPlatform/Resources/Attachment/Attachment.php create mode 100644 tests/Integration/ApiPlatform/AttachmentEndpointTest.php diff --git a/src/ApiPlatform/Resources/Attachment/Attachment.php b/src/ApiPlatform/Resources/Attachment/Attachment.php new file mode 100644 index 000000000..98455002e --- /dev/null +++ b/src/ApiPlatform/Resources/Attachment/Attachment.php @@ -0,0 +1,133 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/AFL-3.0 Academic Free License version 3.0 + */ + +declare(strict_types=1); + +namespace PrestaShop\Module\APIResources\ApiPlatform\Resources\Attachment; + +use ApiPlatform\Metadata\ApiProperty; +use ApiPlatform\Metadata\ApiResource; +use PrestaShop\PrestaShop\Core\Domain\Attachment\Command\AddAttachmentCommand; +use PrestaShop\PrestaShop\Core\Domain\Attachment\Command\DeleteAttachmentCommand; +use PrestaShop\PrestaShop\Core\Domain\Attachment\Command\EditAttachmentCommand; +use PrestaShop\PrestaShop\Core\Domain\Attachment\Exception\AttachmentConstraintException; +use PrestaShop\PrestaShop\Core\Domain\Attachment\Exception\AttachmentNotFoundException; +use PrestaShop\PrestaShop\Core\Domain\Attachment\Query\GetAttachmentForEditing; +use PrestaShopBundle\ApiPlatform\Metadata\CQRSCreate; +use PrestaShopBundle\ApiPlatform\Metadata\CQRSDelete; +use PrestaShopBundle\ApiPlatform\Metadata\CQRSGet; +use PrestaShopBundle\ApiPlatform\Metadata\CQRSPartialUpdate; +use PrestaShopBundle\ApiPlatform\Metadata\LocalizedValue; +use Symfony\Component\HttpFoundation\File\File; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Serializer\Normalizer\ObjectNormalizer; +use Symfony\Component\Validator\Constraints as Assert; + +#[ApiResource( + operations: [ + new CQRSGet( + uriTemplate: '/attachments/{attachmentId}', + requirements: ['attachmentId' => '\d+'], + CQRSQuery: GetAttachmentForEditing::class, + scopes: ['attachment_read'], + CQRSQueryMapping: self::QUERY_MAPPING, + ), + new CQRSCreate( + uriTemplate: '/attachments', + validationContext: ['groups' => ['Default', 'Create']], + CQRSCommand: AddAttachmentCommand::class, + CQRSQuery: GetAttachmentForEditing::class, + scopes: ['attachment_write'], + inputFormats: ['multipart' => ['multipart/form-data']], + // Form data values are all strings so we disable type enforcement. + denormalizationContext: [ObjectNormalizer::DISABLE_TYPE_ENFORCEMENT => true], + CQRSQueryMapping: self::QUERY_MAPPING, + CQRSCommandMapping: self::CREATE_COMMAND_MAPPING, + ), + new CQRSPartialUpdate( + uriTemplate: '/attachments/{attachmentId}', + requirements: ['attachmentId' => '\d+'], + read: false, + CQRSCommand: EditAttachmentCommand::class, + CQRSQuery: GetAttachmentForEditing::class, + scopes: ['attachment_write'], + inputFormats: ['multipart' => ['multipart/form-data']], + denormalizationContext: [ObjectNormalizer::DISABLE_TYPE_ENFORCEMENT => true], + CQRSQueryMapping: self::QUERY_MAPPING, + CQRSCommandMapping: self::UPDATE_COMMAND_MAPPING, + ), + new CQRSDelete( + uriTemplate: '/attachments/{attachmentId}', + requirements: ['attachmentId' => '\d+'], + CQRSCommand: DeleteAttachmentCommand::class, + scopes: ['attachment_write'], + ), + ], + normalizationContext: ['skip_null_values' => false], + exceptionToStatus: [ + AttachmentNotFoundException::class => Response::HTTP_NOT_FOUND, + AttachmentConstraintException::class => Response::HTTP_UNPROCESSABLE_ENTITY, + ], +)] +class Attachment +{ + #[ApiProperty(identifier: true)] + public int $attachmentId; + + #[LocalizedValue] + #[Assert\NotBlank(groups: ['Create'])] + public array $names; + + #[LocalizedValue] + public array $descriptions; + + public ?File $file; + + public string $fileName; + + public const QUERY_MAPPING = [ + '[name]' => '[names]', + '[description]' => '[descriptions]', + ]; + + // AddAttachmentCommand takes the localized names/descriptions as constructor arguments and the + // file information through the multi-parameter setter setFileInformation(pathName, fileSize, + // mimeType, originalName). The mapper fills that setter from a nested "fileInformation" object + // whose keys match the parameter names, reading them from the uploaded file. + public const CREATE_COMMAND_MAPPING = [ + '[names]' => '[localizedNames]', + '[descriptions]' => '[localizedDescriptions]', + '[file].pathName' => '[fileInformation][pathName]', + '[file].size' => '[fileInformation][fileSize]', + '[file].mimeType' => '[fileInformation][mimeType]', + '[file].clientOriginalName' => '[fileInformation][originalName]', + ]; + + // EditAttachmentCommand exposes the file through setFileInfo(pathName, mimeType, originalFileName, + // fileSize) - a different setter name and parameter order than the add command. + public const UPDATE_COMMAND_MAPPING = [ + '[names]' => '[localizedNames]', + '[descriptions]' => '[localizedDescriptions]', + '[file].pathName' => '[fileInfo][pathName]', + '[file].mimeType' => '[fileInfo][mimeType]', + '[file].clientOriginalName' => '[fileInfo][originalFileName]', + '[file].size' => '[fileInfo][fileSize]', + ]; +} diff --git a/tests/Integration/ApiPlatform/AttachmentEndpointTest.php b/tests/Integration/ApiPlatform/AttachmentEndpointTest.php new file mode 100644 index 000000000..5aee195f5 --- /dev/null +++ b/tests/Integration/ApiPlatform/AttachmentEndpointTest.php @@ -0,0 +1,145 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/AFL-3.0 Academic Free License version 3.0 + */ + +declare(strict_types=1); + +namespace PsApiResourcesTest\Integration\ApiPlatform; + +use Symfony\Component\HttpFoundation\Response; +use Tests\Resources\DatabaseDump; + +class AttachmentEndpointTest extends ApiTestCase +{ + public static function setUpBeforeClass(): void + { + parent::setUpBeforeClass(); + DatabaseDump::restoreTables(['attachment', 'attachment_lang']); + } + + public static function tearDownAfterClass(): void + { + parent::tearDownAfterClass(); + DatabaseDump::restoreTables(['attachment', 'attachment_lang']); + } + + public static function getProtectedEndpoints(): iterable + { + yield 'create endpoint' => [ + 'POST', + '/attachments', + 'multipart/form-data', + ]; + + yield 'get endpoint' => [ + 'GET', + '/attachments/1', + ]; + + yield 'update endpoint' => [ + 'PATCH', + '/attachments/1', + 'multipart/form-data', + ]; + + yield 'delete endpoint' => [ + 'DELETE', + '/attachments/1', + ]; + } + + public function testAddAttachment(): int + { + $uploadedFile = $this->prepareUploadedFile(__DIR__ . '/../../Resources/assets/image/Hummingbird_cushion.jpg'); + + $attachment = $this->requestApi('POST', '/attachments', null, ['attachment_write'], Response::HTTP_CREATED, [ + 'headers' => [ + 'content-type' => 'multipart/form-data', + ], + 'extra' => [ + 'files' => [ + 'file' => $uploadedFile, + ], + 'parameters' => [ + 'names' => [ + 'en-US' => 'Documentation', + ], + 'descriptions' => [ + 'en-US' => 'A product documentation file', + ], + ], + ], + ]); + + $this->assertArrayHasKey('attachmentId', $attachment); + $this->assertIsInt($attachment['attachmentId']); + $this->assertGreaterThan(0, $attachment['attachmentId']); + + return $attachment['attachmentId']; + } + + /** + * @depends testAddAttachment + */ + public function testGetAttachment(int $attachmentId): int + { + $attachment = $this->getItem('/attachments/' . $attachmentId, ['attachment_read']); + + $this->assertArrayHasKey('names', $attachment); + $this->assertArrayHasKey('fileName', $attachment); + $this->assertSame('Documentation', reset($attachment['names'])); + + return $attachmentId; + } + + /** + * @depends testGetAttachment + */ + public function testUpdateAttachment(int $attachmentId): int + { + $attachment = $this->requestApi('PATCH', '/attachments/' . $attachmentId, null, ['attachment_write'], Response::HTTP_OK, [ + 'headers' => [ + 'content-type' => 'multipart/form-data', + ], + 'extra' => [ + 'parameters' => [ + 'names' => [ + 'en-US' => 'Updated documentation', + ], + 'descriptions' => [ + 'en-US' => 'An updated description', + ], + ], + ], + ]); + + $this->assertSame('Updated documentation', reset($attachment['names'])); + + return $attachmentId; + } + + /** + * @depends testUpdateAttachment + */ + public function testDeleteAttachment(int $attachmentId): void + { + $this->requestApi('DELETE', '/attachments/' . $attachmentId, null, ['attachment_write'], Response::HTTP_NO_CONTENT); + $this->getItem('/attachments/' . $attachmentId, ['attachment_read'], Response::HTTP_NOT_FOUND); + } +} From e92fa04fe5d0ad7e1dbaead750b07e49c93d93f3 Mon Sep 17 00:00:00 2001 From: boo-code Date: Mon, 29 Jun 2026 10:29:11 +0200 Subject: [PATCH 2/4] Add attachment list and bulk-delete endpoints Complete the attachment Admin API resource with the two operations that were missing alongside the CRUD endpoints: a paginated list sourced from the attachment grid, and a bulk-delete endpoint. These port the AttachmentList and BulkAttachments resources from #105, adapted to the current conventions (strict types, camelCase fileSize with ApiResourceMapping, CQRSDelete bulk operation, Count(min:1) validation). The integration test is extended to cover the list (field and filter mapping) and bulk-delete (including the empty-payload validation error). Co-authored-by: Codencode <2463965+Codencode@users.noreply.github.com> --- .../Resources/Attachment/AttachmentList.php | 63 ++++++++++++++ .../Resources/Attachment/BulkAttachments.php | 56 ++++++++++++ .../ApiPlatform/AttachmentEndpointTest.php | 85 +++++++++++++++++++ 3 files changed, 204 insertions(+) create mode 100644 src/ApiPlatform/Resources/Attachment/AttachmentList.php create mode 100644 src/ApiPlatform/Resources/Attachment/BulkAttachments.php diff --git a/src/ApiPlatform/Resources/Attachment/AttachmentList.php b/src/ApiPlatform/Resources/Attachment/AttachmentList.php new file mode 100644 index 000000000..831dbe6bf --- /dev/null +++ b/src/ApiPlatform/Resources/Attachment/AttachmentList.php @@ -0,0 +1,63 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/AFL-3.0 Academic Free License version 3.0 + */ + +declare(strict_types=1); + +namespace PrestaShop\Module\APIResources\ApiPlatform\Resources\Attachment; + +use ApiPlatform\Metadata\ApiProperty; +use ApiPlatform\Metadata\ApiResource; +use PrestaShop\PrestaShop\Core\Search\Filters\AttachmentFilters; +use PrestaShopBundle\ApiPlatform\Metadata\PaginatedList; + +#[ApiResource( + operations: [ + new PaginatedList( + uriTemplate: '/attachments', + scopes: [ + 'attachment_read', + ], + ApiResourceMapping: self::MAPPING, + gridDataFactory: 'prestashop.core.grid.data_factory.attachment_decorator', + filtersClass: AttachmentFilters::class, + filtersMapping: [ + '[attachmentId]' => '[id_attachment]', + ], + ), + ] +)] +class AttachmentList +{ + #[ApiProperty(identifier: true)] + public int $attachmentId; + + public string $name; + + public string $file; + + public string $fileSize; + + public string $products; + + public const MAPPING = [ + '[id_attachment]' => '[attachmentId]', + '[file_size]' => '[fileSize]', + ]; +} diff --git a/src/ApiPlatform/Resources/Attachment/BulkAttachments.php b/src/ApiPlatform/Resources/Attachment/BulkAttachments.php new file mode 100644 index 000000000..a34260645 --- /dev/null +++ b/src/ApiPlatform/Resources/Attachment/BulkAttachments.php @@ -0,0 +1,56 @@ + + * @copyright Since 2007 PrestaShop SA and Contributors + * @license https://opensource.org/licenses/AFL-3.0 Academic Free License version 3.0 + */ + +declare(strict_types=1); + +namespace PrestaShop\Module\APIResources\ApiPlatform\Resources\Attachment; + +use ApiPlatform\Metadata\ApiProperty; +use ApiPlatform\Metadata\ApiResource; +use PrestaShop\PrestaShop\Core\Domain\Attachment\Command\BulkDeleteAttachmentsCommand; +use PrestaShop\PrestaShop\Core\Domain\Attachment\Exception\AttachmentNotFoundException; +use PrestaShopBundle\ApiPlatform\Metadata\CQRSDelete; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Validator\Constraints as Assert; + +#[ApiResource( + operations: [ + new CQRSDelete( + uriTemplate: '/attachments/bulk-delete', + CQRSCommand: BulkDeleteAttachmentsCommand::class, + scopes: [ + 'attachment_write', + ], + ), + ], + exceptionToStatus: [ + AttachmentNotFoundException::class => Response::HTTP_NOT_FOUND, + ], +)] +class BulkAttachments +{ + /** + * @var int[] + */ + #[ApiProperty(openapiContext: ['type' => 'array', 'items' => ['type' => 'integer'], 'example' => [1, 3]])] + #[Assert\NotBlank] + #[Assert\Count(min: 1)] + public array $attachmentIds = []; +} diff --git a/tests/Integration/ApiPlatform/AttachmentEndpointTest.php b/tests/Integration/ApiPlatform/AttachmentEndpointTest.php index 5aee195f5..5075ee48b 100644 --- a/tests/Integration/ApiPlatform/AttachmentEndpointTest.php +++ b/tests/Integration/ApiPlatform/AttachmentEndpointTest.php @@ -62,6 +62,16 @@ public static function getProtectedEndpoints(): iterable 'DELETE', '/attachments/1', ]; + + yield 'list endpoint' => [ + 'GET', + '/attachments', + ]; + + yield 'bulk delete endpoint' => [ + 'DELETE', + '/attachments/bulk-delete', + ]; } public function testAddAttachment(): int @@ -137,9 +147,84 @@ public function testUpdateAttachment(int $attachmentId): int /** * @depends testUpdateAttachment */ + public function testListAttachments(int $attachmentId): int + { + $attachments = $this->listItems('/attachments?orderBy=attachmentId&sortOrder=desc', ['attachment_read']); + $this->assertGreaterThanOrEqual(1, $attachments['totalItems']); + $this->assertEquals('attachmentId', $attachments['orderBy']); + + // The attachment created above is the most recent, so it comes first in the descending list. + $listed = $attachments['items'][0]; + $this->assertEquals($attachmentId, $listed['attachmentId']); + $this->assertSame('Updated documentation', $listed['name']); + // The grid decorator exposes file_size and products as formatted strings. + $this->assertArrayHasKey('file', $listed); + $this->assertArrayHasKey('fileSize', $listed); + $this->assertArrayHasKey('products', $listed); + + // The filter parameter is mapped to the grid's id_attachment column. + $filtered = $this->listItems('/attachments', ['attachment_read'], [ + 'attachmentId' => $attachmentId, + ]); + $this->assertEquals(1, $filtered['totalItems']); + $this->assertEquals($attachmentId, $filtered['items'][0]['attachmentId']); + + return $attachmentId; + } + + /** + * @depends testListAttachments + */ public function testDeleteAttachment(int $attachmentId): void { $this->requestApi('DELETE', '/attachments/' . $attachmentId, null, ['attachment_write'], Response::HTTP_NO_CONTENT); $this->getItem('/attachments/' . $attachmentId, ['attachment_read'], Response::HTTP_NOT_FOUND); } + + public function testBulkDeleteAttachments(): void + { + $attachmentIds = [ + $this->createAttachment('Bulk attachment one'), + $this->createAttachment('Bulk attachment two'), + $this->createAttachment('Bulk attachment three'), + ]; + + $this->bulkDeleteItems('/attachments/bulk-delete', ['attachmentIds' => $attachmentIds], ['attachment_write']); + + foreach ($attachmentIds as $attachmentId) { + $this->getItem('/attachments/' . $attachmentId, ['attachment_read'], Response::HTTP_NOT_FOUND); + } + } + + public function testBulkDeleteAttachmentsValidationErrors(): void + { + // An empty id list must be rejected with a 422 validation error. + $this->bulkDeleteItems('/attachments/bulk-delete', ['attachmentIds' => []], ['attachment_write'], Response::HTTP_UNPROCESSABLE_ENTITY); + } + + private function createAttachment(string $name): int + { + $uploadedFile = $this->prepareUploadedFile(__DIR__ . '/../../Resources/assets/image/Hummingbird_cushion.jpg'); + + $attachment = $this->requestApi('POST', '/attachments', null, ['attachment_write'], Response::HTTP_CREATED, [ + 'headers' => [ + 'content-type' => 'multipart/form-data', + ], + 'extra' => [ + 'files' => [ + 'file' => $uploadedFile, + ], + 'parameters' => [ + 'names' => [ + 'en-US' => $name, + ], + 'descriptions' => [ + 'en-US' => 'A product documentation file', + ], + ], + ], + ]); + + return $attachment['attachmentId']; + } } From 873e18591bd011ac3d2e364c65f3927024eae82b Mon Sep 17 00:00:00 2001 From: Audrius Date: Sun, 26 Jul 2026 16:11:51 +0300 Subject: [PATCH 3/4] Skip the attachment POST tests on cores without the for-editing CQRS fix --- .../ApiPlatform/AttachmentEndpointTest.php | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/Integration/ApiPlatform/AttachmentEndpointTest.php b/tests/Integration/ApiPlatform/AttachmentEndpointTest.php index 5075ee48b..8d1ab4381 100644 --- a/tests/Integration/ApiPlatform/AttachmentEndpointTest.php +++ b/tests/Integration/ApiPlatform/AttachmentEndpointTest.php @@ -22,6 +22,7 @@ namespace PsApiResourcesTest\Integration\ApiPlatform; +use PrestaShop\PrestaShop\Core\Domain\Attachment\Query\GetAttachmentForEditing; use Symfony\Component\HttpFoundation\Response; use Tests\Resources\DatabaseDump; @@ -74,8 +75,30 @@ public static function getProtectedEndpoints(): iterable ]; } + /** + * The CQRS API normalizer builds GetAttachmentForEditing from the command result using the + * `attachmentId` key. Cores whose constructor still takes `$attachmentIdValue` cannot be + * instantiated from that data, so POST /attachments answers 500 there. 9.0.3 will never carry + * the fix (the 9.0.x branch was deleted), hence a capability probe rather than a version check. + */ + private function skipIfAttachmentIdNotExposed(): void + { + $constructor = (new \ReflectionClass(GetAttachmentForEditing::class))->getConstructor(); + $parameters = null !== $constructor ? $constructor->getParameters() : []; + + if ([] === $parameters || 'attachmentId' !== $parameters[0]->getName()) { + $this->markTestSkipped( + 'Requires the core attachment for-editing CQRS alignment' + . ' (PrestaShop/PrestaShop#41889 on develop, #42124 on 9.1.x):' + . ' GetAttachmentForEditing must accept an "attachmentId" parameter.' + ); + } + } + public function testAddAttachment(): int { + $this->skipIfAttachmentIdNotExposed(); + $uploadedFile = $this->prepareUploadedFile(__DIR__ . '/../../Resources/assets/image/Hummingbird_cushion.jpg'); $attachment = $this->requestApi('POST', '/attachments', null, ['attachment_write'], Response::HTTP_CREATED, [ @@ -204,6 +227,8 @@ public function testBulkDeleteAttachmentsValidationErrors(): void private function createAttachment(string $name): int { + $this->skipIfAttachmentIdNotExposed(); + $uploadedFile = $this->prepareUploadedFile(__DIR__ . '/../../Resources/assets/image/Hummingbird_cushion.jpg'); $attachment = $this->requestApi('POST', '/attachments', null, ['attachment_write'], Response::HTTP_CREATED, [ From 7ba30d4745c630593b4d910f1d1ca5859504a174 Mon Sep 17 00:00:00 2001 From: boo-code Date: Thu, 29 Oct 2026 19:46:45 +0100 Subject: [PATCH 4/4] Address review: gate the item operations on 9.2.1, update through a multipart POST, validate like the back office --- .../Resources/Attachment/Attachment.php | 39 ++- .../Resources/Attachment/AttachmentList.php | 9 +- .../Resources/Attachment/BulkAttachments.php | 5 + .../ApiPlatform/AttachmentEndpointTest.php | 240 +++++++++--------- 4 files changed, 161 insertions(+), 132 deletions(-) diff --git a/src/ApiPlatform/Resources/Attachment/Attachment.php b/src/ApiPlatform/Resources/Attachment/Attachment.php index 98455002e..387db60dd 100644 --- a/src/ApiPlatform/Resources/Attachment/Attachment.php +++ b/src/ApiPlatform/Resources/Attachment/Attachment.php @@ -29,11 +29,12 @@ use PrestaShop\PrestaShop\Core\Domain\Attachment\Command\EditAttachmentCommand; use PrestaShop\PrestaShop\Core\Domain\Attachment\Exception\AttachmentConstraintException; use PrestaShop\PrestaShop\Core\Domain\Attachment\Exception\AttachmentNotFoundException; +use PrestaShop\PrestaShop\Core\Domain\Attachment\Exception\EmptyFileException; use PrestaShop\PrestaShop\Core\Domain\Attachment\Query\GetAttachmentForEditing; use PrestaShopBundle\ApiPlatform\Metadata\CQRSCreate; use PrestaShopBundle\ApiPlatform\Metadata\CQRSDelete; use PrestaShopBundle\ApiPlatform\Metadata\CQRSGet; -use PrestaShopBundle\ApiPlatform\Metadata\CQRSPartialUpdate; +use PrestaShopBundle\ApiPlatform\Metadata\CQRSUpdate; use PrestaShopBundle\ApiPlatform\Metadata\LocalizedValue; use Symfony\Component\HttpFoundation\File\File; use Symfony\Component\HttpFoundation\Response; @@ -48,6 +49,11 @@ CQRSQuery: GetAttachmentForEditing::class, scopes: ['attachment_read'], CQRSQueryMapping: self::QUERY_MAPPING, + extraProperties: [ + // GetAttachmentForEditing accepts the attachmentId the API builds it from only from PrestaShop 9.2.1 + // (PrestaShop/PrestaShop#42124), older cores answer 500 + 'minVersion' => '9.2.1', + ], ), new CQRSCreate( uriTemplate: '/attachments', @@ -60,11 +66,21 @@ denormalizationContext: [ObjectNormalizer::DISABLE_TYPE_ENFORCEMENT => true], CQRSQueryMapping: self::QUERY_MAPPING, CQRSCommandMapping: self::CREATE_COMMAND_MAPPING, + extraProperties: [ + // GetAttachmentForEditing accepts the attachmentId the API builds it from only from PrestaShop 9.2.1 + // (PrestaShop/PrestaShop#42124), older cores answer 500 + 'minVersion' => '9.2.1', + ], ), - new CQRSPartialUpdate( + // A POST, since PHP only parses the multipart body of a POST request. EditAttachmentHandler writes the + // names and the descriptions whatever was sent, so both are required, the file is optional. + new CQRSUpdate( + method: CQRSUpdate::METHOD_POST, uriTemplate: '/attachments/{attachmentId}', requirements: ['attachmentId' => '\d+'], + status: Response::HTTP_OK, read: false, + validationContext: ['groups' => ['Default', 'Update']], CQRSCommand: EditAttachmentCommand::class, CQRSQuery: GetAttachmentForEditing::class, scopes: ['attachment_write'], @@ -72,6 +88,11 @@ denormalizationContext: [ObjectNormalizer::DISABLE_TYPE_ENFORCEMENT => true], CQRSQueryMapping: self::QUERY_MAPPING, CQRSCommandMapping: self::UPDATE_COMMAND_MAPPING, + extraProperties: [ + // GetAttachmentForEditing accepts the attachmentId the API builds it from only from PrestaShop 9.2.1 + // (PrestaShop/PrestaShop#42124), older cores answer 500 + 'minVersion' => '9.2.1', + ], ), new CQRSDelete( uriTemplate: '/attachments/{attachmentId}', @@ -84,6 +105,7 @@ exceptionToStatus: [ AttachmentNotFoundException::class => Response::HTTP_NOT_FOUND, AttachmentConstraintException::class => Response::HTTP_UNPROCESSABLE_ENTITY, + EmptyFileException::class => Response::HTTP_UNPROCESSABLE_ENTITY, ], )] class Attachment @@ -92,13 +114,22 @@ class Attachment public int $attachmentId; #[LocalizedValue] - #[Assert\NotBlank(groups: ['Create'])] + #[Assert\NotBlank(groups: ['Create', 'Update'])] public array $names; #[LocalizedValue] + #[Assert\NotNull(groups: ['Create', 'Update'])] public array $descriptions; - public ?File $file; + /** + * Write-only: the file, sent as the `file` part of the multipart request. Required to add an attachment, + * optional on the update, which keeps the current file without it. The responses expose the stored file + * through fileName. Not nullable, like VirtualProductFile::$file: an initialized null is normalized into the + * response despite readable: false. + */ + #[ApiProperty(readable: false, writable: false)] + #[Assert\NotNull(groups: ['Create'])] + public File $file; public string $fileName; diff --git a/src/ApiPlatform/Resources/Attachment/AttachmentList.php b/src/ApiPlatform/Resources/Attachment/AttachmentList.php index 831dbe6bf..c72eb592f 100644 --- a/src/ApiPlatform/Resources/Attachment/AttachmentList.php +++ b/src/ApiPlatform/Resources/Attachment/AttachmentList.php @@ -35,7 +35,8 @@ 'attachment_read', ], ApiResourceMapping: self::MAPPING, - gridDataFactory: 'prestashop.core.grid.data_factory.attachment_decorator', + // The undecorated factory: the decorator turns the size and the product count into display labels + gridDataFactory: 'prestashop.core.grid.data_factory.attachment', filtersClass: AttachmentFilters::class, filtersMapping: [ '[attachmentId]' => '[id_attachment]', @@ -52,9 +53,11 @@ class AttachmentList public string $file; - public string $fileSize; + #[ApiProperty(openapiContext: ['description' => 'Size of the file, in bytes.'])] + public int $fileSize; - public string $products; + #[ApiProperty(openapiContext: ['description' => 'Number of products the attachment is associated with.'])] + public int $products; public const MAPPING = [ '[id_attachment]' => '[attachmentId]', diff --git a/src/ApiPlatform/Resources/Attachment/BulkAttachments.php b/src/ApiPlatform/Resources/Attachment/BulkAttachments.php index a34260645..29ae84074 100644 --- a/src/ApiPlatform/Resources/Attachment/BulkAttachments.php +++ b/src/ApiPlatform/Resources/Attachment/BulkAttachments.php @@ -26,6 +26,7 @@ use ApiPlatform\Metadata\ApiResource; use PrestaShop\PrestaShop\Core\Domain\Attachment\Command\BulkDeleteAttachmentsCommand; use PrestaShop\PrestaShop\Core\Domain\Attachment\Exception\AttachmentNotFoundException; +use PrestaShop\PrestaShop\Core\Domain\Attachment\Exception\BulkDeleteAttachmentsException; use PrestaShopBundle\ApiPlatform\Metadata\CQRSDelete; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Validator\Constraints as Assert; @@ -35,6 +36,7 @@ new CQRSDelete( uriTemplate: '/attachments/bulk-delete', CQRSCommand: BulkDeleteAttachmentsCommand::class, + allowEmptyBody: false, scopes: [ 'attachment_write', ], @@ -42,6 +44,8 @@ ], exceptionToStatus: [ AttachmentNotFoundException::class => Response::HTTP_NOT_FOUND, + // The handler deletes what it can and reports the failed ids together at the end + BulkDeleteAttachmentsException::class => Response::HTTP_UNPROCESSABLE_ENTITY, ], )] class BulkAttachments @@ -52,5 +56,6 @@ class BulkAttachments #[ApiProperty(openapiContext: ['type' => 'array', 'items' => ['type' => 'integer'], 'example' => [1, 3]])] #[Assert\NotBlank] #[Assert\Count(min: 1)] + #[Assert\All([new Assert\Type('integer'), new Assert\Positive()])] public array $attachmentIds = []; } diff --git a/tests/Integration/ApiPlatform/AttachmentEndpointTest.php b/tests/Integration/ApiPlatform/AttachmentEndpointTest.php index 8d1ab4381..7ae4e6816 100644 --- a/tests/Integration/ApiPlatform/AttachmentEndpointTest.php +++ b/tests/Integration/ApiPlatform/AttachmentEndpointTest.php @@ -22,12 +22,13 @@ namespace PsApiResourcesTest\Integration\ApiPlatform; -use PrestaShop\PrestaShop\Core\Domain\Attachment\Query\GetAttachmentForEditing; use Symfony\Component\HttpFoundation\Response; use Tests\Resources\DatabaseDump; class AttachmentEndpointTest extends ApiTestCase { + private const ASSET = __DIR__ . '/../../Resources/assets/image/Hummingbird_cushion.jpg'; + public static function setUpBeforeClass(): void { parent::setUpBeforeClass(); @@ -42,87 +43,24 @@ public static function tearDownAfterClass(): void public static function getProtectedEndpoints(): iterable { - yield 'create endpoint' => [ - 'POST', - '/attachments', - 'multipart/form-data', - ]; - - yield 'get endpoint' => [ - 'GET', - '/attachments/1', - ]; - - yield 'update endpoint' => [ - 'PATCH', - '/attachments/1', - 'multipart/form-data', - ]; - - yield 'delete endpoint' => [ - 'DELETE', - '/attachments/1', - ]; - - yield 'list endpoint' => [ - 'GET', - '/attachments', - ]; - - yield 'bulk delete endpoint' => [ - 'DELETE', - '/attachments/bulk-delete', - ]; - } - - /** - * The CQRS API normalizer builds GetAttachmentForEditing from the command result using the - * `attachmentId` key. Cores whose constructor still takes `$attachmentIdValue` cannot be - * instantiated from that data, so POST /attachments answers 500 there. 9.0.3 will never carry - * the fix (the 9.0.x branch was deleted), hence a capability probe rather than a version check. - */ - private function skipIfAttachmentIdNotExposed(): void - { - $constructor = (new \ReflectionClass(GetAttachmentForEditing::class))->getConstructor(); - $parameters = null !== $constructor ? $constructor->getParameters() : []; - - if ([] === $parameters || 'attachmentId' !== $parameters[0]->getName()) { - $this->markTestSkipped( - 'Requires the core attachment for-editing CQRS alignment' - . ' (PrestaShop/PrestaShop#41889 on develop, #42124 on 9.1.x):' - . ' GetAttachmentForEditing must accept an "attachmentId" parameter.' - ); + // The single item operations are filtered out below PrestaShop 9.2.1, where they answer 404 instead of 401 + if (self::isVersionAtLeast('9.2.1')) { + yield 'create endpoint' => ['POST', '/attachments', 'multipart/form-data']; + yield 'get endpoint' => ['GET', '/attachments/1']; + yield 'update endpoint' => ['POST', '/attachments/1', 'multipart/form-data']; } + yield 'delete endpoint' => ['DELETE', '/attachments/1']; + yield 'list endpoint' => ['GET', '/attachments']; + yield 'bulk delete endpoint' => ['DELETE', '/attachments/bulk-delete']; } public function testAddAttachment(): int { - $this->skipIfAttachmentIdNotExposed(); - - $uploadedFile = $this->prepareUploadedFile(__DIR__ . '/../../Resources/assets/image/Hummingbird_cushion.jpg'); - - $attachment = $this->requestApi('POST', '/attachments', null, ['attachment_write'], Response::HTTP_CREATED, [ - 'headers' => [ - 'content-type' => 'multipart/form-data', - ], - 'extra' => [ - 'files' => [ - 'file' => $uploadedFile, - ], - 'parameters' => [ - 'names' => [ - 'en-US' => 'Documentation', - ], - 'descriptions' => [ - 'en-US' => 'A product documentation file', - ], - ], - ], - ]); + $this->markTestSkippedByMinVersion('9.2.1'); - $this->assertArrayHasKey('attachmentId', $attachment); + $attachment = $this->sendAttachment('/attachments', $this->getFormFields('Documentation', 'A product documentation file'), true, Response::HTTP_CREATED); $this->assertIsInt($attachment['attachmentId']); - $this->assertGreaterThan(0, $attachment['attachmentId']); + $this->assertEquals($this->expectedAttachment($attachment['attachmentId'], 'Documentation', 'A product documentation file'), $attachment); return $attachment['attachmentId']; } @@ -132,11 +70,10 @@ public function testAddAttachment(): int */ public function testGetAttachment(int $attachmentId): int { - $attachment = $this->getItem('/attachments/' . $attachmentId, ['attachment_read']); - - $this->assertArrayHasKey('names', $attachment); - $this->assertArrayHasKey('fileName', $attachment); - $this->assertSame('Documentation', reset($attachment['names'])); + $this->assertEquals( + $this->expectedAttachment($attachmentId, 'Documentation', 'A product documentation file'), + $this->getItem('/attachments/' . $attachmentId, ['attachment_read']) + ); return $attachmentId; } @@ -146,23 +83,12 @@ public function testGetAttachment(int $attachmentId): int */ public function testUpdateAttachment(int $attachmentId): int { - $attachment = $this->requestApi('PATCH', '/attachments/' . $attachmentId, null, ['attachment_write'], Response::HTTP_OK, [ - 'headers' => [ - 'content-type' => 'multipart/form-data', - ], - 'extra' => [ - 'parameters' => [ - 'names' => [ - 'en-US' => 'Updated documentation', - ], - 'descriptions' => [ - 'en-US' => 'An updated description', - ], - ], - ], - ]); - - $this->assertSame('Updated documentation', reset($attachment['names'])); + // Without a file the current one is kept + $expected = $this->expectedAttachment($attachmentId, 'Updated documentation', 'An updated description'); + $this->assertEquals($expected, $this->sendAttachment('/attachments/' . $attachmentId, $this->getFormFields('Updated documentation', 'An updated description'), false)); + $this->assertEquals($expected, $this->getItem('/attachments/' . $attachmentId, ['attachment_read'])); + // Not covered with a file: AttachmentFileUploader stores it with move_uploaded_file(), which stores nothing for a + // test upload, so replacing it would only unlink a missing file return $attachmentId; } @@ -178,12 +104,11 @@ public function testListAttachments(int $attachmentId): int // The attachment created above is the most recent, so it comes first in the descending list. $listed = $attachments['items'][0]; - $this->assertEquals($attachmentId, $listed['attachmentId']); + $this->assertSame($attachmentId, $listed['attachmentId']); $this->assertSame('Updated documentation', $listed['name']); - // The grid decorator exposes file_size and products as formatted strings. - $this->assertArrayHasKey('file', $listed); - $this->assertArrayHasKey('fileSize', $listed); - $this->assertArrayHasKey('products', $listed); + $this->assertSame(filesize(self::ASSET), $listed['fileSize']); + $this->assertSame(0, $listed['products']); + $this->assertIsString($listed['file']); // The filter parameter is mapped to the grid's id_attachment column. $filtered = $this->listItems('/attachments', ['attachment_read'], [ @@ -198,14 +123,54 @@ public function testListAttachments(int $attachmentId): int /** * @depends testListAttachments */ + public function testUpdateAttachmentWithInvalidData(int $attachmentId): int + { + // EditAttachmentHandler writes the names and the descriptions whatever was sent, so both are required + $response = $this->sendAttachment('/attachments/' . $attachmentId, ['names' => ['en-US' => 'Updated documentation']], false, Response::HTTP_UNPROCESSABLE_ENTITY); + $this->assertValidationErrors([ + ['propertyPath' => 'descriptions', 'message' => 'This value should not be null.'], + ], $response); + $response = $this->sendAttachment('/attachments/' . $attachmentId, ['descriptions' => ['en-US' => 'An updated description']], false, Response::HTTP_UNPROCESSABLE_ENTITY); + $this->assertValidationErrors([ + ['propertyPath' => 'names', 'message' => 'This value should not be blank.'], + ], $response); + $this->assertSame('Updated documentation', $this->getItem('/attachments/' . $attachmentId, ['attachment_read'])['names']['en-US']); + + return $attachmentId; + } + + /** + * @depends testUpdateAttachmentWithInvalidData + */ public function testDeleteAttachment(int $attachmentId): void { $this->requestApi('DELETE', '/attachments/' . $attachmentId, null, ['attachment_write'], Response::HTTP_NO_CONTENT); $this->getItem('/attachments/' . $attachmentId, ['attachment_read'], Response::HTTP_NOT_FOUND); } + public function testAddAttachmentWithInvalidData(): void + { + $this->markTestSkippedByMinVersion('9.2.1'); + + $response = $this->sendAttachment('/attachments', $this->getFormFields('Documentation', 'A product documentation file'), false, Response::HTTP_UNPROCESSABLE_ENTITY); + $this->assertValidationErrors([ + ['propertyPath' => 'file', 'message' => 'This value should not be null.'], + ], $response); + } + + public function testNotFoundAttachment(): void + { + $this->markTestSkippedByMinVersion('9.2.1'); + + $this->getItem('/attachments/999999', ['attachment_read'], Response::HTTP_NOT_FOUND); + $this->sendAttachment('/attachments/999999', $this->getFormFields('Documentation', 'A product documentation file'), false, Response::HTTP_NOT_FOUND); + $this->deleteItem('/attachments/999999', ['attachment_write'], Response::HTTP_NOT_FOUND); + } + public function testBulkDeleteAttachments(): void { + $this->markTestSkippedByMinVersion('9.2.1'); + $attachmentIds = [ $this->createAttachment('Bulk attachment one'), $this->createAttachment('Bulk attachment two'), @@ -217,39 +182,64 @@ public function testBulkDeleteAttachments(): void foreach ($attachmentIds as $attachmentId) { $this->getItem('/attachments/' . $attachmentId, ['attachment_read'], Response::HTTP_NOT_FOUND); } + + // BulkDeleteAttachmentsHandler deletes the known ids and reports the unknown ones together at the end + $attachmentId = $this->createAttachment('Bulk attachment four'); + $this->bulkDeleteItems('/attachments/bulk-delete', ['attachmentIds' => [$attachmentId, 999999]], ['attachment_write'], Response::HTTP_UNPROCESSABLE_ENTITY); + $this->getItem('/attachments/' . $attachmentId, ['attachment_read'], Response::HTTP_NOT_FOUND); } public function testBulkDeleteAttachmentsValidationErrors(): void { - // An empty id list must be rejected with a 422 validation error. $this->bulkDeleteItems('/attachments/bulk-delete', ['attachmentIds' => []], ['attachment_write'], Response::HTTP_UNPROCESSABLE_ENTITY); + + $response = $this->bulkDeleteItems('/attachments/bulk-delete', ['attachmentIds' => [0]], ['attachment_write'], Response::HTTP_UNPROCESSABLE_ENTITY); + $this->assertValidationErrors([ + ['propertyPath' => 'attachmentIds[0]', 'message' => 'This value should be positive.'], + ], $response); + $response = $this->bulkDeleteItems('/attachments/bulk-delete', ['attachmentIds' => ['3abc']], ['attachment_write'], Response::HTTP_UNPROCESSABLE_ENTITY); + $this->assertValidationErrors([ + ['propertyPath' => 'attachmentIds[0]', 'message' => 'This value should be of type integer.'], + ], $response); } private function createAttachment(string $name): int { - $this->skipIfAttachmentIdNotExposed(); - - $uploadedFile = $this->prepareUploadedFile(__DIR__ . '/../../Resources/assets/image/Hummingbird_cushion.jpg'); - - $attachment = $this->requestApi('POST', '/attachments', null, ['attachment_write'], Response::HTTP_CREATED, [ - 'headers' => [ - 'content-type' => 'multipart/form-data', - ], - 'extra' => [ - 'files' => [ - 'file' => $uploadedFile, - ], - 'parameters' => [ - 'names' => [ - 'en-US' => $name, - ], - 'descriptions' => [ - 'en-US' => 'A product documentation file', - ], - ], - ], + return $this->sendAttachment('/attachments', $this->getFormFields($name, 'A product documentation file'), true, Response::HTTP_CREATED)['attachmentId']; + } + + /** + * Sends the multipart POST of the creation and of the update. + */ + private function sendAttachment(string $uri, array $parameters, bool $withFile, int $expectedHttpCode = Response::HTTP_OK): array + { + $extra = ['parameters' => $parameters]; + if ($withFile) { + $extra['files'] = ['file' => $this->prepareUploadedFile(self::ASSET)]; + } + + return $this->requestApi('POST', $uri, null, ['attachment_write'], $expectedHttpCode, [ + 'headers' => ['content-type' => 'multipart/form-data'], + 'extra' => $extra, ]); + } - return $attachment['attachmentId']; + private function getFormFields(string $name, string $description): array + { + return [ + 'names' => ['en-US' => $name], + 'descriptions' => ['en-US' => $description], + ]; + } + + private function expectedAttachment(int $attachmentId, string $name, string $description): array + { + return [ + 'attachmentId' => $attachmentId, + // The languages left out get the name of the default language + 'names' => ['en-US' => $name, 'fr-FR' => $name], + 'descriptions' => ['en-US' => $description, 'fr-FR' => ''], + 'fileName' => basename(self::ASSET), + ]; } }