Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
},
"require": {
"php": "^8.1",
"phplist/core": "dev-main",
"phplist/core": "dev-dev",
"friendsofsymfony/rest-bundle": "*",
"symfony/test-pack": "^1.0",
"symfony/process": "^6.4",
Expand Down
286 changes: 286 additions & 0 deletions src/Messaging/Controller/CampaignActionController.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,286 @@
<?php

declare(strict_types=1);

namespace PhpList\RestBundle\Messaging\Controller;

use Doctrine\ORM\EntityManagerInterface;
use OpenApi\Attributes as OA;
use PhpList\Core\Domain\Messaging\Message\SyncCampaignProcessorMessage;
use PhpList\Core\Domain\Messaging\Model\Message;
use PhpList\Core\Domain\Messaging\Model\Message\MessageStatus;
use PhpList\Core\Domain\Messaging\Service\Manager\MessageManager;
use PhpList\Core\Security\Authentication;
use PhpList\RestBundle\Common\Controller\BaseController;
use PhpList\RestBundle\Common\Validator\RequestValidator;
use PhpList\RestBundle\Messaging\Request\Message\MessageMetadataRequest;
use PhpList\RestBundle\Messaging\Request\ResendMessageToListsRequest;
use PhpList\RestBundle\Messaging\Serializer\MessageNormalizer;
use PhpList\RestBundle\Messaging\Service\CampaignService;
use Symfony\Bridge\Doctrine\Attribute\MapEntity;
use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Messenger\MessageBusInterface;
use Symfony\Component\Routing\Attribute\Route;

/**
* This controller provides REST API to manage campaign actions.
*
* @author Tatevik Grigoryan <tatevik@phplist.com>
*/
#[Route('/campaigns', name: 'campaign_')]
class CampaignActionController extends BaseController
{
public function __construct(
Authentication $authentication,
RequestValidator $validator,
private readonly CampaignService $campaignService,
private readonly MessageBusInterface $messageBus,
private readonly EntityManagerInterface $entityManager,
private readonly MessageManager $messageManager,
private readonly MessageNormalizer $messageNormalizer
) {
parent::__construct($authentication, $validator);
}

#[Route('/{messageId}/copy', name: 'copy_campaign', requirements: ['messageId' => '\d+'], methods: ['POST'])]
#[OA\Post(
path: '/api/v2/campaigns/{messageId}/copy',
description: '🚧 **Status: Beta** – This method is under development. Avoid using in production. ' .
'Copies campaign/message by id into a draft message.',
summary: 'Copies campaign/message by id.',
tags: ['campaigns'],
parameters: [
new OA\Parameter(
name: 'php-auth-pw',
description: 'Session key obtained from login',
in: 'header',
required: true,
schema: new OA\Schema(type: 'string')
),
new OA\Parameter(
name: 'messageId',
description: 'message ID',
in: 'path',
required: true,
schema: new OA\Schema(type: 'string')
)
],
responses: [
new OA\Response(
response: 201,
description: 'Success',
content: new OA\JsonContent(ref: '#/components/schemas/Message')
),
new OA\Response(
response: 403,
description: 'Failure',
content: new OA\JsonContent(ref: '#/components/schemas/UnauthorizedResponse')
)
]
)]
public function copyMessage(
Request $request,
#[MapEntity(mapping: ['messageId' => 'id'])] ?Message $message = null
): JsonResponse {
$authUser = $this->requireAuthentication($request);
if ($message === null) {
throw $this->createNotFoundException('Campaign not found.');
}

$message = $this->messageManager->copyAsDraftMessage($message, $authUser);
$this->entityManager->flush();

return $this->json($this->campaignService->getMessage($message), Response::HTTP_CREATED);
}

