Skip to content

Commit 26a2cc9

Browse files
tatevikg1tatevikg1
committed
After review 0
1 parent 49476e6 commit 26a2cc9

16 files changed

Lines changed: 660 additions & 218 deletions

‎config/parameters.yml.dist‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ parameters:
3737
env(REST_API_BASE_URL): 'http://api.phplist.local/api/v2'
3838
app.frontend_base_url: '%%env(FRONT_END_BASE_URL)%%'
3939
env(FRONT_END_BASE_URL): 'http://frontend.phplist.local'
40-
parallel_use_with_phplist3: '%%env(bool:parallel_use_with_phplist3)%%'
40+
parallel_use_with_phplist3: '%%env(parallel_use_with_phplist3)%%'
4141
env(parallel_use_with_phplist3): '0'
4242

4343
# Email configuration

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

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

77
use Doctrine\ORM\EntityManagerInterface;
8+
use LogicException;
89
use PhpList\Core\Domain\Identity\Model\Administrator;
910
use PhpList\Core\Domain\Subscription\Model\SubscribePage;
1011
use PhpList\Core\Domain\Subscription\Model\SubscribePageData;
@@ -56,6 +57,9 @@ public function createPage(string $title, bool $active = false, ?Administrator $
5657

5758
public function syncPageData(array $data, SubscribePage $page): void
5859
{
60+
if ($page->getId() === null) {
61+
throw new LogicException('Page must be persisted before syncing data');
62+
}
5963
$existingPageData = [];
6064
foreach ($this->getPageData($page) as $pageData) {
6165
$existingPageData[$pageData->getName()] = $pageData;

‎tests/Unit/Domain/Identity/Repository/AdminAttributeValueRepositoryTest.php‎

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,11 @@ public function testFindOneByAdminIdAndAttributeId(): void
5454
$administrator = $this->createMock(Administrator::class);
5555
$administrator->method('getId')->willReturn($adminId);
5656

57-
$expectedResult = new AdminAttributeValue($attributeDefinition, $administrator, 'value');
57+
$expectedResult = new AdminAttributeValue(
58+
$attributeDefinition,
59+
$administrator,
60+
'value'
61+
);
5862

5963
$this->entityManager->expects($this->once())
6064
->method('createQueryBuilder')
@@ -70,13 +74,17 @@ public function testFindOneByAdminIdAndAttributeId(): void
7074
->with(AdminAttributeValue::class, 'aav')
7175
->willReturn($this->queryBuilder);
7276

77+
$joinCalls = [];
78+
7379
$this->queryBuilder->expects($this->exactly(2))
7480
->method('join')
75-
->withConsecutive(
76-
['aav.administrator', 'admin'],
77-
['aav.attributeDefinition', 'attr']
78-
)
79-
->willReturn($this->queryBuilder);
81+
->willReturnCallback(
82+
function (string $join, string $alias) use (&$joinCalls) {
83+
$joinCalls[] = [$join, $alias];
84+
85+
return $this->queryBuilder;
86+
}
87+
);
8088

8189
$this->queryBuilder->expects($this->once())
8290
->method('where')
@@ -88,13 +96,17 @@ public function testFindOneByAdminIdAndAttributeId(): void
8896
->with('attr.id = :attributeId')
8997
->willReturn($this->queryBuilder);
9098

99+
$setParameterCalls = [];
100+
91101
$this->queryBuilder->expects($this->exactly(2))
92102
->method('setParameter')
93-
->withConsecutive(
94-
['adminId', $adminId],
95-
['attributeId', $attributeId]
96-
)
97-
->willReturn($this->queryBuilder);
103+
->willReturnCallback(
104+
function (string $key, mixed $value) use (&$setParameterCalls) {
105+
$setParameterCalls[] = [$key, $value];
106+
107+
return $this->queryBuilder;
108+
}
109+
);
98110

99111
$this->queryBuilder->expects($this->once())
100112
->method('getQuery')
@@ -104,7 +116,26 @@ public function testFindOneByAdminIdAndAttributeId(): void
104116
->method('getOneOrNullResult')
105117
->willReturn($expectedResult);
106118

107-
$result = $this->subject->findOneByAdminIdAndAttributeId($adminId, $attributeId);
119+
$result = $this->subject->findOneByAdminIdAndAttributeId(
120+
$adminId,
121+
$attributeId
122+
);
123+
124+
$this->assertSame(
125+
[
126+
['aav.administrator', 'admin'],
127+
['aav.attributeDefinition', 'attr'],
128+
],
129+
$joinCalls,
130+
);
131+
132+
$this->assertSame(
133+
[
134+
['adminId', $adminId],
135+
['attributeId', $attributeId],
136+
],
137+
$setParameterCalls,
138+
);
108139

109140
$this->assertSame($expectedResult, $result);
110141
}

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

Lines changed: 73 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -108,59 +108,108 @@ public function testSendsToListOwnersWhenFlagEnabled(): void
108108
public function testFallsBackToAdminAddressesWhenNoOwnersOrFlagFalse(): void
109109
{
110110
$configProvider = $this->createMock(ConfigProvider::class);
111+
111112
$configProvider->method('isEnabled')
112113
->with(ConfigOption::SendAdminCopies)
113114
->willReturn(true);
114115

116+
$getValueCalls = [];
117+
115118
$configProvider->expects(self::exactly(2))
116119
->method('getValue')
117-
->withConsecutive([ConfigOption::AdminAddress], [ConfigOption::AdminAddresses])
118-
->willReturnOnConsecutiveCalls(
119-
'single@example.com',
120-
' admin1@example.com, , admin2@example.com ,admin1@example.com '
120+
->willReturnCallback(
121+
function (ConfigOption $key) use (&$getValueCalls): string {
122+
$getValueCalls[] = $key;
123+
124+
return match ($key) {
125+
ConfigOption::AdminAddress => 'single@example.com',
126+
ConfigOption::AdminAddresses =>
127+
' admin1@example.com, , admin2@example.com ,admin1@example.com ',
128+
default => '',
129+
};
130+
}
121131
);
122132

123-
$expectedRecipients = ['admin1@example.com', 'admin2@example.com', 'single@example.com'];
133+
$expectedRecipients = [
134+
'admin1@example.com',
135+
'admin2@example.com',
136+
'single@example.com',
137+
];
124138

125139
$systemEmailBuilder = $this->createMock(SystemEmailBuilder::class);
126-
$systemEmailBuilder->expects(self::exactly(count($expectedRecipients)))
140+
141+
$buildCalls = [];
142+
143+
$systemEmailBuilder
144+
->expects(self::exactly(count($expectedRecipients)))
127145
->method('buildSystemEmail')
128-
->with(self::callback(function (MessagePrecacheDto $data): bool {
129-
return str_starts_with($data->subject, 'phpList ');
130-
}))
131-
->willReturn(new Email());
146+
->willReturnCallback(
147+
function (MessagePrecacheDto $data) use (&$buildCalls): Email {
148+
$buildCalls[] = $data;
149+
150+
return new Email();
151+
}
152+
);
132153

133154
$mailer = $this->createMock(MailerInterface::class);
155+
156+
$sendCalls = [];
157+
134158
$bounce = 'bounce@domain.test';
135-
$i = 0;
159+
136160
$mailer->expects(self::exactly(count($expectedRecipients)))
137161
->method('send')
138-
->with(
139-
self::isInstanceOf(Email::class),
140-
self::callback(function (Envelope $envelope) use ($expectedRecipients, &$i, $bounce): bool {
141-
$sender = $envelope->getSender();
142-
$recipient = $envelope->getRecipients()[0] ?? null;
143-
$expected = $expectedRecipients[$i++] ?? null;
144-
return $sender !== null
145-
&& $sender->getAddress() === $bounce
146-
&& $recipient !== null
147-
&& $recipient->getAddress() === $expected;
148-
})
162+
->willReturnCallback(
163+
function (Email $email, Envelope $envelope) use (&$sendCalls): void {
164+
$sendCalls[] = [$email, $envelope];
165+
}
149166
);
150167

151168
$sender = new AdminCopyEmailSender(
152169
configProvider: $configProvider,
153170
systemEmailBuilder: $systemEmailBuilder,
154171
mailer: $mailer,
155172
logger: $this->createMock(LoggerInterface::class),
156-
// ensure fallback path regardless of list owners
157173
sendListAdminCopy: false,
158174
bounceEmail: $bounce,
159175
);
160176

161-
// Even if lists have owners, flag=false should ignore them and use AdminAddress(es)
162177
$listWithOwner = $this->createListWithOwner('ignored@example.com');
178+
163179
$sender->__invoke('System Update', 'Body', [$listWithOwner]);
180+
181+
$this->assertSame(
182+
[
183+
ConfigOption::AdminAddress,
184+
ConfigOption::AdminAddresses,
185+
],
186+
$getValueCalls,
187+
);
188+
189+
$this->assertCount(count($expectedRecipients), $buildCalls);
190+
191+
foreach ($buildCalls as $call) {
192+
$this->assertInstanceOf(MessagePrecacheDto::class, $call);
193+
$this->assertStringStartsWith('phpList ', $call->subject);
194+
}
195+
196+
$this->assertCount(count($expectedRecipients), $sendCalls);
197+
198+
foreach ($sendCalls as $index => [$email, $envelope]) {
199+
$this->assertInstanceOf(Email::class, $email);
200+
201+
$senderAddress = $envelope->getSender();
202+
$recipient = $envelope->getRecipients()[0] ?? null;
203+
204+
$this->assertNotNull($senderAddress);
205+
$this->assertSame($bounce, $senderAddress->getAddress());
206+
207+
$this->assertNotNull($recipient);
208+
$this->assertSame(
209+
$expectedRecipients[$index],
210+
$recipient->getAddress()
211+
);
212+
}
164213
}
165214

166215
private function createListWithOwner(string $email): SubscriberList

0 commit comments

Comments
 (0)