Skip to content

Commit d99ae7e

Browse files
tatevikg1tatevikg1
committed
Fix: subscribers getFilteredAfterId
1 parent 9af573e commit d99ae7e

4 files changed

Lines changed: 103 additions & 34 deletions

File tree

‎src/Domain/Common/Model/Filter/PaginatedFilter.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
namespace PhpList\Core\Domain\Common\Model\Filter;
66

7+
use InvalidArgumentException;
8+
79
class PaginatedFilter implements FilterRequestInterface
810
{
911
private int $lastId = 0;
@@ -23,6 +25,10 @@ public function getLastId(): int
2325

2426
public function setLimit(int $limit): self
2527
{
28+
if ($limit <= 0) {
29+
throw new InvalidArgumentException('Limit must be greater than 0.');
30+
}
31+
2632
$this->limit = $limit;
2733

2834
return $this;

‎src/Domain/Subscription/Model/Filter/SubscriberFilter.php‎

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

77
use DateTimeImmutable;
8+
use InvalidArgumentException;
89
use PhpList\Core\Domain\Common\Model\Filter\FilterRequestInterface;
910
use PhpList\Core\Domain\Common\Model\Filter\PaginatedFilter;
1011

@@ -20,6 +21,7 @@ class SubscriberFilter extends PaginatedFilter implements FilterRequestInterface
2021
private ?DateTimeImmutable $updatedDateTo;
2122
private ?bool $isConfirmed;
2223
private ?bool $isBlacklisted;
24+
/** @var list<string> */
2325
private array $columns;
2426
private ?string $findColumn;
2527
private ?string $findValue;
@@ -40,6 +42,11 @@ public function __construct(
4042
int $lastId = 0,
4143
int $limit = 50,
4244
) {
45+
$allowedFindColumns = ['email', 'foreignKey', 'uniqueId'];
46+
if ($findColumn !== null && !in_array($findColumn, $allowedFindColumns, true)) {
47+
throw new InvalidArgumentException('Invalid search column.');
48+
}
49+
4350
$this->listId = $listId;
4451
$this->subscribedDateFrom = $subscribedDateFrom;
4552
$this->subscribedDateTo = $subscribedDateTo;
@@ -101,6 +108,7 @@ public function getIsBlacklisted(): ?bool
101108
return $this->isBlacklisted;
102109
}
103110

111+
/** @return list<string> */
104112
public function getColumns(): array
105113
{
106114
return $this->columns;

‎src/Domain/Subscription/Repository/SubscriberRepository.php‎

Lines changed: 87 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -81,59 +81,91 @@ public function getSubscribersBySubscribedListId(int $listId): array
8181
*/
8282
public function getFilteredAfterId(FilterRequestInterface $filter): PaginatedResult
8383
{
84-
$lastId = $filter->getLastId();
85-
$limit = $filter->getLimit();
8684
if (!$filter instanceof SubscriberFilter) {
87-
throw new InvalidArgumentException('Expected SubscriberFilterRequest.');
85+
throw new InvalidArgumentException('Expected SubscriberFilter.');
8886
}
8987

90-
$queryBuilder = $this->createQueryBuilder('subscriber')
91-
->leftJoin('subscriber.subscriptions', 'subscription')
92-
->leftJoin('subscription.subscriberList', 'list');
88+
$lastId = $filter->getLastId();
89+
$limit = $filter->getLimit();
9390

94-
if ($filter->getListId() !== null) {
95-
$queryBuilder->andWhere('list.id = :listId')
96-
->setParameter('listId', $filter->getListId());
97-
if ($filter->getSubscribedDateFrom() !== null) {
98-
$queryBuilder->andWhere('subscription.createdAt > :subscribedAtFrom')
99-
->setParameter('subscribedAtFrom', $filter->getSubscribedDateFrom());
91+
$applyFilters = function (QueryBuilder $queryBuilder) use ($filter): void {
92+
$queryBuilder
93+
->leftJoin('subscriber.subscriptions', 'subscription')
94+
->leftJoin('subscription.subscriberList', 'list');
95+
96+
$this->applyListIdFilter($filter, $queryBuilder);
97+
$this->applyTimeFilter($filter, $queryBuilder);
98+
99+
if ($filter->getIsConfirmed() !== null) {
100+
$queryBuilder
101+
->andWhere('subscriber.confirmed = :isConfirmed')
102+
->setParameter('isConfirmed', $filter->getIsConfirmed());
100103
}
101-
if ($filter->getSubscribedDateTo() !== null) {
102-
$queryBuilder->andWhere('subscription.createdAt < :subscribedAtTo')
103-
->setParameter('subscribedAtTo', $filter->getSubscribedDateTo());
104+
105+
if ($filter->getIsBlacklisted() !== null) {
106+
$queryBuilder
107+
->andWhere('subscriber.blacklisted = :isBlacklisted')
108+
->setParameter('isBlacklisted', $filter->getIsBlacklisted());
104109
}
105-
}
106110

107-
$this->applyTimeFilter($filter, $queryBuilder);
111+
if ($filter->getFindColumn() && $filter->getFindValue()) {
112+
$queryBuilder
113+
->andWhere(sprintf('subscriber.%s LIKE :search', $filter->getFindColumn()))
114+
->setParameter('search', '%' . $filter->getFindValue() . '%');
115+
}
116+
};
108117

109-
if ($filter->getIsConfirmed() !== null) {
110-
$queryBuilder->andWhere('subscriber.confirmed = :isConfirmed')
111-
->setParameter('isConfirmed', $filter->getIsConfirmed());
112-
}
113-
if ($filter->getIsBlacklisted() !== null) {
114-
$queryBuilder->andWhere('subscriber.blacklisted = :isBlacklisted')
115-
->setParameter('isBlacklisted', $filter->getIsBlacklisted());
116-
}
117-
if ($filter->getFindColumn() && $filter->getFindValue()) {
118-
$queryBuilder->andWhere('subscriber.' . $filter->getFindColumn() . ' LIKE :search')
119-
->setParameter('search', '%' . $filter->getFindValue() . '%');
120-
}
118+
$countQb = $this->createQueryBuilder('subscriber')
119+
->select('COUNT(DISTINCT subscriber.id)');
120+
121+
$applyFilters($countQb);
121122

122-
$countQb = clone $queryBuilder;
123123
$total = (int) $countQb
124-
->select('COUNT(DISTINCT subscriber.id)')
125124
->getQuery()
126125
->getSingleScalarResult();
127126

128-
/** @var list<Subscriber> $items */
129-
$items = $queryBuilder
127+
$idsQb = $this->createQueryBuilder('subscriber')
128+
->select('DISTINCT subscriber.id');
129+
130+
$applyFilters($idsQb);
131+
132+
$rawIds = $idsQb
130133
->andWhere('subscriber.id > :lastId')
131134
->setParameter('lastId', $lastId)
132135
->orderBy('subscriber.id', 'ASC')
133-
->setMaxResults($limit)
136+
->setMaxResults($limit + 1)
137+
->getQuery()
138+
->getScalarResult();
139+
140+
$ids = array_map(static fn(array $row): int => (int) $row['id'], $rawIds);
141+
142+
$hasMore = count($ids) > $limit;
143+
if ($hasMore) {
144+
array_pop($ids);
145+
}
146+
147+
if ($ids === []) {
148+
return new PaginatedResult(
149+
items: [],
150+
total: $total,
151+
limit: $limit,
152+
lastId: $lastId,
153+
);
154+
}
155+
156+
/** @var list<Subscriber> $items */
157+
$items = $this->createQueryBuilder('subscriber')
158+
->select('DISTINCT subscriber, subscription, list')
159+
->leftJoin('subscriber.subscriptions', 'subscription')
160+
->leftJoin('subscription.subscriberList', 'list')
161+
->andWhere('subscriber.id IN (:ids)')
162+
->setParameter('ids', $ids)
163+
->orderBy('subscriber.id', 'ASC')
134164
->getQuery()
135165
->getResult();
136166

167+
usort($items, static fn(Subscriber $first, Subscriber $second): int => $first->getId() <=> $second->getId());
168+
137169
return new PaginatedResult(
138170
items: $items,
139171
total: $total,
@@ -142,6 +174,27 @@ public function getFilteredAfterId(FilterRequestInterface $filter): PaginatedRes
142174
);
143175
}
144176

177+
private function applyListIdFilter(SubscriberFilter $filter, QueryBuilder $queryBuilder): void
178+
{
179+
if ($filter->getListId() !== null) {
180+
$queryBuilder
181+
->andWhere('list.id = :listId')
182+
->setParameter('listId', $filter->getListId());
183+
184+
if ($filter->getSubscribedDateFrom() !== null) {
185+
$queryBuilder
186+
->andWhere('subscription.createdAt > :subscribedAtFrom')
187+
->setParameter('subscribedAtFrom', $filter->getSubscribedDateFrom());
188+
}
189+
190+
if ($filter->getSubscribedDateTo() !== null) {
191+
$queryBuilder
192+
->andWhere('subscription.createdAt < :subscribedAtTo')
193+
->setParameter('subscribedAtTo', $filter->getSubscribedDateTo());
194+
}
195+
}
196+
}
197+
145198
private function applyTimeFilter(SubscriberFilter $filter, QueryBuilder $queryBuilder): void
146199
{
147200
if ($filter->getCreatedDateFrom() !== null) {

‎tests/Unit/Domain/Subscription/Service/SubscriberCsvExporterTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use PhpList\Core\Domain\Subscription\Repository\SubscriberAttributeDefinitionRepository;
1313
use PhpList\Core\Domain\Subscription\Repository\SubscriberRepository;
1414
use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberAttributeManager;
15+
use PhpList\Core\Domain\Subscription\Service\Resolver\AttributeValueResolver;
1516
use PhpList\Core\Domain\Subscription\Service\SubscriberCsvExporter;
1617
use PHPUnit\Framework\MockObject\MockObject;
1718
use PHPUnit\Framework\TestCase;
@@ -33,6 +34,7 @@ protected function setUp(): void
3334

3435
$this->subject = new SubscriberCsvExporter(
3536
$this->attributeManagerMock,
37+
$this->createMock(AttributeValueResolver::class),
3638
$this->subscriberRepositoryMock,
3739
$this->attributeDefinitionRepositoryMock,
3840
$this->createMock(LoggerInterface::class)

0 commit comments

Comments
 (0)