#[Route('/{messageId}/status', name: 'update_status', requirements: ['messageId' => '\d+'], methods: ['PATCH'])]
#[OA\Patch(
path: '/api/v2/campaigns/{messageId}/status',
description: '🚧 **Status: Beta** – This method is under development. Avoid using in production. ' .
'Updates campaign/message status by id.',
summary: 'Update campaign status by id.',
requestBody: new OA\RequestBody(
description: 'Update message status.',
required: true,
content: new OA\JsonContent(ref: '#/components/schemas/MessageMetadataRequest')
),
tags: ['campaigns'],
parameters: [
new OA\Parameter(
name: 'php-auth-pw',
description: 'Session key obtained from login',
in: 'header',
required: true,
schema: new OA\Schema(
type: 'string'
)
),
new OA\Parameter(
name: 'messageId',
description: 'message ID',
in: 'path',
required: true,
schema: new OA\Schema(type: 'string')
)
],
responses: [
new OA\Response(
response: 200,
description: 'Success',
content: new OA\JsonContent(ref: '#/components/schemas/Message')
),
new OA\Response(
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')
),
new OA\Response(
response: 422,
description: 'Failure',
content: new OA\JsonContent(ref: '#/components/schemas/ValidationErrorResponse')
),
]
)]
public function updateMessageStatus(
Request $request,
#[MapEntity(mapping: ['messageId' => 'id'])] ?Message $message = null,
): JsonResponse {
$this->requireAuthentication($request);
if ($message === null) {
throw $this->createNotFoundException('Message not found.');
}

/** @var MessageMetadataRequest $messageMetadataRequest */
$messageMetadataRequest = $this->validator->validate($request, MessageMetadataRequest::class);

$message = $this->messageManager->updateStatus(
$message,
MessageStatus::from($messageMetadataRequest->status),
);
$this->entityManager->flush();

return $this->json($this->messageNormalizer->normalize($message), Response::HTTP_OK);
Comment on lines +163 to +169
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check MessageMetadataRequest validation constraints for status
ast-grep --pattern $'class MessageMetadataRequest {
  $$$
}'

# Also check for any validation attributes on the status property
rg -A 10 'class MessageMetadataRequest' --type php

Repository: phpList/rest-api

Length of output: 972


🏁 Script executed:

#!/bin/bash
# Get the complete MessageMetadataRequest class
cat src/Messaging/Request/Message/MessageMetadataRequest.php

# Also find and display the MessageStatus enum
fd -t f 'MessageStatus.php' --type php

Repository: phpList/rest-api

Length of output: 1178


🏁 Script executed:

#!/bin/bash
# Find MessageStatus enum file
fd 'MessageStatus' -x file {} \;

# Search for MessageStatus enum definition
rg 'enum MessageStatus' -A 20

Repository: phpList/rest-api

Length of output: 42


🏁 Script executed:

#!/bin/bash
# Search for any Choice validator on status field
rg 'Choice|Enum' src/Messaging/Request/Message/MessageMetadataRequest.php

# Search for exception handling around MessageStatus::from
rg -B 5 -A 5 'MessageStatus::from' src/

# Check if there's any validation elsewhere for enum values
rg '#\[Assert.*status' src/

# Look for any exception handling in the controller
rg -B 10 -A 10 'MessageStatus::from' src/Messaging/Controller/CampaignActionController.php

Repository: phpList/rest-api

Length of output: 2778


🏁 Script executed:

#!/bin/bash
# Check for global exception handlers that might catch ValueError
rg 'ValueError|catch.*Value' src/ --type php

# Check for enum value constraint definitions
rg 'Choice|Enum' src/Messaging/Request/ -A 2

Repository: phpList/rest-api

Length of output: 566


🏁 Script executed:

#!/bin/bash
# Double-check MessageMetadataRequest doesn't have Choice constraint elsewhere
cat src/Messaging/Request/Message/MessageMetadataRequest.php | grep -i "choice\|enum"

# Verify the exact line range being reviewed
sed -n '163,169p' src/Messaging/Controller/CampaignActionController.php

Repository: phpList/rest-api

Length of output: 348


Add Assert\Choice validation to the status field in MessageMetadataRequest

The status field currently only has #[Assert\NotBlank] validation. Without an Assert\Choice constraint (like used in MessageFormatRequest), invalid enum backing values slip through validation and cause MessageStatus::from() to throw an unhandled ValueError, returning a 500 instead of a proper 400/422 response.

