Skip to content

Refactor/admin attributes routes #163

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TatevikGr merged 6 commits into from
Nov 21, 2025
Merged

Refactor/admin attributes routes #163

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e4874b2
Update: admin routes + swagger
tatevikg1 Nov 20, 2025
e4d4b0a
Update: shorten admin param name
tatevikg1 Nov 20, 2025
65921b7
Update: remove ids from update dtos
tatevikg1 Nov 21, 2025
1d54435
Fix: tests
tatevikg1 Nov 21, 2025
ccfc48e
Fix: template request
tatevikg1 Nov 21, 2025
bfdaa4b
Fix: template request content type
tatevikg1 Nov 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions src/Common/Validator/RequestValidator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,9 @@ public function validate(Request $request, string $dtoClass): RequestInterface
}
$routeParams = $request->attributes->get('_route_params') ?? [];

if (isset($routeParams['subscriberId'])) {
$routeParams['subscriberId'] = (int) $routeParams['subscriberId'];
}
if (isset($routeParams['messageId'])) {
$routeParams['messageId'] = (int) $routeParams['messageId'];
}
if (isset($routeParams['listId'])) {
$routeParams['listId'] = (int) $routeParams['listId'];
}
if (isset($routeParams['administratorId'])) {
$routeParams['administratorId'] = (int) $routeParams['administratorId'];
}

$data = array_merge($routeParams, $body ?? []);

Expand Down
33 changes: 18 additions & 15 deletions src/Identity/Controller/AdminAttributeValueController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Routing\Attribute\Route;

#[Route('/administrators/attribute-values', name: 'admin_attribute_value_')]
#[Route('/administrators', name: 'admin_attribute_value_')]
class AdminAttributeValueController extends BaseController
{
private AdminAttributeManager $attributeManager;
Expand All @@ -46,13 +46,13 @@ public function __construct(
}

#[Route(
path: '/{adminId}/{definitionId}',
path: '/{adminId}/attributes/{definitionId}',
name: 'create',
requirements: ['adminId' => '\d+', 'definitionId' => '\d+'],
methods: ['POST', 'PUT'],
)]
#[OA\Post(
path: '/api/v2/administrators/attribute-values/{adminId}/{definitionId}',
path: '/api/v2/administrators/{adminId}/attributes/{definitionId}',
description: '🚧 **Status: Beta** – This method is under development. Avoid using in production. ' .
'Returns created/updated admin attribute.',
summary: 'Create/update an admin attribute.',
Expand Down Expand Up @@ -133,13 +133,13 @@ public function createOrUpdate(
}

#[Route(
path: '/{adminId}/{definitionId}',
path: '/{adminId}/attributes/{definitionId}',
name: 'delete',
requirements: ['adminId' => '\d+', 'definitionId' => '\d+'],
methods: ['DELETE'],
)]
#[OA\Delete(
path: '/api/v2/administrators/attribute-values/{adminId}/{definitionId}',
path: '/api/v2/administrators/{adminId}/attributes/{definitionId}',
description: '🚧 **Status: Beta** – This method is under development. Avoid using in production. ' .
'Deletes a single admin attribute.',
summary: 'Deletes an attribute.',
Expand Down Expand Up @@ -203,9 +203,9 @@ public function delete(
return $this->json(null, Response::HTTP_NO_CONTENT);
}

#[Route('/{adminId}', name: 'get__list', requirements: ['adminId' => '\d+'], methods: ['GET'])]
#[Route('/{adminId}/attributes', name: 'get_list', requirements: ['adminId' => '\d+'], methods: ['GET'])]
#[OA\Get(
path: '/api/v2/administrators/attribute-values/{adminId}',
path: '/api/v2/administrators/{adminId}/attributes',
description: '🚧 **Status: Beta** – This method is under development. Avoid using in production. ' .
'Returns a JSON list of all admin attributes.',
summary: 'Gets a list of all admin attributes.',
Expand Down Expand Up @@ -260,6 +260,11 @@ public function delete(
response: 403,
description: 'Failure',
content: new OA\JsonContent(ref: '#/components/schemas/UnauthorizedResponse')
),
new OA\Response(
response: 404,
description: 'Failure',
content: new OA\JsonContent(ref: '#/components/schemas/NotFoundErrorResponse')
)
]
)]
Expand All @@ -276,18 +281,18 @@ public function getPaginated(

return $this->json(
$this->paginatedDataProvider->getPaginatedList(
$request,
$this->normalizer,
AdminAttributeValue::class,
$filter
request: $request,
normalizer:$this->normalizer,
className: AdminAttributeValue::class,
filter: $filter
),
Response::HTTP_OK
);
}

