From 16d3809fac91030d0a1c125fd7901be79f320fd4 Mon Sep 17 00:00:00 2001 From: Tatevik Date: Wed, 20 May 2026 13:21:53 +0400 Subject: [PATCH 1/3] remove extra code --- .../Service/Manager/SubscribePageManager.php | 14 --------- .../Manager/SubscribePageManagerTest.php | 31 ------------------- 2 files changed, 45 deletions(-) diff --git a/src/Domain/Subscription/Service/Manager/SubscribePageManager.php b/src/Domain/Subscription/Service/Manager/SubscribePageManager.php index 4f9474d5..523ba2e7 100644 --- a/src/Domain/Subscription/Service/Manager/SubscribePageManager.php +++ b/src/Domain/Subscription/Service/Manager/SubscribePageManager.php @@ -10,8 +10,6 @@ use PhpList\Core\Domain\Subscription\Model\SubscribePageData; use PhpList\Core\Domain\Subscription\Repository\SubscriberPageDataRepository; use PhpList\Core\Domain\Subscription\Repository\SubscriberPageRepository; -use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; -use Symfony\Contracts\Translation\TranslatorInterface; class SubscribePageManager { @@ -19,7 +17,6 @@ public function __construct( private readonly SubscriberPageRepository $pageRepository, private readonly SubscriberPageDataRepository $pageDataRepository, private readonly EntityManagerInterface $entityManager, - private readonly TranslatorInterface $translator, ) { } @@ -35,17 +32,6 @@ public function createPage(string $title, bool $active = false, ?Administrator $ return $page; } - public function getPage(int $id): SubscribePage - { - /** @var SubscribePage|null $page */ - $page = $this->pageRepository->find($id); - if (!$page) { - throw new NotFoundHttpException($this->translator->trans('Subscribe page not found')); - } - - return $page; - } - public function updatePage( SubscribePage $page, ?string $title = null, diff --git a/tests/Unit/Domain/Subscription/Service/Manager/SubscribePageManagerTest.php b/tests/Unit/Domain/Subscription/Service/Manager/SubscribePageManagerTest.php index 0c29242a..8d0dff73 100644 --- a/tests/Unit/Domain/Subscription/Service/Manager/SubscribePageManagerTest.php +++ b/tests/Unit/Domain/Subscription/Service/Manager/SubscribePageManagerTest.php @@ -13,8 +13,6 @@ use PhpList\Core\Domain\Subscription\Service\Manager\SubscribePageManager; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; -use Symfony\Component\Translation\Translator; class SubscribePageManagerTest extends TestCase { @@ -33,7 +31,6 @@ protected function setUp(): void pageRepository: $this->pageRepository, pageDataRepository: $this->pageDataRepository, entityManager: $this->entityManager, - translator: new Translator('en'), ); } @@ -53,34 +50,6 @@ public function testCreatePageCreatesAndSaves(): void $this->assertSame($owner, $page->getOwner()); } - public function testGetPageReturnsPage(): void - { - $page = new SubscribePage(); - $this->pageRepository - ->expects($this->once()) - ->method('find') - ->with(123) - ->willReturn($page); - - $result = $this->manager->getPage(123); - - $this->assertSame($page, $result); - } - - public function testGetPageThrowsWhenNotFound(): void - { - $this->pageRepository - ->expects($this->once()) - ->method('find') - ->with(999) - ->willReturn(null); - - $this->expectException(NotFoundHttpException::class); - $this->expectExceptionMessage('Subscribe page not found'); - - $this->manager->getPage(999); - } - public function testUpdatePageUpdatesProvidedFieldsAndFlushes(): void { $originalOwner = new Administrator(); From e87632092443189305d65a11eeb1fda88295a9cc Mon Sep 17 00:00:00 2001 From: Tatevik Date: Thu, 21 May 2026 18:24:44 +0400 Subject: [PATCH 2/3] syncPageData --- .../Service/Manager/SubscribePageManager.php | 28 +++ .../Manager/SubscribePageManagerTest.php | 191 ++++++++++++++++++ 2 files changed, 219 insertions(+) diff --git a/src/Domain/Subscription/Service/Manager/SubscribePageManager.php b/src/Domain/Subscription/Service/Manager/SubscribePageManager.php index 523ba2e7..d46b3f90 100644 --- a/src/Domain/Subscription/Service/Manager/SubscribePageManager.php +++ b/src/Domain/Subscription/Service/Manager/SubscribePageManager.php @@ -32,6 +32,34 @@ public function createPage(string $title, bool $active = false, ?Administrator $ return $page; } + public function syncPageData(array $data, SubscribePage $page): void + { + $existingPageData = []; + foreach ($this->getPageData($page) as $pageData) { + $existingPageData[$pageData->getName()] = $pageData; + } + + foreach ($data as $pageDataKey => $value) { + if (isset($existingPageData[$pageDataKey])) { + $pageData = $existingPageData[$pageDataKey]; + $pageData->setData($value); + unset($existingPageData[$pageDataKey]); + continue; + } + + $pageData = (new SubscribePageData()) + ->setId($page->getId()) + ->setName($pageDataKey) + ->setData($value); + + $this->pageDataRepository->persist($pageData); + } + + foreach ($existingPageData as $pageData) { + $this->entityManager->remove($pageData); + } + } + public function updatePage( SubscribePage $page, ?string $title = null, diff --git a/tests/Unit/Domain/Subscription/Service/Manager/SubscribePageManagerTest.php b/tests/Unit/Domain/Subscription/Service/Manager/SubscribePageManagerTest.php index 8d0dff73..443f03af 100644 --- a/tests/Unit/Domain/Subscription/Service/Manager/SubscribePageManagerTest.php +++ b/tests/Unit/Domain/Subscription/Service/Manager/SubscribePageManagerTest.php @@ -194,4 +194,195 @@ public function testSetPageDataCreatesNewWhenMissingAndPersistsAndFlushes(): voi $this->assertSame('greeting', $result->getName()); $this->assertSame('hello', $result->getData()); } + + public function testSyncPageDataWithEmptyExistingDataCreatesNewEntries(): void + { + $page = $this->getMockBuilder(SubscribePage::class) + ->onlyMethods(['getId']) + ->getMock(); + $page->method('getId')->willReturn(42); + + $this->pageDataRepository + ->expects($this->once()) + ->method('getByPage') + ->with($page) + ->willReturn([]); + + $this->pageDataRepository + ->expects($this->exactly(2)) + ->method('persist') + ->with($this->isInstanceOf(SubscribePageData::class)); + + $this->entityManager + ->expects($this->never()) + ->method('remove'); + + $data = [ + 'field1' => 'value1', + 'field2' => 'value2', + ]; + + $this->manager->syncPageData($data, $page); + } + + public function testSyncPageDataUpdatesExistingEntries(): void + { + $page = new SubscribePage(); + $pageData1 = new SubscribePageData(); + $pageData1->setName('field1')->setData('old_value1'); + $pageData2 = new SubscribePageData(); + $pageData2->setName('field2')->setData('old_value2'); + + $this->pageDataRepository + ->expects($this->once()) + ->method('getByPage') + ->with($page) + ->willReturn([$pageData1, $pageData2]); + + $this->pageDataRepository + ->expects($this->never()) + ->method('persist'); + + $this->entityManager + ->expects($this->never()) + ->method('remove'); + + $data = [ + 'field1' => 'new_value1', + 'field2' => 'new_value2', + ]; + + $this->manager->syncPageData($data, $page); + + $this->assertSame('new_value1', $pageData1->getData()); + $this->assertSame('new_value2', $pageData2->getData()); + } + + public function testSyncPageDataRemovesExistingEntriesNotInNewData(): void + { + $page = new SubscribePage(); + $pageData1 = new SubscribePageData(); + $pageData1->setName('field1')->setData('value1'); + $pageData2 = new SubscribePageData(); + $pageData2->setName('field2')->setData('value2'); + + $this->pageDataRepository + ->expects($this->once()) + ->method('getByPage') + ->with($page) + ->willReturn([$pageData1, $pageData2]); + + $this->pageDataRepository + ->expects($this->never()) + ->method('persist'); + + $this->entityManager + ->expects($this->exactly(1)) + ->method('remove') + ->with($pageData2); + + $data = [ + 'field1' => 'value1', + ]; + + $this->manager->syncPageData($data, $page); + } + + public function testSyncPageDataWithMixedOperationsCreateUpdateDelete(): void + { + $page = $this->getMockBuilder(SubscribePage::class) + ->onlyMethods(['getId']) + ->getMock(); + $page->method('getId')->willReturn(10); + + $existingData1 = new SubscribePageData(); + $existingData1->setName('keep_and_update')->setData('old_value'); + $existingData2 = new SubscribePageData(); + $existingData2->setName('to_delete')->setData('delete_me'); + + $this->pageDataRepository + ->expects($this->once()) + ->method('getByPage') + ->with($page) + ->willReturn([$existingData1, $existingData2]); + + $this->pageDataRepository + ->expects($this->once()) + ->method('persist') + ->with($this->isInstanceOf(SubscribePageData::class)); + + $this->entityManager + ->expects($this->once()) + ->method('remove') + ->with($existingData2); + + $data = [ + 'keep_and_update' => 'updated_value', + 'new_field' => 'new_value', + ]; + + $this->manager->syncPageData($data, $page); + + $this->assertSame('updated_value', $existingData1->getData()); + } + + public function testSyncPageDataWithEmptyDataRemovesAllExistingEntries(): void + { + $page = new SubscribePage(); + $pageData1 = new SubscribePageData(); + $pageData1->setName('field1')->setData('value1'); + $pageData2 = new SubscribePageData(); + $pageData2->setName('field2')->setData('value2'); + + $this->pageDataRepository + ->expects($this->once()) + ->method('getByPage') + ->with($page) + ->willReturn([$pageData1, $pageData2]); + + $this->pageDataRepository + ->expects($this->never()) + ->method('persist'); + + $this->entityManager + ->expects($this->exactly(2)) + ->method('remove') + ->withConsecutive([$pageData1], [$pageData2]); + + $data = []; + + $this->manager->syncPageData($data, $page); + } + + public function testSyncPageDataPreservesExistingDataObjectsWhenKeepingEntries(): void + { + $page = new SubscribePage(); + $originalPageData = new SubscribePageData(); + $originalPageData->setId(99)->setName('color')->setData('red'); + + $this->pageDataRepository + ->expects($this->once()) + ->method('getByPage') + ->with($page) + ->willReturn([$originalPageData]); + + $this->pageDataRepository + ->expects($this->never()) + ->method('persist'); + + $this->entityManager + ->expects($this->never()) + ->method('remove'); + + $data = [ + 'color' => 'blue', + ]; + + $this->manager->syncPageData($data, $page); + + // Should have updated the same object, not created a new one + $this->assertSame(99, $originalPageData->getId()); + $this->assertSame('color', $originalPageData->getName()); + $this->assertSame('blue', $originalPageData->getData()); + } } From 4a1084b356c2e2a3d5c0eab6ff79a06d273497b4 Mon Sep 17 00:00:00 2001 From: Tatevik Date: Thu, 21 May 2026 19:59:17 +0400 Subject: [PATCH 3/3] load data into page --- .../Subscription/Model/SubscribePage.php | 16 ++++ .../Repository/SubscriberPageRepository.php | 91 +++++++++++++++++-- .../Service/Manager/SubscribePageManager.php | 7 +- 3 files changed, 106 insertions(+), 8 deletions(-) diff --git a/src/Domain/Subscription/Model/SubscribePage.php b/src/Domain/Subscription/Model/SubscribePage.php index 979b3c4c..3b484920 100644 --- a/src/Domain/Subscription/Model/SubscribePage.php +++ b/src/Domain/Subscription/Model/SubscribePage.php @@ -30,6 +30,9 @@ class SubscribePage implements DomainModel, Identity, OwnableInterface #[ORM\JoinColumn(name: 'owner', referencedColumnName: 'id', nullable: true)] private ?Administrator $owner = null; + /** @var SubscribePageData[] */ + private array $data = []; + public function getId(): ?int { return $this->id; @@ -50,6 +53,12 @@ public function getOwner(): ?Administrator return $this->owner; } + /** @return SubscribePageData[] */ + public function getData(): array + { + return $this->data; + } + public function setTitle(string $title): self { $this->title = $title; @@ -67,4 +76,11 @@ public function setOwner(?Administrator $owner): self $this->owner = $owner; return $this; } + + /** @param SubscribePageData[] $data */ + public function setData(array $data): self + { + $this->data = $data; + return $this; + } } diff --git a/src/Domain/Subscription/Repository/SubscriberPageRepository.php b/src/Domain/Subscription/Repository/SubscriberPageRepository.php index 136b589c..c9f39449 100644 --- a/src/Domain/Subscription/Repository/SubscriberPageRepository.php +++ b/src/Domain/Subscription/Repository/SubscriberPageRepository.php @@ -4,6 +4,8 @@ namespace PhpList\Core\Domain\Subscription\Repository; +use PhpList\Core\Domain\Common\Model\Filter\FilterRequestInterface; +use PhpList\Core\Domain\Common\Model\PaginatedResult; use PhpList\Core\Domain\Common\Repository\AbstractRepository; use PhpList\Core\Domain\Common\Repository\CursorPaginationTrait; use PhpList\Core\Domain\Common\Repository\Interfaces\PaginatableRepositoryInterface; @@ -14,17 +16,92 @@ class SubscriberPageRepository extends AbstractRepository implements Paginatable { use CursorPaginationTrait; - /** @return array{page: SubscribePage, data: SubscribePageData}[] */ - public function findPagesWithData(int $pageId): array + public function findPageWithData(int $pageId): ?SubscribePage { - return $this->createQueryBuilder('p') - ->select('p AS page, d AS data') - ->from(SubscribePage::class, 'p') - ->from(SubscribePageData::class, 'd') + $result = $this->createQueryBuilder('p') + ->select('p AS page', 'd AS data') + ->leftJoin( + SubscribePageData::class, + 'd', + 'ON', + 'd.id = p.id' + ) ->where('p.id = :id') - ->andWhere('d.id = p.id') ->setParameter('id', $pageId) ->getQuery() ->getResult(); + + if ($result === []) { + return null; + } + + return $this->loadPageData($result); + } + + public function getFilteredAfterId(FilterRequestInterface $filter): PaginatedResult + { + $queryBuilder = $this->createQueryBuilder('p'); + + $countQb = clone $queryBuilder; + $total = (int) $countQb + ->select('COUNT(DISTINCT p.id)') + ->getQuery() + ->getSingleScalarResult(); + + $rows = $queryBuilder + ->select('p AS page', 'd AS data') + ->leftJoin( + SubscribePageData::class, + 'd', + 'ON', + 'd.id = p.id' + ) + ->andWhere('p.id > :afterId') + ->setParameter('afterId', $filter->getLastId()) + ->orderBy('p.id', 'ASC') + ->setMaxResults($filter->getLimit()) + ->getQuery() + ->getResult(); + + $grouped = []; + foreach ($rows as $row) { + /** @var SubscribePage $page */ + $page = $row['page'] ?? null; + $data = $row['data'] ?? null; + if ($page !== null) { + $grouped[$page->getId()][] = $row; + } + if ($data !== null) { + $grouped[$data->getId()][] = ['data' => $data]; + } + } + + $pages = []; + foreach ($grouped as $group) { + $pages[] = $this->loadPageData($group); + } + + return new PaginatedResult( + items: array_values($pages), + total: $total, + limit: $filter->getLimit(), + lastId: $filter->getLastId(), + ); + } + + private function loadPageData(array $result): SubscribePage + { + /** @var SubscribePage $page */ + $page = array_shift($result)['page']; + + $data = []; + foreach ($result as $row) { + if (isset($row['data'])) { + $data[] = $row['data']; + } + } + $page->setData($data); + + return $page; } } diff --git a/src/Domain/Subscription/Service/Manager/SubscribePageManager.php b/src/Domain/Subscription/Service/Manager/SubscribePageManager.php index d46b3f90..8d5b78b4 100644 --- a/src/Domain/Subscription/Service/Manager/SubscribePageManager.php +++ b/src/Domain/Subscription/Service/Manager/SubscribePageManager.php @@ -20,6 +20,11 @@ public function __construct( ) { } + public function findPage(int $id): ?SubscribePage + { + return $this->pageRepository->findPageWithData($id); + } + public function createPage(string $title, bool $active = false, ?Administrator $owner = null): SubscribePage { $page = new SubscribePage(); @@ -92,7 +97,7 @@ public function deletePage(SubscribePage $page): void /** @return SubscribePageData[] */ public function getPageData(SubscribePage $page): array { - return $this->pageDataRepository->getByPage($page,); + return $this->pageDataRepository->getByPage($page); } public function setPageData(SubscribePage $page, string $name, ?string $value): SubscribePageData