Add constraint: #[Assert\Choice(['draft', 'sent', ...])] with valid enum backing values.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Messaging/Controller/CampaignActionController.php` around lines 163 -
169, Add an Assert\Choice constraint to the MessageMetadataRequest::$status
validation so invalid enum backing values are rejected before
MessageStatus::from() is called: update the MessageMetadataRequest class to
mirror the approach used in MessageFormatRequest by adding
#[Assert\Choice([...])] with the valid backing strings from the MessageStatus
enum (use the exact enum backing values, e.g. 'draft','sent', etc.), ensuring
the controller never receives bad values that would cause MessageStatus::from()
to throw.

}

#[Route('/{messageId}/send', name: 'send_campaign', requirements: ['messageId' => '\d+'], methods: ['POST'])]
#[OA\Post(
path: '/api/v2/campaigns/{messageId}/send',
description: '🚧 **Status: Beta** – This method is under development. Avoid using in production. ' .
'Processes/sends campaign/message by id.',
summary: 'Processes/sends campaign/message by id.',
tags: ['campaigns'],
parameters: [
new OA\Parameter(
name: 'php-auth-pw',
description: 'Session key obtained from login',
in: 'header',
required: true,
schema: new OA\Schema(type: 'string')
),
new OA\Parameter(
name: 'messageId',
description: 'message ID',
in: 'path',
required: true,
schema: new OA\Schema(type: 'string')
)
],
responses: [
new OA\Response(
response: 200,
description: 'Success',
content: new OA\JsonContent(ref: '#/components/schemas/Message')
),
new OA\Response(
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')
),
]
Comment on lines +195 to +211
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Missing 404 response in OpenAPI documentation

The sendMessage endpoint can return 404 when the campaign is not found (line 226), but this response isn't documented. Same applies to copyMessage (lines 67-78).

📝 Add 404 response to OpenAPI
         responses: [
             new OA\Response(
                 response: 200,
                 description: 'Success',
                 content: new OA\JsonContent(ref: '#/components/schemas/Message')
             ),
             new OA\Response(
                 response: 403,
                 description: 'Failure',
                 content: new OA\JsonContent(ref: '#/components/schemas/UnauthorizedResponse')
-            )
+            ),
+            new OA\Response(
+                response: 404,
+                description: 'Campaign not found',
+                content: new OA\JsonContent(ref: '#/components/schemas/NotFoundErrorResponse')
+            )
         ]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