#[Route('/{adminId}/{definitionId}', name: 'get_one', methods: ['GET'])]
#[Route('/{adminId}/attributes/{definitionId}', name: 'get_one', methods: ['GET'])]
#[OA\Get(
path: '/api/v2/administrators/attribute-values/{adminId}/{definitionId}',
path: '/api/v2/administrators/{adminId}/attributes/{definitionId}',
description: '🚧 **Status: Beta** – This method is under development. Avoid using in production. ' .
'Returns a single attribute.',
summary: 'Gets admin attribute.',
Expand Down Expand Up @@ -355,8 +360,6 @@ public function getAttributeDefinition(
adminId: $admin->getId(),
attributeDefinitionId: $definition->getId()
);
$this->attributeManager->delete($attribute);
$this->entityManager->flush();

return $this->json(
$this->normalizer->normalize($attribute),
Expand Down
24 changes: 12 additions & 12 deletions src/Identity/Controller/AdministratorController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,16 +157,16 @@ public function createAdministrator(
return $this->json($json, Response::HTTP_CREATED);
}

#[Route('/{administratorId}', name: 'get_one', requirements: ['administratorId' => '\d+'], methods: ['GET'])]
#[Route('/{adminId}', name: 'get_one', requirements: ['adminId' => '\d+'], methods: ['GET'])]
#[OA\Get(
path: '/api/v2/administrators/{administratorId}',
path: '/api/v2/administrators/{adminId}',
description: '🚧 **Status: Beta** – This method is under development. Avoid using in production. ' .
'Get administrator by ID.',
summary: 'Get Administrator',
tags: ['administrators'],
parameters: [
new OA\Parameter(
name: 'administratorId',
name: 'adminId',
description: 'Administrator ID',
in: 'path',
required: true,
Expand Down Expand Up @@ -194,7 +194,7 @@ public function createAdministrator(
)]
public function getAdministrator(
Request $request,
#[MapEntity(mapping: ['administratorId' => 'id'])] ?Administrator $administrator,
#[MapEntity(mapping: ['adminId' => 'id'])] ?Administrator $administrator,
): JsonResponse {
$this->requireAuthentication($request);

Expand All @@ -206,9 +206,9 @@ public function getAdministrator(
return $this->json($json, Response::HTTP_OK);
}

#[Route('/{administratorId}', name: 'update', requirements: ['administratorId' => '\d+'], methods: ['PUT'])]
#[Route('/{adminId}', name: 'update', requirements: ['adminId' => '\d+'], methods: ['PUT'])]
#[OA\Put(
path: '/api/v2/administrators/{administratorId}',
path: '/api/v2/administrators/{adminId}',
description: '🚧 **Status: Beta** – This method is under development. Avoid using in production. ' .
'Update an administrator.',
summary: 'Update Administrator',
Expand All @@ -220,7 +220,7 @@ public function getAdministrator(
tags: ['administrators'],
parameters: [
new OA\Parameter(
name: 'administratorId',
name: 'adminId',
description: 'Administrator ID',
in: 'path',
required: true,
Expand Down Expand Up @@ -248,7 +248,7 @@ public function getAdministrator(
)]
public function updateAdministrator(
Request $request,
#[MapEntity(mapping: ['administratorId' => 'id'])] ?Administrator $administrator,
#[MapEntity(mapping: ['adminId' => 'id'])] ?Administrator $administrator,
): JsonResponse {
$this->requireAuthentication($request);

Expand All @@ -263,16 +263,16 @@ public function updateAdministrator(
return $this->json($this->normalizer->normalize($administrator), Response::HTTP_OK);
}

#[Route('/{administratorId}', name: 'delete', requirements: ['administratorId' => '\d+'], methods: ['DELETE'])]
#[Route('/{adminId}', name: 'delete', requirements: ['adminId' => '\d+'], methods: ['DELETE'])]
#[OA\Delete(
path: '/api/v2/administrators/{administratorId}',
path: '/api/v2/administrators/{adminId}',
description: '🚧 **Status: Beta** – This method is under development. Avoid using in production. ' .
'Delete an administrator.',
summary: 'Delete Administrator',
tags: ['administrators'],
parameters: [
new OA\Parameter(
name: 'administratorId',
name: 'adminId',
description: 'Administrator ID',
in: 'path',
required: true,
Expand All @@ -299,7 +299,7 @@ public function updateAdministrator(
)]
public function deleteAdministrator(
Request $request,
#[MapEntity(mapping: ['administratorId' => 'id'])] ?Administrator $administrator
#[MapEntity(mapping: ['adminId' => 'id'])] ?Administrator $administrator
): JsonResponse {
$this->requireAuthentication($request);

Expand Down
3 changes: 0 additions & 3 deletions src/Identity/Request/UpdateAdministratorRequest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,6 @@
)]
class UpdateAdministratorRequest implements RequestInterface
{
public int $administratorId;

#[Assert\Length(min: 3, max: 255)]
#[UniqueLoginName]
public ?string $loginName = null;
Expand Down Expand Up @@ -89,7 +87,6 @@ class UpdateAdministratorRequest implements RequestInterface
public function getDto(): UpdateAdministratorDto
{
return new UpdateAdministratorDto(
administratorId: $this->administratorId,
loginName: $this->loginName,
password: $this->password,
email: $this->email,
Expand Down
16 changes: 16 additions & 0 deletions src/Messaging/Controller/CampaignController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,21 @@ public function getMessages(Request $request): JsonResponse
response: 403,
description: 'Failure',
content: new OA\JsonContent(ref: '#/components/schemas/UnauthorizedResponse')
),
new OA\Response(
response: 404,
description: 'Failure',
content: new OA\JsonContent(ref: '#/components/schemas/NotFoundErrorResponse')
)
]
)]
public function getMessage(
Request $request,
#[MapEntity(mapping: ['messageId' => 'id'])] ?Message $message = null
): JsonResponse {
if ($message === null) {
throw $this->createNotFoundException('Campaign not found.');
}
$this->requireAuthentication($request);

return $this->json($this->campaignService->getMessage($message), Response::HTTP_OK);
Expand Down Expand Up @@ -271,6 +279,11 @@ public function createMessage(Request $request): JsonResponse
description: 'Failure',
content: new OA\JsonContent(ref: '#/components/schemas/UnauthorizedResponse')
),
new OA\Response(
response: 404,
description: 'Failure',
content: new OA\JsonContent(ref: '#/components/schemas/NotFoundErrorResponse')
),
new OA\Response(
response: 422,
description: 'Failure',
Expand All @@ -282,6 +295,9 @@ public function updateMessage(
Request $request,
#[MapEntity(mapping: ['messageId' => 'id'])] ?Message $message = null,
): JsonResponse {
if ($message === null) {
throw $this->createNotFoundException('Campaign not found.');
}
$authUser = $this->requireAuthentication($request);

/** @var UpdateMessageRequest $updateMessageRequest */
Expand Down
52 changes: 4 additions & 48 deletions src/Messaging/Controller/TemplateController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,9 @@ public function getTemplates(Request $request): JsonResponse

return $this->json(
$this->paginatedDataProvider->getPaginatedList(
$request,
$this->normalizer,
Template::class,
request: $request,
normalizer: $this->normalizer,
className: Template::class,
),
Response::HTTP_OK
);
Expand Down Expand Up @@ -178,51 +178,7 @@ public function getTemplate(
required: true,
content: new OA\MediaType(
mediaType: 'multipart/form-data',
schema: new OA\Schema(
required: ['title'],
properties: [
new OA\Property(
property: 'title',
type: 'string',
example: 'Newsletter Template'
),
new OA\Property(
property: 'content',
type: 'string',
example: '<html><body>[CONTENT]</body></html>'
),
new OA\Property(
property: 'text',
type: 'string',
example: '[CONTENT]'
),
new OA\Property(
property: 'file',
description: 'Optional file upload for HTML content',
type: 'string',
format: 'binary'
),
new OA\Property(
property: 'check_links',
description: 'Check that all links have full URLs',
type: 'boolean',
example: true
),
new OA\Property(
property: 'check_images',
description: 'Check that all images have full URLs',
type: 'boolean',
example: false
),
new OA\Property(
property: 'check_external_images',
description: 'Check that all external images exist',
type: 'boolean',
example: true
),
],
type: 'object'
)
schema: new OA\Schema(ref: '#/components/schemas/CreateTemplateRequest')
)
),
tags: ['templates'],
Expand Down
35 changes: 35 additions & 0 deletions src/Messaging/Request/CreateTemplateRequest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,47 @@

namespace PhpList\RestBundle\Messaging\Request;

use OpenApi\Attributes as OA;
use PhpList\Core\Domain\Messaging\Model\Dto\CreateTemplateDto;
use PhpList\RestBundle\Common\Request\RequestInterface;
use PhpList\RestBundle\Messaging\Validator\Constraint\ContainsPlaceholder;
use Symfony\Component\HttpFoundation\File\UploadedFile;
use Symfony\Component\Validator\Constraints as Assert;

#[OA\Schema(
schema: 'CreateTemplateRequest',
required: ['title'],
properties: [
new OA\Property(property: 'title', type: 'string', example: 'Newsletter Template'),
new OA\Property(property: 'content', type: 'string', example: '<html><body>[CONTENT]</body></html>'),
new OA\Property(property: 'text', type: 'string', example: '[CONTENT]'),
new OA\Property(
property: 'file',
description: 'Optional file upload for HTML content',
type: 'string',
format: 'binary'
),
new OA\Property(
property: 'check_links',
description: 'Check that all links have full URLs',
type: 'boolean',
example: true
),
new OA\Property(
property: 'check_images',
description: 'Check that all images have full URLs',
type: 'boolean',
example: false
),
new OA\Property(
property: 'check_external_images',
description: 'Check that all external images exist',
type: 'boolean',
example: true
),
],
type: 'object'
)]
Comment on lines 14 to 52
Copy link

@coderabbitai coderabbitai bot Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Clarify whether content is required or optional.

The content property (line 55) is declared as non-nullable string but isn't marked as required in the OpenAPI schema nor validated with NotBlank or NotNull. This creates ambiguity:

  • If content is required, add it to the required array and/or add validation constraints
  • If content is optional, declare it as ?string to match the OpenAPI schema

Apply this diff if content should be required:

 #[OA\Schema(
     schema: 'CreateTemplateRequest',
-    required: ['title'],
+    required: ['title', 'content'],
     properties: [

Or this diff if content should be optional:

-    #[ContainsPlaceholder]
-    public string $content;
+    #[ContainsPlaceholder]
+    public ?string $content = null;

And update line 69 in getDto() accordingly.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/Messaging/Request/CreateTemplateRequest.php around lines 14-47, the OA
Schema declares content as a string but its required status is ambiguous; pick
one approach and apply it: if content should be required, add 'content' to the
schema's required array, add the appropriate Symfony validation annotation
(NotBlank or NotNull) to the content property, and ensure getDto() (line ~69)
returns a non-null string; if content should be optional, change the property
declaration to ?string, update any validation annotations to allow null, and
update getDto() (line ~69) to return ?string accordingly so the PHP type and
OpenAPI schema stay consistent.

class CreateTemplateRequest implements RequestInterface
{
#[Assert\NotBlank(normalizer: 'trim')]
Expand Down
3 changes: 0 additions & 3 deletions src/Messaging/Request/UpdateMessageRequest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,9 @@

class UpdateMessageRequest extends CreateMessageRequest
{
public int $messageId;

public function getDto(): MessageDtoInterface
{
return new UpdateMessageDto(
messageId: $this->messageId,
content: $this->content->getDto(),
format: $this->format->getDto(),
metadata: $this->metadata->getDto(),
Expand Down
Loading
Loading