Skip to content

Commit 158fac6

Browse files
TatevikGrtatevikg1
TatevikGr
and
tatevikg1
authored
Refactor: id in update requests + template::update method (#372)
 Refactor Removed ID fields from several update DTOs and standardized update patterns so managers accept entity instances directly. New Features Added a dedicated template update DTO and enhanced template update flow (conditional field updates, content resolution, link/image validation, image replacement). Tests Unit tests updated to match new DTO shapes and manager signatures. Chores Disabled docstring coverage check in CI configuration. * Update: code rabbit config --------- Co-authored-by: Tatevik <tatevikg1@gmail.com>
1 parent 4c07ff3 commit 158fac6

11 files changed

Lines changed: 111 additions & 32 deletions

File tree

‎.coderabbit.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,3 +73,7 @@ reviews:
7373
# code_guidelines:
7474
# filePatterns:
7575
# - ".github/AGENT.md"
76+
77+
checks:
78+
docstring_coverage:
79+
enabled: false

‎src/Domain/Identity/Model/Dto/UpdateAdministratorDto.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
final class UpdateAdministratorDto
88
{
99
public function __construct(
10-
public readonly int $administratorId,
1110
public readonly ?string $loginName = null,
1211
public readonly ?string $password = null,
1312
public readonly ?string $email = null,

‎src/Domain/Messaging/Model/Dto/UpdateMessageDto.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
class UpdateMessageDto implements MessageDtoInterface
1414
{
1515
public function __construct(
16-
public readonly int $messageId,
1716
public readonly MessageContentDto $content,
1817
public readonly MessageFormatDto $format,
1918
public readonly MessageMetadataDto $metadata,

‎src/Domain/Messaging/Model/Dto/UpdateTemplateDto.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PhpList\Core\Domain\Messaging\Model\Dto;
6+
7+
class UpdateTemplateDto
8+
{
9+
/**
10+
* @SuppressWarnings("BooleanArgumentFlag")
11+
*/
12+
public function __construct(
13+
public readonly ?string $title = null,
14+
public readonly ?string $content = null,
15+
public readonly ?string $text = null,
16+
public readonly ?string $fileContent = null,
17+
public readonly bool $shouldCheckLinks = false,
18+
public readonly bool $shouldCheckImages = false,
19+
public readonly bool $shouldCheckExternalImages = false,
20+
) {
21+
}
22+
}

‎src/Domain/Messaging/Service/Manager/TemplateManager.php‎

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,11 @@
66

77
use PhpList\Core\Domain\Common\Model\ValidationContext;
88
use PhpList\Core\Domain\Messaging\Model\Dto\CreateTemplateDto;
9+
use PhpList\Core\Domain\Messaging\Model\Dto\UpdateTemplateDto;
910
use PhpList\Core\Domain\Messaging\Model\Template;
1011
use PhpList\Core\Domain\Messaging\Repository\TemplateRepository;
1112
use PhpList\Core\Domain\Messaging\Validator\TemplateImageValidator;
1213
use PhpList\Core\Domain\Messaging\Validator\TemplateLinkValidator;
13-
use PhpList\Core\Domain\Subscription\Model\Dto\UpdateSubscriberDto;
14-
use PhpList\Core\Domain\Subscription\Model\Subscriber;
1514

1615
class TemplateManager
1716
{
@@ -35,11 +34,11 @@ public function __construct(
3534
public function create(CreateTemplateDto $createTemplateDto): Template
3635
{
3736
$template = (new Template($createTemplateDto->title))
38-
->setContent($createTemplateDto->content)
3937
->setText($createTemplateDto->text);
4038

41-
if ($createTemplateDto->fileContent) {
42-
$template->setContent($createTemplateDto->fileContent);
39+
$content = $createTemplateDto->fileContent ?? $createTemplateDto->content;
40+
if ($content !== null) {
41+
$template->setContent($content);
4342
}
4443

4544
$context = (new ValidationContext())
@@ -59,19 +58,38 @@ public function create(CreateTemplateDto $createTemplateDto): Template
5958
return $template;
6059
}
6160

62-
public function update(UpdateSubscriberDto $updateSubscriberDto): Subscriber
61+
public function update(Template $template, UpdateTemplateDto $updateTemplateDto): Template
6362
{
64-
/** @var Subscriber $subscriber */
65-
$subscriber = $this->templateRepository->find($updateSubscriberDto->subscriberId);
63+
if ($updateTemplateDto->title !== null) {
64+
$template->setTitle($updateTemplateDto->title);
65+
}
66+
67+
if ($updateTemplateDto->text !== null) {
68+
$template->setText($updateTemplateDto->text);
69+
}
70+
71+
$content = $updateTemplateDto->fileContent ?? $updateTemplateDto->content;
72+
if ($content !== null) {
73+
$template->setContent($content);
74+
}
75+
76+
$context = (new ValidationContext())
77+
->set('checkLinks', $updateTemplateDto->shouldCheckLinks)
78+
->set('checkImages', $updateTemplateDto->shouldCheckImages)
79+
->set('checkExternalImages', $updateTemplateDto->shouldCheckExternalImages);
80+
81+
$this->templateLinkValidator->validate($template->getContent() ?? '', $context);
82+
83+
$imageUrls = $this->templateImageManager->extractAllImages($template->getContent() ?? '');
84+
$this->templateImageValidator->validate($imageUrls, $context);
6685

67-
$subscriber->setEmail($updateSubscriberDto->email);
68-
$subscriber->setConfirmed($updateSubscriberDto->confirmed);
69-
$subscriber->setBlacklisted($updateSubscriberDto->blacklisted);
70-
$subscriber->setHtmlEmail($updateSubscriberDto->htmlEmail);
71-
$subscriber->setDisabled($updateSubscriberDto->disabled);
72-
$subscriber->setExtraData($updateSubscriberDto->additionalData);
86+
foreach ($template->getImages() as $image) {
87+
$this->templateImageManager->delete($image);
88+
}
89+
90+
$this->templateImageManager->createImagesFromImagePaths($imageUrls, $template);
7391

74-
return $subscriber;
92+
return $template;
7593
}
7694

7795
public function delete(Template $template): void

‎src/Domain/Subscription/Model/Dto/UpdateSubscriberDto.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
class UpdateSubscriberDto
88
{
99
public function __construct(
10-
public readonly int $subscriberId,
1110
public readonly string $email,
1211
public readonly bool $confirmed,
1312
public readonly bool $blacklisted,

‎src/Domain/Subscription/Service/Manager/SubscriberManager.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,11 @@ public function getSubscriberById(int $subscriberId): ?Subscriber
5858
return $this->subscriberRepository->find($subscriberId);
5959
}
6060

61-
public function updateSubscriber(UpdateSubscriberDto $subscriberDto, Administrator $admin): Subscriber
62-
{
63-
/** @var Subscriber $subscriber */
64-
$subscriber = $this->subscriberRepository->find($subscriberDto->subscriberId);
65-
61+
public function updateSubscriber(
62+
Subscriber $subscriber,
63+
UpdateSubscriberDto $subscriberDto,
64+
Administrator $admin
65+
): Subscriber {
6666
$subscriber->setEmail($subscriberDto->email);
6767
$subscriber->setConfirmed($subscriberDto->confirmed);
6868
$subscriber->setBlacklisted($subscriberDto->blacklisted);

‎tests/Unit/Domain/Identity/Service/AdministratorManagerTest.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ public function testUpdateAdministrator(): void
5454
$admin->setPasswordHash('old_hash');
5555

5656
$dto = new UpdateAdministratorDto(
57-
administratorId: 1,
5857
loginName: 'new',
5958
password: 'newpass',
6059
email: 'new@example.com',

‎tests/Unit/Domain/Messaging/Service/Manager/MessageManagerTest.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@ public function testUpdateMessageReturnsUpdatedMessage(): void
104104
);
105105

106106
$updateRequest = new UpdateMessageDto(
107-
messageId: 1,
108107
content: $content,
109108
format: $format,
110109
metadata: $metadata,

‎tests/Unit/Domain/Messaging/Service/Manager/TemplateManagerTest.php‎

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
namespace PhpList\Core\Tests\Unit\Domain\Messaging\Service\Manager;
66

77
use PhpList\Core\Domain\Messaging\Model\Dto\CreateTemplateDto;
8+
use PhpList\Core\Domain\Messaging\Model\Dto\UpdateTemplateDto;
89
use PhpList\Core\Domain\Messaging\Model\Template;
910
use PhpList\Core\Domain\Messaging\Repository\TemplateRepository;
1011
use PhpList\Core\Domain\Messaging\Service\Manager\TemplateImageManager;
@@ -30,13 +31,53 @@ protected function setUp(): void
3031
$this->templateImageValidator = $this->createMock(TemplateImageValidator::class);
3132

3233
$this->manager = new TemplateManager(
33-
$this->templateRepository,
34-
$this->templateImageManager,
35-
$this->templateLinkValidator,
36-
$this->templateImageValidator
34+
templateRepository: $this->templateRepository,
35+
templateImageManager: $this->templateImageManager,
36+
templateLinkValidator: $this->templateLinkValidator,
37+
templateImageValidator: $this->templateImageValidator
3738
);
3839
}
3940

41+
public function testUpdateTemplateSuccessfully(): void
42+
{
43+
$existing = (new Template('Old Title'))
44+
->setContent('<html><body>Old</body></html>')
45+
->setText(null);
46+
47+
$dto = new UpdateTemplateDto(
48+
title: 'New Title',
49+
content: '<html><body>New</body></html>',
50+
text: 'New text',
51+
fileContent: null,
52+
shouldCheckLinks: true,
53+
shouldCheckImages: true,
54+
shouldCheckExternalImages: true,
55+
);
56+
57+
$this->templateLinkValidator->expects($this->once())
58+
->method('validate')
59+
->with($dto->content, $this->anything());
60+
61+
$this->templateImageManager->expects($this->once())
62+
->method('extractAllImages')
63+
->with($dto->content)
64+
->willReturn(['a.png']);
65+
66+
$this->templateImageValidator->expects($this->once())
67+
->method('validate')
68+
->with(['a.png'], $this->anything());
69+
70+
$this->templateImageManager->expects($this->once())
71+
->method('createImagesFromImagePaths')
72+
->with(['a.png'], $this->isInstanceOf(Template::class));
73+
74+
$updated = $this->manager->update(template: $existing, updateTemplateDto: $dto);
75+
76+
$this->assertSame('New Title', $updated->getTitle());
77+
$this->assertSame('New text', $updated->getText());
78+
$this->assertSame($dto->content, $updated->getContent());
79+
}
80+
4081
public function testCreateTemplateSuccessfully(): void
4182
{
4283
$request = new CreateTemplateDto(

0 commit comments

Comments
 (0)