responses: [
new OA\Response(
response: 200,
description: 'Success',
content: new OA\JsonContent(ref: '#/components/schemas/Message')
),
new OA\Response(
response: 403,
description: 'Failure',
content: new OA\JsonContent(ref: '#/components/schemas/UnauthorizedResponse')
)
]
responses: [
new OA\Response(
response: 200,
description: 'Success',
content: new OA\JsonContent(ref: '#/components/schemas/Message')
),
new OA\Response(
response: 403,
description: 'Failure',
content: new OA\JsonContent(ref: '#/components/schemas/UnauthorizedResponse')
),
new OA\Response(
response: 404,
description: 'Campaign not found',
content: new OA\JsonContent(ref: '#/components/schemas/NotFoundErrorResponse')
)
]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Messaging/Controller/CampaignActionController.php` around lines 207 -
218, Add a 404 OpenAPI response to the responses arrays for both sendMessage and
copyMessage in CampaignActionController.php: update the responses list inside
the OA\Response annotations for the sendMessage and copyMessage endpoints to
include a new OA\Response(response: 404, description: 'Not Found', content: new
OA\JsonContent(ref: '#/components/schemas/NotFoundResponse')) so the
documentation reflects the controller's 404 return when a campaign or resource
is not found.

)]
public function sendMessage(
Request $request,
#[MapEntity(mapping: ['messageId' => 'id'])] ?Message $message = null
): JsonResponse {
$this->requireAuthentication($request);
if ($message === null) {
throw $this->createNotFoundException('Campaign not found.');
}

$this->messageBus->dispatch(new SyncCampaignProcessorMessage($message->getId()));

return $this->json($this->campaignService->getMessage($message), Response::HTTP_OK);
}

#[Route('/{messageId}/resend', name: 'resend_campaign', requirements: ['messageId' => '\d+'], methods: ['POST'])]
#[OA\Post(
path: '/api/v2/campaigns/{messageId}/resend',
description: '🚧 **Status: Beta** – This method is under development. Avoid using in production. ' .
'Processes/sends campaign/message by id to specified mailing lists.',
summary: 'Processes/sends campaign/message by id to lists.',
requestBody: new OA\RequestBody(
description: 'List ids to send this campaign to.',
required: true,
content: new OA\JsonContent(ref: '#/components/schemas/ResendMessageToListsRequest')
),
tags: ['campaigns'],
parameters: [
new OA\Parameter(
name: 'php-auth-pw',
description: 'Session key obtained from login',
in: 'header',
required: true,
schema: new OA\Schema(type: 'string')
),
new OA\Parameter(
name: 'messageId',
description: 'message ID',
in: 'path',
required: true,
schema: new OA\Schema(type: 'string')
)
],
responses: [
new OA\Response(
response: 200,
description: 'Success',
content: new OA\JsonContent(ref: '#/components/schemas/Message')
),
new OA\Response(
response: 403,
description: 'Failure',
content: new OA\JsonContent(ref: '#/components/schemas/UnauthorizedResponse')
)
]
Comment on lines +255 to +266
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Missing 404 response in OpenAPI documentation.

The endpoint throws not-found (line 272) when the campaign doesn't exist, but this isn't documented in the responses. Should mirror sendMessage's OpenAPI spec for consistency.

📝 Proposed fix
             new OA\Response(
                 response: 403,
                 description: 'Failure',
                 content: new OA\JsonContent(ref: '#/components/schemas/UnauthorizedResponse')
-            )
+            ),
+            new OA\Response(
+                response: 404,
+                description: 'Campaign not found',
+                content: new OA\JsonContent(ref: '#/components/schemas/NotFoundErrorResponse')
+            )
         ]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
responses: [
new OA\Response(
response: 200,
description: 'Success',
content: new OA\JsonContent(ref: '#/components/schemas/Message')
),
new OA\Response(
response: 403,
description: 'Failure',
content: new OA\JsonContent(ref: '#/components/schemas/UnauthorizedResponse')
)
]
responses: [
new OA\Response(
response: 200,
description: 'Success',
content: new OA\JsonContent(ref: '#/components/schemas/Message')
),
new OA\Response(
response: 403,
description: 'Failure',
content: new OA\JsonContent(ref: '#/components/schemas/UnauthorizedResponse')
),
new OA\Response(
response: 404,
description: 'Campaign not found',
content: new OA\JsonContent(ref: '#/components/schemas/NotFoundErrorResponse')
)
]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Messaging/Controller/CampaignActionController.php` around lines 253 -
264, The OpenAPI responses for the endpoint in CampaignActionController:: (the
responses array shown) are missing the 404 not-found response; update the
responses block to include a 404 OA\Response with description 'Not Found' and
content referencing the same schema used by sendMessage's spec (e.g.,
OA\JsonContent(ref: '#/components/schemas/NotFoundResponse') or whichever
NotFound schema sendMessage uses) so the documented responses mirror sendMessage
and cover the case where the campaign is not found (the code path that throws
the not-found error).

)]
public function resendMessageToLists(
Request $request,
#[MapEntity(mapping: ['messageId' => 'id'])] ?Message $message = null
): JsonResponse {
$this->requireAuthentication($request);
if ($message === null) {
throw $this->createNotFoundException('Campaign not found.');
}

/** @var ResendMessageToListsRequest $resendToListsRequest */
$resendToListsRequest = $this->validator->validate($request, ResendMessageToListsRequest::class);

$this->messageBus->dispatch(
new SyncCampaignProcessorMessage($message->getId(), $resendToListsRequest->listIds)
);

return $this->json($this->campaignService->getMessage($message), Response::HTTP_OK);
}
}
Loading
Loading