From 953a7cc8fd12ddd748f8811640a3d8e28dff4bb2 Mon Sep 17 00:00:00 2001 From: Supun Wimalasena Date: Wed, 25 Feb 2026 13:26:41 +0100 Subject: [PATCH 01/23] wip, refactor, create subscriber update, etc. --- .../Console/Controller/ApiKeyController.php | 6 +- .../Console/Controller/ApprovalController.php | 18 +++--- .../Console/Controller/IssueController.php | 8 +-- .../Controller/NewsletterController.php | 8 +-- .../Controller/SendingProfileController.php | 14 ++--- .../Controller/SubscriberController.php | 55 ++++++++++++------- .../Subscriber/CreateSubscriberIfExists.php | 10 ++++ .../Subscriber/CreateSubscriberInput.php | 17 ++++-- backend/src/Service/ApiKey/ApiKeyService.php | 8 +-- .../src/Service/Approval/ApprovalService.php | 20 +++---- backend/src/Service/Import/ImportService.php | 6 +- backend/src/Service/Issue/IssueService.php | 20 +++---- backend/src/Service/Issue/SendService.php | 12 ++-- .../Service/Newsletter/NewsletterService.php | 8 +-- .../NewsletterList/NewsletterListService.php | 2 +- .../SendingProfile/SendingProfileService.php | 16 +++--- .../Service/Subscriber/SubscriberService.php | 16 +++--- .../src/Service/Template/TemplateService.php | 2 +- backend/src/Util/OptionalPropertyTrait.php | 4 +- .../Subscriber/CreateSubscriberTest.php | 20 ++++++- compose.yaml | 2 +- 21 files changed, 163 insertions(+), 109 deletions(-) create mode 100644 backend/src/Api/Console/Input/Subscriber/CreateSubscriberIfExists.php diff --git a/backend/src/Api/Console/Controller/ApiKeyController.php b/backend/src/Api/Console/Controller/ApiKeyController.php index ab10addf..5ac8b905 100644 --- a/backend/src/Api/Console/Controller/ApiKeyController.php +++ b/backend/src/Api/Console/Controller/ApiKeyController.php @@ -54,13 +54,13 @@ public function getApiKeys(Newsletter $newsletter): JsonResponse public function updateApiKey(#[MapRequestPayload] UpdateApiKeyInput $input, ApiKey $apiKey): JsonResponse { $updates = new UpdateApiKeyDto(); - if ($input->hasProperty('is_enabled')) { + if ($input->has('is_enabled')) { $updates->enabled = $input->is_enabled; } - if ($input->hasProperty('scopes')) { + if ($input->has('scopes')) { $updates->scopes = $input->scopes; } - if ($input->hasProperty('name')) { + if ($input->has('name')) { $updates->name = $input->name; } diff --git a/backend/src/Api/Console/Controller/ApprovalController.php b/backend/src/Api/Console/Controller/ApprovalController.php index df801584..dab13e07 100644 --- a/backend/src/Api/Console/Controller/ApprovalController.php +++ b/backend/src/Api/Console/Controller/ApprovalController.php @@ -102,39 +102,39 @@ public function updateApproval( $updates = new UpdateApprovalDto(); - if ($input->hasProperty('company_name')) { + if ($input->has('company_name')) { $updates->companyName = $input->company_name; } - if ($input->hasProperty('country')) { + if ($input->has('country')) { $updates->country = $input->country; } - if ($input->hasProperty('website')) { + if ($input->has('website')) { $updates->website = $input->website; } - if ($input->hasProperty('social_links')) { + if ($input->has('social_links')) { $updates->socialLinks = $input->social_links; } - if ($input->hasProperty('type_of_content')) { + if ($input->has('type_of_content')) { $updates->typeOfContent = $input->type_of_content; } - if ($input->hasProperty('frequency')) { + if ($input->has('frequency')) { $updates->frequency = $input->frequency; } - if ($input->hasProperty('existing_list')) { + if ($input->has('existing_list')) { $updates->existingList = $input->existing_list; } - if ($input->hasProperty('sample')) { + if ($input->has('sample')) { $updates->sample = $input->sample; } - if ($input->hasProperty('why_post')) { + if ($input->has('why_post')) { $updates->whyPost = $input->why_post; } diff --git a/backend/src/Api/Console/Controller/IssueController.php b/backend/src/Api/Console/Controller/IssueController.php index f3c7c32d..958db24c 100644 --- a/backend/src/Api/Console/Controller/IssueController.php +++ b/backend/src/Api/Console/Controller/IssueController.php @@ -96,15 +96,15 @@ public function updateIssue( { $updates = new UpdateIssueDto(); - if ($input->hasProperty('subject')) { + if ($input->has('subject')) { $updates->subject = $input->subject; } - if ($input->hasProperty('content')) { + if ($input->has('content')) { $updates->content = $input->content; } - if ($input->hasProperty('sending_profile_id')) { + if ($input->has('sending_profile_id')) { $sendingProfile = $this->sendingProfileService->getSendingProfileOfNewsletterById( $newsletter, $input->sending_profile_id @@ -117,7 +117,7 @@ public function updateIssue( $updates->sendingProfile = $sendingProfile; } - if ($input->hasProperty('lists')) { + if ($input->has('lists')) { $missingListIds = $this->newsletterListService->getMissingListIdsOfNewsletter($newsletter, $input->lists); if ($missingListIds !== null) { diff --git a/backend/src/Api/Console/Controller/NewsletterController.php b/backend/src/Api/Console/Controller/NewsletterController.php index bdb08449..d16eec6e 100644 --- a/backend/src/Api/Console/Controller/NewsletterController.php +++ b/backend/src/Api/Console/Controller/NewsletterController.php @@ -93,19 +93,19 @@ public function updateNewsletter( ): JsonResponse { $updates = new UpdateNewsletterDto(); - if ($input->hasProperty('name')) { + if ($input->has('name')) { $updates->name = $input->name; } - if ($input->hasProperty('subdomain')) { + if ($input->has('subdomain')) { if ($this->newsletterService->isSubdomainTaken($input->subdomain)) { throw new UnprocessableEntityHttpException('Subdomain is already taken.'); } $updates->subdomain = $input->subdomain; } - if ($input->hasProperty('language_code')) { + if ($input->has('language_code')) { $updates->language_code = $input->language_code; } - if ($input->hasProperty('is_rtl')) { + if ($input->has('is_rtl')) { $updates->is_rtl = $input->is_rtl; } $newsletter = $this->newsletterService->updateNewsletter($newsletter, $updates); diff --git a/backend/src/Api/Console/Controller/SendingProfileController.php b/backend/src/Api/Console/Controller/SendingProfileController.php index 793d5485..4fd7cfd5 100644 --- a/backend/src/Api/Console/Controller/SendingProfileController.php +++ b/backend/src/Api/Console/Controller/SendingProfileController.php @@ -83,33 +83,33 @@ public function updateSendingProfile( { $updates = new UpdateSendingProfileDto(); - if ($input->hasProperty('from_email')) { + if ($input->has('from_email')) { $domain = $this->getDomainFromEmail($input->from_email); $updates->customDomain = $domain; $updates->fromEmail = $input->from_email; } - if ($input->hasProperty('from_name')) { + if ($input->has('from_name')) { $updates->fromName = $input->from_name; } - if ($input->hasProperty('reply_to_email')) { + if ($input->has('reply_to_email')) { $updates->replyToEmail = $input->reply_to_email; } - if ($input->hasProperty('brand_name')) { + if ($input->has('brand_name')) { $updates->brandName = $input->brand_name; } - if ($input->hasProperty('brand_logo')) { + if ($input->has('brand_logo')) { $updates->brandLogo = $input->brand_logo; } - if ($input->hasProperty('brand_url')) { + if ($input->has('brand_url')) { $updates->brandUrl = $input->brand_url; } - if ($input->hasProperty('is_default')) { + if ($input->has('is_default')) { $updates->isDefault = $input->is_default; } diff --git a/backend/src/Api/Console/Controller/SubscriberController.php b/backend/src/Api/Console/Controller/SubscriberController.php index 12edad36..9163a7e1 100644 --- a/backend/src/Api/Console/Controller/SubscriberController.php +++ b/backend/src/Api/Console/Controller/SubscriberController.php @@ -5,6 +5,7 @@ use App\Api\Console\Authorization\Scope; use App\Api\Console\Authorization\ScopeRequired; use App\Api\Console\Input\Subscriber\BulkActionSubscriberInput; +use App\Api\Console\Input\Subscriber\CreateSubscriberIfExists; use App\Api\Console\Input\Subscriber\CreateSubscriberInput; use App\Api\Console\Input\Subscriber\UpdateSubscriberInput; use App\Api\Console\Object\SubscriberObject; @@ -79,6 +80,7 @@ public function createSubscriber( Newsletter $newsletter ): JsonResponse { + $missingListIds = $this ->newsletterListService ->getMissingListIdsOfNewsletter($newsletter, $input->list_ids); @@ -87,23 +89,38 @@ public function createSubscriber( throw new UnprocessableEntityHttpException("List with id {$missingListIds[0]} not found"); } - $subscriberDB = $this->subscriberService->getSubscriberByEmail($newsletter, $input->email); - if ($subscriberDB !== null) { - throw new UnprocessableEntityHttpException("Subscriber with email {$input->email} already exists"); - } - + $subscriber = $this->subscriberService->getSubscriberByEmail($newsletter, $input->email); $lists = $this->newsletterListService->getListsByIds($input->list_ids); - $subscriber = $this->subscriberService->createSubscriber( - $newsletter, - $input->email, - $lists, - SubscriberStatus::PENDING, - $input->source ?? SubscriberSource::CONSOLE, - $input->subscribe_ip, - $input->subscribed_at ? \DateTimeImmutable::createFromTimestamp($input->subscribed_at) : null, - $input->unsubscribed_at ? \DateTimeImmutable::createFromTimestamp($input->unsubscribed_at) : null, - ); + if ($subscriber === null) { + + // create subscriber + $subscriber = $this->subscriberService->createSubscriber( + $newsletter, + $input->email, + $lists, + SubscriberStatus::PENDING, + source: $input->source ?? SubscriberSource::CONSOLE, + subscribeIp: $input->subscribe_ip ?? null, + subscribedAt: $input->has('subscribed_at') ? \DateTimeImmutable::createFromTimestamp($input->subscribed_at) : null, + unsubscribedAt: $input->has('unsubscribed_at') ? \DateTimeImmutable::createFromTimestamp($input->unsubscribed_at) : null, + ); + + } elseif ($input->if_exists === CreateSubscriberIfExists::UPDATE) { + + // update + $updates = new UpdateSubscriberDto(); + $updates->lists = $lists; + $updates->status = $input->status; + $updates->subscribedAt = $input->subscribed_at; + $updates->unsubscribedAt = $input->unsubscribed_at; + + // TODO: + + + } else { + throw new UnprocessableEntityHttpException("Subscriber with email {$input->email} already exists"); + } return $this->json(new SubscriberObject($subscriber)); } @@ -118,7 +135,7 @@ public function updateSubscriber( { $updates = new UpdateSubscriberDto(); - if ($input->hasProperty('email')) { + if ($input->has('email')) { $subscriberDB = $this->subscriberService->getSubscriberByEmail($newsletter, $input->email); if ($subscriberDB !== null) { throw new UnprocessableEntityHttpException("Subscriber with email {$input->email} already exists"); @@ -127,7 +144,7 @@ public function updateSubscriber( $updates->email = $input->email; } - if ($input->hasProperty('list_ids')) { + if ($input->has('list_ids')) { $missingListIds = $this->newsletterListService->getMissingListIdsOfNewsletter( $newsletter, $input->list_ids @@ -140,7 +157,7 @@ public function updateSubscriber( $updates->lists = $this->newsletterListService->getListsByIds($input->list_ids); } - if ($input->hasProperty('status')) { + if ($input->has('status')) { if ($input->status === SubscriberStatus::SUBSCRIBED && $subscriber->getOptInAt() === null) { throw new UnprocessableEntityHttpException('Subscribers without opt-in can not be updated to SUBSCRIBED status.'); } @@ -150,7 +167,7 @@ public function updateSubscriber( $metadataDefinitions = $this->subscriberMetadataService->getMetadataDefinitions($newsletter); - if ($input->hasProperty('metadata')) { + if ($input->has('metadata')) { try { $this->subscriberMetadataService->validateMetadata($newsletter, $input->metadata); } catch (\Exception $e) { diff --git a/backend/src/Api/Console/Input/Subscriber/CreateSubscriberIfExists.php b/backend/src/Api/Console/Input/Subscriber/CreateSubscriberIfExists.php new file mode 100644 index 00000000..4d87ad75 --- /dev/null +++ b/backend/src/Api/Console/Input/Subscriber/CreateSubscriberIfExists.php @@ -0,0 +1,10 @@ +hasProperty('enabled')) { + if ($updates->has('enabled')) { $apiKey->setIsEnabled($updates->enabled); } - if ($updates->hasProperty('scopes')) { + if ($updates->has('scopes')) { $apiKey->setScopes($updates->scopes); } - if ($updates->hasProperty('name')) { + if ($updates->has('name')) { $apiKey->setName($updates->name); } - if ($updates->hasProperty('lastAccessedAt')) { + if ($updates->has('lastAccessedAt')) { $apiKey->setLastAccessedAt($updates->lastAccessedAt); } diff --git a/backend/src/Service/Approval/ApprovalService.php b/backend/src/Service/Approval/ApprovalService.php index f7c1de7a..fc3c529f 100644 --- a/backend/src/Service/Approval/ApprovalService.php +++ b/backend/src/Service/Approval/ApprovalService.php @@ -137,29 +137,29 @@ public function updateApproval( UpdateApprovalDto $updates ): Approval { - if ($updates->hasProperty('companyName')) { + if ($updates->has('companyName')) { $approval->setCompanyName($updates->companyName); } - if ($updates->hasProperty('country')) { + if ($updates->has('country')) { $approval->setCountry($updates->country); } - if ($updates->hasProperty('website')) { + if ($updates->has('website')) { $approval->setWebsite($updates->website); } - if ($updates->hasProperty('socialLinks')) { + if ($updates->has('socialLinks')) { $approval->setSocialLinks($updates->socialLinks); } - if ($updates->hasProperty('status')) { + if ($updates->has('status')) { $approval->setStatus($updates->status); } $otherInfo = $approval->getOtherInfo() ?? []; - if ($updates->hasProperty('typeOfContent')) { + if ($updates->has('typeOfContent')) { if ($updates->typeOfContent === null) { unset($otherInfo['type_of_content']); } else { @@ -167,7 +167,7 @@ public function updateApproval( } } - if ($updates->hasProperty('frequency')) { + if ($updates->has('frequency')) { if ($updates->frequency === null) { unset($otherInfo['frequency']); } else { @@ -175,7 +175,7 @@ public function updateApproval( } } - if ($updates->hasProperty('existingList')) { + if ($updates->has('existingList')) { if ($updates->existingList === null) { unset($otherInfo['existing_list']); } else { @@ -183,7 +183,7 @@ public function updateApproval( } } - if ($updates->hasProperty('sample')) { + if ($updates->has('sample')) { if ($updates->sample === null) { unset($otherInfo['sample']); } else { @@ -191,7 +191,7 @@ public function updateApproval( } } - if ($updates->hasProperty('whyPost')) { + if ($updates->has('whyPost')) { if ($updates->whyPost === null) { unset($otherInfo['why_post']); } else { diff --git a/backend/src/Service/Import/ImportService.php b/backend/src/Service/Import/ImportService.php index f002bc13..718f84b3 100644 --- a/backend/src/Service/Import/ImportService.php +++ b/backend/src/Service/Import/ImportService.php @@ -92,13 +92,13 @@ public function createSubscriberImport( public function updateSubscriberImport(SubscriberImport $subscriberImport, UpdateSubscriberImportDto $updates): SubscriberImport { - if ($updates->hasProperty('status')) { + if ($updates->has('status')) { $subscriberImport->setStatus($updates->status); } - if ($updates->hasProperty('fields')) { + if ($updates->has('fields')) { $subscriberImport->setFields($updates->fields); } - if ($updates->hasProperty('errorMessage')) { + if ($updates->has('errorMessage')) { $subscriberImport->setErrorMessage($updates->errorMessage); } $subscriberImport->setUpdatedAt($this->now()); diff --git a/backend/src/Service/Issue/IssueService.php b/backend/src/Service/Issue/IssueService.php index 953ff277..dfb96856 100644 --- a/backend/src/Service/Issue/IssueService.php +++ b/backend/src/Service/Issue/IssueService.php @@ -58,43 +58,43 @@ public function createIssueDraft(Newsletter $newsletter): Issue public function updateIssue(Issue $issue, UpdateIssueDto $updates): Issue { - if ($updates->hasProperty('subject')) { + if ($updates->has('subject')) { $issue->setSubject($updates->subject); } - if ($updates->hasProperty('content')) { + if ($updates->has('content')) { $issue->setContent($updates->content); } - if ($updates->hasProperty('sendingProfile')) { + if ($updates->has('sendingProfile')) { $issue->setSendingProfile($updates->sendingProfile); } - if ($updates->hasProperty('status')) { + if ($updates->has('status')) { $issue->setStatus($updates->status); } - if ($updates->hasProperty('lists')) { + if ($updates->has('lists')) { $issue->setListids($updates->lists); } - if ($updates->hasProperty('html')) { + if ($updates->has('html')) { $issue->setHtml($updates->html); } - if ($updates->hasProperty('text')) { + if ($updates->has('text')) { $issue->setText($updates->text); } - if ($updates->hasProperty('sendingAt')) { + if ($updates->has('sendingAt')) { $issue->setSendingAt($updates->sendingAt); } - if ($updates->hasProperty('totalSendable')) { + if ($updates->has('totalSendable')) { $issue->setTotalSendable($updates->totalSendable); } - if ($updates->hasProperty('sentAt')) { + if ($updates->has('sentAt')) { $issue->setSentAt($updates->sentAt); } diff --git a/backend/src/Service/Issue/SendService.php b/backend/src/Service/Issue/SendService.php index eec3c36a..17595803 100644 --- a/backend/src/Service/Issue/SendService.php +++ b/backend/src/Service/Issue/SendService.php @@ -174,27 +174,27 @@ public function getIssueProgress(Issue $issue): ?array public function updateSend(Send $send, UpdateSendDto $updates): Send { - if ($updates->hasProperty('status')) { + if ($updates->has('status')) { $send->setStatus($updates->status); } - if ($updates->hasProperty('deliveredAt')) { + if ($updates->has('deliveredAt')) { $send->setDeliveredAt($updates->deliveredAt); } - if ($updates->hasProperty('failedAt')) { + if ($updates->has('failedAt')) { $send->setFailedAt($updates->failedAt); } - if ($updates->hasProperty('bouncedAt')) { + if ($updates->has('bouncedAt')) { $send->setBouncedAt($updates->bouncedAt); } - if ($updates->hasProperty('complainedAt')) { + if ($updates->has('complainedAt')) { $send->setComplainedAt($updates->complainedAt); } - if ($updates->hasProperty('hardBounce')) { + if ($updates->has('hardBounce')) { $send->setHardBounce($updates->hardBounce); } diff --git a/backend/src/Service/Newsletter/NewsletterService.php b/backend/src/Service/Newsletter/NewsletterService.php index a14e5bee..790c913f 100644 --- a/backend/src/Service/Newsletter/NewsletterService.php +++ b/backend/src/Service/Newsletter/NewsletterService.php @@ -266,11 +266,11 @@ public function updateNewsletterMeta(Newsletter $newsletter, UpdateNewsletterMet public function updateNewsletter(Newsletter $newsletter, UpdateNewsletterDto $updates): Newsletter { - if ($updates->hasProperty('name')) { + if ($updates->has('name')) { $newsletter->setName($updates->name); } - if ($updates->hasProperty('subdomain')) { + if ($updates->has('subdomain')) { $newsletter->setSubdomain($updates->subdomain); $systemSendingProfile = $this->sendingProfileService->getSystemSendingProfileOfNewsletter($newsletter); @@ -281,11 +281,11 @@ public function updateNewsletter(Newsletter $newsletter, UpdateNewsletterDto $up ->updateSendingProfile($systemSendingProfile, $sendingProfileUpdates); } - if ($updates->hasProperty('language_code')) { + if ($updates->has('language_code')) { $newsletter->setLanguageCode($updates->language_code); } - if ($updates->hasProperty('is_rtl')) { + if ($updates->has('is_rtl')) { $newsletter->setIsRtl($updates->is_rtl); } diff --git a/backend/src/Service/NewsletterList/NewsletterListService.php b/backend/src/Service/NewsletterList/NewsletterListService.php index a6935eea..4e6612a1 100644 --- a/backend/src/Service/NewsletterList/NewsletterListService.php +++ b/backend/src/Service/NewsletterList/NewsletterListService.php @@ -126,7 +126,7 @@ public function getMissingListIdsOfNewsletter(Newsletter $newsletter, array $lis } /** - * Note that we should validate the lists are within the newsletter (using isListsAvailable) before calling this method + * Note that we should validate the lists are within the newsletter (using getMissingListIdsOfNewsletter) before calling this method * @param array $listIds * @return ArrayCollection */ diff --git a/backend/src/Service/SendingProfile/SendingProfileService.php b/backend/src/Service/SendingProfile/SendingProfileService.php index f670aaf4..dcc4f51d 100644 --- a/backend/src/Service/SendingProfile/SendingProfileService.php +++ b/backend/src/Service/SendingProfile/SendingProfileService.php @@ -86,35 +86,35 @@ public function updateSendingProfile( UpdateSendingProfileDto $updates ): SendingProfile { - if ($updates->hasProperty('fromEmail')) { + if ($updates->has('fromEmail')) { $sendingProfile->setFromEmail($updates->fromEmail); } - if ($updates->hasProperty('fromName')) { + if ($updates->has('fromName')) { $sendingProfile->setFromName($updates->fromName); } - if ($updates->hasProperty('replyToEmail')) { + if ($updates->has('replyToEmail')) { $sendingProfile->setReplyToEmail($updates->replyToEmail); } - if ($updates->hasProperty('brandName')) { + if ($updates->has('brandName')) { $sendingProfile->setBrandName($updates->brandName); } - if ($updates->hasProperty('brandLogo')) { + if ($updates->has('brandLogo')) { $sendingProfile->setBrandLogo($updates->brandLogo); } - if ($updates->hasProperty('brandUrl')) { + if ($updates->has('brandUrl')) { $sendingProfile->setBrandUrl($updates->brandUrl); } - if ($updates->hasProperty('customDomain')) { + if ($updates->has('customDomain')) { $sendingProfile->setDomain($updates->customDomain); } - if ($updates->hasProperty('isDefault')) { + if ($updates->has('isDefault')) { // only true is supported assert($updates->isDefault === true); $sendingProfile->setIsDefault($updates->isDefault); diff --git a/backend/src/Service/Subscriber/SubscriberService.php b/backend/src/Service/Subscriber/SubscriberService.php index 1cc62b9e..43d33945 100644 --- a/backend/src/Service/Subscriber/SubscriberService.php +++ b/backend/src/Service/Subscriber/SubscriberService.php @@ -157,15 +157,15 @@ public function getSubscribers( public function updateSubscriber(Subscriber $subscriber, UpdateSubscriberDto $updates): Subscriber { - if ($updates->hasProperty('email')) { + if ($updates->has('email')) { $subscriber->setEmail($updates->email); } - if ($updates->hasProperty('status')) { + if ($updates->has('status')) { $subscriber->setStatus($updates->status); } - if ($updates->hasProperty('lists')) { + if ($updates->has('lists')) { // Clear & re-add lists foreach ($subscriber->getLists() as $list) { $subscriber->removeList($list); @@ -175,23 +175,23 @@ public function updateSubscriber(Subscriber $subscriber, UpdateSubscriberDto $up } } - if ($updates->hasProperty('subscribedAt')) { + if ($updates->has('subscribedAt')) { $subscriber->setSubscribedAt($updates->subscribedAt); } - if ($updates->hasProperty('optInAt')) { + if ($updates->has('optInAt')) { $subscriber->setOptInAt($updates->optInAt); } - if ($updates->hasProperty('unsubscribedAt')) { + if ($updates->has('unsubscribedAt')) { $subscriber->setUnsubscribedAt($updates->unsubscribedAt); } - if ($updates->hasProperty('unsubscribedReason')) { + if ($updates->has('unsubscribedReason')) { $subscriber->setUnsubscribeReason($updates->unsubscribedReason); } - if ($updates->hasProperty('metadata')) { + if ($updates->has('metadata')) { $metadata = $subscriber->getMetadata(); foreach ($updates->metadata as $key => $value) { $metadata[$key] = $value; diff --git a/backend/src/Service/Template/TemplateService.php b/backend/src/Service/Template/TemplateService.php index 49e9b6d3..235098b1 100644 --- a/backend/src/Service/Template/TemplateService.php +++ b/backend/src/Service/Template/TemplateService.php @@ -59,7 +59,7 @@ public function readDefaultTemplate(): string public function updateTemplate(Template $template, UpdateTemplateDto $updates): Template { - if ($updates->hasProperty('template')) { + if ($updates->has('template')) { $template->setTemplate($updates->template); } diff --git a/backend/src/Util/OptionalPropertyTrait.php b/backend/src/Util/OptionalPropertyTrait.php index 49175041..3c39f721 100644 --- a/backend/src/Util/OptionalPropertyTrait.php +++ b/backend/src/Util/OptionalPropertyTrait.php @@ -8,7 +8,7 @@ trait OptionalPropertyTrait /** * Checks if the property is INITIALIZED */ - public function hasProperty(string $property): bool + public function has(string $property): bool { try { $_ = $this->{$property}; @@ -18,4 +18,4 @@ public function hasProperty(string $property): bool } } -} \ No newline at end of file +} diff --git a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php index 15d3d2ad..d47c2157 100644 --- a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php +++ b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php @@ -24,7 +24,25 @@ class CreateSubscriberTest extends WebTestCase { - // TODO: tests for authentication + public function test_test(): void + { + $newsletter = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + + $response = $this->consoleApi( + $newsletter, + 'POST', + '/subscribers', + [ + 'email' => 'test@email.com', + 'list_ids' => [$list->getId()], + 'subscribe_ip' => null, // '222.222.222.222' + ] + ); + + dd($response->getContent()); + + } public function testCreateSubscriberMinimal(): void { diff --git a/compose.yaml b/compose.yaml index 84fe2b97..50bb42c6 100644 --- a/compose.yaml +++ b/compose.yaml @@ -33,7 +33,7 @@ services: target: backend-dev volumes: - ./backend:/app/backend - # - ../internal:/app/backend/vendor/hyvor/internal:ro + - ../internal:/app/backend/vendor/hyvor/internal:ro - ./shared:/app/shared labels: traefik.enable: true From a8060cf15032bb6576f672900f2966e1a2f7d07e Mon Sep 17 00:00:00 2001 From: Supun Wimalasena Date: Wed, 25 Feb 2026 15:38:13 +0100 Subject: [PATCH 02/23] side project: sentry config --- backend/composer.json | 3 +- backend/composer.lock | 343 +++++++++++++++++++++++++++- backend/config/bundles.php | 1 + backend/config/packages/monolog.php | 1 - backend/config/packages/sentry.yaml | 39 ++++ backend/config/reference.php | 83 +++++++ backend/symfony.lock | 12 + 7 files changed, 479 insertions(+), 3 deletions(-) create mode 100644 backend/config/packages/sentry.yaml diff --git a/backend/composer.json b/backend/composer.json index 40bf0f2f..f109f740 100644 --- a/backend/composer.json +++ b/backend/composer.json @@ -45,7 +45,8 @@ "twig/cssinliner-extra": "^3.21", "twig/extra-bundle": "^3.21", "symfony/dom-crawler": "7.4.*", - "zenstruck/messenger-monitor-bundle": "^0.6.0" + "zenstruck/messenger-monitor-bundle": "^0.6.0", + "sentry/sentry-symfony": "^5.9" }, "bump-after-update": true, "sort-packages": true, diff --git a/backend/composer.lock b/backend/composer.lock index cec1baf5..19eb728c 100644 --- a/backend/composer.lock +++ b/backend/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "cdd012dd5bf3ebdc6cb6a7c0079c7639", + "content-hash": "84ff26b07990da9b940ce6cb14f5bcb2", "packages": [ { "name": "aws/aws-crt-php", @@ -2511,6 +2511,66 @@ }, "time": "2026-02-04T15:14:59+00:00" }, + { + "name": "jean85/pretty-package-versions", + "version": "2.1.1", + "source": { + "type": "git", + "url": "https://github.com/Jean85/pretty-package-versions.git", + "reference": "4d7aa5dab42e2a76d99559706022885de0e18e1a" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/Jean85/pretty-package-versions/zipball/4d7aa5dab42e2a76d99559706022885de0e18e1a", + "reference": "4d7aa5dab42e2a76d99559706022885de0e18e1a", + "shasum": "" + }, + "require": { + "composer-runtime-api": "^2.1.0", + "php": "^7.4|^8.0" + }, + "require-dev": { + "friendsofphp/php-cs-fixer": "^3.2", + "jean85/composer-provided-replaced-stub-package": "^1.0", + "phpstan/phpstan": "^2.0", + "phpunit/phpunit": "^7.5|^8.5|^9.6", + "rector/rector": "^2.0", + "vimeo/psalm": "^4.3 || ^5.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.x-dev" + } + }, + "autoload": { + "psr-4": { + "Jean85\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Alessandro Lai", + "email": "alessandro.lai85@gmail.com" + } + ], + "description": "A library to get pretty versions strings of installed dependencies", + "keywords": [ + "composer", + "package", + "release", + "versions" + ], + "support": { + "issues": "https://github.com/Jean85/pretty-package-versions/issues", + "source": "https://github.com/Jean85/pretty-package-versions/tree/2.1.1" + }, + "time": "2025-03-19T14:43:43+00:00" + }, { "name": "league/flysystem", "version": "3.31.0", @@ -4067,6 +4127,199 @@ ], "time": "2023-12-12T12:06:11+00:00" }, + { + "name": "sentry/sentry", + "version": "4.21.0", + "source": { + "type": "git", + "url": "https://github.com/getsentry/sentry-php.git", + "reference": "2bf405fc4d38f00073a7d023cf321e59f614d54c" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/getsentry/sentry-php/zipball/2bf405fc4d38f00073a7d023cf321e59f614d54c", + "reference": "2bf405fc4d38f00073a7d023cf321e59f614d54c", + "shasum": "" + }, + "require": { + "ext-curl": "*", + "ext-json": "*", + "ext-mbstring": "*", + "guzzlehttp/psr7": "^1.8.4|^2.1.1", + "jean85/pretty-package-versions": "^1.5|^2.0.4", + "php": "^7.2|^8.0", + "psr/log": "^1.0|^2.0|^3.0", + "symfony/options-resolver": "^4.4.30|^5.0.11|^6.0|^7.0|^8.0" + }, + "conflict": { + "raven/raven": "*" + }, + "require-dev": { + "friendsofphp/php-cs-fixer": "^3.4", + "guzzlehttp/promises": "^2.0.3", + "guzzlehttp/psr7": "^1.8.4|^2.1.1", + "monolog/monolog": "^1.6|^2.0|^3.0", + "nyholm/psr7": "^1.8", + "phpbench/phpbench": "^1.0", + "phpstan/phpstan": "^1.3", + "phpunit/phpunit": "^8.5.52|^9.6.34", + "spiral/roadrunner-http": "^3.6", + "spiral/roadrunner-worker": "^3.6", + "vimeo/psalm": "^4.17" + }, + "suggest": { + "monolog/monolog": "Allow sending log messages to Sentry by using the included Monolog handler." + }, + "type": "library", + "autoload": { + "files": [ + "src/functions.php" + ], + "psr-4": { + "Sentry\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Sentry", + "email": "accounts@sentry.io" + } + ], + "description": "PHP SDK for Sentry (http://sentry.io)", + "homepage": "http://sentry.io", + "keywords": [ + "crash-reporting", + "crash-reports", + "error-handler", + "error-monitoring", + "log", + "logging", + "profiling", + "sentry", + "tracing" + ], + "support": { + "issues": "https://github.com/getsentry/sentry-php/issues", + "source": "https://github.com/getsentry/sentry-php/tree/4.21.0" + }, + "funding": [ + { + "url": "https://sentry.io/", + "type": "custom" + }, + { + "url": "https://sentry.io/pricing/", + "type": "custom" + } + ], + "time": "2026-02-24T15:32:51+00:00" + }, + { + "name": "sentry/sentry-symfony", + "version": "5.9.0", + "source": { + "type": "git", + "url": "https://github.com/getsentry/sentry-symfony.git", + "reference": "75a73de23b9af414b3c8b15c26187a4ae6c65732" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/getsentry/sentry-symfony/zipball/75a73de23b9af414b3c8b15c26187a4ae6c65732", + "reference": "75a73de23b9af414b3c8b15c26187a4ae6c65732", + "shasum": "" + }, + "require": { + "guzzlehttp/psr7": "^2.1.1", + "jean85/pretty-package-versions": "^1.5||^2.0", + "php": "^7.2||^8.0", + "sentry/sentry": "^4.20.0", + "symfony/cache-contracts": "^1.1||^2.4||^3.0", + "symfony/config": "^4.4.20||^5.0.11||^6.0||^7.0||^8.0", + "symfony/console": "^4.4.20||^5.0.11||^6.0||^7.0||^8.0", + "symfony/dependency-injection": "^4.4.20||^5.0.11||^6.0||^7.0||^8.0", + "symfony/event-dispatcher": "^4.4.20||^5.0.11||^6.0||^7.0||^8.0", + "symfony/http-kernel": "^4.4.20||^5.0.11||^6.0||^7.0||^8.0", + "symfony/polyfill-php80": "^1.22", + "symfony/psr-http-message-bridge": "^1.2||^2.0||^6.4||^7.0||^8.0", + "symfony/yaml": "^4.4.20||^5.0.11||^6.0||^7.0||^8.0" + }, + "require-dev": { + "doctrine/dbal": "^2.13||^3.3||^4.0", + "doctrine/doctrine-bundle": "^2.6||^3.0", + "friendsofphp/php-cs-fixer": "^2.19||^3.40", + "masterminds/html5": "^2.8", + "phpstan/extension-installer": "^1.0", + "phpstan/phpstan": "1.12.5", + "phpstan/phpstan-phpunit": "1.4.0", + "phpstan/phpstan-symfony": "1.4.10", + "phpunit/phpunit": "^8.5.40||^9.6.21", + "symfony/browser-kit": "^4.4.20||^5.0.11||^6.0||^7.0||^8.0", + "symfony/cache": "^4.4.20||^5.0.11||^6.0||^7.0||^8.0", + "symfony/dom-crawler": "^4.4.20||^5.0.11||^6.0||^7.0||^8.0", + "symfony/framework-bundle": "^4.4.20||^5.0.11||^6.0||^7.0||^8.0", + "symfony/http-client": "^4.4.20||^5.0.11||^6.0||^7.0||^8.0", + "symfony/messenger": "^4.4.20||^5.0.11||^6.0||^7.0||^8.0", + "symfony/monolog-bundle": "^3.4||^4.0", + "symfony/phpunit-bridge": "^5.2.6||^6.0||^7.0||^8.0", + "symfony/process": "^4.4.20||^5.0.11||^6.0||^7.0||^8.0", + "symfony/security-core": "^4.4.20||^5.0.11||^6.0||^7.0||^8.0", + "symfony/security-http": "^4.4.20||^5.0.11||^6.0||^7.0||^8.0", + "symfony/twig-bundle": "^4.4.20||^5.0.11||^6.0||^7.0||^8.0", + "vimeo/psalm": "^4.3||^5.16.0" + }, + "suggest": { + "doctrine/doctrine-bundle": "Allow distributed tracing of database queries using Sentry.", + "monolog/monolog": "Allow sending log messages to Sentry by using the included Monolog handler.", + "symfony/cache": "Allow distributed tracing of cache pools using Sentry.", + "symfony/twig-bundle": "Allow distributed tracing of Twig template rendering using Sentry." + }, + "type": "symfony-bundle", + "autoload": { + "files": [ + "src/aliases.php" + ], + "psr-4": { + "Sentry\\SentryBundle\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Sentry", + "email": "accounts@sentry.io" + } + ], + "description": "Symfony integration for Sentry (http://getsentry.com)", + "homepage": "http://getsentry.com", + "keywords": [ + "errors", + "logging", + "sentry", + "symfony" + ], + "support": { + "issues": "https://github.com/getsentry/sentry-symfony/issues", + "source": "https://github.com/getsentry/sentry-symfony/tree/5.9.0" + }, + "funding": [ + { + "url": "https://sentry.io/", + "type": "custom" + }, + { + "url": "https://sentry.io/pricing/", + "type": "custom" + } + ], + "time": "2026-02-23T12:32:36+00:00" + }, { "name": "symfony/cache", "version": "v7.4.5", @@ -7590,6 +7843,94 @@ ], "time": "2026-01-27T16:16:02+00:00" }, + { + "name": "symfony/psr-http-message-bridge", + "version": "v7.4.4", + "source": { + "type": "git", + "url": "https://github.com/symfony/psr-http-message-bridge.git", + "reference": "929ffe10bbfbb92e711ac3818d416f9daffee067" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/psr-http-message-bridge/zipball/929ffe10bbfbb92e711ac3818d416f9daffee067", + "reference": "929ffe10bbfbb92e711ac3818d416f9daffee067", + "shasum": "" + }, + "require": { + "php": ">=8.2", + "psr/http-message": "^1.0|^2.0", + "symfony/http-foundation": "^6.4|^7.0|^8.0" + }, + "conflict": { + "php-http/discovery": "<1.15", + "symfony/http-kernel": "<6.4" + }, + "require-dev": { + "nyholm/psr7": "^1.1", + "php-http/discovery": "^1.15", + "psr/log": "^1.1.4|^2|^3", + "symfony/browser-kit": "^6.4|^7.0|^8.0", + "symfony/config": "^6.4|^7.0|^8.0", + "symfony/event-dispatcher": "^6.4|^7.0|^8.0", + "symfony/framework-bundle": "^6.4.13|^7.1.6|^8.0", + "symfony/http-kernel": "^6.4.13|^7.1.6|^8.0", + "symfony/runtime": "^6.4.13|^7.1.6|^8.0" + }, + "type": "symfony-bridge", + "autoload": { + "psr-4": { + "Symfony\\Bridge\\PsrHttpMessage\\": "" + }, + "exclude-from-classmap": [ + "/Tests/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Fabien Potencier", + "email": "fabien@symfony.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "PSR HTTP message bridge", + "homepage": "https://symfony.com", + "keywords": [ + "http", + "http-message", + "psr-17", + "psr-7" + ], + "support": { + "source": "https://github.com/symfony/psr-http-message-bridge/tree/v7.4.4" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://github.com/nicolas-grekas", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2026-01-03T23:30:35+00:00" + }, { "name": "symfony/rate-limiter", "version": "v7.4.5", diff --git a/backend/config/bundles.php b/backend/config/bundles.php index 0e09a636..2db7f272 100644 --- a/backend/config/bundles.php +++ b/backend/config/bundles.php @@ -17,4 +17,5 @@ Symfony\UX\TwigComponent\TwigComponentBundle::class => ['all' => true], Twig\Extra\TwigExtraBundle\TwigExtraBundle::class => ['all' => true], Zenstruck\Messenger\Monitor\ZenstruckMessengerMonitorBundle::class => ['all' => true], + Sentry\SentryBundle\SentryBundle::class => ['prod' => true], ]; diff --git a/backend/config/packages/monolog.php b/backend/config/packages/monolog.php index 1c667be4..fb0a8726 100644 --- a/backend/config/packages/monolog.php +++ b/backend/config/packages/monolog.php @@ -9,7 +9,6 @@ ->type('buffer') ->handler('final') ->level("%env(LOG_LEVEL)%") - ->bubble(false) ->channels()->elements(['app']); $monolog->handler('non_app') ->type('buffer') diff --git a/backend/config/packages/sentry.yaml b/backend/config/packages/sentry.yaml new file mode 100644 index 00000000..81ad7b5c --- /dev/null +++ b/backend/config/packages/sentry.yaml @@ -0,0 +1,39 @@ +when@dev: + sentry: + dsn: '%env(SENTRY_DSN)%' + options: + # Add request headers, cookies, IP address and the authenticated user + # see https://docs.sentry.io/platforms/php/data-management/data-collected/ for more info + # send_default_pii: true + ignore_exceptions: + - 'Symfony\Component\ErrorHandler\Error\FatalError' + - 'Symfony\Component\Debug\Exception\FatalErrorException' +# +# # If you are using Monolog, you also need this additional configuration to log the errors correctly: +# # https://docs.sentry.io/platforms/php/guides/symfony/integrations/monolog/ +# register_error_listener: false +# register_error_handler: false +# + monolog: + handlers: + # Use this only if you don't want to use structured logging and instead receive + # certain log levels as errors. + sentry: + type: service + id: Sentry\Monolog\Handler +# +# # Use this for structured log integration +# sentry_logs: +# type: service +# id: Sentry\SentryBundle\Monolog\LogsHandler +# +# # Enable one of the two services below, depending on your choice above + services: + Sentry\Monolog\Handler: + arguments: + $hub: '@Sentry\State\HubInterface' + $level: !php/const Monolog\Logger::ERROR + $fillExtraContext: true # Enables sending monolog context to Sentry +# Sentry\SentryBundle\Monolog\LogsHandler: +# arguments: +# - !php/const Monolog\Logger::INFO diff --git a/backend/config/reference.php b/backend/config/reference.php index 61e9ec8e..a97740a1 100644 --- a/backend/config/reference.php +++ b/backend/config/reference.php @@ -1316,6 +1316,85 @@ * expired_worker_ttl?: int|Param, // How long to keep expired workers in cache (in seconds). // Default: 3600 * }, * } + * @psalm-type SentryConfig = array{ + * dsn?: scalar|Param|null, // If this value is not provided, the SDK will try to read it from the SENTRY_DSN environment variable. If that variable also does not exist, the SDK will not send any events. + * register_error_listener?: bool|Param, // Default: true + * register_error_handler?: bool|Param, // Default: true + * logger?: scalar|Param|null, // The service ID of the PSR-3 logger used to log messages coming from the SDK client. Be aware that setting the same logger of the application may create a circular loop when an event fails to be sent. // Default: null + * options?: array{ + * integrations?: mixed, // Default: [] + * default_integrations?: bool|Param, + * prefixes?: list, + * sample_rate?: float|Param, // The sampling factor to apply to events. A value of 0 will deny sending any event, and a value of 1 will send all events. + * enable_tracing?: bool|Param, + * traces_sample_rate?: float|Param, // The sampling factor to apply to transactions. A value of 0 will deny sending any transaction, and a value of 1 will send all transactions. + * traces_sampler?: scalar|Param|null, + * profiles_sample_rate?: float|Param, // The sampling factor to apply to profiles. A value of 0 will deny sending any profiles, and a value of 1 will send all profiles. Profiles are sampled in relation to traces_sample_rate + * enable_logs?: bool|Param, + * enable_metrics?: bool|Param, // Default: true + * attach_stacktrace?: bool|Param, + * attach_metric_code_locations?: bool|Param, + * context_lines?: int|Param, + * environment?: scalar|Param|null, // Default: "%kernel.environment%" + * logger?: scalar|Param|null, + * spotlight?: bool|Param, + * spotlight_url?: scalar|Param|null, + * release?: scalar|Param|null, // Default: "%env(default::SENTRY_RELEASE)%" + * server_name?: scalar|Param|null, + * ignore_exceptions?: list, + * ignore_transactions?: list, + * before_send?: scalar|Param|null, + * before_send_transaction?: scalar|Param|null, + * before_send_check_in?: scalar|Param|null, + * before_send_metrics?: scalar|Param|null, + * before_send_log?: scalar|Param|null, + * before_send_metric?: scalar|Param|null, + * trace_propagation_targets?: mixed, + * tags?: array, + * error_types?: scalar|Param|null, + * max_breadcrumbs?: int|Param, + * before_breadcrumb?: mixed, + * in_app_exclude?: list, + * in_app_include?: list, + * send_default_pii?: bool|Param, + * max_value_length?: int|Param, + * transport?: scalar|Param|null, + * http_client?: scalar|Param|null, + * http_proxy?: scalar|Param|null, + * http_proxy_authentication?: scalar|Param|null, + * http_connect_timeout?: float|Param, // The maximum number of seconds to wait while trying to connect to a server. It works only when using the default transport. + * http_timeout?: float|Param, // The maximum execution time for the request+response as a whole. It works only when using the default transport. + * http_ssl_verify_peer?: bool|Param, + * http_compression?: bool|Param, + * capture_silenced_errors?: bool|Param, + * max_request_body_size?: "none"|"never"|"small"|"medium"|"always"|Param, + * class_serializers?: array, + * }, + * messenger?: bool|array{ + * enabled?: bool|Param, // Default: true + * capture_soft_fails?: bool|Param, // Default: true + * isolate_breadcrumbs_by_message?: bool|Param, // Default: false + * }, + * tracing?: bool|array{ + * enabled?: bool|Param, // Default: true + * dbal?: bool|array{ + * enabled?: bool|Param, // Default: true + * connections?: list, + * }, + * twig?: bool|array{ + * enabled?: bool|Param, // Default: true + * }, + * cache?: bool|array{ + * enabled?: bool|Param, // Default: true + * }, + * http_client?: bool|array{ + * enabled?: bool|Param, // Default: true + * }, + * console?: array{ + * excluded_commands?: list, + * }, + * }, + * } * @psalm-type ConfigType = array{ * imports?: ImportsConfig, * parameters?: ParametersConfig, @@ -1331,6 +1410,7 @@ * twig_component?: TwigComponentConfig, * twig_extra?: TwigExtraConfig, * zenstruck_messenger_monitor?: ZenstruckMessengerMonitorConfig, + * sentry?: SentryConfig, * "when@dev"?: array{ * imports?: ImportsConfig, * parameters?: ParametersConfig, @@ -1349,6 +1429,7 @@ * twig_component?: TwigComponentConfig, * twig_extra?: TwigExtraConfig, * zenstruck_messenger_monitor?: ZenstruckMessengerMonitorConfig, + * sentry?: SentryConfig, * }, * "when@prod"?: array{ * imports?: ImportsConfig, @@ -1365,6 +1446,7 @@ * twig_component?: TwigComponentConfig, * twig_extra?: TwigExtraConfig, * zenstruck_messenger_monitor?: ZenstruckMessengerMonitorConfig, + * sentry?: SentryConfig, * }, * "when@test"?: array{ * imports?: ImportsConfig, @@ -1384,6 +1466,7 @@ * twig_component?: TwigComponentConfig, * twig_extra?: TwigExtraConfig, * zenstruck_messenger_monitor?: ZenstruckMessengerMonitorConfig, + * sentry?: SentryConfig, * }, * ... Date: Thu, 26 Feb 2026 10:04:58 +0100 Subject: [PATCH 03/23] subscriber lists done --- backend/.env | 9 +- backend/.gitignore | 1 + backend/config/reference.php | 3 - backend/migrations/Version20260225000000.php | 35 +++ .../Controller/SubscriberController.php | 64 +++++- .../Subscriber/AddSubscriberListInput.php | 10 + .../Subscriber/RemoveSubscriberListInput.php | 10 + .../Subscriber/RemoveSubscriberListReason.php | 9 + .../SubscriberListIfUnsubscribed.php | 9 + .../src/Entity/SubscriberListUnsubscribed.php | 65 ++++++ .../NewsletterList/NewsletterListService.php | 17 ++ .../Service/Subscriber/SubscriberService.php | 60 ++++++ .../Subscriber/AddSubscriberListTest.php | 201 ++++++++++++++++++ .../Subscriber/CreateSubscriberTest.php | 2 - .../Subscriber/RemoveSubscriberListTest.php | 192 +++++++++++++++++ .../SubscriberListUnsubscribedFactory.php | 38 ++++ 16 files changed, 714 insertions(+), 11 deletions(-) create mode 100644 backend/migrations/Version20260225000000.php create mode 100644 backend/src/Api/Console/Input/Subscriber/AddSubscriberListInput.php create mode 100644 backend/src/Api/Console/Input/Subscriber/RemoveSubscriberListInput.php create mode 100644 backend/src/Api/Console/Input/Subscriber/RemoveSubscriberListReason.php create mode 100644 backend/src/Api/Console/Input/Subscriber/SubscriberListIfUnsubscribed.php create mode 100644 backend/src/Entity/SubscriberListUnsubscribed.php create mode 100644 backend/tests/Api/Console/Subscriber/AddSubscriberListTest.php create mode 100644 backend/tests/Api/Console/Subscriber/RemoveSubscriberListTest.php create mode 100644 backend/tests/Factory/SubscriberListUnsubscribedFactory.php diff --git a/backend/.env b/backend/.env index 44f99e24..1926fa02 100644 --- a/backend/.env +++ b/backend/.env @@ -31,7 +31,6 @@ URL_ARCHIVE=https://hyvorpost.email # One of: debug, info, notice, warning, error, critical, alert, emergency LOG_LEVEL=info - ### ============ HYVOR CLOUD ============ ### # Deployment type @@ -54,13 +53,11 @@ COMMS_KEY= # @deprecated. Migrate to DEPLOYMENT env IS_CLOUD=true - ### ============ DATABASE ============ ### # PostgreSQL database is the single source of truth DATABASE_URL="" - ### ============ MAIL ============ ### # Hyvor Relay configuration @@ -84,7 +81,6 @@ NOTIFICATION_MAIL_REPLY_TO= # If not set, if will fallback to RELAY_API_KEY NOTIFICATION_RELAY_API_KEY= - ### ============ FILE STORAGE ============ ### # You can use any S3-compatibly storage like @@ -97,6 +93,10 @@ S3_ACCESS_KEY_ID=key-id S3_SECRET_ACCESS_KEY=access-key S3_BUCKET=hyvor-post +### ============ INTEGRATIONS ============ ### + +# Sentry-compatible DSN for error tracking +SENTRY_DSN= ### ============ SCALING ============ ### @@ -104,7 +104,6 @@ S3_BUCKET=hyvor-post # Default is x2 CPUs WORKERS= - ### ============ DOCKER IMAGE ============ ### # Defaults (do not change or add to docs): diff --git a/backend/.gitignore b/backend/.gitignore index 05abde28..67fd15a8 100644 --- a/backend/.gitignore +++ b/backend/.gitignore @@ -2,6 +2,7 @@ /.env.dev.local /.env.local.php /.env.*.local +/.env.local /config/secrets/prod/prod.decrypt.private.php /public/bundles/ /var/ diff --git a/backend/config/reference.php b/backend/config/reference.php index a97740a1..36bb2776 100644 --- a/backend/config/reference.php +++ b/backend/config/reference.php @@ -1410,7 +1410,6 @@ * twig_component?: TwigComponentConfig, * twig_extra?: TwigExtraConfig, * zenstruck_messenger_monitor?: ZenstruckMessengerMonitorConfig, - * sentry?: SentryConfig, * "when@dev"?: array{ * imports?: ImportsConfig, * parameters?: ParametersConfig, @@ -1429,7 +1428,6 @@ * twig_component?: TwigComponentConfig, * twig_extra?: TwigExtraConfig, * zenstruck_messenger_monitor?: ZenstruckMessengerMonitorConfig, - * sentry?: SentryConfig, * }, * "when@prod"?: array{ * imports?: ImportsConfig, @@ -1466,7 +1464,6 @@ * twig_component?: TwigComponentConfig, * twig_extra?: TwigExtraConfig, * zenstruck_messenger_monitor?: ZenstruckMessengerMonitorConfig, - * sentry?: SentryConfig, * }, * ...addSql(<<addSql('DROP TABLE list_subscriber_unsubscribed'); + } +} diff --git a/backend/src/Api/Console/Controller/SubscriberController.php b/backend/src/Api/Console/Controller/SubscriberController.php index 9163a7e1..29d76aa7 100644 --- a/backend/src/Api/Console/Controller/SubscriberController.php +++ b/backend/src/Api/Console/Controller/SubscriberController.php @@ -4,9 +4,13 @@ use App\Api\Console\Authorization\Scope; use App\Api\Console\Authorization\ScopeRequired; +use App\Api\Console\Input\Subscriber\AddSubscriberListInput; use App\Api\Console\Input\Subscriber\BulkActionSubscriberInput; use App\Api\Console\Input\Subscriber\CreateSubscriberIfExists; use App\Api\Console\Input\Subscriber\CreateSubscriberInput; +use App\Api\Console\Input\Subscriber\RemoveSubscriberListInput; +use App\Api\Console\Input\Subscriber\RemoveSubscriberListReason; +use App\Api\Console\Input\Subscriber\SubscriberListIfUnsubscribed; use App\Api\Console\Input\Subscriber\UpdateSubscriberInput; use App\Api\Console\Object\SubscriberObject; use App\Entity\Newsletter; @@ -31,7 +35,7 @@ class SubscriberController extends AbstractController public function __construct( private SubscriberService $subscriberService, private NewsletterListService $newsletterListService, - private SubscriberMetadataService $subscriberMetadataService + private SubscriberMetadataService $subscriberMetadataService, ) { } @@ -272,4 +276,62 @@ public function bulkActions(Newsletter $newsletter, #[MapRequestPayload] BulkAct throw new BadRequestHttpException("Unhandled action"); } + + #[Route('/subscribers/{id}/lists', methods: 'POST')] + #[ScopeRequired(Scope::SUBSCRIBERS_WRITE)] + public function addSubscriberList( + Subscriber $subscriber, + Newsletter $newsletter, + #[MapRequestPayload] AddSubscriberListInput $input + ): JsonResponse + { + if ($input->id === null && $input->name === null) { + throw new UnprocessableEntityHttpException('Either id or name must be provided'); + } + + $list = $this->newsletterListService->getListByIdOrName($newsletter, $input->id, $input->name); + + if ($list === null) { + throw new UnprocessableEntityHttpException('List not found'); + } + + try { + $this->subscriberService->addSubscriberToList( + $subscriber, + $list, + $input->if_unsubscribed === SubscriberListIfUnsubscribed::ERROR + ); + } catch (\RuntimeException $e) { + throw new UnprocessableEntityHttpException($e->getMessage()); + } + + return $this->json(new SubscriberObject($subscriber)); + } + + #[Route('/subscribers/{id}/lists', methods: 'DELETE')] + #[ScopeRequired(Scope::SUBSCRIBERS_WRITE)] + public function removeSubscriberList( + Subscriber $subscriber, + Newsletter $newsletter, + #[MapRequestPayload] RemoveSubscriberListInput $input + ): JsonResponse + { + if ($input->id === null && $input->name === null) { + throw new UnprocessableEntityHttpException('Either id or name must be provided'); + } + + $list = $this->newsletterListService->getListByIdOrName($newsletter, $input->id, $input->name); + + if ($list === null) { + throw new UnprocessableEntityHttpException('List not found'); + } + + $this->subscriberService->removeSubscriberFromList( + $subscriber, + $list, + $input->reason === RemoveSubscriberListReason::UNSUBSCRIBE + ); + + return $this->json(new SubscriberObject($subscriber)); + } } diff --git a/backend/src/Api/Console/Input/Subscriber/AddSubscriberListInput.php b/backend/src/Api/Console/Input/Subscriber/AddSubscriberListInput.php new file mode 100644 index 00000000..db45af81 --- /dev/null +++ b/backend/src/Api/Console/Input/Subscriber/AddSubscriberListInput.php @@ -0,0 +1,10 @@ +id; + } + + public function getList(): NewsletterList + { + return $this->list; + } + + public function setList(NewsletterList $list): static + { + $this->list = $list; + return $this; + } + + public function getSubscriber(): Subscriber + { + return $this->subscriber; + } + + public function setSubscriber(Subscriber $subscriber): static + { + $this->subscriber = $subscriber; + return $this; + } + + public function getCreatedAt(): \DateTimeImmutable + { + return $this->created_at; + } + + public function setCreatedAt(\DateTimeImmutable $created_at): static + { + $this->created_at = $created_at; + return $this; + } +} diff --git a/backend/src/Service/NewsletterList/NewsletterListService.php b/backend/src/Service/NewsletterList/NewsletterListService.php index 4e6612a1..fc2b7c11 100644 --- a/backend/src/Service/NewsletterList/NewsletterListService.php +++ b/backend/src/Service/NewsletterList/NewsletterListService.php @@ -137,6 +137,23 @@ public function getListsByIds(array $listIds): ArrayCollection ); } + public function getListByIdOrName(Newsletter $newsletter, ?int $id, ?string $name): ?NewsletterList + { + assert($id !== null || $name !== null, 'Either id or name must be provided'); + + if ($id !== null) { + return $this->em->getRepository(NewsletterList::class)->findOneBy([ + 'id' => $id, + 'newsletter' => $newsletter, + ]); + } + + return $this->em->getRepository(NewsletterList::class)->findOneBy([ + 'name' => $name, + 'newsletter' => $newsletter, + ]); + } + /** * @param int[] $listIds * @return array key: list_id, value: subscriber_count diff --git a/backend/src/Service/Subscriber/SubscriberService.php b/backend/src/Service/Subscriber/SubscriberService.php index 43d33945..1ff7528f 100644 --- a/backend/src/Service/Subscriber/SubscriberService.php +++ b/backend/src/Service/Subscriber/SubscriberService.php @@ -8,6 +8,7 @@ use App\Entity\Send; use App\Entity\Subscriber; use App\Entity\SubscriberExport; +use App\Entity\SubscriberListUnsubscribed; use App\Entity\Type\SubscriberExportStatus; use App\Entity\Type\SubscriberSource; use App\Entity\Type\SubscriberStatus; @@ -301,6 +302,65 @@ public function getExports(Newsletter $newsletter): array ->findBy(['newsletter' => $newsletter], ['created_at' => 'DESC']); } + /** + * @throws \RuntimeException if $checkUnsubscribed is true and the subscriber has a prior unsubscription record + */ + public function addSubscriberToList( + Subscriber $subscriber, + NewsletterList $list, + bool $checkUnsubscribed + ): void + { + if ($checkUnsubscribed) { + $record = $this->em->getRepository(SubscriberListUnsubscribed::class)->findOneBy([ + 'list' => $list, + 'subscriber' => $subscriber, + ]); + + if ($record !== null) { + throw new \RuntimeException('Subscriber has previously unsubscribed from this list'); + } + } + + $subscriber->addList($list); + $subscriber->setUpdatedAt($this->now()); + + $this->em->persist($subscriber); + $this->em->flush(); + } + + public function removeSubscriberFromList( + Subscriber $subscriber, + NewsletterList $list, + bool $recordUnsubscription + ): void + { + $subscriber->removeList($list); + $subscriber->setUpdatedAt($this->now()); + + $this->em->persist($subscriber); + $this->em->flush(); + + + if ($recordUnsubscription) { + $existing = $this->em->getRepository(SubscriberListUnsubscribed::class)->findOneBy([ + 'list' => $list, + 'subscriber' => $subscriber, + ]); + + if ($existing === null) { + $unsubscribed = new SubscriberListUnsubscribed() + ->setList($list) + ->setSubscriber($subscriber) + ->setCreatedAt($this->now()); + + $this->em->persist($unsubscribed); + $this->em->persist($list); // test fails otherwise, since this is used in removeElement, but sure why + $this->em->flush(); + } + } + } + public function getSubscriberById(int $id): ?Subscriber { return $this->subscriberRepository->find($id); diff --git a/backend/tests/Api/Console/Subscriber/AddSubscriberListTest.php b/backend/tests/Api/Console/Subscriber/AddSubscriberListTest.php new file mode 100644 index 00000000..84e16ba2 --- /dev/null +++ b/backend/tests/Api/Console/Subscriber/AddSubscriberListTest.php @@ -0,0 +1,201 @@ + $newsletter]); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); + + $response = $this->consoleApi( + $newsletter, + 'POST', + '/subscribers/' . $subscriber->getId() . '/lists', + ['id' => $list->getId()] + ); + + $this->assertSame(200, $response->getStatusCode()); + + $subscriberDb = $this->em->getRepository(Subscriber::class)->find($subscriber->getId()); + $this->assertInstanceOf(Subscriber::class, $subscriberDb); + $this->assertCount(1, $subscriberDb->getLists()); + $this->assertSame($list->getId(), $subscriberDb->getLists()->first()->getId()); + } + + public function testAddSubscriberToListByName(): void + { + $newsletter = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne(['newsletter' => $newsletter, 'name' => 'My List']); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); + + $response = $this->consoleApi( + $newsletter, + 'POST', + '/subscribers/' . $subscriber->getId() . '/lists', + ['name' => 'My List'] + ); + + $this->assertSame(200, $response->getStatusCode()); + + $subscriberDb = $this->em->getRepository(Subscriber::class)->find($subscriber->getId()); + $this->assertInstanceOf(Subscriber::class, $subscriberDb); + $this->assertCount(1, $subscriberDb->getLists()); + $this->assertSame($list->getId(), $subscriberDb->getLists()->first()->getId()); + } + + public function testAddSubscriberToListValidation(): void + { + $newsletter = NewsletterFactory::createOne(); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); + + $response = $this->consoleApi( + $newsletter, + 'POST', + '/subscribers/' . $subscriber->getId() . '/lists', + [] + ); + + $this->assertSame(422, $response->getStatusCode()); + $this->assertSame('Either id or name must be provided', $this->getJson()['message']); + } + + public function testAddSubscriberToListNotFound(): void + { + $newsletter = NewsletterFactory::createOne(); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); + + $response = $this->consoleApi( + $newsletter, + 'POST', + '/subscribers/' . $subscriber->getId() . '/lists', + ['id' => 999999] + ); + + $this->assertSame(422, $response->getStatusCode()); + $this->assertSame('List not found', $this->getJson()['message']); + } + + public function testAddSubscriberToListAlreadyInList(): void + { + $newsletter = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter, 'lists' => [$list]]); + + $response = $this->consoleApi( + $newsletter, + 'POST', + '/subscribers/' . $subscriber->getId() . '/lists', + ['id' => $list->getId()] + ); + + $this->assertSame(200, $response->getStatusCode()); + + // Still only in the list once (idempotent) + $subscriberDb = $this->em->getRepository(Subscriber::class)->find($subscriber->getId()); + $this->assertInstanceOf(Subscriber::class, $subscriberDb); + $this->assertCount(1, $subscriberDb->getLists()); + } + + public function testAddSubscriberToListIfUnsubscribedError(): void + { + $newsletter = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); + + SubscriberListUnsubscribedFactory::createOne([ + 'list' => $list, + 'subscriber' => $subscriber, + ]); + + $response = $this->consoleApi( + $newsletter, + 'POST', + '/subscribers/' . $subscriber->getId() . '/lists', + ['id' => $list->getId(), 'if_unsubscribed' => 'error'] + ); + + $this->assertSame(422, $response->getStatusCode()); + $this->assertSame( + 'Subscriber has previously unsubscribed from this list', + $this->getJson()['message'] + ); + } + + public function testAddSubscriberToListIfUnsubscribedForceCreate(): void + { + $newsletter = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); + + SubscriberListUnsubscribedFactory::createOne([ + 'list' => $list, + 'subscriber' => $subscriber, + ]); + + $response = $this->consoleApi( + $newsletter, + 'POST', + '/subscribers/' . $subscriber->getId() . '/lists', + ['id' => $list->getId(), 'if_unsubscribed' => 'force_create'] + ); + + $this->assertSame(200, $response->getStatusCode()); + + $subscriberDb = $this->em->getRepository(Subscriber::class)->find($subscriber->getId()); + $this->assertInstanceOf(Subscriber::class, $subscriberDb); + $this->assertCount(1, $subscriberDb->getLists()); + } + + public function testCannotAddSubscriberOfOtherNewsletter(): void + { + $newsletter1 = NewsletterFactory::createOne(); + $newsletter2 = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne(['newsletter' => $newsletter1]); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter1]); + + $response = $this->consoleApi( + $newsletter2, + 'POST', + '/subscribers/' . $subscriber->getId() . '/lists', + ['id' => $list->getId()] + ); + + $this->assertSame(403, $response->getStatusCode()); + $this->assertSame('Entity does not belong to the newsletter', $this->getJson()['message']); + } + + public function testCannotAddListOfOtherNewsletter(): void + { + $newsletter1 = NewsletterFactory::createOne(); + $newsletter2 = NewsletterFactory::createOne(); + $listOfNewsletter2 = NewsletterListFactory::createOne(['newsletter' => $newsletter2]); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter1]); + + $response = $this->consoleApi( + $newsletter1, + 'POST', + '/subscribers/' . $subscriber->getId() . '/lists', + ['id' => $listOfNewsletter2->getId()] + ); + + $this->assertSame(422, $response->getStatusCode()); + $this->assertSame('List not found', $this->getJson()['message']); + } +} diff --git a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php index d47c2157..b29d266c 100644 --- a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php +++ b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php @@ -40,8 +40,6 @@ public function test_test(): void ] ); - dd($response->getContent()); - } public function testCreateSubscriberMinimal(): void diff --git a/backend/tests/Api/Console/Subscriber/RemoveSubscriberListTest.php b/backend/tests/Api/Console/Subscriber/RemoveSubscriberListTest.php new file mode 100644 index 00000000..d19db0d1 --- /dev/null +++ b/backend/tests/Api/Console/Subscriber/RemoveSubscriberListTest.php @@ -0,0 +1,192 @@ + $newsletter]); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter, 'lists' => [$list]]); + + $response = $this->consoleApi( + $newsletter, + 'DELETE', + '/subscribers/' . $subscriber->getId() . '/lists', + ['id' => $list->getId()] + ); + + $this->assertSame(200, $response->getStatusCode()); + + $this->em->clear(); + $subscriberDb = $this->em->getRepository(Subscriber::class)->find($subscriber->getId()); + $this->assertInstanceOf(Subscriber::class, $subscriberDb); + $this->assertCount(0, $subscriberDb->getLists()); + } + + public function testRemoveSubscriberFromListByName(): void + { + $newsletter = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne(['newsletter' => $newsletter, 'name' => 'Remove Me']); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter, 'lists' => [$list]]); + + $response = $this->consoleApi( + $newsletter, + 'DELETE', + '/subscribers/' . $subscriber->getId() . '/lists', + ['name' => 'Remove Me'] + ); + + $this->assertSame(200, $response->getStatusCode()); + + $this->em->clear(); + $subscriberDb = $this->em->getRepository(Subscriber::class)->find($subscriber->getId()); + $this->assertInstanceOf(Subscriber::class, $subscriberDb); + $this->assertCount(0, $subscriberDb->getLists()); + } + + public function testRemoveSubscriberFromListValidation(): void + { + $newsletter = NewsletterFactory::createOne(); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); + + $response = $this->consoleApi( + $newsletter, + 'DELETE', + '/subscribers/' . $subscriber->getId() . '/lists', + [] + ); + + $this->assertSame(422, $response->getStatusCode()); + $this->assertSame('Either id or name must be provided', $this->getJson()['message']); + } + + public function testRemoveSubscriberFromListNotFound(): void + { + $newsletter = NewsletterFactory::createOne(); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); + + $response = $this->consoleApi( + $newsletter, + 'DELETE', + '/subscribers/' . $subscriber->getId() . '/lists', + ['id' => 999999] + ); + + $this->assertSame(422, $response->getStatusCode()); + $this->assertSame('List not found', $this->getJson()['message']); + } + + public function testRemoveSubscriberFromListNotInList(): void + { + $newsletter = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); + + $response = $this->consoleApi( + $newsletter, + 'DELETE', + '/subscribers/' . $subscriber->getId() . '/lists', + ['id' => $list->getId()] + ); + + $this->assertSame(200, $response->getStatusCode()); + } + + public function testRemoveSubscriberFromListWithReasonUnsubscribe(): void + { + $newsletter = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter, 'lists' => [$list]]); + + $response = $this->consoleApi( + $newsletter, + 'DELETE', + '/subscribers/' . $subscriber->getId() . '/lists', + ['id' => $list->getId(), 'reason' => 'unsubscribe'] + ); + + $this->assertSame(200, $response->getStatusCode()); + + $record = $this->em->getRepository(SubscriberListUnsubscribed::class)->findOneBy([ + 'list' => $list->_real(), + 'subscriber' => $subscriber->_real(), + ]); + + $this->assertInstanceOf(SubscriberListUnsubscribed::class, $record); + } + + public function testRemoveSubscriberFromListWithReasonOther(): void + { + $newsletter = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter, 'lists' => [$list]]); + + $response = $this->consoleApi( + $newsletter, + 'DELETE', + '/subscribers/' . $subscriber->getId() . '/lists', + ['id' => $list->getId(), 'reason' => 'other'] + ); + + $this->assertSame(200, $response->getStatusCode()); + + $record = $this->em->getRepository(SubscriberListUnsubscribed::class)->findOneBy([ + 'list' => $list->_real(), + 'subscriber' => $subscriber->_real(), + ]); + + $this->assertNull($record); + } + + public function testCannotRemoveSubscriberOfOtherNewsletter(): void + { + $newsletter1 = NewsletterFactory::createOne(); + $newsletter2 = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne(['newsletter' => $newsletter1]); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter1, 'lists' => [$list]]); + + $response = $this->consoleApi( + $newsletter2, + 'DELETE', + '/subscribers/' . $subscriber->getId() . '/lists', + ['id' => $list->getId()] + ); + + $this->assertSame(403, $response->getStatusCode()); + $this->assertSame('Entity does not belong to the newsletter', $this->getJson()['message']); + } + + public function testCannotRemoveListOfOtherNewsletter(): void + { + $newsletter1 = NewsletterFactory::createOne(); + $newsletter2 = NewsletterFactory::createOne(); + $listOfNewsletter2 = NewsletterListFactory::createOne(['newsletter' => $newsletter2]); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter1]); + + $response = $this->consoleApi( + $newsletter1, + 'DELETE', + '/subscribers/' . $subscriber->getId() . '/lists', + ['id' => $listOfNewsletter2->getId()] + ); + + $this->assertSame(422, $response->getStatusCode()); + $this->assertSame('List not found', $this->getJson()['message']); + } +} diff --git a/backend/tests/Factory/SubscriberListUnsubscribedFactory.php b/backend/tests/Factory/SubscriberListUnsubscribedFactory.php new file mode 100644 index 00000000..ee3f0660 --- /dev/null +++ b/backend/tests/Factory/SubscriberListUnsubscribedFactory.php @@ -0,0 +1,38 @@ + + */ +final class SubscriberListUnsubscribedFactory extends PersistentProxyObjectFactory +{ + public function __construct() + { + } + + public static function class(): string + { + return SubscriberListUnsubscribed::class; + } + + /** + * @return array + */ + protected function defaults(): array + { + return [ + 'list' => NewsletterListFactory::new(), + 'subscriber' => SubscriberFactory::new(), + 'created_at' => \DateTimeImmutable::createFromMutable(self::faker()->dateTime()), + ]; + } + + protected function initialize(): static + { + return $this; + } +} From 9d882bc39e1d673cb60a96ddc1593b820174d302 Mon Sep 17 00:00:00 2001 From: Supun Wimalasena Date: Thu, 26 Feb 2026 10:48:25 +0100 Subject: [PATCH 04/23] clean subscriber endpoints --- .../Controller/SubscriberController.php | 45 ++++----- .../Subscriber/CreateSubscriberInput.php | 10 -- .../Subscriber/UpdateSubscriberInput.php | 10 -- .../Public/Controller/Form/FormController.php | 1 - .../Subscriber/Dto/UpdateSubscriberDto.php | 6 +- .../Service/Subscriber/SubscriberService.php | 10 -- .../Subscriber/CreateSubscriberTest.php | 89 ++---------------- .../Subscriber/UpdateSubscriberTest.php | 92 ++----------------- .../docs/[...slug]/content/ConsoleApi.svelte | 48 +++++++++- 9 files changed, 78 insertions(+), 233 deletions(-) diff --git a/backend/src/Api/Console/Controller/SubscriberController.php b/backend/src/Api/Console/Controller/SubscriberController.php index 29d76aa7..6a3a0e36 100644 --- a/backend/src/Api/Console/Controller/SubscriberController.php +++ b/backend/src/Api/Console/Controller/SubscriberController.php @@ -85,16 +85,7 @@ public function createSubscriber( ): JsonResponse { - $missingListIds = $this - ->newsletterListService - ->getMissingListIdsOfNewsletter($newsletter, $input->list_ids); - - if ($missingListIds !== null) { - throw new UnprocessableEntityHttpException("List with id {$missingListIds[0]} not found"); - } - $subscriber = $this->subscriberService->getSubscriberByEmail($newsletter, $input->email); - $lists = $this->newsletterListService->getListsByIds($input->list_ids); if ($subscriber === null) { @@ -102,7 +93,7 @@ public function createSubscriber( $subscriber = $this->subscriberService->createSubscriber( $newsletter, $input->email, - $lists, + [], SubscriberStatus::PENDING, source: $input->source ?? SubscriberSource::CONSOLE, subscribeIp: $input->subscribe_ip ?? null, @@ -114,13 +105,24 @@ public function createSubscriber( // update $updates = new UpdateSubscriberDto(); - $updates->lists = $lists; - $updates->status = $input->status; - $updates->subscribedAt = $input->subscribed_at; - $updates->unsubscribedAt = $input->unsubscribed_at; - // TODO: + if ($updates->has('status')) { + $updates->status = $input->status; + } + + if ($updates->has('subscribe_ip')) { + $updates->subscribeIp = $input->subscribe_ip; + } + + if ($updates->has('subscribed_at')) { + $updates->subscribedAt = $input->subscribed_at ? \DateTimeImmutable::createFromTimestamp($input->subscribed_at) : null; + } + + if ($updates->has('unsubscribed_at')) { + $updates->unsubscribedAt = $input->unsubscribed_at ? \DateTimeImmutable::createFromTimestamp($input->unsubscribed_at) : null; + } + $subscriber = $this->subscriberService->updateSubscriber($subscriber, $updates); } else { throw new UnprocessableEntityHttpException("Subscriber with email {$input->email} already exists"); @@ -148,19 +150,6 @@ public function updateSubscriber( $updates->email = $input->email; } - if ($input->has('list_ids')) { - $missingListIds = $this->newsletterListService->getMissingListIdsOfNewsletter( - $newsletter, - $input->list_ids - ); - - if ($missingListIds !== null) { - throw new UnprocessableEntityHttpException("List with id {$missingListIds[0]} not found"); - } - - $updates->lists = $this->newsletterListService->getListsByIds($input->list_ids); - } - if ($input->has('status')) { if ($input->status === SubscriberStatus::SUBSCRIBED && $subscriber->getOptInAt() === null) { throw new UnprocessableEntityHttpException('Subscribers without opt-in can not be updated to SUBSCRIBED status.'); diff --git a/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php b/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php index 04f8c890..3ca8ba40 100644 --- a/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php +++ b/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php @@ -17,16 +17,6 @@ class CreateSubscriberInput #[Assert\Length(max: 255)] public string $email; - /** - * @var int[] $list_ids - */ - #[Assert\NotBlank] - #[Assert\All([ - new Assert\NotBlank(), - new Assert\Type('int'), - ])] - public array $list_ids; - public SubscriberStatus $status = SubscriberStatus::PENDING; public ?SubscriberSource $source; diff --git a/backend/src/Api/Console/Input/Subscriber/UpdateSubscriberInput.php b/backend/src/Api/Console/Input/Subscriber/UpdateSubscriberInput.php index e2b9014d..f4958852 100644 --- a/backend/src/Api/Console/Input/Subscriber/UpdateSubscriberInput.php +++ b/backend/src/Api/Console/Input/Subscriber/UpdateSubscriberInput.php @@ -14,16 +14,6 @@ class UpdateSubscriberInput #[Assert\Email] public string $email; - /** - * @var int[] $list_ids - */ - #[Assert\Count(min: 1, minMessage: "There should be at least one list.")] - #[Assert\All([ - new Assert\NotBlank(), - new Assert\Type('int'), - ])] - public array $list_ids; - public SubscriberStatus $status; /** diff --git a/backend/src/Api/Public/Controller/Form/FormController.php b/backend/src/Api/Public/Controller/Form/FormController.php index d0e1beee..997d7d73 100644 --- a/backend/src/Api/Public/Controller/Form/FormController.php +++ b/backend/src/Api/Public/Controller/Form/FormController.php @@ -5,7 +5,6 @@ namespace App\Api\Public\Controller\Form; use App\Api\Public\Input\Form\FormInitInput; -use App\Api\Public\Input\Form\FormRenderInput; use App\Api\Public\Input\Form\FormSubscribeInput; use App\Api\Public\Object\Form\FormListObject; use App\Api\Public\Object\Form\FormSubscriberObject; diff --git a/backend/src/Service/Subscriber/Dto/UpdateSubscriberDto.php b/backend/src/Service/Subscriber/Dto/UpdateSubscriberDto.php index f4b758fa..d42fd0f8 100644 --- a/backend/src/Service/Subscriber/Dto/UpdateSubscriberDto.php +++ b/backend/src/Service/Subscriber/Dto/UpdateSubscriberDto.php @@ -13,12 +13,8 @@ class UpdateSubscriberDto public string $email; - /** - * @var iterable - */ - public iterable $lists; - public SubscriberStatus $status; + public ?string $subscribeIp; public ?\DateTimeImmutable $subscribedAt; diff --git a/backend/src/Service/Subscriber/SubscriberService.php b/backend/src/Service/Subscriber/SubscriberService.php index 1ff7528f..cc301f55 100644 --- a/backend/src/Service/Subscriber/SubscriberService.php +++ b/backend/src/Service/Subscriber/SubscriberService.php @@ -166,16 +166,6 @@ public function updateSubscriber(Subscriber $subscriber, UpdateSubscriberDto $up $subscriber->setStatus($updates->status); } - if ($updates->has('lists')) { - // Clear & re-add lists - foreach ($subscriber->getLists() as $list) { - $subscriber->removeList($list); - } - foreach ($updates->lists as $list) { - $subscriber->addList($list); - } - } - if ($updates->has('subscribedAt')) { $subscriber->setSubscribedAt($updates->subscribedAt); } diff --git a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php index b29d266c..83f1dc10 100644 --- a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php +++ b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php @@ -11,7 +11,6 @@ use App\Service\Subscriber\Message\SubscriberCreatedMessage; use App\Service\Subscriber\SubscriberService; use App\Tests\Case\WebTestCase; -use App\Tests\Factory\NewsletterListFactory; use App\Tests\Factory\NewsletterFactory; use App\Tests\Factory\SubscriberFactory; use PHPUnit\Framework\Attributes\CoversClass; @@ -24,39 +23,17 @@ class CreateSubscriberTest extends WebTestCase { - public function test_test(): void - { - $newsletter = NewsletterFactory::createOne(); - $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); - - $response = $this->consoleApi( - $newsletter, - 'POST', - '/subscribers', - [ - 'email' => 'test@email.com', - 'list_ids' => [$list->getId()], - 'subscribe_ip' => null, // '222.222.222.222' - ] - ); - - } - public function testCreateSubscriberMinimal(): void { $this->mockRelayClient(); $newsletter = NewsletterFactory::createOne(); - $list1 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); - $list2 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); - $response = $this->consoleApi( $newsletter, 'POST', '/subscribers', [ 'email' => 'test@email.com', - 'list_ids' => [$list1->getId(), $list2->getId()] ] ); @@ -72,11 +49,7 @@ public function testCreateSubscriberMinimal(): void $this->assertSame('test@email.com', $subscriber->getEmail()); $this->assertSame(SubscriberStatus::PENDING, $subscriber->getStatus()); $this->assertSame('console', $subscriber->getSource()->value); - - $subscriberLists = $subscriber->getLists(); - $this->assertCount(2, $subscriberLists); - $this->assertSame($list1->getId(), $subscriberLists[0]?->getId()); - $this->assertSame($list2->getId(), $subscriberLists[1]?->getId()); + $this->assertCount(0, $subscriber->getLists()); $transport = $this->transport('async'); $transport->queue()->assertCount(1); @@ -88,7 +61,6 @@ public function testCreateSubscriberWithAllInputs(): void { $this->mockRelayClient(); $newsletter = NewsletterFactory::createOne(); - $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); $subscribedAt = new \DateTimeImmutable('2021-08-27 12:00:00'); $unsubscribedAt = new \DateTimeImmutable('2021-08-29 12:00:00'); @@ -99,7 +71,6 @@ public function testCreateSubscriberWithAllInputs(): void '/subscribers', [ 'email' => 'supun@hyvor.com', - 'list_ids' => [$list->getId()], 'source' => 'form', 'subscribe_ip' => '79.255.1.1', 'subscribed_at' => $subscribedAt->getTimestamp(), @@ -134,7 +105,7 @@ public function testCreateSubscriberWithAllInputs(): void $this->assertInstanceOf(SubscriberCreatedMessage::class, $message); } - public function testInputValidationEmptyEmailAndListIds(): void + public function testInputValidationEmptyEmail(): void { $this->validateInput( fn(Newsletter $newsletter) => [], @@ -143,38 +114,21 @@ public function testInputValidationEmptyEmailAndListIds(): void 'property' => 'email', 'message' => 'This value should not be blank.', ], - [ - 'property' => 'list_ids', - 'message' => 'This value should not be blank.', - ] ] ); } - public function testInputValidationInvalidEmailAndListIds(): void + public function testInputValidationInvalidEmail(): void { $this->validateInput( fn(Newsletter $newsletter) => [ 'email' => 'not-email', - 'list_ids' => [ - null, - 1, - 'string', - ], ], [ [ 'property' => 'email', 'message' => 'This value is not a valid email address.', ], - [ - 'property' => 'list_ids[0]', - 'message' => 'This value should not be blank.', - ], - [ - 'property' => 'list_ids[2]', - 'message' => 'This value should be of type int.', - ], ] ); } @@ -184,7 +138,6 @@ public function testInputValidationEmailTooLong(): void $this->validateInput( fn(Newsletter $newsletter) => [ 'email' => str_repeat('a', 256) . '@hyvor.com', - 'list_ids' => [1], ], [ [ @@ -200,7 +153,6 @@ public function testInputValidationOptionalValues(): void $this->validateInput( fn(Newsletter $newsletter) => [ 'email' => 'supun@hyvor.com', - 'list_ids' => [1], 'source' => 'invalid-source', 'subscribe_ip' => '127.0.0.1', 'subscribed_at' => 'invalid-date', @@ -234,7 +186,6 @@ public function testValidatesIp( $this->validateInput( fn(Newsletter $newsletter) => [ 'email' => 'supun@hyvor.com', - 'list_ids' => [1], 'subscribe_ip' => $ip, ], [ @@ -269,38 +220,13 @@ private function validateInput( $this->assertHasViolation($violations[0]['property'], $violations[0]['message']); } - public function testCreateSubscriberInvalidList(): void - { - $newsletter1 = NewsletterFactory::createOne(); - $newsletter2 = NewsletterFactory::createOne(); - - $newsletterList1 = NewsletterListFactory::createOne(['newsletter' => $newsletter2]); - - $response = $this->consoleApi( - $newsletter1, - 'POST', - '/subscribers', - [ - 'email' => 'supun@hyvor.com', - 'list_ids' => [$newsletterList1->getId()] - ] - ); - - $this->assertSame(422, $response->getStatusCode()); - $this->assertSame('List with id ' . $newsletterList1->getId() . ' not found', $this->getJson()['message']); - } - public function testCreateSubscriberDuplicateEmail(): void { $newsletter = NewsletterFactory::createOne(); - $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); - $subscriber = SubscriberFactory::createOne( - [ - 'newsletter' => $newsletter, - 'email' => 'thibault@hyvor.com', - 'lists' => [$list], - ] - ); + $subscriber = SubscriberFactory::createOne([ + 'newsletter' => $newsletter, + 'email' => 'thibault@hyvor.com', + ]); $response = $this->consoleApi( $newsletter, @@ -308,7 +234,6 @@ public function testCreateSubscriberDuplicateEmail(): void '/subscribers', [ 'email' => 'thibault@hyvor.com', - 'list_ids' => [$list->getId()], ] ); diff --git a/backend/tests/Api/Console/Subscriber/UpdateSubscriberTest.php b/backend/tests/Api/Console/Subscriber/UpdateSubscriberTest.php index 6056b32f..f6053e8b 100644 --- a/backend/tests/Api/Console/Subscriber/UpdateSubscriberTest.php +++ b/backend/tests/Api/Console/Subscriber/UpdateSubscriberTest.php @@ -14,8 +14,6 @@ use App\Tests\Factory\SubscriberFactory; use App\Tests\Factory\SubscriberMetadataDefinitionFactory; use PHPUnit\Framework\Attributes\CoversClass; -use Symfony\Component\Clock\Clock; -use Symfony\Component\Clock\MockClock; use Symfony\Component\Clock\Test\ClockSensitiveTrait; #[CoversClass(SubscriberController::class)] @@ -26,20 +24,15 @@ class UpdateSubscriberTest extends WebTestCase { use ClockSensitiveTrait; - // TODO: tests for authentication - - public function testUpdateList(): void + public function testUpdateStatus(): void { static::mockTime(new \DateTimeImmutable('2025-02-21')); $newsletter = NewsletterFactory::createOne(); - $list1 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); - $list2 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); - $subscriber = SubscriberFactory::createOne([ 'newsletter' => $newsletter, - 'lists' => [$list1], 'status' => SubscriberStatus::UNSUBSCRIBED, + 'opt_in_at' => new \DateTimeImmutable('2025-01-01'), ]); $response = $this->consoleApi( @@ -47,47 +40,19 @@ public function testUpdateList(): void 'PATCH', '/subscribers/' . $subscriber->getId(), [ - 'email' => 'new@email.com', - 'list_ids' => [$list1->getId(), $list2->getId()], 'status' => 'subscribed', ] ); $this->assertSame(200, $response->getStatusCode()); - $json = $this->getJson(); - $this->assertSame('new@email.com', $json['email']); $repository = $this->em->getRepository(Subscriber::class); - $subscriber = $repository->find($json['id']); + $subscriber = $repository->find($subscriber->getId()); $this->assertInstanceOf(Subscriber::class, $subscriber); - $this->assertSame('new@email.com', $subscriber->getEmail()); $this->assertSame('subscribed', $subscriber->getStatus()->value); - $this->assertCount(2, $subscriber->getLists()); - $this->assertContains($list1->_real(), $subscriber->getLists()); - $this->assertContains($list2->_real(), $subscriber->getLists()); $this->assertSame('2025-02-21 00:00:00', $subscriber->getUpdatedAt()->format('Y-m-d H:i:s')); } - public function testCannotUpdateSubscriberToEmptyList(): void - { - $this->validateInput( - fn(Newsletter $newsletter) => [ - 'email' => 'mybademail', - 'list_ids' => [], - ], - [ - [ - 'property' => 'email', - 'message' => 'This value is not a valid email address.', - ], - [ - 'property' => 'list_ids', - 'message' => 'There should be at least one list.', - ], - ] - ); - } - public function testValidatesStatus(): void { $this->validateInput( @@ -127,42 +92,14 @@ private function validateInput( $this->assertHasViolation($violations[0]['property'], $violations[0]['message']); } - public function testUpdateSubscriberInvalidListId(): void - { - $newsletter1 = NewsletterFactory::createOne(); - $newsletter2 = NewsletterFactory::createOne(); - - $newsletterList = NewsletterListFactory::createOne(['newsletter' => $newsletter2]); - $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter1]); - - $response = $this->consoleApi( - $newsletter1, - 'PATCH', - '/subscribers/' . $subscriber->getId(), - [ - 'list_ids' => [$newsletterList->getId()], - ] - ); - - $this->assertSame(422, $response->getStatusCode()); - $json = $this->getJson(); - - $this->assertSame( - 'List with id ' . $newsletterList->getId() . ' not found', - $json['message'] - ); - } - public function testCannotUpdateSubscriberOfOtherNewsletter(): void { $newsletter1 = NewsletterFactory::createOne(); $newsletter2 = NewsletterFactory::createOne(); - $newsletterList = NewsletterListFactory::createOne(['newsletter' => $newsletter1]); $subscriber = SubscriberFactory::createOne([ 'newsletter' => $newsletter1, 'email' => 'ishini@hyvor.com', - 'lists' => [$newsletterList], ]); $response = $this->consoleApi( @@ -207,10 +144,8 @@ public function testUpdateSubscriberWithTakenEmail(): void public function test_update_subscriber_metadata(): void { $newsletter = NewsletterFactory::createOne(); - $list1 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); - $list2 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); - $metadata = SubscriberMetadataDefinitionFactory::createOne([ + SubscriberMetadataDefinitionFactory::createOne([ 'key' => 'name', 'name' => 'Name', 'newsletter' => $newsletter, @@ -218,7 +153,6 @@ public function test_update_subscriber_metadata(): void $subscriber = SubscriberFactory::createOne([ 'newsletter' => $newsletter, - 'lists' => [$list1], 'status' => SubscriberStatus::UNSUBSCRIBED, ]); @@ -231,10 +165,7 @@ public function test_update_subscriber_metadata(): void 'PATCH', '/subscribers/' . $subscriber->getId(), [ - 'email' => 'new@email.com', - 'list_ids' => [$list1->getId(), $list2->getId()], - 'status' => 'subscribed', - 'metadata' => $metaUpdate + 'metadata' => $metaUpdate, ] ); @@ -248,32 +179,24 @@ public function test_update_subscriber_metadata(): void public function test_update_subscriber_metadata_invalid_name(): void { $newsletter = NewsletterFactory::createOne(); - $list1 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); - $list2 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); $subscriber = SubscriberFactory::createOne([ 'newsletter' => $newsletter, - 'lists' => [$list1], 'status' => SubscriberStatus::UNSUBSCRIBED, ]); - $metadata = SubscriberMetadataDefinitionFactory::createOne([ + SubscriberMetadataDefinitionFactory::createOne([ 'key' => 'age', 'name' => 'Age', 'newsletter' => $newsletter, ]); - - $metaUpdate = [ - 'name' => 'Thibault', - ]; - $response = $this->consoleApi( $newsletter, 'PATCH', '/subscribers/' . $subscriber->getId(), [ - 'metadata' => $metaUpdate, + 'metadata' => ['name' => 'Thibault'], ] ); @@ -285,7 +208,6 @@ public function test_update_subscriber_metadata_invalid_name(): void ); } - public function test_update_subscriber_metadata_invalid_type(): void { // TODO: Implement this test when other metadata types are implemented diff --git a/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte b/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte index e2a890ab..a87ca52f 100644 --- a/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte +++ b/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte @@ -294,6 +294,12 @@
  • POST /subscribers/bulk - Bulk update subscribers
    • +
  • + POST /subscribers/{'{id}'}/lists - Add a subscriber to a list +
    • +
  • + DELETE /subscribers/{'{id}'}/lists - Remove a subscriber from a list +

    • Objects:

    @@ -334,7 +340,6 @@ code={` type Request = { email: string; - list_ids: number[]; source?: 'console' | 'form' | 'import'; // default: 'console' subscribe_ip?: string | null; subscribed_at?: number | null; // unix timestamp @@ -353,7 +358,6 @@ code={` type Request = { email?: string; - list_ids?: number[]; status?: 'subscribed' | 'unsubscribed' | 'pending'; metadata?: Record; } @@ -394,6 +398,46 @@ `} /> +

    Add a subscriber to a list

    + +POST /subscribers/{'{id}'}/lists + + + +

    Remove a subscriber from a list

    + +DELETE /subscribers/{'{id}'}/lists + + +

    Subscriber Metadata

    From 68fb6ec3269473add806f03fb6036e3fe899b18c Mon Sep 17 00:00:00 2001 From: Supun Wimalasena Date: Thu, 26 Feb 2026 14:35:36 +0100 Subject: [PATCH 05/23] create subscriber endpoint updated --- .../Controller/SubscriberController.php | 20 ++-- .../Subscriber/Dto/UpdateSubscriberDto.php | 4 +- .../Service/Subscriber/SubscriberService.php | 8 ++ .../Subscriber/CreateSubscriberTest.php | 101 ++++++++++++++++++ 4 files changed, 125 insertions(+), 8 deletions(-) diff --git a/backend/src/Api/Console/Controller/SubscriberController.php b/backend/src/Api/Console/Controller/SubscriberController.php index 6a3a0e36..f8bfc4f3 100644 --- a/backend/src/Api/Console/Controller/SubscriberController.php +++ b/backend/src/Api/Console/Controller/SubscriberController.php @@ -106,20 +106,26 @@ public function createSubscriber( // update $updates = new UpdateSubscriberDto(); - if ($updates->has('status')) { - $updates->status = $input->status; + $updates->status = $input->status; + + if ($input->has('source')) { + $updates->source = $input->source; } - if ($updates->has('subscribe_ip')) { + if ($input->has('subscribe_ip')) { $updates->subscribeIp = $input->subscribe_ip; } - if ($updates->has('subscribed_at')) { - $updates->subscribedAt = $input->subscribed_at ? \DateTimeImmutable::createFromTimestamp($input->subscribed_at) : null; + if ($input->has('subscribed_at')) { + $updates->subscribedAt = $input->subscribed_at !== null + ? \DateTimeImmutable::createFromTimestamp($input->subscribed_at) + : null; } - if ($updates->has('unsubscribed_at')) { - $updates->unsubscribedAt = $input->unsubscribed_at ? \DateTimeImmutable::createFromTimestamp($input->unsubscribed_at) : null; + if ($input->has('unsubscribed_at')) { + $updates->unsubscribedAt = $input->unsubscribed_at !== null + ? \DateTimeImmutable::createFromTimestamp($input->unsubscribed_at) + : null; } $subscriber = $this->subscriberService->updateSubscriber($subscriber, $updates); diff --git a/backend/src/Service/Subscriber/Dto/UpdateSubscriberDto.php b/backend/src/Service/Subscriber/Dto/UpdateSubscriberDto.php index d42fd0f8..2b3fc895 100644 --- a/backend/src/Service/Subscriber/Dto/UpdateSubscriberDto.php +++ b/backend/src/Service/Subscriber/Dto/UpdateSubscriberDto.php @@ -3,6 +3,7 @@ namespace App\Service\Subscriber\Dto; use App\Entity\NewsletterList; +use App\Entity\Type\SubscriberSource; use App\Entity\Type\SubscriberStatus; use App\Util\OptionalPropertyTrait; @@ -14,13 +15,14 @@ class UpdateSubscriberDto public string $email; public SubscriberStatus $status; + public SubscriberSource $source; public ?string $subscribeIp; public ?\DateTimeImmutable $subscribedAt; public ?\DateTimeImmutable $optInAt; - public \DateTimeImmutable $unsubscribedAt; + public ?\DateTimeImmutable $unsubscribedAt; public ?string $unsubscribedReason; diff --git a/backend/src/Service/Subscriber/SubscriberService.php b/backend/src/Service/Subscriber/SubscriberService.php index cc301f55..089acdf2 100644 --- a/backend/src/Service/Subscriber/SubscriberService.php +++ b/backend/src/Service/Subscriber/SubscriberService.php @@ -166,6 +166,14 @@ public function updateSubscriber(Subscriber $subscriber, UpdateSubscriberDto $up $subscriber->setStatus($updates->status); } + if ($updates->has('source')) { + $subscriber->setSource($updates->source); + } + + if ($updates->has('subscribeIp')) { + $subscriber->setSubscribeIp($updates->subscribeIp); + } + if ($updates->has('subscribedAt')) { $subscriber->setSubscribedAt($updates->subscribedAt); } diff --git a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php index 83f1dc10..a215f2ce 100644 --- a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php +++ b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php @@ -220,6 +220,107 @@ private function validateInput( $this->assertHasViolation($violations[0]['property'], $violations[0]['message']); } + public function test_updates_if_exists(): void + { + $newsletter = NewsletterFactory::createOne(); + + $subscriber = SubscriberFactory::createOne([ + 'newsletter' => $newsletter, + 'email' => 'supun@hyvor.com', + 'status' => SubscriberStatus::UNSUBSCRIBED, + 'subscribe_ip' => '1.2.3.4', + ]); + + $subscribedAt = new \DateTimeImmutable('2024-01-01 00:00:00'); + + $response = $this->consoleApi( + $newsletter, + 'POST', + '/subscribers', + [ + 'email' => 'supun@hyvor.com', + 'if_exists' => 'update', + 'status' => 'pending', + 'subscribe_ip' => '79.255.1.1', + 'subscribed_at' => $subscribedAt->getTimestamp(), + 'source' => 'import' + ] + ); + + $this->assertSame(200, $response->getStatusCode()); + + $repository = $this->em->getRepository(Subscriber::class); + $updated = $repository->find($subscriber->getId()); + $this->assertInstanceOf(Subscriber::class, $updated); + $this->assertSame(SubscriberStatus::PENDING, $updated->getStatus()); + $this->assertSame('79.255.1.1', $updated->getSubscribeIp()); + $this->assertSame('2024-01-01 00:00:00', $updated->getSubscribedAt()?->format('Y-m-d H:i:s')); + $this->assertSame(SubscriberSource::IMPORT, $updated->getSource()); + } + + public function testCreateSubscriberIfExistsUpdateClearsTimestamps(): void + { + $newsletter = NewsletterFactory::createOne(); + + $subscriber = SubscriberFactory::createOne([ + 'newsletter' => $newsletter, + 'email' => 'supun@hyvor.com', + 'subscribed_at' => new \DateTimeImmutable('2024-01-01'), + 'unsubscribed_at' => new \DateTimeImmutable('2024-06-01'), + ]); + + $response = $this->consoleApi( + $newsletter, + 'POST', + '/subscribers', + [ + 'email' => 'supun@hyvor.com', + 'if_exists' => 'update', + 'subscribed_at' => null, + 'unsubscribed_at' => null, + ] + ); + + $this->assertSame(200, $response->getStatusCode()); + + $repository = $this->em->getRepository(Subscriber::class); + $updated = $repository->find($subscriber->getId()); + $this->assertInstanceOf(Subscriber::class, $updated); + $this->assertNull($updated->getSubscribedAt()); + $this->assertNull($updated->getUnsubscribedAt()); + } + + public function testCreateSubscriberIfExistsUpdateDoesNotChangeUnsentFields(): void + { + $newsletter = NewsletterFactory::createOne(); + + $subscriber = SubscriberFactory::createOne([ + 'newsletter' => $newsletter, + 'email' => 'supun@hyvor.com', + 'subscribe_ip' => '1.2.3.4', + 'subscribed_at' => new \DateTimeImmutable('2024-01-01'), + ]); + + $response = $this->consoleApi( + $newsletter, + 'POST', + '/subscribers', + [ + 'email' => 'supun@hyvor.com', + 'if_exists' => 'update', + // subscribe_ip and subscribed_at not sent + ] + ); + + $this->assertSame(200, $response->getStatusCode()); + + $repository = $this->em->getRepository(Subscriber::class); + $updated = $repository->find($subscriber->getId()); + $this->assertInstanceOf(Subscriber::class, $updated); + $this->assertSame('1.2.3.4', $updated->getSubscribeIp()); + $this->assertSame('2024-01-01 00:00:00', $updated->getSubscribedAt()?->format('Y-m-d H:i:s')); + } + public function testCreateSubscriberDuplicateEmail(): void { $newsletter = NewsletterFactory::createOne(); From bb195f199e1779357773d64df358fd063ab0d438 Mon Sep 17 00:00:00 2001 From: Supun Wimalasena Date: Thu, 26 Feb 2026 14:44:45 +0100 Subject: [PATCH 06/23] wip --- backend/config/packages/sentry.yaml | 2 +- backend/src/Entity/SubscriberListUnsubscribed.php | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/backend/config/packages/sentry.yaml b/backend/config/packages/sentry.yaml index 81ad7b5c..23a0dd9e 100644 --- a/backend/config/packages/sentry.yaml +++ b/backend/config/packages/sentry.yaml @@ -1,4 +1,4 @@ -when@dev: +when@prod: sentry: dsn: '%env(SENTRY_DSN)%' options: diff --git a/backend/src/Entity/SubscriberListUnsubscribed.php b/backend/src/Entity/SubscriberListUnsubscribed.php index eb4e78cc..efcf1015 100644 --- a/backend/src/Entity/SubscriberListUnsubscribed.php +++ b/backend/src/Entity/SubscriberListUnsubscribed.php @@ -30,6 +30,11 @@ public function getId(): int return $this->id; } + public function setId(int $id): void + { + $this->id = $id; + } + public function getList(): NewsletterList { return $this->list; From 17e762908e5cba801b81e96c7243d62a74dba572 Mon Sep 17 00:00:00 2001 From: Supun Wimalasena Date: Fri, 27 Feb 2026 12:37:25 +0100 Subject: [PATCH 07/23] wip --- .../Controller/SubscriberController.php | 18 ++++--- .../Public/Controller/Form/FormController.php | 37 +++++++------ backend/src/Command/Dev/DevSeedCommand.php | 2 +- .../Service/Subscriber/SubscriberService.php | 53 ++++++++++++++----- 4 files changed, 73 insertions(+), 37 deletions(-) diff --git a/backend/src/Api/Console/Controller/SubscriberController.php b/backend/src/Api/Console/Controller/SubscriberController.php index f8bfc4f3..5ccd9744 100644 --- a/backend/src/Api/Console/Controller/SubscriberController.php +++ b/backend/src/Api/Console/Controller/SubscriberController.php @@ -290,16 +290,18 @@ public function addSubscriberList( throw new UnprocessableEntityHttpException('List not found'); } - try { - $this->subscriberService->addSubscriberToList( - $subscriber, - $list, - $input->if_unsubscribed === SubscriberListIfUnsubscribed::ERROR - ); - } catch (\RuntimeException $e) { - throw new UnprocessableEntityHttpException($e->getMessage()); + if ( + $input->if_unsubscribed === SubscriberListIfUnsubscribed::ERROR && + $this->subscriberService->hasSubscriberUnsubscribedFromList($subscriber, $list) + ) { + throw new BadRequestHttpException('Subscriber was previously unsubscribed and can not be added to the list again'); } + $this->subscriberService->addSubscriberToList( + $subscriber, + $list, + ); + return $this->json(new SubscriberObject($subscriber)); } diff --git a/backend/src/Api/Public/Controller/Form/FormController.php b/backend/src/Api/Public/Controller/Form/FormController.php index 997d7d73..4c6cb44a 100644 --- a/backend/src/Api/Public/Controller/Form/FormController.php +++ b/backend/src/Api/Public/Controller/Form/FormController.php @@ -31,13 +31,11 @@ class FormController extends AbstractController use ClockAwareTrait; public function __construct( - private NewsletterService $newsletterService, + private NewsletterService $newsletterService, private NewsletterListService $newsletterListService, - private SubscriberService $subscriberService, - private AppConfig $appConfig, - ) - { - } + private SubscriberService $subscriberService, + private AppConfig $appConfig, + ) {} #[Route('/form/init', methods: 'POST')] public function init(#[MapRequestPayload] FormInitInput $input): JsonResponse @@ -53,7 +51,7 @@ public function init(#[MapRequestPayload] FormInitInput $input): JsonResponse if ($listIds !== null) { $missingListIds = $this->newsletterListService->getMissingListIdsOfNewsletter( $newsletter, - $listIds + $listIds, ); if ($missingListIds !== null) { throw new UnprocessableEntityHttpException("List with id {$missingListIds[0]} not found"); @@ -74,9 +72,8 @@ public function init(#[MapRequestPayload] FormInitInput $input): JsonResponse #[Route('/form/subscribe', methods: 'POST')] public function subscribe( #[MapRequestPayload] FormSubscribeInput $input, - Request $request, - ): JsonResponse - { + Request $request, + ): JsonResponse { $ip = $request->getClientIp(); $newsletter = $this->newsletterService->getNewsletterBySubdomain($input->newsletter_subdomain); @@ -87,7 +84,7 @@ public function subscribe( $listIds = $input->list_ids; $missingListIds = $this->newsletterListService->getMissingListIdsOfNewsletter( $newsletter, - $listIds + $listIds, ); if ($missingListIds !== null) { @@ -101,12 +98,22 @@ public function subscribe( if ($subscriber) { $update = new UpdateSubscriberDto(); - $update->status = $subscriber->getOptInAt() !== null ? SubscriberStatus::SUBSCRIBED : SubscriberStatus::PENDING; + + // if the user is already subscribed, we do not want to change the status + if ($subscriber->getStatus() !== SubscriberStatus::SUBSCRIBED) { + // if the user has previously opted-in + // we can directly set the status to subscribed + $update->status = + $subscriber->getOptInAt() !== null ? + SubscriberStatus::SUBSCRIBED : + SubscriberStatus::PENDING; + } + $update->lists = $lists; $this->subscriberService->updateSubscriber( $subscriber, - $update + $update, ); } else { $subscriber = $this->subscriberService->createSubscriber( @@ -115,7 +122,7 @@ public function subscribe( $lists, SubscriberStatus::PENDING, SubscriberSource::FORM, - $ip + $ip, ); } @@ -138,7 +145,7 @@ public function renderForm(Request $request): Response getSubdomain()} instance={$instance}> - HTML; + HTML; return new Response($response); } diff --git a/backend/src/Command/Dev/DevSeedCommand.php b/backend/src/Command/Dev/DevSeedCommand.php index 52a1ed4b..ff86be9a 100644 --- a/backend/src/Command/Dev/DevSeedCommand.php +++ b/backend/src/Command/Dev/DevSeedCommand.php @@ -28,7 +28,7 @@ * @codeCoverageIgnore */ #[AsCommand( - name: 'app:dev:seed', + name: 'dev:seed', description: 'Seeds the database with test data for development purposes.' )] class DevSeedCommand extends Command diff --git a/backend/src/Service/Subscriber/SubscriberService.php b/backend/src/Service/Subscriber/SubscriberService.php index 089acdf2..6fac24b6 100644 --- a/backend/src/Service/Subscriber/SubscriberService.php +++ b/backend/src/Service/Subscriber/SubscriberService.php @@ -19,7 +19,6 @@ use Doctrine\Common\Collections\ArrayCollection; use Doctrine\ORM\EntityManagerInterface; use Symfony\Component\Clock\ClockAwareTrait; -use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\Messenger\MessageBusInterface; class SubscriberService @@ -301,25 +300,43 @@ public function getExports(Newsletter $newsletter): array } /** - * @throws \RuntimeException if $checkUnsubscribed is true and the subscriber has a prior unsubscription record + * @param Subscriber $subscriber + * @param NewsletterList[] $lists */ - public function addSubscriberToList( - Subscriber $subscriber, - NewsletterList $list, - bool $checkUnsubscribed + public function setSubscriberLists( + Subscriber $subscriber, + array $lists ): void { - if ($checkUnsubscribed) { - $record = $this->em->getRepository(SubscriberListUnsubscribed::class)->findOneBy([ - 'list' => $list, - 'subscriber' => $subscriber, - ]); - if ($record !== null) { - throw new \RuntimeException('Subscriber has previously unsubscribed from this list'); + $listIds = array_map(fn(NewsletterList $list) => $list->getId(), $lists); + + // remove lists that are not in the new list + foreach ($subscriber->getLists() as $existingList) { + if (!in_array($existingList->getId(), $listIds)) { + $subscriber->removeList($existingList); } } + // add new lists + foreach ($lists as $list) { + if (!$subscriber->getLists()->contains($list)) { + $subscriber->addList($list); + } + } + + $subscriber->setUpdatedAt($this->now()); + + $this->em->persist($subscriber); + $this->em->flush(); + + } + + public function addSubscriberToList( + Subscriber $subscriber, + NewsletterList $list, + ): void + { $subscriber->addList($list); $subscriber->setUpdatedAt($this->now()); @@ -359,6 +376,16 @@ public function removeSubscriberFromList( } } + public function hasSubscriberUnsubscribedFromList(Subscriber $subscriber, NewsletterList $list): bool + { + $record = $this->em->getRepository(SubscriberListUnsubscribed::class)->findOneBy([ + 'list' => $list, + 'subscriber' => $subscriber, + ]); + + return $record !== null; + } + public function getSubscriberById(int $id): ?Subscriber { return $this->subscriberRepository->find($id); From 9a90b95d222f018291e87600836d644a3b7fe6b4 Mon Sep 17 00:00:00 2001 From: Supun Wimalasena Date: Mon, 2 Mar 2026 16:03:21 +0100 Subject: [PATCH 08/23] cleanup lists --- .../Subscriber/AddSubscriberListInput.php | 10 - .../Subscriber/CreateSubscriberIfExists.php | 10 - .../Subscriber/RemoveSubscriberListInput.php | 10 - .../Subscriber/RemoveSubscriberListReason.php | 9 - .../SubscriberListIfUnsubscribed.php | 9 - .../Subscriber/UpdateSubscriberInput.php | 23 -- .../Subscriber/AddSubscriberListTest.php | 201 ---------------- .../Subscriber/RemoveSubscriberListTest.php | 192 ---------------- .../Subscriber/UpdateSubscriberTest.php | 216 ------------------ 9 files changed, 680 deletions(-) delete mode 100644 backend/src/Api/Console/Input/Subscriber/AddSubscriberListInput.php delete mode 100644 backend/src/Api/Console/Input/Subscriber/CreateSubscriberIfExists.php delete mode 100644 backend/src/Api/Console/Input/Subscriber/RemoveSubscriberListInput.php delete mode 100644 backend/src/Api/Console/Input/Subscriber/RemoveSubscriberListReason.php delete mode 100644 backend/src/Api/Console/Input/Subscriber/SubscriberListIfUnsubscribed.php delete mode 100644 backend/src/Api/Console/Input/Subscriber/UpdateSubscriberInput.php delete mode 100644 backend/tests/Api/Console/Subscriber/AddSubscriberListTest.php delete mode 100644 backend/tests/Api/Console/Subscriber/RemoveSubscriberListTest.php delete mode 100644 backend/tests/Api/Console/Subscriber/UpdateSubscriberTest.php diff --git a/backend/src/Api/Console/Input/Subscriber/AddSubscriberListInput.php b/backend/src/Api/Console/Input/Subscriber/AddSubscriberListInput.php deleted file mode 100644 index db45af81..00000000 --- a/backend/src/Api/Console/Input/Subscriber/AddSubscriberListInput.php +++ /dev/null @@ -1,10 +0,0 @@ - - */ - public array $metadata; -} diff --git a/backend/tests/Api/Console/Subscriber/AddSubscriberListTest.php b/backend/tests/Api/Console/Subscriber/AddSubscriberListTest.php deleted file mode 100644 index 84e16ba2..00000000 --- a/backend/tests/Api/Console/Subscriber/AddSubscriberListTest.php +++ /dev/null @@ -1,201 +0,0 @@ - $newsletter]); - $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); - - $response = $this->consoleApi( - $newsletter, - 'POST', - '/subscribers/' . $subscriber->getId() . '/lists', - ['id' => $list->getId()] - ); - - $this->assertSame(200, $response->getStatusCode()); - - $subscriberDb = $this->em->getRepository(Subscriber::class)->find($subscriber->getId()); - $this->assertInstanceOf(Subscriber::class, $subscriberDb); - $this->assertCount(1, $subscriberDb->getLists()); - $this->assertSame($list->getId(), $subscriberDb->getLists()->first()->getId()); - } - - public function testAddSubscriberToListByName(): void - { - $newsletter = NewsletterFactory::createOne(); - $list = NewsletterListFactory::createOne(['newsletter' => $newsletter, 'name' => 'My List']); - $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); - - $response = $this->consoleApi( - $newsletter, - 'POST', - '/subscribers/' . $subscriber->getId() . '/lists', - ['name' => 'My List'] - ); - - $this->assertSame(200, $response->getStatusCode()); - - $subscriberDb = $this->em->getRepository(Subscriber::class)->find($subscriber->getId()); - $this->assertInstanceOf(Subscriber::class, $subscriberDb); - $this->assertCount(1, $subscriberDb->getLists()); - $this->assertSame($list->getId(), $subscriberDb->getLists()->first()->getId()); - } - - public function testAddSubscriberToListValidation(): void - { - $newsletter = NewsletterFactory::createOne(); - $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); - - $response = $this->consoleApi( - $newsletter, - 'POST', - '/subscribers/' . $subscriber->getId() . '/lists', - [] - ); - - $this->assertSame(422, $response->getStatusCode()); - $this->assertSame('Either id or name must be provided', $this->getJson()['message']); - } - - public function testAddSubscriberToListNotFound(): void - { - $newsletter = NewsletterFactory::createOne(); - $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); - - $response = $this->consoleApi( - $newsletter, - 'POST', - '/subscribers/' . $subscriber->getId() . '/lists', - ['id' => 999999] - ); - - $this->assertSame(422, $response->getStatusCode()); - $this->assertSame('List not found', $this->getJson()['message']); - } - - public function testAddSubscriberToListAlreadyInList(): void - { - $newsletter = NewsletterFactory::createOne(); - $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); - $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter, 'lists' => [$list]]); - - $response = $this->consoleApi( - $newsletter, - 'POST', - '/subscribers/' . $subscriber->getId() . '/lists', - ['id' => $list->getId()] - ); - - $this->assertSame(200, $response->getStatusCode()); - - // Still only in the list once (idempotent) - $subscriberDb = $this->em->getRepository(Subscriber::class)->find($subscriber->getId()); - $this->assertInstanceOf(Subscriber::class, $subscriberDb); - $this->assertCount(1, $subscriberDb->getLists()); - } - - public function testAddSubscriberToListIfUnsubscribedError(): void - { - $newsletter = NewsletterFactory::createOne(); - $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); - $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); - - SubscriberListUnsubscribedFactory::createOne([ - 'list' => $list, - 'subscriber' => $subscriber, - ]); - - $response = $this->consoleApi( - $newsletter, - 'POST', - '/subscribers/' . $subscriber->getId() . '/lists', - ['id' => $list->getId(), 'if_unsubscribed' => 'error'] - ); - - $this->assertSame(422, $response->getStatusCode()); - $this->assertSame( - 'Subscriber has previously unsubscribed from this list', - $this->getJson()['message'] - ); - } - - public function testAddSubscriberToListIfUnsubscribedForceCreate(): void - { - $newsletter = NewsletterFactory::createOne(); - $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); - $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); - - SubscriberListUnsubscribedFactory::createOne([ - 'list' => $list, - 'subscriber' => $subscriber, - ]); - - $response = $this->consoleApi( - $newsletter, - 'POST', - '/subscribers/' . $subscriber->getId() . '/lists', - ['id' => $list->getId(), 'if_unsubscribed' => 'force_create'] - ); - - $this->assertSame(200, $response->getStatusCode()); - - $subscriberDb = $this->em->getRepository(Subscriber::class)->find($subscriber->getId()); - $this->assertInstanceOf(Subscriber::class, $subscriberDb); - $this->assertCount(1, $subscriberDb->getLists()); - } - - public function testCannotAddSubscriberOfOtherNewsletter(): void - { - $newsletter1 = NewsletterFactory::createOne(); - $newsletter2 = NewsletterFactory::createOne(); - $list = NewsletterListFactory::createOne(['newsletter' => $newsletter1]); - $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter1]); - - $response = $this->consoleApi( - $newsletter2, - 'POST', - '/subscribers/' . $subscriber->getId() . '/lists', - ['id' => $list->getId()] - ); - - $this->assertSame(403, $response->getStatusCode()); - $this->assertSame('Entity does not belong to the newsletter', $this->getJson()['message']); - } - - public function testCannotAddListOfOtherNewsletter(): void - { - $newsletter1 = NewsletterFactory::createOne(); - $newsletter2 = NewsletterFactory::createOne(); - $listOfNewsletter2 = NewsletterListFactory::createOne(['newsletter' => $newsletter2]); - $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter1]); - - $response = $this->consoleApi( - $newsletter1, - 'POST', - '/subscribers/' . $subscriber->getId() . '/lists', - ['id' => $listOfNewsletter2->getId()] - ); - - $this->assertSame(422, $response->getStatusCode()); - $this->assertSame('List not found', $this->getJson()['message']); - } -} diff --git a/backend/tests/Api/Console/Subscriber/RemoveSubscriberListTest.php b/backend/tests/Api/Console/Subscriber/RemoveSubscriberListTest.php deleted file mode 100644 index d19db0d1..00000000 --- a/backend/tests/Api/Console/Subscriber/RemoveSubscriberListTest.php +++ /dev/null @@ -1,192 +0,0 @@ - $newsletter]); - $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter, 'lists' => [$list]]); - - $response = $this->consoleApi( - $newsletter, - 'DELETE', - '/subscribers/' . $subscriber->getId() . '/lists', - ['id' => $list->getId()] - ); - - $this->assertSame(200, $response->getStatusCode()); - - $this->em->clear(); - $subscriberDb = $this->em->getRepository(Subscriber::class)->find($subscriber->getId()); - $this->assertInstanceOf(Subscriber::class, $subscriberDb); - $this->assertCount(0, $subscriberDb->getLists()); - } - - public function testRemoveSubscriberFromListByName(): void - { - $newsletter = NewsletterFactory::createOne(); - $list = NewsletterListFactory::createOne(['newsletter' => $newsletter, 'name' => 'Remove Me']); - $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter, 'lists' => [$list]]); - - $response = $this->consoleApi( - $newsletter, - 'DELETE', - '/subscribers/' . $subscriber->getId() . '/lists', - ['name' => 'Remove Me'] - ); - - $this->assertSame(200, $response->getStatusCode()); - - $this->em->clear(); - $subscriberDb = $this->em->getRepository(Subscriber::class)->find($subscriber->getId()); - $this->assertInstanceOf(Subscriber::class, $subscriberDb); - $this->assertCount(0, $subscriberDb->getLists()); - } - - public function testRemoveSubscriberFromListValidation(): void - { - $newsletter = NewsletterFactory::createOne(); - $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); - - $response = $this->consoleApi( - $newsletter, - 'DELETE', - '/subscribers/' . $subscriber->getId() . '/lists', - [] - ); - - $this->assertSame(422, $response->getStatusCode()); - $this->assertSame('Either id or name must be provided', $this->getJson()['message']); - } - - public function testRemoveSubscriberFromListNotFound(): void - { - $newsletter = NewsletterFactory::createOne(); - $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); - - $response = $this->consoleApi( - $newsletter, - 'DELETE', - '/subscribers/' . $subscriber->getId() . '/lists', - ['id' => 999999] - ); - - $this->assertSame(422, $response->getStatusCode()); - $this->assertSame('List not found', $this->getJson()['message']); - } - - public function testRemoveSubscriberFromListNotInList(): void - { - $newsletter = NewsletterFactory::createOne(); - $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); - $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); - - $response = $this->consoleApi( - $newsletter, - 'DELETE', - '/subscribers/' . $subscriber->getId() . '/lists', - ['id' => $list->getId()] - ); - - $this->assertSame(200, $response->getStatusCode()); - } - - public function testRemoveSubscriberFromListWithReasonUnsubscribe(): void - { - $newsletter = NewsletterFactory::createOne(); - $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); - $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter, 'lists' => [$list]]); - - $response = $this->consoleApi( - $newsletter, - 'DELETE', - '/subscribers/' . $subscriber->getId() . '/lists', - ['id' => $list->getId(), 'reason' => 'unsubscribe'] - ); - - $this->assertSame(200, $response->getStatusCode()); - - $record = $this->em->getRepository(SubscriberListUnsubscribed::class)->findOneBy([ - 'list' => $list->_real(), - 'subscriber' => $subscriber->_real(), - ]); - - $this->assertInstanceOf(SubscriberListUnsubscribed::class, $record); - } - - public function testRemoveSubscriberFromListWithReasonOther(): void - { - $newsletter = NewsletterFactory::createOne(); - $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); - $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter, 'lists' => [$list]]); - - $response = $this->consoleApi( - $newsletter, - 'DELETE', - '/subscribers/' . $subscriber->getId() . '/lists', - ['id' => $list->getId(), 'reason' => 'other'] - ); - - $this->assertSame(200, $response->getStatusCode()); - - $record = $this->em->getRepository(SubscriberListUnsubscribed::class)->findOneBy([ - 'list' => $list->_real(), - 'subscriber' => $subscriber->_real(), - ]); - - $this->assertNull($record); - } - - public function testCannotRemoveSubscriberOfOtherNewsletter(): void - { - $newsletter1 = NewsletterFactory::createOne(); - $newsletter2 = NewsletterFactory::createOne(); - $list = NewsletterListFactory::createOne(['newsletter' => $newsletter1]); - $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter1, 'lists' => [$list]]); - - $response = $this->consoleApi( - $newsletter2, - 'DELETE', - '/subscribers/' . $subscriber->getId() . '/lists', - ['id' => $list->getId()] - ); - - $this->assertSame(403, $response->getStatusCode()); - $this->assertSame('Entity does not belong to the newsletter', $this->getJson()['message']); - } - - public function testCannotRemoveListOfOtherNewsletter(): void - { - $newsletter1 = NewsletterFactory::createOne(); - $newsletter2 = NewsletterFactory::createOne(); - $listOfNewsletter2 = NewsletterListFactory::createOne(['newsletter' => $newsletter2]); - $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter1]); - - $response = $this->consoleApi( - $newsletter1, - 'DELETE', - '/subscribers/' . $subscriber->getId() . '/lists', - ['id' => $listOfNewsletter2->getId()] - ); - - $this->assertSame(422, $response->getStatusCode()); - $this->assertSame('List not found', $this->getJson()['message']); - } -} diff --git a/backend/tests/Api/Console/Subscriber/UpdateSubscriberTest.php b/backend/tests/Api/Console/Subscriber/UpdateSubscriberTest.php deleted file mode 100644 index f6053e8b..00000000 --- a/backend/tests/Api/Console/Subscriber/UpdateSubscriberTest.php +++ /dev/null @@ -1,216 +0,0 @@ - $newsletter, - 'status' => SubscriberStatus::UNSUBSCRIBED, - 'opt_in_at' => new \DateTimeImmutable('2025-01-01'), - ]); - - $response = $this->consoleApi( - $newsletter, - 'PATCH', - '/subscribers/' . $subscriber->getId(), - [ - 'status' => 'subscribed', - ] - ); - - $this->assertSame(200, $response->getStatusCode()); - - $repository = $this->em->getRepository(Subscriber::class); - $subscriber = $repository->find($subscriber->getId()); - $this->assertInstanceOf(Subscriber::class, $subscriber); - $this->assertSame('subscribed', $subscriber->getStatus()->value); - $this->assertSame('2025-02-21 00:00:00', $subscriber->getUpdatedAt()->format('Y-m-d H:i:s')); - } - - public function testValidatesStatus(): void - { - $this->validateInput( - fn(Newsletter $newsletter) => [ - 'status' => 'invalid', - ], - [ - [ - 'property' => 'status', - 'message' => 'This value should be of type int|string.', - ], - ] - ); - } - - /** - * @param callable(Newsletter): array $input - * @param array $violations - * @return void - */ - private function validateInput( - callable $input, - array $violations - ): void - { - $newsletter = NewsletterFactory::createOne(); - $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); - - $response = $this->consoleApi( - $newsletter, - 'PATCH', - '/subscribers/' . $subscriber->getId(), - $input($newsletter), - ); - - $this->assertSame(422, $response->getStatusCode()); - $this->assertHasViolation($violations[0]['property'], $violations[0]['message']); - } - - public function testCannotUpdateSubscriberOfOtherNewsletter(): void - { - $newsletter1 = NewsletterFactory::createOne(); - $newsletter2 = NewsletterFactory::createOne(); - - $subscriber = SubscriberFactory::createOne([ - 'newsletter' => $newsletter1, - 'email' => 'ishini@hyvor.com', - ]); - - $response = $this->consoleApi( - $newsletter2, - 'PATCH', - '/subscribers/' . $subscriber->getId(), - [ - 'email' => 'supun@hyvor.com', - ] - ); - - $this->assertSame(403, $response->getStatusCode()); - $this->assertSame('Entity does not belong to the newsletter', $this->getJson()['message']); - - $repository = $this->em->getRepository(Subscriber::class); - $subscriber = $repository->find($subscriber->getId()); - $this->assertSame('ishini@hyvor.com', $subscriber?->getEmail()); - } - - public function testUpdateSubscriberWithTakenEmail(): void - { - $newsletter = NewsletterFactory::createOne(); - $subscriber1 = SubscriberFactory::createOne(['newsletter' => $newsletter, 'email' => 'thibault@hyvor.com']); - $subscriber2 = SubscriberFactory::createOne(['newsletter' => $newsletter, 'email' => 'supun@hyvor.com']); - - $response = $this->consoleApi( - $newsletter, - 'PATCH', - '/subscribers/' . $subscriber1->getId(), - [ - 'email' => 'supun@hyvor.com', - ] - ); - - $this->assertSame(422, $response->getStatusCode()); - $this->assertSame( - 'Subscriber with email ' . $subscriber2->getEmail() . ' already exists', - $this->getJson()['message'] - ); - } - - public function test_update_subscriber_metadata(): void - { - $newsletter = NewsletterFactory::createOne(); - - SubscriberMetadataDefinitionFactory::createOne([ - 'key' => 'name', - 'name' => 'Name', - 'newsletter' => $newsletter, - ]); - - $subscriber = SubscriberFactory::createOne([ - 'newsletter' => $newsletter, - 'status' => SubscriberStatus::UNSUBSCRIBED, - ]); - - $metaUpdate = [ - 'name' => 'Thibault', - ]; - - $response = $this->consoleApi( - $newsletter, - 'PATCH', - '/subscribers/' . $subscriber->getId(), - [ - 'metadata' => $metaUpdate, - ] - ); - - $this->assertSame(200, $response->getStatusCode()); - - $subscriber = $this->em->getRepository(Subscriber::class)->find($subscriber->getId()); - $this->assertInstanceOf(Subscriber::class, $subscriber); - $this->assertSame($subscriber->getMetadata(), $metaUpdate); - } - - public function test_update_subscriber_metadata_invalid_name(): void - { - $newsletter = NewsletterFactory::createOne(); - - $subscriber = SubscriberFactory::createOne([ - 'newsletter' => $newsletter, - 'status' => SubscriberStatus::UNSUBSCRIBED, - ]); - - SubscriberMetadataDefinitionFactory::createOne([ - 'key' => 'age', - 'name' => 'Age', - 'newsletter' => $newsletter, - ]); - - $response = $this->consoleApi( - $newsletter, - 'PATCH', - '/subscribers/' . $subscriber->getId(), - [ - 'metadata' => ['name' => 'Thibault'], - ] - ); - - $this->assertSame(422, $response->getStatusCode()); - $json = $this->getJson(); - $this->assertSame( - 'Metadata definition with key name not found', - $json['message'] - ); - } - - public function test_update_subscriber_metadata_invalid_type(): void - { - // TODO: Implement this test when other metadata types are implemented - $this->markTestSkipped(); - } -} From 0052ec7ea13533fb03b6f444df761fb1ec24b54f Mon Sep 17 00:00:00 2001 From: Supun Wimalasena Date: Mon, 2 Mar 2026 16:03:37 +0100 Subject: [PATCH 09/23] API docs --- .../docs/[...slug]/content/ConsoleApi.svelte | 144 ++++++++++-------- 1 file changed, 81 insertions(+), 63 deletions(-) diff --git a/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte b/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte index a87ca52f..01971edc 100644 --- a/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte +++ b/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte @@ -284,9 +284,8 @@

    Objects:

    @@ -331,7 +324,7 @@ `} /> -

    Create a subscriber

    +

    Create or update a subscriber

    POST /subscribers @@ -339,32 +332,97 @@ language="ts" code={` type Request = { + // If a subscriber with the given email already exists, it will be updated. + // Otherwise, a new subscriber will be created. email: string; - source?: 'console' | 'form' | 'import'; // default: 'console' + + // The lists that the subscriber has subscribed to + // Send an array of list IDs (number) or names (string) + lists: (number | string)[]; + + // The subscriber's subscription status + // set \`send_pending_confirmation_email=true\` to send a confirmation email + // default: pending + status?: 'pending' | 'subscribed' | 'unsubscribed'; + + // the source of the subscriber (default: 'console') + source?: 'console' | 'form' | 'import'; + + // subscriber's IP address subscribe_ip?: string | null; + + // unix timestamp of when the subscriber opted in + // if not set, it will be set to the current time if status is 'subscribed' subscribed_at?: number | null; // unix timestamp + + // unix timestamp of when the subscriber unsubscribed + // if not set, it will be set to the current time if status is 'unsubscribed' unsubscribed_at?: number | null; // unix timestamp - } - type Response = Subscriber - `} -/> -

    Update a subscriber

    + // additional metadata for the subscriber + // keys must be defined in the Subscriber Metadata Definitions section (or using the API) + metadata?: Record; -PATCH /subscribers/{'{id}'} + // ============ SETTINGS =========== + // change how the endpoint behaves -; + // define how to handle the case when the subscriber has + // previously unsubscribed from a list that is provided + // see below for more info + // default: 'ignore' + list_add_strategy_if_unsubscribed: 'ignore' | 'force_add'; + + // define the reason for removing the subscriber from a list + // see below + // default: 'unsubscribe' + list_remove_reason: 'unsubscribe' | 'other'; + + // whether to send a confirmation email when adding a subscriber with 'pending' status + // or when changing an existing subscriber's status to 'pending'. + // default: false + send_pending_confirmation_email?: boolean; } type Response = Subscriber `} /> +
    Managing list unsubscriptions and re-subscriptions
    + +

    + list_add_strategy_if_unsubscribed: +

    + +
      +
    • + ignore - use this strategy for most auto-subscribing cases (e.g. automatically subscribing + a user to a list when they start a trial). This makes sures that if the user has previously unsubscribed + from the list, they will not be re-subscribed. +
      • +
    • + force_add - use this strategy if the user is explicitly asking to subscribe to the + list again (e.g. they checked a checkbox to subscribe to the newsletter). This will add the subscriber + to the list even if they have previously unsubscribed. +
      • +
    + +

    + list_remove_reason: +

    + +
      +
    • + unsubscribe - use this reason if the subscriber is explicitly asking to be + removed from the list (e.g. they unchecked a checkbox to unsubscribe). This will record an + unsubscription, blocking future re-adds unless + list_add_strategy_if_unsubscribed=force_add. Hyvor Post's default unsubscribe + form uses this. +
      • +
    • + other - use this reason if you want to remove the subscriber from the list without + recording an unsubscription. +
      • +
    +

    Delete a subscriber

    DELETE /subscribers/{'{id}'} @@ -398,46 +456,6 @@ `} /> -

    Add a subscriber to a list

    - -POST /subscribers/{'{id}'}/lists - - - -

    Remove a subscriber from a list

    - -DELETE /subscribers/{'{id}'}/lists - - -

    Subscriber Metadata

    From 9a43ad471d45b95e4876b647aca752b8580159af Mon Sep 17 00:00:00 2001 From: Supun Wimalasena Date: Tue, 3 Mar 2026 09:33:19 +0100 Subject: [PATCH 10/23] wip endpoint --- .../Controller/SubscriberController.php | 211 +++----- .../Subscriber/CreateSubscriberInput.php | 45 +- .../ListAddStrategyIfUnsubscribed.php | 9 + .../Input/Subscriber/ListRemoveReason.php | 9 + .../Input/Subscriber/MetadataStrategy.php | 9 + .../App/Messenger/MessageTransport.php | 8 + .../Event/SubscriberCreatedEvent.php | 25 + .../Service/Subscriber/SubscriberService.php | 106 ++-- .../Subscriber/CreateSubscriberTest.php | 495 ++++++++++++------ .../docs/[...slug]/content/ConsoleApi.svelte | 138 ++++- 10 files changed, 676 insertions(+), 379 deletions(-) create mode 100644 backend/src/Api/Console/Input/Subscriber/ListAddStrategyIfUnsubscribed.php create mode 100644 backend/src/Api/Console/Input/Subscriber/ListRemoveReason.php create mode 100644 backend/src/Api/Console/Input/Subscriber/MetadataStrategy.php create mode 100644 backend/src/Service/App/Messenger/MessageTransport.php create mode 100644 backend/src/Service/Subscriber/Event/SubscriberCreatedEvent.php diff --git a/backend/src/Api/Console/Controller/SubscriberController.php b/backend/src/Api/Console/Controller/SubscriberController.php index 5ccd9744..515ab276 100644 --- a/backend/src/Api/Console/Controller/SubscriberController.php +++ b/backend/src/Api/Console/Controller/SubscriberController.php @@ -4,14 +4,10 @@ use App\Api\Console\Authorization\Scope; use App\Api\Console\Authorization\ScopeRequired; -use App\Api\Console\Input\Subscriber\AddSubscriberListInput; use App\Api\Console\Input\Subscriber\BulkActionSubscriberInput; -use App\Api\Console\Input\Subscriber\CreateSubscriberIfExists; use App\Api\Console\Input\Subscriber\CreateSubscriberInput; -use App\Api\Console\Input\Subscriber\RemoveSubscriberListInput; -use App\Api\Console\Input\Subscriber\RemoveSubscriberListReason; -use App\Api\Console\Input\Subscriber\SubscriberListIfUnsubscribed; -use App\Api\Console\Input\Subscriber\UpdateSubscriberInput; +use App\Api\Console\Input\Subscriber\ListAddStrategyIfUnsubscribed; +use App\Api\Console\Input\Subscriber\ListRemoveReason; use App\Api\Console\Object\SubscriberObject; use App\Entity\Newsletter; use App\Entity\Subscriber; @@ -19,6 +15,7 @@ use App\Entity\Type\SubscriberStatus; use App\Service\NewsletterList\NewsletterListService; use App\Service\Subscriber\Dto\UpdateSubscriberDto; +use App\Service\Subscriber\Message\SubscriberCreatedMessage; use App\Service\Subscriber\SubscriberService; use App\Service\SubscriberMetadata\SubscriberMetadataService; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; @@ -27,18 +24,18 @@ use Symfony\Component\HttpKernel\Attribute\MapRequestPayload; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; use Symfony\Component\HttpKernel\Exception\UnprocessableEntityHttpException; +use Symfony\Component\Messenger\MessageBusInterface; use Symfony\Component\Routing\Attribute\Route; class SubscriberController extends AbstractController { public function __construct( - private SubscriberService $subscriberService, - private NewsletterListService $newsletterListService, + private SubscriberService $subscriberService, + private NewsletterListService $newsletterListService, private SubscriberMetadataService $subscriberMetadataService, - ) - { - } + private MessageBusInterface $messageBus, + ) {} #[Route('/subscribers', methods: 'GET')] #[ScopeRequired(Scope::SUBSCRIBERS_READ)] @@ -70,7 +67,7 @@ public function getSubscribers(Request $request, Newsletter $newsletter): JsonRe $listId, $search, $limit, - $offset + $offset, ) ->map(fn($subscriber) => new SubscriberObject($subscriber)); @@ -81,29 +78,36 @@ public function getSubscribers(Request $request, Newsletter $newsletter): JsonRe #[ScopeRequired(Scope::SUBSCRIBERS_WRITE)] public function createSubscriber( #[MapRequestPayload] CreateSubscriberInput $input, - Newsletter $newsletter - ): JsonResponse - { + Newsletter $newsletter, + ): JsonResponse { + // Resolve lists + $resolvedLists = []; + foreach ($input->lists as $listIdOrName) { + $id = is_int($listIdOrName) ? $listIdOrName : null; + $name = is_string($listIdOrName) ? $listIdOrName : null; + $list = $this->newsletterListService->getListByIdOrName($newsletter, $id, $name); + if ($list === null) { + throw new UnprocessableEntityHttpException("List not found: {$listIdOrName}"); + } + $resolvedLists[] = $list; + } $subscriber = $this->subscriberService->getSubscriberByEmail($newsletter, $input->email); if ($subscriber === null) { - - // create subscriber $subscriber = $this->subscriberService->createSubscriber( $newsletter, $input->email, - [], - SubscriberStatus::PENDING, + $resolvedLists, + $input->status, source: $input->source ?? SubscriberSource::CONSOLE, - subscribeIp: $input->subscribe_ip ?? null, - subscribedAt: $input->has('subscribed_at') ? \DateTimeImmutable::createFromTimestamp($input->subscribed_at) : null, - unsubscribedAt: $input->has('unsubscribed_at') ? \DateTimeImmutable::createFromTimestamp($input->unsubscribed_at) : null, + subscribeIp: $input->has('subscribe_ip') ? $input->subscribe_ip : null, + subscribedAt: $input->getSubscribedAt(), + unsubscribedAt: $input->getUnsubscribedAt(), + sendConfirmationEmail: $input->send_pending_confirmation_email, ); - - } elseif ($input->if_exists === CreateSubscriberIfExists::UPDATE) { - - // update + } else { + // Update existing subscriber with provided fields $updates = new UpdateSubscriberDto(); $updates->status = $input->status; @@ -128,54 +132,41 @@ public function createSubscriber( : null; } - $subscriber = $this->subscriberService->updateSubscriber($subscriber, $updates); - - } else { - throw new UnprocessableEntityHttpException("Subscriber with email {$input->email} already exists"); - } - - return $this->json(new SubscriberObject($subscriber)); - } - - #[Route('/subscribers/{id}', methods: 'PATCH')] - #[ScopeRequired(Scope::SUBSCRIBERS_WRITE)] - public function updateSubscriber( - Subscriber $subscriber, - Newsletter $newsletter, - #[MapRequestPayload] UpdateSubscriberInput $input - ): JsonResponse - { - $updates = new UpdateSubscriberDto(); - - if ($input->has('email')) { - $subscriberDB = $this->subscriberService->getSubscriberByEmail($newsletter, $input->email); - if ($subscriberDB !== null) { - throw new UnprocessableEntityHttpException("Subscriber with email {$input->email} already exists"); - } - - $updates->email = $input->email; - } - - if ($input->has('status')) { - if ($input->status === SubscriberStatus::SUBSCRIBED && $subscriber->getOptInAt() === null) { - throw new UnprocessableEntityHttpException('Subscribers without opt-in can not be updated to SUBSCRIBED status.'); + if ($input->has('metadata')) { + $updates->metadata = $input->metadata; } - $updates->status = $input->status; + $subscriber = $this->subscriberService->updateSubscriber($subscriber, $updates); } - $metadataDefinitions = $this->subscriberMetadataService->getMetadataDefinitions($newsletter); +// // Sync lists +// $resolvedListIds = array_map(fn($l) => $l->getId(), $resolvedLists); +// $currentListIds = $subscriber->getLists()->map(fn($l) => $l->getId())->toArray(); +// +// // Add new lists +// foreach ($resolvedLists as $list) { +// if (!in_array($list->getId(), $currentListIds)) { +// if ( +// $input->list_add_strategy_if_unsubscribed === ListAddStrategyIfUnsubscribed::IGNORE && +// $this->subscriberService->hasSubscriberUnsubscribedFromList($subscriber, $list) +// ) { +// continue; +// } +// $this->subscriberService->addSubscriberToList($subscriber, $list); +// } +// } +// +// // Remove lists no longer in the resolved set +// foreach ($subscriber->getLists()->toArray() as $existingList) { +// if (!in_array($existingList->getId(), $resolvedListIds)) { +// $this->subscriberService->removeSubscriberFromList( +// $subscriber, +// $existingList, +// $input->list_remove_reason === ListRemoveReason::UNSUBSCRIBE, +// ); +// } +// } - if ($input->has('metadata')) { - try { - $this->subscriberMetadataService->validateMetadata($newsletter, $input->metadata); - } catch (\Exception $e) { - throw new UnprocessableEntityHttpException($e->getMessage()); - } - $updates->metadata = $input->metadata; - } - - $subscriber = $this->subscriberService->updateSubscriber($subscriber, $updates); return $this->json(new SubscriberObject($subscriber)); } @@ -189,8 +180,10 @@ public function deleteSubscriber(Subscriber $subscriber): JsonResponse #[Route('/subscribers/bulk', methods: 'POST')] #[ScopeRequired(Scope::SUBSCRIBERS_WRITE)] - public function bulkActions(Newsletter $newsletter, #[MapRequestPayload] BulkActionSubscriberInput $input): JsonResponse - { + public function bulkActions( + Newsletter $newsletter, + #[MapRequestPayload] BulkActionSubscriberInput $input, + ): JsonResponse { if (count($input->subscribers_ids) >= $this->subscriberService::BULK_SUBSCRIBER_LIMIT) { throw new UnprocessableEntityHttpException("Subscribers limit exceeded"); } @@ -202,7 +195,9 @@ public function bulkActions(Newsletter $newsletter, #[MapRequestPayload] BulkAct $subscriber = array_find($currentSubscribers, fn($s) => $s->getId() === $subscriberId); if ($subscriber === null) { - throw new UnprocessableEntityHttpException("Subscriber with ID {$subscriberId} not found in the newsletter"); + throw new UnprocessableEntityHttpException( + "Subscriber with ID {$subscriberId} not found in the newsletter", + ); } $subscribers[] = $subscriber; @@ -213,13 +208,14 @@ public function bulkActions(Newsletter $newsletter, #[MapRequestPayload] BulkAct return $this->json([ 'status' => 'success', 'message' => 'Subscribers deleted successfully', - 'subscribers' => [] + 'subscribers' => [], ]); } if ($input->action == 'status_change') { - if ($input->status == null) + if ($input->status == null) { throw new UnprocessableEntityHttpException("Status must be provided for status change action"); + } $status = SubscriberStatus::tryFrom($input->status); if (!$status) { @@ -241,13 +237,14 @@ public function bulkActions(Newsletter $newsletter, #[MapRequestPayload] BulkAct return $this->json([ 'status' => 'success', 'message' => 'Subscribers status updated successfully', - 'subscribers' => array_map(fn($s) => new SubscriberObject($s), $subscribers) + 'subscribers' => array_map(fn($s) => new SubscriberObject($s), $subscribers), ]); } if ($input->action == 'metadata_update') { - if ($input->metadata == null) + if ($input->metadata == null) { throw new UnprocessableEntityHttpException("Metadata must be provided for metadata update action"); + } foreach ($subscribers as $subscriber) { $updates = new UpdateSubscriberDto(); @@ -265,70 +262,10 @@ public function bulkActions(Newsletter $newsletter, #[MapRequestPayload] BulkAct return $this->json([ 'status' => 'success', 'message' => 'Subscribers metadata updated successfully', - 'subscribers' => array_map(fn($s) => new SubscriberObject($s), $subscribers) + 'subscribers' => array_map(fn($s) => new SubscriberObject($s), $subscribers), ]); } throw new BadRequestHttpException("Unhandled action"); } - - #[Route('/subscribers/{id}/lists', methods: 'POST')] - #[ScopeRequired(Scope::SUBSCRIBERS_WRITE)] - public function addSubscriberList( - Subscriber $subscriber, - Newsletter $newsletter, - #[MapRequestPayload] AddSubscriberListInput $input - ): JsonResponse - { - if ($input->id === null && $input->name === null) { - throw new UnprocessableEntityHttpException('Either id or name must be provided'); - } - - $list = $this->newsletterListService->getListByIdOrName($newsletter, $input->id, $input->name); - - if ($list === null) { - throw new UnprocessableEntityHttpException('List not found'); - } - - if ( - $input->if_unsubscribed === SubscriberListIfUnsubscribed::ERROR && - $this->subscriberService->hasSubscriberUnsubscribedFromList($subscriber, $list) - ) { - throw new BadRequestHttpException('Subscriber was previously unsubscribed and can not be added to the list again'); - } - - $this->subscriberService->addSubscriberToList( - $subscriber, - $list, - ); - - return $this->json(new SubscriberObject($subscriber)); - } - - #[Route('/subscribers/{id}/lists', methods: 'DELETE')] - #[ScopeRequired(Scope::SUBSCRIBERS_WRITE)] - public function removeSubscriberList( - Subscriber $subscriber, - Newsletter $newsletter, - #[MapRequestPayload] RemoveSubscriberListInput $input - ): JsonResponse - { - if ($input->id === null && $input->name === null) { - throw new UnprocessableEntityHttpException('Either id or name must be provided'); - } - - $list = $this->newsletterListService->getListByIdOrName($newsletter, $input->id, $input->name); - - if ($list === null) { - throw new UnprocessableEntityHttpException('List not found'); - } - - $this->subscriberService->removeSubscriberFromList( - $subscriber, - $list, - $input->reason === RemoveSubscriberListReason::UNSUBSCRIBE - ); - - return $this->json(new SubscriberObject($subscriber)); - } } diff --git a/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php b/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php index 3ca8ba40..ce36f698 100644 --- a/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php +++ b/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php @@ -5,12 +5,14 @@ use App\Entity\Type\SubscriberSource; use App\Entity\Type\SubscriberStatus; use App\Util\OptionalPropertyTrait; +use Symfony\Component\Clock\ClockAwareTrait; use Symfony\Component\Validator\Constraints as Assert; class CreateSubscriberInput { use OptionalPropertyTrait; + use ClockAwareTrait; #[Assert\NotBlank] #[Assert\Email] @@ -19,15 +21,48 @@ class CreateSubscriberInput public SubscriberStatus $status = SubscriberStatus::PENDING; - public ?SubscriberSource $source; + public ?SubscriberSource $source = null; #[Assert\Ip(version: Assert\Ip::ALL_ONLY_PUBLIC)] - public ?string $subscribe_ip; + private ?string $subscribe_ip; - public ?int $subscribed_at; + private ?int $subscribed_at; - public ?int $unsubscribed_at; + private ?int $unsubscribed_at; - public CreateSubscriberIfExists $if_exists = CreateSubscriberIfExists::ERROR; + /** + * @var ?(int|string)[] + */ + public ?array $lists = null; + + /** + * @var array|null + */ + public ?array $metadata = null; + + public ListAddStrategyIfUnsubscribed $list_add_strategy_if_unsubscribed = ListAddStrategyIfUnsubscribed::IGNORE; + + public ListRemoveReason $list_remove_reason = ListRemoveReason::UNSUBSCRIBE; + + public MetadataStrategy $metadata_strategy = MetadataStrategy::MERGE; + + public bool $send_pending_confirmation_email = false; + + public function getSubscriberIp(): ?string + { + return $this->has('subscribe_ip') ? $this->subscribe_ip : null; + } + + public function getSubscribedAt(): ?\DateTimeImmutable + { + $subscribedAt = $this->has('subscribed_at') ? $this->subscribed_at : null; + return $subscribedAt ? new \DateTimeImmutable()->setTimestamp($this->subscribed_at) : null; + } + + public function getUnsubscribedAt(): ?\DateTimeImmutable + { + $unsubscribedAt = $this->has('unsubscribed_at') ? $this->unsubscribed_at : null; + return $unsubscribedAt ? new \DateTimeImmutable()->setTimestamp($this->unsubscribed_at) : null; + } } diff --git a/backend/src/Api/Console/Input/Subscriber/ListAddStrategyIfUnsubscribed.php b/backend/src/Api/Console/Input/Subscriber/ListAddStrategyIfUnsubscribed.php new file mode 100644 index 00000000..78f12fb2 --- /dev/null +++ b/backend/src/Api/Console/Input/Subscriber/ListAddStrategyIfUnsubscribed.php @@ -0,0 +1,9 @@ +subscriber; + } + + public function shouldSendConfirmationEmail(): bool + { + return $this->sendConfirmationEmail; + } + +} diff --git a/backend/src/Service/Subscriber/SubscriberService.php b/backend/src/Service/Subscriber/SubscriberService.php index 6fac24b6..1c1128fd 100644 --- a/backend/src/Service/Subscriber/SubscriberService.php +++ b/backend/src/Service/Subscriber/SubscriberService.php @@ -14,11 +14,13 @@ use App\Entity\Type\SubscriberStatus; use App\Repository\SubscriberRepository; use App\Service\Subscriber\Dto\UpdateSubscriberDto; +use App\Service\Subscriber\Event\SubscriberCreatedEvent; use App\Service\Subscriber\Message\ExportSubscribersMessage; use App\Service\Subscriber\Message\SubscriberCreatedMessage; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\ORM\EntityManagerInterface; use Symfony\Component\Clock\ClockAwareTrait; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\Messenger\MessageBusInterface; class SubscriberService @@ -29,27 +31,25 @@ class SubscriberService public const BULK_SUBSCRIBER_LIMIT = 100; public function __construct( - private EntityManagerInterface $em, - private SubscriberRepository $subscriberRepository, - private MessageBusInterface $messageBus, - ) - { - } + private EntityManagerInterface $em, + private SubscriberRepository $subscriberRepository, + private EventDispatcherInterface $ed, + ) {} /** * @param iterable $lists */ public function createSubscriber( - Newsletter $newsletter, - string $email, - iterable $lists, - SubscriberStatus $status, - SubscriberSource $source, - ?string $subscribeIp = null, + Newsletter $newsletter, + string $email, + iterable $lists, + SubscriberStatus $status, + SubscriberSource $source, + ?string $subscribeIp = null, ?\DateTimeImmutable $subscribedAt = null, - ?\DateTimeImmutable $unsubscribedAt = null - ): Subscriber - { + ?\DateTimeImmutable $unsubscribedAt = null, + bool $sendConfirmationEmail = true, + ): Subscriber { $subscriber = new Subscriber() ->setNewsletter($newsletter) ->setEmail($email) @@ -83,7 +83,7 @@ public function createSubscriber( $this->em->persist($subscriber); $this->em->flush(); - $this->messageBus->dispatch(new SubscriberCreatedMessage($subscriber->getId())); + $this->ed->dispatch(new SubscriberCreatedEvent($subscriber, $sendConfirmationEmail)); return $subscriber; } @@ -102,7 +102,8 @@ public function deleteSubscribers(array $subscribers): void $ids = array_map(fn(Subscriber $s) => $s->getId(), $subscribers); $qb = $this->em->createQueryBuilder(); - $qb->delete(Subscriber::class, 's') + $qb + ->delete(Subscriber::class, 's') ->where($qb->expr()->in('s.id', ':ids')) ->setParameter('ids', $ids); @@ -113,14 +114,13 @@ public function deleteSubscribers(array $subscribers): void * @return ArrayCollection */ public function getSubscribers( - Newsletter $newsletter, + Newsletter $newsletter, ?SubscriberStatus $status, - ?int $listId, - ?string $search, - int $limit, - int $offset - ): ArrayCollection - { + ?int $listId, + ?string $search, + int $limit, + int $offset, + ): ArrayCollection { $qb = $this->subscriberRepository->createQueryBuilder('s'); $qb @@ -133,18 +133,21 @@ public function getSubscribers( ->setFirstResult($offset); if ($status !== null) { - $qb->andWhere('s.status = :status') + $qb + ->andWhere('s.status = :status') ->setParameter('status', $status->value); } if ($listId !== null) { - $qb->andWhere('l.id = :listId') + $qb + ->andWhere('l.id = :listId') ->andWhere('l.deleted_at IS NULL') ->setParameter('listId', $listId); } if ($search !== null) { - $qb->andWhere('s.email LIKE :search') + $qb + ->andWhere('s.email LIKE :search') ->setParameter('search', '%' . $search . '%'); } @@ -211,11 +214,10 @@ public function getSubscriberByEmail(Newsletter $newsletter, string $email): ?Su } public function unsubscribeBySend( - Send $send, + Send $send, ?\DateTimeImmutable $at = null, - ?string $reason = null - ): void - { + ?string $reason = null, + ): void { $subscriber = $send->getSubscriber(); $update = new UpdateSubscriberDto(); @@ -229,14 +231,14 @@ public function unsubscribeBySend( } public function unsubscribeByEmail( - string $email, + string $email, ?\DateTimeImmutable $at = null, - ?string $reason = null - ): void - { + ?string $reason = null, + ): void { $qb = $this->em->createQueryBuilder(); - $qb->update(Subscriber::class, 's') + $qb + ->update(Subscriber::class, 's') ->set('s.status', ':status') ->set('s.opt_in_at', ':optInAt') ->set('s.unsubscribed_at', ':unsubscribedAt') @@ -270,9 +272,8 @@ public function exportSubscribers(Newsletter $newsletter): SubscriberExport public function markSubscriberExportAsFailed( SubscriberExport $subscriberExport, - string $errorMessage - ): void - { + string $errorMessage, + ): void { $subscriberExport->setStatus(SubscriberExportStatus::FAILED); $subscriberExport->setErrorMessage($errorMessage); $this->em->persist($subscriberExport); @@ -281,9 +282,8 @@ public function markSubscriberExportAsFailed( public function markSubscriberExportAsCompleted( SubscriberExport $subscriberExport, - Media $media - ): void - { + Media $media, + ): void { $subscriberExport->setStatus(SubscriberExportStatus::COMPLETED); $subscriberExport->setMedia($media); $this->em->persist($subscriberExport); @@ -295,7 +295,8 @@ public function markSubscriberExportAsCompleted( */ public function getExports(Newsletter $newsletter): array { - return $this->em->getRepository(SubscriberExport::class) + return $this->em + ->getRepository(SubscriberExport::class) ->findBy(['newsletter' => $newsletter], ['created_at' => 'DESC']); } @@ -305,10 +306,8 @@ public function getExports(Newsletter $newsletter): array */ public function setSubscriberLists( Subscriber $subscriber, - array $lists - ): void - { - + array $lists, + ): void { $listIds = array_map(fn(NewsletterList $list) => $list->getId(), $lists); // remove lists that are not in the new list @@ -329,14 +328,12 @@ public function setSubscriberLists( $this->em->persist($subscriber); $this->em->flush(); - } public function addSubscriberToList( - Subscriber $subscriber, + Subscriber $subscriber, NewsletterList $list, - ): void - { + ): void { $subscriber->addList($list); $subscriber->setUpdatedAt($this->now()); @@ -345,11 +342,10 @@ public function addSubscriberToList( } public function removeSubscriberFromList( - Subscriber $subscriber, + Subscriber $subscriber, NewsletterList $list, - bool $recordUnsubscription - ): void - { + bool $recordUnsubscription, + ): void { $subscriber->removeList($list); $subscriber->setUpdatedAt($this->now()); diff --git a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php index a215f2ce..53e1dc12 100644 --- a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php +++ b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php @@ -5,14 +5,20 @@ use App\Api\Console\Controller\SubscriberController; use App\Entity\Newsletter; use App\Entity\Subscriber; +use App\Entity\SubscriberListUnsubscribed; use App\Entity\Type\SubscriberSource; use App\Entity\Type\SubscriberStatus; use App\Repository\SubscriberRepository; +use App\Service\App\Messenger\MessageTransport; +use App\Service\NewsletterList\NewsletterListService; +use App\Service\Subscriber\Event\SubscriberCreatedEvent; use App\Service\Subscriber\Message\SubscriberCreatedMessage; use App\Service\Subscriber\SubscriberService; use App\Tests\Case\WebTestCase; use App\Tests\Factory\NewsletterFactory; +use App\Tests\Factory\NewsletterListFactory; use App\Tests\Factory\SubscriberFactory; +use App\Tests\Factory\SubscriberListUnsubscribedFactory; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\TestWith; @@ -20,21 +26,29 @@ #[CoversClass(SubscriberService::class)] #[CoversClass(SubscriberRepository::class)] #[CoversClass(Subscriber::class)] +#[CoversClass(NewsletterListService::class)] class CreateSubscriberTest extends WebTestCase { - public function testCreateSubscriberMinimal(): void + public function test_create_subscriber(): void { - $this->mockRelayClient(); $newsletter = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne([ + 'newsletter' => $newsletter, + 'name' => 'List 1', + ]); + $response = $this->consoleApi( $newsletter, 'POST', '/subscribers', [ 'email' => 'test@email.com', - ] + 'lists' => [ + 'List 1', + ], + ], ); $this->assertSame(200, $response->getStatusCode()); @@ -49,18 +63,73 @@ public function testCreateSubscriberMinimal(): void $this->assertSame('test@email.com', $subscriber->getEmail()); $this->assertSame(SubscriberStatus::PENDING, $subscriber->getStatus()); $this->assertSame('console', $subscriber->getSource()->value); - $this->assertCount(0, $subscriber->getLists()); - $transport = $this->transport('async'); - $transport->queue()->assertCount(1); - $message = $transport->queue()->first()->getMessage(); - $this->assertInstanceOf(SubscriberCreatedMessage::class, $message); + $lists = $subscriber->getLists(); + $this->assertCount(1, $lists); + $this->assertSame('List 1', $lists->first()?->getName()); + + $event = $this->getEd()->getFirstEvent(SubscriberCreatedEvent::class); + $this->assertNotNull($event); + $this->assertFalse($event->shouldSendConfirmationEmail()); + } + + public function testCreateSubscriberWithListsById(): void + { + $newsletter = NewsletterFactory::createOne(); + $list1 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + $list2 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + + $response = $this->consoleApi( + $newsletter, + 'POST', + '/subscribers', + [ + 'email' => 'test@email.com', + 'lists' => [$list1->getId(), $list2->getId()], + ], + ); + + $this->assertSame(200, $response->getStatusCode()); + + $this->em->clear(); + $json = $this->getJson(); + $subscriber = $this->em->getRepository(Subscriber::class)->find($json['id']); + $this->assertInstanceOf(Subscriber::class, $subscriber); + $this->assertCount(2, $subscriber->getLists()); + $listIds = $subscriber->getLists()->map(fn($l) => $l->getId())->toArray(); + $this->assertContains($list1->getId(), $listIds); + $this->assertContains($list2->getId(), $listIds); + } + + public function testCreateSubscriberWithListsByName(): void + { + $newsletter = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne(['newsletter' => $newsletter, 'name' => 'My Newsletter']); + + $response = $this->consoleApi( + $newsletter, + 'POST', + '/subscribers', + [ + 'email' => 'test@email.com', + 'lists' => ['My Newsletter'], + ], + ); + + $this->assertSame(200, $response->getStatusCode()); + + $this->em->clear(); + $json = $this->getJson(); + $subscriber = $this->em->getRepository(Subscriber::class)->find($json['id']); + $this->assertInstanceOf(Subscriber::class, $subscriber); + $this->assertCount(1, $subscriber->getLists()); + $this->assertSame($list->getId(), $subscriber->getLists()->first()->getId()); } public function testCreateSubscriberWithAllInputs(): void { - $this->mockRelayClient(); $newsletter = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); $subscribedAt = new \DateTimeImmutable('2021-08-27 12:00:00'); $unsubscribedAt = new \DateTimeImmutable('2021-08-29 12:00:00'); @@ -75,7 +144,11 @@ public function testCreateSubscriberWithAllInputs(): void 'subscribe_ip' => '79.255.1.1', 'subscribed_at' => $subscribedAt->getTimestamp(), 'unsubscribed_at' => $unsubscribedAt->getTimestamp(), - ] + 'lists' => [$list->getId()], + 'list_add_strategy_if_unsubscribed' => 'force_add', + 'list_remove_reason' => 'other', + 'send_pending_confirmation_email' => false, + ], ); $this->assertSame(200, $response->getStatusCode()); @@ -89,8 +162,8 @@ public function testCreateSubscriberWithAllInputs(): void $this->assertSame($subscribedAt->getTimestamp(), $json['subscribed_at']); $this->assertSame($unsubscribedAt->getTimestamp(), $json['unsubscribed_at']); - $repository = $this->em->getRepository(Subscriber::class); - $subscriber = $repository->find($json['id']); + $this->em->clear(); + $subscriber = $this->em->getRepository(Subscriber::class)->find($json['id']); $this->assertInstanceOf(Subscriber::class, $subscriber); $this->assertSame('supun@hyvor.com', $subscriber->getEmail()); $this->assertSame(SubscriberStatus::PENDING, $subscriber->getStatus()); @@ -98,251 +171,329 @@ public function testCreateSubscriberWithAllInputs(): void $this->assertSame('79.255.1.1', $subscriber->getSubscribeIp()); $this->assertSame('2021-08-27 12:00:00', $subscriber->getSubscribedAt()?->format('Y-m-d H:i:s')); $this->assertSame('2021-08-29 12:00:00', $subscriber->getUnsubscribedAt()?->format('Y-m-d H:i:s')); - - $transport = $this->transport('async'); - $transport->queue()->assertCount(1); - $message = $transport->queue()->first()->getMessage(); - $this->assertInstanceOf(SubscriberCreatedMessage::class, $message); + $this->assertCount(1, $subscriber->getLists()); } - public function testInputValidationEmptyEmail(): void + public function testUpdateExistingSubscriber(): void { - $this->validateInput( - fn(Newsletter $newsletter) => [], - [ - [ - 'property' => 'email', - 'message' => 'This value should not be blank.', - ], - ] - ); - } + $newsletter = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); - public function testInputValidationInvalidEmail(): void - { - $this->validateInput( - fn(Newsletter $newsletter) => [ - 'email' => 'not-email', - ], - [ - [ - 'property' => 'email', - 'message' => 'This value is not a valid email address.', - ], - ] - ); - } + $subscriber = SubscriberFactory::createOne([ + 'newsletter' => $newsletter, + 'email' => 'supun@hyvor.com', + 'status' => SubscriberStatus::UNSUBSCRIBED, + 'subscribe_ip' => '1.2.3.4', + ]); - public function testInputValidationEmailTooLong(): void - { - $this->validateInput( - fn(Newsletter $newsletter) => [ - 'email' => str_repeat('a', 256) . '@hyvor.com', - ], - [ - [ - 'property' => 'email', - 'message' => 'This value is too long. It should have 255 characters or less.', - ], - ] - ); - } + $subscribedAt = new \DateTimeImmutable('2024-01-01 00:00:00'); - public function testInputValidationOptionalValues(): void - { - $this->validateInput( - fn(Newsletter $newsletter) => [ + $response = $this->consoleApi( + $newsletter, + 'POST', + '/subscribers', + [ 'email' => 'supun@hyvor.com', - 'source' => 'invalid-source', - 'subscribe_ip' => '127.0.0.1', - 'subscribed_at' => 'invalid-date', - 'unsubscribed_at' => 'invalid-date', + 'status' => 'pending', + 'subscribe_ip' => '79.255.1.1', + 'subscribed_at' => $subscribedAt->getTimestamp(), + 'source' => 'import', + 'lists' => [$list->getId()], ], - [ - [ - 'property' => 'source', - 'message' => 'This value should be of type int|string.', - ], - [ - 'property' => 'subscribed_at', - 'message' => 'This value should be of type int|null.', - ], - [ - 'property' => 'unsubscribed_at', - 'message' => 'This value should be of type int|null.', - ], - ] ); + + $this->assertSame(200, $response->getStatusCode()); + + $this->em->clear(); + $updated = $this->em->getRepository(Subscriber::class)->find($subscriber->getId()); + $this->assertInstanceOf(Subscriber::class, $updated); + $this->assertSame(SubscriberStatus::PENDING, $updated->getStatus()); + $this->assertSame('79.255.1.1', $updated->getSubscribeIp()); + $this->assertSame('2024-01-01 00:00:00', $updated->getSubscribedAt()?->format('Y-m-d H:i:s')); + $this->assertSame(SubscriberSource::IMPORT, $updated->getSource()); + $this->assertCount(1, $updated->getLists()); + $this->assertSame($list->getId(), $updated->getLists()->first()->getId()); } - #[TestWith(['not a valid ip'])] - #[TestWith(['127.0.0.1'])] // private ip - #[TestWith(['::1'])] // localhost - #[TestWith(['169.254.255.255'])] // reserved ip - public function testValidatesIp( - string $ip - ): void + public function testListAddStrategyIgnore(): void { - $this->validateInput( - fn(Newsletter $newsletter) => [ - 'email' => 'supun@hyvor.com', - 'subscribe_ip' => $ip, - ], + $newsletter = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); + + SubscriberListUnsubscribedFactory::createOne([ + 'list' => $list, + 'subscriber' => $subscriber, + ]); + + $response = $this->consoleApi( + $newsletter, + 'POST', + '/subscribers', [ - [ - 'property' => 'subscribe_ip', - 'message' => 'This value is not a valid IP address.', - ], - ] + 'email' => $subscriber->getEmail(), + 'lists' => [$list->getId()], + 'list_add_strategy_if_unsubscribed' => 'ignore', + ], ); + + $this->assertSame(200, $response->getStatusCode()); + + $this->em->clear(); + $updated = $this->em->getRepository(Subscriber::class)->find($subscriber->getId()); + $this->assertInstanceOf(Subscriber::class, $updated); + // Subscriber should NOT be added to the list (was previously unsubscribed, strategy=ignore) + $this->assertCount(0, $updated->getLists()); } - /** - * @param callable(Newsletter): array $input - * @param array $violations - * @return void - */ - private function validateInput( - callable $input, - array $violations - ): void + public function testListAddStrategyForceAdd(): void { $newsletter = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); + + SubscriberListUnsubscribedFactory::createOne([ + 'list' => $list, + 'subscriber' => $subscriber, + ]); $response = $this->consoleApi( $newsletter, 'POST', '/subscribers', - $input($newsletter), + [ + 'email' => $subscriber->getEmail(), + 'lists' => [$list->getId()], + 'list_add_strategy_if_unsubscribed' => 'force_add', + ], ); - $this->assertSame(422, $response->getStatusCode()); - $this->assertHasViolation($violations[0]['property'], $violations[0]['message']); + $this->assertSame(200, $response->getStatusCode()); + + $this->em->clear(); + $updated = $this->em->getRepository(Subscriber::class)->find($subscriber->getId()); + $this->assertInstanceOf(Subscriber::class, $updated); + // Subscriber SHOULD be added even though previously unsubscribed + $this->assertCount(1, $updated->getLists()); + $this->assertSame($list->getId(), $updated->getLists()->first()->getId()); } - public function test_updates_if_exists(): void + public function testListRemoveReasonUnsubscribe(): void { $newsletter = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter, 'lists' => [$list]]); - $subscriber = SubscriberFactory::createOne([ - 'newsletter' => $newsletter, - 'email' => 'supun@hyvor.com', - 'status' => SubscriberStatus::UNSUBSCRIBED, - 'subscribe_ip' => '1.2.3.4', + $response = $this->consoleApi( + $newsletter, + 'POST', + '/subscribers', + [ + 'email' => $subscriber->getEmail(), + 'lists' => [], // empty = remove from all lists + 'list_remove_reason' => 'unsubscribe', + ], + ); + + $this->assertSame(200, $response->getStatusCode()); + + $this->em->clear(); + $updated = $this->em->getRepository(Subscriber::class)->find($subscriber->getId()); + $this->assertInstanceOf(Subscriber::class, $updated); + $this->assertCount(0, $updated->getLists()); + + // Should record unsubscription + $record = $this->em->getRepository(SubscriberListUnsubscribed::class)->findOneBy([ + 'list' => $list->_real(), + 'subscriber' => $updated, ]); + $this->assertInstanceOf(SubscriberListUnsubscribed::class, $record); + } - $subscribedAt = new \DateTimeImmutable('2024-01-01 00:00:00'); + public function testListRemoveReasonOther(): void + { + $newsletter = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter, 'lists' => [$list]]); $response = $this->consoleApi( $newsletter, 'POST', '/subscribers', [ - 'email' => 'supun@hyvor.com', - 'if_exists' => 'update', - 'status' => 'pending', - 'subscribe_ip' => '79.255.1.1', - 'subscribed_at' => $subscribedAt->getTimestamp(), - 'source' => 'import' - ] + 'email' => $subscriber->getEmail(), + 'lists' => [], // empty = remove from all lists + 'list_remove_reason' => 'other', + ], ); $this->assertSame(200, $response->getStatusCode()); - $repository = $this->em->getRepository(Subscriber::class); - $updated = $repository->find($subscriber->getId()); + $this->em->clear(); + $updated = $this->em->getRepository(Subscriber::class)->find($subscriber->getId()); $this->assertInstanceOf(Subscriber::class, $updated); - $this->assertSame(SubscriberStatus::PENDING, $updated->getStatus()); - $this->assertSame('79.255.1.1', $updated->getSubscribeIp()); - $this->assertSame('2024-01-01 00:00:00', $updated->getSubscribedAt()?->format('Y-m-d H:i:s')); - $this->assertSame(SubscriberSource::IMPORT, $updated->getSource()); + $this->assertCount(0, $updated->getLists()); + + // Should NOT record unsubscription + $record = $this->em->getRepository(SubscriberListUnsubscribed::class)->findOneBy([ + 'list' => $list->_real(), + 'subscriber' => $updated, + ]); + $this->assertNull($record); } - public function testCreateSubscriberIfExistsUpdateClearsTimestamps(): void + public function testSendPendingConfirmationEmail(): void { + $this->mockRelayClient(); $newsletter = NewsletterFactory::createOne(); - $subscriber = SubscriberFactory::createOne([ - 'newsletter' => $newsletter, - 'email' => 'supun@hyvor.com', - 'subscribed_at' => new \DateTimeImmutable('2024-01-01'), - 'unsubscribed_at' => new \DateTimeImmutable('2024-06-01'), - ]); - $response = $this->consoleApi( $newsletter, 'POST', '/subscribers', [ - 'email' => 'supun@hyvor.com', - 'if_exists' => 'update', - 'subscribed_at' => null, - 'unsubscribed_at' => null, - ] + 'email' => 'test@email.com', + 'lists' => [], + 'send_pending_confirmation_email' => true, + ], ); $this->assertSame(200, $response->getStatusCode()); - $repository = $this->em->getRepository(Subscriber::class); - $updated = $repository->find($subscriber->getId()); - $this->assertInstanceOf(Subscriber::class, $updated); - $this->assertNull($updated->getSubscribedAt()); - $this->assertNull($updated->getUnsubscribedAt()); + $transport = $this->transport('async'); + $transport->queue()->assertCount(1); + $message = $transport->queue()->first()->getMessage(); + $this->assertInstanceOf(SubscriberCreatedMessage::class, $message); } - public function testCreateSubscriberIfExistsUpdateDoesNotChangeUnsentFields(): void + public function testNoConfirmationEmailByDefault(): void { $newsletter = NewsletterFactory::createOne(); - $subscriber = SubscriberFactory::createOne([ - 'newsletter' => $newsletter, - 'email' => 'supun@hyvor.com', - 'subscribe_ip' => '1.2.3.4', - 'subscribed_at' => new \DateTimeImmutable('2024-01-01'), - ]); - $response = $this->consoleApi( $newsletter, 'POST', '/subscribers', [ - 'email' => 'supun@hyvor.com', - 'if_exists' => 'update', - // subscribe_ip and subscribed_at not sent - ] + 'email' => 'test@email.com', + 'lists' => [], + ], ); $this->assertSame(200, $response->getStatusCode()); - $repository = $this->em->getRepository(Subscriber::class); - $updated = $repository->find($subscriber->getId()); - $this->assertInstanceOf(Subscriber::class, $updated); - $this->assertSame('1.2.3.4', $updated->getSubscribeIp()); - $this->assertSame('2024-01-01 00:00:00', $updated->getSubscribedAt()?->format('Y-m-d H:i:s')); + $transport = $this->transport('async'); + $transport->queue()->assertCount(0); } - public function testCreateSubscriberDuplicateEmail(): void + public function testListNotFound(): void { $newsletter = NewsletterFactory::createOne(); - $subscriber = SubscriberFactory::createOne([ - 'newsletter' => $newsletter, - 'email' => 'thibault@hyvor.com', - ]); $response = $this->consoleApi( $newsletter, 'POST', '/subscribers', [ - 'email' => 'thibault@hyvor.com', - ] + 'email' => 'test@email.com', + 'lists' => [999999], + ], ); $this->assertSame(422, $response->getStatusCode()); - $this->assertSame( - 'Subscriber with email ' . $subscriber->getEmail() . ' already exists', - $this->getJson()['message'] + $this->assertStringContainsString('List not found', $this->getJson()['message']); + } + + public function testInputValidationEmptyEmail(): void + { + $this->validateInput( + fn(Newsletter $newsletter) => ['lists' => []], + [ + [ + 'property' => 'email', + 'message' => 'This value should not be blank.', + ], + ], + ); + } + + public function testInputValidationInvalidEmail(): void + { + $this->validateInput( + fn(Newsletter $newsletter) + => [ + 'email' => 'not-email', + 'lists' => [], + ], + [ + [ + 'property' => 'email', + 'message' => 'This value is not a valid email address.', + ], + ], + ); + } + + public function testInputValidationEmailTooLong(): void + { + $this->validateInput( + fn(Newsletter $newsletter) + => [ + 'email' => str_repeat('a', 256) . '@hyvor.com', + 'lists' => [], + ], + [ + [ + 'property' => 'email', + 'message' => 'This value is too long. It should have 255 characters or less.', + ], + ], ); } + #[TestWith(['not a valid ip'])] + #[TestWith(['127.0.0.1'])] // private ip + #[TestWith(['::1'])] // localhost + #[TestWith(['169.254.255.255'])] // reserved ip + public function testValidatesIp( + string $ip, + ): void { + $this->validateInput( + fn(Newsletter $newsletter) + => [ + 'email' => 'supun@hyvor.com', + 'lists' => [], + 'subscribe_ip' => $ip, + ], + [ + [ + 'property' => 'subscribe_ip', + 'message' => 'This value is not a valid IP address.', + ], + ], + ); + } + + /** + * @param callable(Newsletter): array $input + * @param array $violations + * @return void + */ + private function validateInput( + callable $input, + array $violations, + ): void { + $newsletter = NewsletterFactory::createOne(); + + $response = $this->consoleApi( + $newsletter, + 'POST', + '/subscribers', + $input($newsletter), + ); + + $this->assertSame(422, $response->getStatusCode()); + $this->assertHasViolation($violations[0]['property'], $violations[0]['message']); + } + } diff --git a/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte b/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte index 01971edc..171f38bc 100644 --- a/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte +++ b/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte @@ -1,5 +1,5 @@

    Console API

    @@ -336,13 +336,18 @@ // Otherwise, a new subscriber will be created. email: string; - // The lists that the subscriber has subscribed to - // Send an array of list IDs (number) or names (string) + // Subscribe to or unsubscribe from lists based + // on the given \`lists_strategy\`. lists: (number | string)[]; + lists_strategy: + | 'sync' // (default) sets the subscriber's lists to the given lists (overwriting existing lists) + | 'add' // adds the subscriber to the given lists + | 'remove'; // removes the subscriber from the given lists + // The subscriber's subscription status // set \`send_pending_confirmation_email=true\` to send a confirmation email - // default: pending + // default: subscribed status?: 'pending' | 'subscribed' | 'unsubscribed'; // the source of the subscriber (default: 'console') @@ -366,16 +371,21 @@ // ============ SETTINGS =========== // change how the endpoint behaves - // define how to handle the case when the subscriber has - // previously unsubscribed from a list that is provided + // if the subscriber was previously removed from a list, + // define the reason(s) for ignoring the re-subscription to that list. // see below for more info - // default: 'ignore' - list_add_strategy_if_unsubscribed: 'ignore' | 'force_add'; + // default: ['unsubscribe', 'bounce'] + list_ignore_resubscribe_on: ('unsubscribe' | 'bounce' | 'auto')[]; // define the reason for removing the subscriber from a list - // see below + // (only when updating, see below for more info) // default: 'unsubscribe' - list_remove_reason: 'unsubscribe' | 'other'; + list_remove_reason: 'unsubscribe' | 'bounce' | 'auto'; + + // whether to overwrite or merge the subscriber's metadata + // when updating an existing subscriber. + // default: 'merge' + metadata_strategy: 'merge' | 'overwrite'; // whether to send a confirmation email when adding a subscriber with 'pending' status // or when changing an existing subscriber's status to 'pending'. @@ -388,6 +398,12 @@
    Managing list unsubscriptions and re-subscriptions
    +

    + For all subscribers, Hyvor Post records the lists they have previously unsubscribed from. This + makes it easier to build automations around list subscriptions while respecting subscribers' + preferences. +

    +

    list_add_strategy_if_unsubscribed:

    @@ -423,6 +439,108 @@ +
    Examples
    + +
    + +
    + This example creates a new subscriber with a subscription to the "Default" list. If a + subscriber exists in with the same email, they will be updated and their lists will be + set to only "Default" (overwriting existing lists). +
    + + +     + + +
    + Assuming you have a list with List ID 123, this example adds the subscriber to that list + without affecting their other list subscriptions. If the subscriber is already + subscribed to the list, no changes will be made. +
    + + +     + + +
    This example simply removes the subscriber from the list named "Paid Users".
    + + +     + + +
    + This example creates a subscriber or updates an existing subscriber with "pending" + status, and will send a confirmation email to the subscriber asking them to confirm + their subscription. +
    + +     + + +
    + By default, this endpoint ignores re-subscription attempts to lists that the subscriber + has previously unsubscribed from (or was removed from due to a bounce). This example + shows how to override that behavior. +
    + + +

    + To force re-adding both previous unsubscribes and bounces, use an empty array for list_ignore_resubscribe_on. +

    +     +
    +

    Delete a subscriber

    DELETE /subscribers/{'{id}'} From bc4c675318a56fe919e970845241547ccee2843d Mon Sep 17 00:00:00 2001 From: Supun Wimalasena Date: Tue, 3 Mar 2026 13:11:57 +0100 Subject: [PATCH 11/23] input planned --- backend/config/reference.php | 125 ++++-------------- .../Controller/SubscriberController.php | 72 +++++++--- .../Subscriber/CreateSubscriberInput.php | 37 +++++- .../ListAddStrategyIfUnsubscribed.php | 9 -- .../Input/Subscriber/ListRemoveReason.php | 1 + .../Subscriber/ListSkipResubscribeOn.php | 10 ++ .../Input/Subscriber/ListsStrategy.php | 12 ++ .../NewsletterList/NewsletterListService.php | 83 +++++++----- .../Service/Subscriber/SubscriberService.php | 4 +- .../Subscriber/CreateSubscriberTest.php | 14 +- .../docs/[...slug]/content/ConsoleApi.svelte | 31 +++-- 11 files changed, 212 insertions(+), 186 deletions(-) delete mode 100644 backend/src/Api/Console/Input/Subscriber/ListAddStrategyIfUnsubscribed.php create mode 100644 backend/src/Api/Console/Input/Subscriber/ListSkipResubscribeOn.php create mode 100644 backend/src/Api/Console/Input/Subscriber/ListsStrategy.php diff --git a/backend/config/reference.php b/backend/config/reference.php index 36bb2776..06bd1875 100644 --- a/backend/config/reference.php +++ b/backend/config/reference.php @@ -208,29 +208,29 @@ * initial_marking?: list, * events_to_dispatch?: list|null, * places?: list, + * name?: scalar|Param|null, + * metadata?: array, * }>, - * transitions: list, * to?: list, * weight?: int|Param, // Default: 1 - * metadata?: list, + * metadata?: array, * }>, - * metadata?: list, + * metadata?: array, * }>, * }, * router?: bool|array{ // Router configuration * enabled?: bool|Param, // Default: false - * resource: scalar|Param|null, + * resource?: scalar|Param|null, * type?: scalar|Param|null, * cache_dir?: scalar|Param|null, // Deprecated: Setting the "framework.router.cache_dir.cache_dir" configuration option is deprecated. It will be removed in version 8.0. // Default: "%kernel.build_dir%" * default_uri?: scalar|Param|null, // The default URI used to generate URLs in a non-HTTP context. // Default: null @@ -360,10 +360,10 @@ * mapping?: array{ * paths?: list, * }, - * default_context?: list, + * default_context?: array, * named_serializers?: array, + * default_context?: array, * include_built_in_normalizers?: bool|Param, // Whether to include the built-in normalizers // Default: true * include_built_in_encoders?: bool|Param, // Whether to include the built-in encoders // Default: true * }>, @@ -427,7 +427,7 @@ * }, * messenger?: bool|array{ // Messenger configuration * enabled?: bool|Param, // Default: true - * routing?: array, * }>, * serializer?: array{ @@ -440,7 +440,7 @@ * transports?: array, + * options?: array, * failure_transport?: scalar|Param|null, // Transport name to send failed messages to (after all retries have failed). // Default: null * retry_strategy?: string|array{ * service?: scalar|Param|null, // Service id to override the retry strategy entirely. // Default: null @@ -462,7 +462,7 @@ * allow_no_senders?: bool|Param, // Default: true * }, * middleware?: list, * }>, * }>, @@ -634,7 +634,7 @@ * lock_factory?: scalar|Param|null, // The service ID of the lock factory used by this limiter (or null to disable locking). // Default: "auto" * cache_pool?: scalar|Param|null, // The cache pool to use for storing the current limiter state. // Default: "cache.rate_limiter" * storage_service?: scalar|Param|null, // The service ID of a custom storage implementation, this precedes any configured "cache_pool". // Default: null - * policy: "fixed_window"|"token_bucket"|"sliding_window"|"compound"|"no_limit"|Param, // The algorithm to be used by this limiter. + * policy?: "fixed_window"|"token_bucket"|"sliding_window"|"compound"|"no_limit"|Param, // The algorithm to be used by this limiter. * limiters?: list, * limit?: int|Param, // The maximum allowed hits in a fixed interval or burst. * interval?: scalar|Param|null, // Configures the fixed interval if "policy" is set to "fixed_window" or "sliding_window". The value must be a number followed by "second", "minute", "hour", "day", "week" or "month" (or their plural equivalent). @@ -679,7 +679,7 @@ * enabled?: bool|Param, // Default: false * message_bus?: scalar|Param|null, // The message bus to use. // Default: "messenger.default_bus" * routing?: array, * }, @@ -694,7 +694,7 @@ * dbal?: array{ * default_connection?: scalar|Param|null, * types?: array, * driver_schemes?: array, @@ -910,7 +910,7 @@ * datetime_functions?: array, * }, * filters?: array, * }>, @@ -1045,7 +1045,7 @@ * use_microseconds?: scalar|Param|null, // Default: true * channels?: list, * handlers?: array, * mailer?: scalar|Param|null, // Default: null * email_prototype?: string|array{ - * id: scalar|Param|null, + * id?: scalar|Param|null, * method?: scalar|Param|null, // Default: null * }, * lazy?: bool|Param, // Default: true @@ -1316,85 +1316,6 @@ * expired_worker_ttl?: int|Param, // How long to keep expired workers in cache (in seconds). // Default: 3600 * }, * } - * @psalm-type SentryConfig = array{ - * dsn?: scalar|Param|null, // If this value is not provided, the SDK will try to read it from the SENTRY_DSN environment variable. If that variable also does not exist, the SDK will not send any events. - * register_error_listener?: bool|Param, // Default: true - * register_error_handler?: bool|Param, // Default: true - * logger?: scalar|Param|null, // The service ID of the PSR-3 logger used to log messages coming from the SDK client. Be aware that setting the same logger of the application may create a circular loop when an event fails to be sent. // Default: null - * options?: array{ - * integrations?: mixed, // Default: [] - * default_integrations?: bool|Param, - * prefixes?: list, - * sample_rate?: float|Param, // The sampling factor to apply to events. A value of 0 will deny sending any event, and a value of 1 will send all events. - * enable_tracing?: bool|Param, - * traces_sample_rate?: float|Param, // The sampling factor to apply to transactions. A value of 0 will deny sending any transaction, and a value of 1 will send all transactions. - * traces_sampler?: scalar|Param|null, - * profiles_sample_rate?: float|Param, // The sampling factor to apply to profiles. A value of 0 will deny sending any profiles, and a value of 1 will send all profiles. Profiles are sampled in relation to traces_sample_rate - * enable_logs?: bool|Param, - * enable_metrics?: bool|Param, // Default: true - * attach_stacktrace?: bool|Param, - * attach_metric_code_locations?: bool|Param, - * context_lines?: int|Param, - * environment?: scalar|Param|null, // Default: "%kernel.environment%" - * logger?: scalar|Param|null, - * spotlight?: bool|Param, - * spotlight_url?: scalar|Param|null, - * release?: scalar|Param|null, // Default: "%env(default::SENTRY_RELEASE)%" - * server_name?: scalar|Param|null, - * ignore_exceptions?: list, - * ignore_transactions?: list, - * before_send?: scalar|Param|null, - * before_send_transaction?: scalar|Param|null, - * before_send_check_in?: scalar|Param|null, - * before_send_metrics?: scalar|Param|null, - * before_send_log?: scalar|Param|null, - * before_send_metric?: scalar|Param|null, - * trace_propagation_targets?: mixed, - * tags?: array, - * error_types?: scalar|Param|null, - * max_breadcrumbs?: int|Param, - * before_breadcrumb?: mixed, - * in_app_exclude?: list, - * in_app_include?: list, - * send_default_pii?: bool|Param, - * max_value_length?: int|Param, - * transport?: scalar|Param|null, - * http_client?: scalar|Param|null, - * http_proxy?: scalar|Param|null, - * http_proxy_authentication?: scalar|Param|null, - * http_connect_timeout?: float|Param, // The maximum number of seconds to wait while trying to connect to a server. It works only when using the default transport. - * http_timeout?: float|Param, // The maximum execution time for the request+response as a whole. It works only when using the default transport. - * http_ssl_verify_peer?: bool|Param, - * http_compression?: bool|Param, - * capture_silenced_errors?: bool|Param, - * max_request_body_size?: "none"|"never"|"small"|"medium"|"always"|Param, - * class_serializers?: array, - * }, - * messenger?: bool|array{ - * enabled?: bool|Param, // Default: true - * capture_soft_fails?: bool|Param, // Default: true - * isolate_breadcrumbs_by_message?: bool|Param, // Default: false - * }, - * tracing?: bool|array{ - * enabled?: bool|Param, // Default: true - * dbal?: bool|array{ - * enabled?: bool|Param, // Default: true - * connections?: list, - * }, - * twig?: bool|array{ - * enabled?: bool|Param, // Default: true - * }, - * cache?: bool|array{ - * enabled?: bool|Param, // Default: true - * }, - * http_client?: bool|array{ - * enabled?: bool|Param, // Default: true - * }, - * console?: array{ - * excluded_commands?: list, - * }, - * }, - * } * @psalm-type ConfigType = array{ * imports?: ImportsConfig, * parameters?: ParametersConfig, @@ -1444,7 +1365,6 @@ * twig_component?: TwigComponentConfig, * twig_extra?: TwigExtraConfig, * zenstruck_messenger_monitor?: ZenstruckMessengerMonitorConfig, - * sentry?: SentryConfig, * }, * "when@test"?: array{ * imports?: ImportsConfig, @@ -1482,7 +1402,10 @@ final class App */ public static function config(array $config): array { - return AppReference::config($config); + /** @var ConfigType $config */ + $config = AppReference::config($config); + + return $config; } } diff --git a/backend/src/Api/Console/Controller/SubscriberController.php b/backend/src/Api/Console/Controller/SubscriberController.php index 515ab276..6a4ae212 100644 --- a/backend/src/Api/Console/Controller/SubscriberController.php +++ b/backend/src/Api/Console/Controller/SubscriberController.php @@ -6,10 +6,11 @@ use App\Api\Console\Authorization\ScopeRequired; use App\Api\Console\Input\Subscriber\BulkActionSubscriberInput; use App\Api\Console\Input\Subscriber\CreateSubscriberInput; -use App\Api\Console\Input\Subscriber\ListAddStrategyIfUnsubscribed; +use App\Api\Console\Input\Subscriber\ListSkipResubscribeOn; use App\Api\Console\Input\Subscriber\ListRemoveReason; use App\Api\Console\Object\SubscriberObject; use App\Entity\Newsletter; +use App\Entity\NewsletterList; use App\Entity\Subscriber; use App\Entity\Type\SubscriberSource; use App\Entity\Type\SubscriberStatus; @@ -80,28 +81,17 @@ public function createSubscriber( #[MapRequestPayload] CreateSubscriberInput $input, Newsletter $newsletter, ): JsonResponse { - // Resolve lists - $resolvedLists = []; - foreach ($input->lists as $listIdOrName) { - $id = is_int($listIdOrName) ? $listIdOrName : null; - $name = is_string($listIdOrName) ? $listIdOrName : null; - $list = $this->newsletterListService->getListByIdOrName($newsletter, $id, $name); - if ($list === null) { - throw new UnprocessableEntityHttpException("List not found: {$listIdOrName}"); - } - $resolvedLists[] = $list; - } - + $lists = $this->resolveLists($newsletter, $input->lists); $subscriber = $this->subscriberService->getSubscriberByEmail($newsletter, $input->email); if ($subscriber === null) { $subscriber = $this->subscriberService->createSubscriber( $newsletter, $input->email, - $resolvedLists, + $lists, $input->status, source: $input->source ?? SubscriberSource::CONSOLE, - subscribeIp: $input->has('subscribe_ip') ? $input->subscribe_ip : null, + subscribeIp: $input->getSubscriberIp(), subscribedAt: $input->getSubscribedAt(), unsubscribedAt: $input->getUnsubscribedAt(), sendConfirmationEmail: $input->send_pending_confirmation_email, @@ -170,6 +160,58 @@ public function createSubscriber( return $this->json(new SubscriberObject($subscriber)); } + /** + * @param (string|int)[] $listIdsOrNames + * @return NewsletterList[] + */ + private function resolveLists(Newsletter $newsletter, array $listIdsOrNames): array + { + $listIds = []; + $listNames = []; + + foreach ($listIdsOrNames as $listIdOrName) { + if (is_int($listIdOrName)) { + $listIds[] = $listIdOrName; + } elseif (is_string($listIdOrName)) { + $listNames[] = $listIdOrName; + } + } + + $resolvedLists = []; + + if (count($listIds) > 0) { + $resolvedLists = $this->newsletterListService->getListsByIds($newsletter, $listIds); + + if (count($resolvedLists) !== count($listIds)) { + $resolvedListIds = array_map(fn($l) => $l->getId(), $resolvedLists); + $missingIds = array_diff($listIds, $resolvedListIds); + throw new UnprocessableEntityHttpException( + "Lists with IDs " . implode(', ', $missingIds) . " not found", + ); + } + } + + if (count($listNames) > 0) { + $listsByName = $this->newsletterListService->getListsByNames($newsletter, $listNames); + + foreach ($listsByName as $list) { + if (!in_array($list, $resolvedLists)) { + $resolvedLists[] = $list; + } + } + + if (count($listsByName) !== count($listNames)) { + $resolvedListNames = array_map(fn($l) => $l->getName(), $listsByName); + $missingNames = array_diff($listNames, $resolvedListNames); + throw new UnprocessableEntityHttpException( + "Lists with names " . implode(', ', $missingNames) . " not found", + ); + } + } + + return $resolvedLists; + } + #[Route('/subscribers/{id}', methods: 'DELETE')] #[ScopeRequired(Scope::SUBSCRIBERS_WRITE)] public function deleteSubscriber(Subscriber $subscriber): JsonResponse diff --git a/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php b/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php index ce36f698..db4dc0e0 100644 --- a/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php +++ b/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php @@ -19,7 +19,12 @@ class CreateSubscriberInput #[Assert\Length(max: 255)] public string $email; - public SubscriberStatus $status = SubscriberStatus::PENDING; + /** + * @var ?(int|string)[] + */ + public ?array $lists = null; + + public SubscriberStatus $status = SubscriberStatus::SUBSCRIBED; public ?SubscriberSource $source = null; @@ -30,17 +35,18 @@ class CreateSubscriberInput private ?int $unsubscribed_at; - /** - * @var ?(int|string)[] - */ - public ?array $lists = null; - /** * @var array|null */ public ?array $metadata = null; - public ListAddStrategyIfUnsubscribed $list_add_strategy_if_unsubscribed = ListAddStrategyIfUnsubscribed::IGNORE; + + // settings + + public ListsStrategy $lists_strategy = ListsStrategy::SYNC; + + #[Assert\All(new Assert\Choice(callback: 'getListResubscribeOnValues'))] + private array $list_skip_resubscribe_on = ['unsubscribe', 'bounce']; public ListRemoveReason $list_remove_reason = ListRemoveReason::UNSUBSCRIBE; @@ -65,4 +71,21 @@ public function getUnsubscribedAt(): ?\DateTimeImmutable return $unsubscribedAt ? new \DateTimeImmutable()->setTimestamp($this->unsubscribed_at) : null; } + /** + * @return ListSkipResubscribeOn[] + */ + public function getListSkipResubscribeOn(): array + { + $listSkipResubscribeOn = $this->has('list_skip_resubscribe_on') ? $this->list_skip_resubscribe_on : []; + return array_map(fn($item) => ListSkipResubscribeOn::tryFrom($item), $listSkipResubscribeOn); + } + + /** + * @return string[] + */ + public function getListResubscribeOnValues(): array + { + return array_map(fn($value) => $value->value, ListSkipResubscribeOn::cases()); + } + } diff --git a/backend/src/Api/Console/Input/Subscriber/ListAddStrategyIfUnsubscribed.php b/backend/src/Api/Console/Input/Subscriber/ListAddStrategyIfUnsubscribed.php deleted file mode 100644 index 78f12fb2..00000000 --- a/backend/src/Api/Console/Input/Subscriber/ListAddStrategyIfUnsubscribed.php +++ /dev/null @@ -1,9 +0,0 @@ -em->getRepository(NewsletterList::class) + return $this->em + ->getRepository(NewsletterList::class) ->count([ 'newsletter' => $newsletter, ]); @@ -32,10 +31,10 @@ public function getListCounter(Newsletter $newsletter): int public function isNameAvailable( Newsletter $newsletter, - string $name - ): bool - { - return $this->em->getRepository(NewsletterList::class) + string $name, + ): bool { + return $this->em + ->getRepository(NewsletterList::class) ->count([ 'newsletter' => $newsletter, 'name' => $name, @@ -44,10 +43,9 @@ public function isNameAvailable( public function createNewsletterList( Newsletter $newsletter, - string $name, - ?string $description - ): NewsletterList - { + string $name, + ?string $description, + ): NewsletterList { $list = new NewsletterList() ->setNewsletter($newsletter) ->setName($name) @@ -79,13 +77,14 @@ public function getListById(int $id): ?NewsletterList public function getListsOfNewsletter(Newsletter $newsletter): ArrayCollection { return new ArrayCollection( - $this->em->getRepository(NewsletterList::class) + $this->em + ->getRepository(NewsletterList::class) ->findBy( [ 'newsletter' => $newsletter, 'deleted_at' => null, - ] - ) + ], + ), ); } @@ -126,32 +125,45 @@ public function getMissingListIdsOfNewsletter(Newsletter $newsletter, array $lis } /** - * Note that we should validate the lists are within the newsletter (using getMissingListIdsOfNewsletter) before calling this method - * @param array $listIds - * @return ArrayCollection + * @param int[] $ids + * @return NewsletterList[] */ - public function getListsByIds(array $listIds): ArrayCollection + public function getListsByIds(Newsletter $newsletter, array $ids): array { - return new ArrayCollection( - $this->em->getRepository(NewsletterList::class)->findBy(['id' => $listIds]) - ); + $qb = $this->em->createQueryBuilder(); + $qb + ->select('l') + ->from(NewsletterList::class, 'l') + ->where('l.newsletter = :newsletter') + ->andWhere($qb->expr()->in('l.id', ':ids')) + ->setParameter('newsletter', $newsletter) + ->setParameter('ids', $ids); + + /** @var array $result */ + $result = $qb->getQuery()->getResult(); + + return $result; } - public function getListByIdOrName(Newsletter $newsletter, ?int $id, ?string $name): ?NewsletterList + /** + * @param string[] $names + * @return NewsletterList[] + */ + public function getListsByNames(Newsletter $newsletter, array $names): array { - assert($id !== null || $name !== null, 'Either id or name must be provided'); + $qb = $this->em->createQueryBuilder(); + $qb + ->select('l') + ->from(NewsletterList::class, 'l') + ->where('l.newsletter = :newsletter') + ->andWhere($qb->expr()->in('l.name', ':names')) + ->setParameter('newsletter', $newsletter) + ->setParameter('names', $names); - if ($id !== null) { - return $this->em->getRepository(NewsletterList::class)->findOneBy([ - 'id' => $id, - 'newsletter' => $newsletter, - ]); - } + /** @var array $result */ + $result = $qb->getQuery()->getResult(); - return $this->em->getRepository(NewsletterList::class)->findOneBy([ - 'name' => $name, - 'newsletter' => $newsletter, - ]); + return $result; } /** @@ -162,7 +174,8 @@ public function getSubscriberCountOfLists(array $listIds): array { $qb = $this->em->createQueryBuilder(); - $qb->select('l.id AS list_id, COUNT(s.id) AS subscriber_count') + $qb + ->select('l.id AS list_id, COUNT(s.id) AS subscriber_count') ->from(NewsletterList::class, 'l') ->leftJoin('l.subscribers', 's', 'WITH', 's.status = :subscribed') ->where($qb->expr()->in('l.id', ':listIds')) diff --git a/backend/src/Service/Subscriber/SubscriberService.php b/backend/src/Service/Subscriber/SubscriberService.php index 1c1128fd..e77a425c 100644 --- a/backend/src/Service/Subscriber/SubscriberService.php +++ b/backend/src/Service/Subscriber/SubscriberService.php @@ -37,12 +37,12 @@ public function __construct( ) {} /** - * @param iterable $lists + * @param array $lists */ public function createSubscriber( Newsletter $newsletter, string $email, - iterable $lists, + array $lists, SubscriberStatus $status, SubscriberSource $source, ?string $subscribeIp = null, diff --git a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php index 53e1dc12..ed623025 100644 --- a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php +++ b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php @@ -30,7 +30,7 @@ class CreateSubscriberTest extends WebTestCase { - public function test_create_subscriber(): void + public function test_create_subscriber_minimal(): void { $newsletter = NewsletterFactory::createOne(); @@ -61,18 +61,24 @@ public function test_create_subscriber(): void $subscriber = $repository->find($json['id']); $this->assertInstanceOf(Subscriber::class, $subscriber); $this->assertSame('test@email.com', $subscriber->getEmail()); - $this->assertSame(SubscriberStatus::PENDING, $subscriber->getStatus()); + $this->assertSame(SubscriberStatus::SUBSCRIBED, $subscriber->getStatus()); $this->assertSame('console', $subscriber->getSource()->value); $lists = $subscriber->getLists(); $this->assertCount(1, $lists); - $this->assertSame('List 1', $lists->first()?->getName()); + $this->assertSame('List 1', $lists[0]?->getName()); $event = $this->getEd()->getFirstEvent(SubscriberCreatedEvent::class); - $this->assertNotNull($event); + $this->assertSame($json['id'], $event->getSubscriber()->getId()); $this->assertFalse($event->shouldSendConfirmationEmail()); } + public function test_create_subscriber_with_all_inputs(): void + { + // + } + + public function testCreateSubscriberWithListsById(): void { $newsletter = NewsletterFactory::createOne(); diff --git a/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte b/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte index 171f38bc..988a9f6c 100644 --- a/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte +++ b/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte @@ -338,19 +338,15 @@ // Subscribe to or unsubscribe from lists based // on the given \`lists_strategy\`. + // an array of list IDs or names. lists: (number | string)[]; - lists_strategy: - | 'sync' // (default) sets the subscriber's lists to the given lists (overwriting existing lists) - | 'add' // adds the subscriber to the given lists - | 'remove'; // removes the subscriber from the given lists - // The subscriber's subscription status - // set \`send_pending_confirmation_email=true\` to send a confirmation email // default: subscribed - status?: 'pending' | 'subscribed' | 'unsubscribed'; + status?: 'subscribed' | 'unsubscribed' | 'pending'; - // the source of the subscriber (default: 'console') + // the source of the subscriber + // default: console source?: 'console' | 'form' | 'import'; // subscriber's IP address @@ -371,16 +367,22 @@ // ============ SETTINGS =========== // change how the endpoint behaves + // how \`lists\` field is processed when updating an existing subscriber's list subscriptions. + // sync: overwrites the lists (default) + // add: adds to the current lists + // remove: removes from the current lists + lists_strategy: 'sync' | 'add' | 'remove'; + // if the subscriber was previously removed from a list, // define the reason(s) for ignoring the re-subscription to that list. // see below for more info // default: ['unsubscribe', 'bounce'] - list_ignore_resubscribe_on: ('unsubscribe' | 'bounce' | 'auto')[]; + list_skip_resubscribe_on: ('unsubscribe' | 'bounce' | 'auto')[]; // define the reason for removing the subscriber from a list // (only when updating, see below for more info) // default: 'unsubscribe' - list_remove_reason: 'unsubscribe' | 'bounce' | 'auto'; + list_remove_reason: 'unsubscribe' | 'bounce' | 'other'; // whether to overwrite or merge the subscriber's metadata // when updating an existing subscriber. @@ -488,7 +490,10 @@ { "email": "example@example.com", "lists": ["Paid Users"], - "lists_strategy": "remove" + "lists_strategy": "remove", + + // unsubscribe, bounce, or other + "list_remove_reason": "unsubscribe" } `} /> @@ -528,14 +533,14 @@ "lists_strategy": "add", // ignore unsubscription if the subscriber was removed from the list due to a bounce // but allow re-adding if they previously unsubscribed themselves - "list_ignore_resubscribe_on": ["bounce"] + "list_skip_resubscribe_on": ["bounce"] } `} />

    To force re-adding both previous unsubscribes and bounces, use an empty array for list_ignore_resubscribe_onlist_skip_resubscribe_on.

    From 6de795796973cc4b11a46f0563c44fd0d73e223d Mon Sep 17 00:00:00 2001 From: Supun Wimalasena Date: Tue, 3 Mar 2026 13:37:51 +0100 Subject: [PATCH 12/23] create subscriber, validate metadata, etc. --- .../Controller/SubscriberController.php | 18 ++++- .../Subscriber/CreateSubscriberInput.php | 19 +++-- backend/src/Entity/Subscriber.php | 6 +- .../Type/SubscriberMetadataDefinitionType.php | 9 ++- .../Service/Subscriber/SubscriberService.php | 22 +++--- .../MetadataValidationFailedException.php | 5 ++ .../SubscriberMetadataService.php | 79 ++++++++++++------- .../Subscriber/CreateSubscriberTest.php | 74 ++++++++++++++++- 8 files changed, 173 insertions(+), 59 deletions(-) create mode 100644 backend/src/Service/SubscriberMetadata/Exception/MetadataValidationFailedException.php diff --git a/backend/src/Api/Console/Controller/SubscriberController.php b/backend/src/Api/Console/Controller/SubscriberController.php index 6a4ae212..9a3f15bb 100644 --- a/backend/src/Api/Console/Controller/SubscriberController.php +++ b/backend/src/Api/Console/Controller/SubscriberController.php @@ -18,6 +18,7 @@ use App\Service\Subscriber\Dto\UpdateSubscriberDto; use App\Service\Subscriber\Message\SubscriberCreatedMessage; use App\Service\Subscriber\SubscriberService; +use App\Service\SubscriberMetadata\Exception\MetadataValidationFailedException; use App\Service\SubscriberMetadata\SubscriberMetadataService; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\JsonResponse; @@ -35,7 +36,6 @@ public function __construct( private SubscriberService $subscriberService, private NewsletterListService $newsletterListService, private SubscriberMetadataService $subscriberMetadataService, - private MessageBusInterface $messageBus, ) {} #[Route('/subscribers', methods: 'GET')] @@ -81,9 +81,20 @@ public function createSubscriber( #[MapRequestPayload] CreateSubscriberInput $input, Newsletter $newsletter, ): JsonResponse { - $lists = $this->resolveLists($newsletter, $input->lists); + $lists = $input->lists ? $this->resolveLists($newsletter, $input->lists) : []; $subscriber = $this->subscriberService->getSubscriberByEmail($newsletter, $input->email); + if ($input->metadata) { + try { + $this->subscriberMetadataService->validateMetadata( + $newsletter, + $input->metadata, + ); + } catch (MetadataValidationFailedException $e) { + throw new UnprocessableEntityHttpException($e->getMessage()); + } + } + if ($subscriber === null) { $subscriber = $this->subscriberService->createSubscriber( $newsletter, @@ -91,9 +102,10 @@ public function createSubscriber( $lists, $input->status, source: $input->source ?? SubscriberSource::CONSOLE, - subscribeIp: $input->getSubscriberIp(), + subscribeIp: $input->getSubscribeIp(), subscribedAt: $input->getSubscribedAt(), unsubscribedAt: $input->getUnsubscribedAt(), + metadata: $input->metadata ?? [], sendConfirmationEmail: $input->send_pending_confirmation_email, ); } else { diff --git a/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php b/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php index db4dc0e0..13eb6d3e 100644 --- a/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php +++ b/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php @@ -4,9 +4,11 @@ use App\Entity\Type\SubscriberSource; use App\Entity\Type\SubscriberStatus; +use App\Service\SubscriberMetadata\SubscriberMetadataService; use App\Util\OptionalPropertyTrait; use Symfony\Component\Clock\ClockAwareTrait; use Symfony\Component\Validator\Constraints as Assert; +use Symfony\Component\Validator\Context\ExecutionContextInterface; class CreateSubscriberInput { @@ -29,22 +31,25 @@ class CreateSubscriberInput public ?SubscriberSource $source = null; #[Assert\Ip(version: Assert\Ip::ALL_ONLY_PUBLIC)] - private ?string $subscribe_ip; + public ?string $subscribe_ip; - private ?int $subscribed_at; + public ?int $subscribed_at; - private ?int $unsubscribed_at; + public ?int $unsubscribed_at; /** - * @var array|null + * @var array|null */ + #[Assert\All(new Assert\Type('scalar'))] public ?array $metadata = null; - // settings public ListsStrategy $lists_strategy = ListsStrategy::SYNC; + /** + * @var string[] + */ #[Assert\All(new Assert\Choice(callback: 'getListResubscribeOnValues'))] private array $list_skip_resubscribe_on = ['unsubscribe', 'bounce']; @@ -54,7 +59,7 @@ class CreateSubscriberInput public bool $send_pending_confirmation_email = false; - public function getSubscriberIp(): ?string + public function getSubscribeIp(): ?string { return $this->has('subscribe_ip') ? $this->subscribe_ip : null; } @@ -77,7 +82,7 @@ public function getUnsubscribedAt(): ?\DateTimeImmutable public function getListSkipResubscribeOn(): array { $listSkipResubscribeOn = $this->has('list_skip_resubscribe_on') ? $this->list_skip_resubscribe_on : []; - return array_map(fn($item) => ListSkipResubscribeOn::tryFrom($item), $listSkipResubscribeOn); + return array_map(fn($item) => ListSkipResubscribeOn::from($item), $listSkipResubscribeOn); } /** diff --git a/backend/src/Entity/Subscriber.php b/backend/src/Entity/Subscriber.php index 9b687d25..512d3225 100644 --- a/backend/src/Entity/Subscriber.php +++ b/backend/src/Entity/Subscriber.php @@ -64,7 +64,7 @@ class Subscriber private ?string $unsubscribe_reason = null; /** - * @var array + * @var array */ #[ORM\Column(type: 'json', options: ['default' => '{}'])] private array $metadata = []; @@ -254,7 +254,7 @@ public function removeList(NewsletterList $list): self } /** - * @return array + * @return array */ public function getMetadata(): array { @@ -262,7 +262,7 @@ public function getMetadata(): array } /** - * @param array $metadata + * @param array $metadata */ public function setMetadata(array $metadata): static { diff --git a/backend/src/Entity/Type/SubscriberMetadataDefinitionType.php b/backend/src/Entity/Type/SubscriberMetadataDefinitionType.php index 1c32d935..1ad82836 100644 --- a/backend/src/Entity/Type/SubscriberMetadataDefinitionType.php +++ b/backend/src/Entity/Type/SubscriberMetadataDefinitionType.php @@ -8,5 +8,12 @@ enum SubscriberMetadataDefinitionType: string case TEXT = 'text'; // Maybe in the future, we will add more types like select, checkbox, etc. + + public function toJsonType(): string + { + return match ($this) { + SubscriberMetadataDefinitionType::TEXT => 'string', + }; + } -} \ No newline at end of file +} diff --git a/backend/src/Service/Subscriber/SubscriberService.php b/backend/src/Service/Subscriber/SubscriberService.php index e77a425c..8e0e6068 100644 --- a/backend/src/Service/Subscriber/SubscriberService.php +++ b/backend/src/Service/Subscriber/SubscriberService.php @@ -38,6 +38,7 @@ public function __construct( /** * @param array $lists + * @param array $metadata */ public function createSubscriber( Newsletter $newsletter, @@ -48,6 +49,7 @@ public function createSubscriber( ?string $subscribeIp = null, ?\DateTimeImmutable $subscribedAt = null, ?\DateTimeImmutable $unsubscribedAt = null, + array $metadata = [], bool $sendConfirmationEmail = true, ): Subscriber { $subscriber = new Subscriber() @@ -56,24 +58,18 @@ public function createSubscriber( ->setCreatedAt($this->now()) ->setUpdatedAt($this->now()) ->setStatus($status) - ->setSource($source); + ->setSubscribedAt($subscribedAt) + ->setUnsubscribedAt($unsubscribedAt) + ->setSubscribeIp($subscribeIp) + ->setSource($source) + ->setMetadata($metadata); // if status is subscribed, subscribed_at should be set to now // if status is unsubscribed, unsubscribed_at should be set to now if ($status === SubscriberStatus::SUBSCRIBED) { - $subscriber->setSubscribedAt($this->now()); + $subscriber->setSubscribedAt($subscribedAt ?? $this->now()); } elseif ($status === SubscriberStatus::UNSUBSCRIBED) { - $subscriber->setUnsubscribedAt($this->now()); - } - - if ($subscribedAt !== null) { - $subscriber->setSubscribedAt($subscribedAt); - } - if ($unsubscribedAt !== null) { - $subscriber->setUnsubscribedAt($unsubscribedAt); - } - if ($subscribeIp !== null) { - $subscriber->setSubscribeIp($subscribeIp); + $subscriber->setUnsubscribedAt($unsubscribedAt ?? $this->now()); } foreach ($lists as $list) { diff --git a/backend/src/Service/SubscriberMetadata/Exception/MetadataValidationFailedException.php b/backend/src/Service/SubscriberMetadata/Exception/MetadataValidationFailedException.php new file mode 100644 index 00000000..ea59983b --- /dev/null +++ b/backend/src/Service/SubscriberMetadata/Exception/MetadataValidationFailedException.php @@ -0,0 +1,5 @@ +findOneBy(['newsletter' => $newsletter, 'key' => $key]); } + /** + * @param string[] $keys + * @return SubscriberMetadataDefinition[] + */ + public function getMetadataDefinitionsByKeys(Newsletter $newsletter, array $keys): array + { + return $this->entityManager + ->getRepository(SubscriberMetadataDefinition::class) + ->findBy(['newsletter' => $newsletter, 'key' => $keys]); + } + public function getMetadataDefinitionsCount(Newsletter $newsletter): int { return $this->entityManager @@ -47,10 +57,9 @@ public function getMetadataDefinitionsCount(Newsletter $newsletter): int public function createMetadataDefinition( Newsletter $newsletter, - string $key, - string $name, - ): SubscriberMetadataDefinition - { + string $key, + string $name, + ): SubscriberMetadataDefinition { $metadataDefinition = new SubscriberMetadataDefinition(); $metadataDefinition->setNewsletter($newsletter); $metadataDefinition->setKey($key); @@ -67,9 +76,8 @@ public function createMetadataDefinition( public function updateMetadataDefinition( SubscriberMetadataDefinition $metadataDefinition, - string $name, - ): void - { + string $name, + ): void { $metadataDefinition->setName($name); $metadataDefinition->setUpdatedAt($this->now()); @@ -82,11 +90,38 @@ public function deleteMetadataDefinition(SubscriberMetadataDefinition $metadataD $this->entityManager->flush(); } - public function validateValueType( - SubscriberMetadataDefinition $metadataDefinition, - mixed $value - ): bool + /** + * @param array $metadata + * @throws MetadataValidationFailedException + */ + public function validateMetadata(Newsletter $newsletter, array $metadata): void { + $keys = array_keys($metadata); + $definitions = $this->getMetadataDefinitionsByKeys($newsletter, $keys); + + if (count($definitions) !== count($keys)) { + $foundKeys = array_map(fn(SubscriberMetadataDefinition $def) => $def->getKey(), $definitions); + $missingKeys = array_diff($keys, $foundKeys); + throw new MetadataValidationFailedException( + "Metadata definitions with keys " . implode(', ', $missingKeys) . " not found", + ); + } + + foreach ($definitions as $definition) { + $value = $metadata[$definition->getKey()] ?? null; + if (!$this->validateValueType($definition, $value)) { + throw new MetadataValidationFailedException( + "Invalid value type for metadata key " . $definition->getKey( + ) . ". Expected type: " . $definition->getType()->toJsonType(), + ); + } + } + } + + private function validateValueType( + SubscriberMetadataDefinition $metadataDefinition, + mixed $value, + ): bool { return match ($metadataDefinition->getType()) { // @phpstan-ignore-next-line SubscriberMetadataDefinitionType::TEXT => is_string($value), @@ -94,20 +129,4 @@ public function validateValueType( default => false, }; } - - /** - * @param array $metadata - */ - public function validateMetadata(Newsletter $newsletter, array $metadata): bool - { - foreach ($metadata as $key => $value) { - $metaDef = $this->getMetadataDefinitionByKey($newsletter, $key); - if ($metaDef === null) - throw new \Exception("Metadata definition with key {$key} not found"); - if (!$this->validateValueType($metaDef, $value)) { - throw new \Exception("Value for metadata key {$key} is not valid"); - } - } - return true; - } } diff --git a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php index ed623025..6c6fd0b9 100644 --- a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php +++ b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php @@ -6,10 +6,10 @@ use App\Entity\Newsletter; use App\Entity\Subscriber; use App\Entity\SubscriberListUnsubscribed; +use App\Entity\Type\SubscriberMetadataDefinitionType; use App\Entity\Type\SubscriberSource; use App\Entity\Type\SubscriberStatus; use App\Repository\SubscriberRepository; -use App\Service\App\Messenger\MessageTransport; use App\Service\NewsletterList\NewsletterListService; use App\Service\Subscriber\Event\SubscriberCreatedEvent; use App\Service\Subscriber\Message\SubscriberCreatedMessage; @@ -19,6 +19,7 @@ use App\Tests\Factory\NewsletterListFactory; use App\Tests\Factory\SubscriberFactory; use App\Tests\Factory\SubscriberListUnsubscribedFactory; +use App\Tests\Factory\SubscriberMetadataDefinitionFactory; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\TestWith; @@ -75,7 +76,76 @@ public function test_create_subscriber_minimal(): void public function test_create_subscriber_with_all_inputs(): void { - // + $this->getEd()->setMockEvents([SubscriberCreatedEvent::class]); + + $newsletter = NewsletterFactory::createOne(); + $list1 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + $list2 = NewsletterListFactory::createOne(['newsletter' => $newsletter, 'name' => 'Named List']); + + $subscribedAt = new \DateTimeImmutable('2023-06-15 10:00:00'); + $unsubscribedAt = new \DateTimeImmutable('2023-06-20 10:00:00'); + + SubscriberMetadataDefinitionFactory::createOne([ + 'newsletter' => $newsletter, + 'key' => 'test-key', + 'type' => SubscriberMetadataDefinitionType::TEXT, + ]); + + $response = $this->consoleApi( + $newsletter, + 'POST', + '/subscribers', + [ + 'email' => 'test@hyvor.com', + 'lists' => [$list1->getId(), 'Named List'], + 'status' => 'pending', + 'source' => 'import', + 'subscribe_ip' => '203.0.113.1', + 'subscribed_at' => $subscribedAt->getTimestamp(), + 'unsubscribed_at' => $unsubscribedAt->getTimestamp(), + 'metadata' => [ + 'test-key' => 'test', + ], + 'send_pending_confirmation_email' => true, + ], + ); + $this->assertSame(200, $response->getStatusCode()); + + $json = $this->getJson(); + $this->assertIsInt($json['id']); + $this->assertSame('test@hyvor.com', $json['email']); + $this->assertSame('pending', $json['status']); + $this->assertSame('import', $json['source']); + $this->assertSame('203.0.113.1', $json['subscribe_ip']); + $this->assertSame($subscribedAt->getTimestamp(), $json['subscribed_at']); + $this->assertSame($unsubscribedAt->getTimestamp(), $json['unsubscribed_at']); + $this->assertCount(2, $json['list_ids']); + $this->assertContains($list1->getId(), $json['list_ids']); + $this->assertContains($list2->getId(), $json['list_ids']); + + $this->em->clear(); + $subscriber = $this->em->getRepository(Subscriber::class)->find($json['id']); + $this->assertInstanceOf(Subscriber::class, $subscriber); + $this->assertSame('test@hyvor.com', $subscriber->getEmail()); + $this->assertSame(SubscriberStatus::PENDING, $subscriber->getStatus()); + $this->assertSame(SubscriberSource::IMPORT, $subscriber->getSource()); + $this->assertSame('203.0.113.1', $subscriber->getSubscribeIp()); + $this->assertSame('2023-06-15 10:00:00', $subscriber->getSubscribedAt()?->format('Y-m-d H:i:s')); + $this->assertSame('2023-06-20 10:00:00', $subscriber->getUnsubscribedAt()?->format('Y-m-d H:i:s')); + $this->assertSame( + [ + 'test-key' => 'test', + ], + $subscriber->getMetadata(), + ); + $listIds = $subscriber->getLists()->map(fn($l) => $l->getId())->toArray(); + $this->assertCount(2, $listIds); + $this->assertContains($list1->getId(), $listIds); + $this->assertContains($list2->getId(), $listIds); + + $event = $this->getEd()->getFirstEvent(SubscriberCreatedEvent::class); + $this->assertSame($json['id'], $event->getSubscriber()->getId()); + $this->assertTrue($event->shouldSendConfirmationEmail()); } From 1f1a437d08b967b7ea5736a178a3dcbe06685002 Mon Sep 17 00:00:00 2001 From: Supun Wimalasena Date: Tue, 3 Mar 2026 14:54:41 +0100 Subject: [PATCH 13/23] update subscriber wip --- .../Controller/SubscriberController.php | 18 ++-- .../Subscriber/CreateSubscriberInput.php | 10 +- .../Api/Console/Object/SubscriberObject.php | 2 - backend/src/Entity/Subscriber.php | 15 --- backend/src/Entity/Type/SubscriberStatus.php | 1 - .../Subscriber/Dto/UpdateSubscriberDto.php | 7 +- .../Service/Subscriber/SubscriberService.php | 12 --- .../Subscriber/CreateSubscriberTest.php | 94 +++++++++++++++++-- backend/tests/Factory/SubscriberFactory.php | 5 +- .../docs/[...slug]/content/ConsoleApi.svelte | 20 ++-- 10 files changed, 105 insertions(+), 79 deletions(-) diff --git a/backend/src/Api/Console/Controller/SubscriberController.php b/backend/src/Api/Console/Controller/SubscriberController.php index 9a3f15bb..58e1e77a 100644 --- a/backend/src/Api/Console/Controller/SubscriberController.php +++ b/backend/src/Api/Console/Controller/SubscriberController.php @@ -100,21 +100,21 @@ public function createSubscriber( $newsletter, $input->email, $lists, - $input->status, + status: $input->status ?? SubscriberStatus::SUBSCRIBED, source: $input->source ?? SubscriberSource::CONSOLE, subscribeIp: $input->getSubscribeIp(), subscribedAt: $input->getSubscribedAt(), - unsubscribedAt: $input->getUnsubscribedAt(), metadata: $input->metadata ?? [], sendConfirmationEmail: $input->send_pending_confirmation_email, ); } else { - // Update existing subscriber with provided fields $updates = new UpdateSubscriberDto(); - $updates->status = $input->status; + if ($input->status) { + $updates->status = $input->status; + } - if ($input->has('source')) { + if ($input->source) { $updates->source = $input->source; } @@ -128,13 +128,7 @@ public function createSubscriber( : null; } - if ($input->has('unsubscribed_at')) { - $updates->unsubscribedAt = $input->unsubscribed_at !== null - ? \DateTimeImmutable::createFromTimestamp($input->unsubscribed_at) - : null; - } - - if ($input->has('metadata')) { + if ($input->metadata) { $updates->metadata = $input->metadata; } diff --git a/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php b/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php index 13eb6d3e..fdca526f 100644 --- a/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php +++ b/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php @@ -26,7 +26,7 @@ class CreateSubscriberInput */ public ?array $lists = null; - public SubscriberStatus $status = SubscriberStatus::SUBSCRIBED; + public ?SubscriberStatus $status = null; public ?SubscriberSource $source = null; @@ -35,8 +35,6 @@ class CreateSubscriberInput public ?int $subscribed_at; - public ?int $unsubscribed_at; - /** * @var array|null */ @@ -70,12 +68,6 @@ public function getSubscribedAt(): ?\DateTimeImmutable return $subscribedAt ? new \DateTimeImmutable()->setTimestamp($this->subscribed_at) : null; } - public function getUnsubscribedAt(): ?\DateTimeImmutable - { - $unsubscribedAt = $this->has('unsubscribed_at') ? $this->unsubscribed_at : null; - return $unsubscribedAt ? new \DateTimeImmutable()->setTimestamp($this->unsubscribed_at) : null; - } - /** * @return ListSkipResubscribeOn[] */ diff --git a/backend/src/Api/Console/Object/SubscriberObject.php b/backend/src/Api/Console/Object/SubscriberObject.php index daba7352..efc944f5 100644 --- a/backend/src/Api/Console/Object/SubscriberObject.php +++ b/backend/src/Api/Console/Object/SubscriberObject.php @@ -20,7 +20,6 @@ class SubscriberObject public ?string $subscribe_ip; public bool $is_opted_in = false; public ?int $subscribed_at; - public ?int $unsubscribed_at; /** * @var array @@ -37,7 +36,6 @@ public function __construct(Subscriber $subscriber) $this->subscribe_ip = $subscriber->getSubscribeIp(); $this->is_opted_in = $subscriber->getOptInAt() !== null; $this->subscribed_at = $subscriber->getSubscribedAt()?->getTimestamp(); - $this->unsubscribed_at = $subscriber->getUnsubscribedAt()?->getTimestamp(); $this->metadata = $subscriber->getMetadata(); } diff --git a/backend/src/Entity/Subscriber.php b/backend/src/Entity/Subscriber.php index 512d3225..e92dec88 100644 --- a/backend/src/Entity/Subscriber.php +++ b/backend/src/Entity/Subscriber.php @@ -48,9 +48,6 @@ class Subscriber #[ORM\Column(nullable: true)] private ?\DateTimeImmutable $opt_in_at = null; - #[ORM\Column(nullable: true)] - private ?\DateTimeImmutable $unsubscribed_at = null; - #[ORM\Column(enumType: SubscriberSource::class)] private SubscriberSource $source; @@ -170,18 +167,6 @@ public function setOptInAt(?\DateTimeImmutable $opt_in_at): static return $this; } - public function getUnsubscribedAt(): ?\DateTimeImmutable - { - return $this->unsubscribed_at; - } - - public function setUnsubscribedAt(?\DateTimeImmutable $unsubscribed_at): static - { - $this->unsubscribed_at = $unsubscribed_at; - - return $this; - } - public function getSource(): SubscriberSource { return $this->source; diff --git a/backend/src/Entity/Type/SubscriberStatus.php b/backend/src/Entity/Type/SubscriberStatus.php index d1237c65..8f5394e9 100644 --- a/backend/src/Entity/Type/SubscriberStatus.php +++ b/backend/src/Entity/Type/SubscriberStatus.php @@ -5,6 +5,5 @@ enum SubscriberStatus: string { case SUBSCRIBED = 'subscribed'; - case UNSUBSCRIBED = 'unsubscribed'; case PENDING = 'pending'; } diff --git a/backend/src/Service/Subscriber/Dto/UpdateSubscriberDto.php b/backend/src/Service/Subscriber/Dto/UpdateSubscriberDto.php index 2b3fc895..b010fe5e 100644 --- a/backend/src/Service/Subscriber/Dto/UpdateSubscriberDto.php +++ b/backend/src/Service/Subscriber/Dto/UpdateSubscriberDto.php @@ -2,7 +2,6 @@ namespace App\Service\Subscriber\Dto; -use App\Entity\NewsletterList; use App\Entity\Type\SubscriberSource; use App\Entity\Type\SubscriberStatus; use App\Util\OptionalPropertyTrait; @@ -12,8 +11,6 @@ class UpdateSubscriberDto use OptionalPropertyTrait; - public string $email; - public SubscriberStatus $status; public SubscriberSource $source; public ?string $subscribeIp; @@ -22,12 +19,10 @@ class UpdateSubscriberDto public ?\DateTimeImmutable $optInAt; - public ?\DateTimeImmutable $unsubscribedAt; - public ?string $unsubscribedReason; /** - * @var array + * @var array */ public array $metadata; diff --git a/backend/src/Service/Subscriber/SubscriberService.php b/backend/src/Service/Subscriber/SubscriberService.php index 8e0e6068..b05a4d8a 100644 --- a/backend/src/Service/Subscriber/SubscriberService.php +++ b/backend/src/Service/Subscriber/SubscriberService.php @@ -48,7 +48,6 @@ public function createSubscriber( SubscriberSource $source, ?string $subscribeIp = null, ?\DateTimeImmutable $subscribedAt = null, - ?\DateTimeImmutable $unsubscribedAt = null, array $metadata = [], bool $sendConfirmationEmail = true, ): Subscriber { @@ -59,7 +58,6 @@ public function createSubscriber( ->setUpdatedAt($this->now()) ->setStatus($status) ->setSubscribedAt($subscribedAt) - ->setUnsubscribedAt($unsubscribedAt) ->setSubscribeIp($subscribeIp) ->setSource($source) ->setMetadata($metadata); @@ -68,8 +66,6 @@ public function createSubscriber( // if status is unsubscribed, unsubscribed_at should be set to now if ($status === SubscriberStatus::SUBSCRIBED) { $subscriber->setSubscribedAt($subscribedAt ?? $this->now()); - } elseif ($status === SubscriberStatus::UNSUBSCRIBED) { - $subscriber->setUnsubscribedAt($unsubscribedAt ?? $this->now()); } foreach ($lists as $list) { @@ -156,10 +152,6 @@ public function getSubscribers( public function updateSubscriber(Subscriber $subscriber, UpdateSubscriberDto $updates): Subscriber { - if ($updates->has('email')) { - $subscriber->setEmail($updates->email); - } - if ($updates->has('status')) { $subscriber->setStatus($updates->status); } @@ -180,10 +172,6 @@ public function updateSubscriber(Subscriber $subscriber, UpdateSubscriberDto $up $subscriber->setOptInAt($updates->optInAt); } - if ($updates->has('unsubscribedAt')) { - $subscriber->setUnsubscribedAt($updates->unsubscribedAt); - } - if ($updates->has('unsubscribedReason')) { $subscriber->setUnsubscribeReason($updates->unsubscribedReason); } diff --git a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php index 6c6fd0b9..96c391bc 100644 --- a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php +++ b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php @@ -22,15 +22,19 @@ use App\Tests\Factory\SubscriberMetadataDefinitionFactory; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\TestWith; +use Symfony\Component\Clock\Test\ClockSensitiveTrait; + +use function Zenstruck\Foundry\Persistence\refresh; #[CoversClass(SubscriberController::class)] #[CoversClass(SubscriberService::class)] -#[CoversClass(SubscriberRepository::class)] -#[CoversClass(Subscriber::class)] +#[CoversClass(SubscriberCreatedEvent::class)] #[CoversClass(NewsletterListService::class)] class CreateSubscriberTest extends WebTestCase { + use ClockSensitiveTrait; + public function test_create_subscriber_minimal(): void { $newsletter = NewsletterFactory::createOne(); @@ -83,7 +87,6 @@ public function test_create_subscriber_with_all_inputs(): void $list2 = NewsletterListFactory::createOne(['newsletter' => $newsletter, 'name' => 'Named List']); $subscribedAt = new \DateTimeImmutable('2023-06-15 10:00:00'); - $unsubscribedAt = new \DateTimeImmutable('2023-06-20 10:00:00'); SubscriberMetadataDefinitionFactory::createOne([ 'newsletter' => $newsletter, @@ -102,7 +105,6 @@ public function test_create_subscriber_with_all_inputs(): void 'source' => 'import', 'subscribe_ip' => '203.0.113.1', 'subscribed_at' => $subscribedAt->getTimestamp(), - 'unsubscribed_at' => $unsubscribedAt->getTimestamp(), 'metadata' => [ 'test-key' => 'test', ], @@ -118,7 +120,6 @@ public function test_create_subscriber_with_all_inputs(): void $this->assertSame('import', $json['source']); $this->assertSame('203.0.113.1', $json['subscribe_ip']); $this->assertSame($subscribedAt->getTimestamp(), $json['subscribed_at']); - $this->assertSame($unsubscribedAt->getTimestamp(), $json['unsubscribed_at']); $this->assertCount(2, $json['list_ids']); $this->assertContains($list1->getId(), $json['list_ids']); $this->assertContains($list2->getId(), $json['list_ids']); @@ -131,7 +132,6 @@ public function test_create_subscriber_with_all_inputs(): void $this->assertSame(SubscriberSource::IMPORT, $subscriber->getSource()); $this->assertSame('203.0.113.1', $subscriber->getSubscribeIp()); $this->assertSame('2023-06-15 10:00:00', $subscriber->getSubscribedAt()?->format('Y-m-d H:i:s')); - $this->assertSame('2023-06-20 10:00:00', $subscriber->getUnsubscribedAt()?->format('Y-m-d H:i:s')); $this->assertSame( [ 'test-key' => 'test', @@ -149,6 +149,88 @@ public function test_create_subscriber_with_all_inputs(): void } + public function test_creates_subscriber_fills_subscribed_at(): void + { + $this->mockTime('2026-01-01'); + + $newsletter = NewsletterFactory::createOne(); + + $response = $this->consoleApi( + $newsletter, + 'POST', + '/subscribers', + [ + 'email' => 'test@email.com', + ], + ); + + $subscriber = $this->em->getRepository(Subscriber::class)->find($this->getJson()['id']); + $this->assertNotNull($subscriber); + $this->assertSame(SubscriberStatus::SUBSCRIBED, $subscriber->getStatus()); + $this->assertSame('2026-01-01', $subscriber->getSubscribedAt()?->format('Y-m-d')); + } + + public function test_updates_subscriber_all(): void + { + $newsletter = NewsletterFactory::createOne(); + $list1 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + $list2 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + + SubscriberMetadataDefinitionFactory::createOne([ + 'newsletter' => $newsletter, + 'key' => 'a', + ]); + + $subscriber = SubscriberFactory::createOne([ + 'email' => 'supun@hyvor.com', + 'newsletter' => $newsletter, + 'lists' => [ + $list1, + ], + 'status' => SubscriberStatus::PENDING, + 'source' => SubscriberSource::FORM, + 'subscribe_ip' => '1.2.3.4', + 'subscribed_at' => new \DateTimeImmutable('2026-01-02'), + ]); + + $this->consoleApi( + $newsletter, + 'POST', + '/subscribers', + [ + 'email' => 'supun@hyvor.com', + 'lists' => [$list2->getId()], // merge + 'status' => 'subscribed', + 'source' => 'console', + 'subscribe_ip' => '2.3.4.5', + 'subscribed_at' => new \DateTimeImmutable('2026-01-01')->getTimestamp(), + 'metadata' => [ + 'a' => 'b', + ], + ], + ); + + $this->assertResponseIsSuccessful(); + + refresh($subscriber); + + $this->assertSame(SubscriberStatus::SUBSCRIBED, $subscriber->getStatus()); + $this->assertSame(SubscriberSource::CONSOLE, $subscriber->getSource()); + $this->assertSame('2.3.4.5', $subscriber->getSubscribeIp()); + $this->assertSame('2026-01-01', $subscriber->getSubscribedAt()?->format('Y-m-d')); + $this->assertSame([ + 'a' => 'b', + ], $subscriber->getMetadata()); + + $lists = $subscriber->getLists(); + $this->assertCount(2, $lists); + + $listIds = $lists->map(fn($l) => $l->getId())->toArray(); + $this->assertContains($list1->getId(), $listIds); + $this->assertContains($list2->getId(), $listIds); + } + + public function testCreateSubscriberWithListsById(): void { $newsletter = NewsletterFactory::createOne(); diff --git a/backend/tests/Factory/SubscriberFactory.php b/backend/tests/Factory/SubscriberFactory.php index 2d8f36ee..45a657e4 100644 --- a/backend/tests/Factory/SubscriberFactory.php +++ b/backend/tests/Factory/SubscriberFactory.php @@ -17,9 +17,7 @@ final class SubscriberFactory extends PersistentProxyObjectFactory * * @todo inject services if required */ - public function __construct() - { - } + public function __construct() {} public static function class(): string { @@ -44,7 +42,6 @@ protected function defaults(): array 'subscribed_at' => \DateTimeImmutable::createFromMutable(self::faker()->dateTime()), 'opt_in_at' => \DateTimeImmutable::createFromMutable(self::faker()->dateTime()), 'unsubscribe_reason' => self::faker()->text(255), - 'unsubscribed_at' => \DateTimeImmutable::createFromMutable(self::faker()->dateTime()), 'updated_at' => \DateTimeImmutable::createFromMutable(self::faker()->dateTime()), ]; } diff --git a/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte b/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte index 988a9f6c..8ec62e21 100644 --- a/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte +++ b/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte @@ -339,11 +339,11 @@ // Subscribe to or unsubscribe from lists based // on the given \`lists_strategy\`. // an array of list IDs or names. - lists: (number | string)[]; + lists?: (number | string)[]; // The subscriber's subscription status // default: subscribed - status?: 'subscribed' | 'unsubscribed' | 'pending'; + status?: 'subscribed' | 'pending'; // the source of the subscriber // default: console @@ -356,10 +356,6 @@ // if not set, it will be set to the current time if status is 'subscribed' subscribed_at?: number | null; // unix timestamp - // unix timestamp of when the subscriber unsubscribed - // if not set, it will be set to the current time if status is 'unsubscribed' - unsubscribed_at?: number | null; // unix timestamp - // additional metadata for the subscriber // keys must be defined in the Subscriber Metadata Definitions section (or using the API) metadata?: Record; @@ -368,26 +364,26 @@ // change how the endpoint behaves // how \`lists\` field is processed when updating an existing subscriber's list subscriptions. - // sync: overwrites the lists (default) - // add: adds to the current lists + // merge: merges the lists (default) + // overwrite: overwrites the lists // remove: removes from the current lists - lists_strategy: 'sync' | 'add' | 'remove'; + lists_strategy?: 'merge' | 'overwrite' | 'remove'; // if the subscriber was previously removed from a list, // define the reason(s) for ignoring the re-subscription to that list. // see below for more info // default: ['unsubscribe', 'bounce'] - list_skip_resubscribe_on: ('unsubscribe' | 'bounce' | 'auto')[]; + list_skip_resubscribe_on?: ('unsubscribe' | 'bounce' | 'auto')[]; // define the reason for removing the subscriber from a list // (only when updating, see below for more info) // default: 'unsubscribe' - list_remove_reason: 'unsubscribe' | 'bounce' | 'other'; + list_remove_reason?: 'unsubscribe' | 'bounce' | 'other'; // whether to overwrite or merge the subscriber's metadata // when updating an existing subscriber. // default: 'merge' - metadata_strategy: 'merge' | 'overwrite'; + metadata_strategy?: 'merge' | 'overwrite'; // whether to send a confirmation email when adding a subscriber with 'pending' status // or when changing an existing subscriber's status to 'pending'. From 3bf8855356fa1936759b7daec4dc2b38980ffba9 Mon Sep 17 00:00:00 2001 From: Supun Wimalasena Date: Tue, 3 Mar 2026 15:15:09 +0100 Subject: [PATCH 14/23] lists strategy, before tests --- backend/migrations/Version20260225000000.php | 20 +++---- .../Controller/SubscriberController.php | 30 ++++++++--- .../Subscriber/CreateSubscriberInput.php | 11 ++-- .../Input/Subscriber/ListsStrategy.php | 4 +- backend/src/Entity/Subscriber.php | 9 ++++ ...bscribed.php => SubscriberListRemoval.php} | 18 ++++++- .../Type/ListRemovalReason.php} | 5 +- .../Subscriber/Dto/UpdateSubscriberDto.php | 4 ++ .../Service/Subscriber/SubscriberService.php | 53 +++++-------------- .../Subscriber/CreateSubscriberTest.php | 18 ++++--- ...y.php => SubscriberListRemovalFactory.php} | 13 +++-- .../docs/[...slug]/content/ConsoleApi.svelte | 4 +- 12 files changed, 107 insertions(+), 82 deletions(-) rename backend/src/Entity/{SubscriberListUnsubscribed.php => SubscriberListRemoval.php} (80%) rename backend/src/{Api/Console/Input/Subscriber/ListSkipResubscribeOn.php => Entity/Type/ListRemovalReason.php} (53%) rename backend/tests/Factory/{SubscriberListUnsubscribedFactory.php => SubscriberListRemovalFactory.php} (62%) diff --git a/backend/migrations/Version20260225000000.php b/backend/migrations/Version20260225000000.php index fff68152..5078457b 100644 --- a/backend/migrations/Version20260225000000.php +++ b/backend/migrations/Version20260225000000.php @@ -16,15 +16,17 @@ public function getDescription(): string public function up(Schema $schema): void { - $this->addSql(<<addSql( + <<lists ? $this->resolveLists($newsletter, $input->lists) : []; + $resolvedLists = $input->lists ? $this->resolveLists($newsletter, $input->lists) : []; $subscriber = $this->subscriberService->getSubscriberByEmail($newsletter, $input->email); if ($input->metadata) { @@ -99,7 +96,7 @@ public function createSubscriber( $subscriber = $this->subscriberService->createSubscriber( $newsletter, $input->email, - $lists, + $resolvedLists, status: $input->status ?? SubscriberStatus::SUBSCRIBED, source: $input->source ?? SubscriberSource::CONSOLE, subscribeIp: $input->getSubscribeIp(), @@ -132,6 +129,27 @@ public function createSubscriber( $updates->metadata = $input->metadata; } + if ($input->lists !== null) { + $newLists = $subscriber->getLists()->toArray(); + + if ($input->lists_strategy === ListsStrategy::MERGE) { + foreach ($resolvedLists as $list) { + if (!array_find($newLists, fn($l) => $l->getId() === $list->getId())) { + $newLists[] = $list; + } + } + } elseif ($input->lists_strategy === ListsStrategy::OVERWRITE) { + $newLists = $resolvedLists; + } else { + $newLists = array_filter( + $newLists, + fn($l) => !array_find($resolvedLists, fn($rl) => $rl->getId() === $l->getId()), + ); + } + + $updates->lists = $newLists; + } + $subscriber = $this->subscriberService->updateSubscriber($subscriber, $updates); } diff --git a/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php b/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php index fdca526f..34dab027 100644 --- a/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php +++ b/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php @@ -2,6 +2,7 @@ namespace App\Api\Console\Input\Subscriber; +use App\Entity\Type\ListRemovalReason; use App\Entity\Type\SubscriberSource; use App\Entity\Type\SubscriberStatus; use App\Service\SubscriberMetadata\SubscriberMetadataService; @@ -43,13 +44,13 @@ class CreateSubscriberInput // settings - public ListsStrategy $lists_strategy = ListsStrategy::SYNC; + public ListsStrategy $lists_strategy = ListsStrategy::MERGE; /** * @var string[] */ #[Assert\All(new Assert\Choice(callback: 'getListResubscribeOnValues'))] - private array $list_skip_resubscribe_on = ['unsubscribe', 'bounce']; + private array $list_skip_resubscribe_on = ['unsubscribe', 'bounce', 'complaint']; public ListRemoveReason $list_remove_reason = ListRemoveReason::UNSUBSCRIBE; @@ -69,12 +70,12 @@ public function getSubscribedAt(): ?\DateTimeImmutable } /** - * @return ListSkipResubscribeOn[] + * @return ListRemovalReason[] */ public function getListSkipResubscribeOn(): array { $listSkipResubscribeOn = $this->has('list_skip_resubscribe_on') ? $this->list_skip_resubscribe_on : []; - return array_map(fn($item) => ListSkipResubscribeOn::from($item), $listSkipResubscribeOn); + return array_map(fn($item) => ListRemovalReason::from($item), $listSkipResubscribeOn); } /** @@ -82,7 +83,7 @@ public function getListSkipResubscribeOn(): array */ public function getListResubscribeOnValues(): array { - return array_map(fn($value) => $value->value, ListSkipResubscribeOn::cases()); + return array_map(fn($value) => $value->value, ListRemovalReason::cases()); } } diff --git a/backend/src/Api/Console/Input/Subscriber/ListsStrategy.php b/backend/src/Api/Console/Input/Subscriber/ListsStrategy.php index 27e35a08..fdb239f7 100644 --- a/backend/src/Api/Console/Input/Subscriber/ListsStrategy.php +++ b/backend/src/Api/Console/Input/Subscriber/ListsStrategy.php @@ -5,8 +5,8 @@ enum ListsStrategy: string { - case SYNC = 'sync'; - case ADD = 'add'; + case MERGE = 'merge'; + case OVERWRITE = 'overwrite'; case REMOVE = 'remove'; } diff --git a/backend/src/Entity/Subscriber.php b/backend/src/Entity/Subscriber.php index e92dec88..58278234 100644 --- a/backend/src/Entity/Subscriber.php +++ b/backend/src/Entity/Subscriber.php @@ -224,6 +224,15 @@ public function getLists(): Collection return $this->lists; } + /** + * @param Collection $lists + */ + public function setLists(Collection $lists): static + { + $this->lists = $lists; + return $this; + } + public function addList(NewsletterList $list): self { if (!$this->lists->contains($list)) { diff --git a/backend/src/Entity/SubscriberListUnsubscribed.php b/backend/src/Entity/SubscriberListRemoval.php similarity index 80% rename from backend/src/Entity/SubscriberListUnsubscribed.php rename to backend/src/Entity/SubscriberListRemoval.php index efcf1015..e0caa8f8 100644 --- a/backend/src/Entity/SubscriberListUnsubscribed.php +++ b/backend/src/Entity/SubscriberListRemoval.php @@ -5,9 +5,9 @@ use Doctrine\ORM\Mapping as ORM; #[ORM\Entity] -#[ORM\Table(name: 'list_subscriber_unsubscribed')] +#[ORM\Table(name: 'subscriber_list_removals')] #[ORM\UniqueConstraint(columns: ['list_id', 'subscriber_id'])] -class SubscriberListUnsubscribed +class SubscriberListRemoval { #[ORM\Id] #[ORM\GeneratedValue] @@ -22,6 +22,9 @@ class SubscriberListUnsubscribed #[ORM\JoinColumn(name: 'subscriber_id', nullable: false, onDelete: 'CASCADE')] private Subscriber $subscriber; + #[ORM\Column(type: 'string')] + private string $reason; + #[ORM\Column] private \DateTimeImmutable $created_at; @@ -57,6 +60,17 @@ public function setSubscriber(Subscriber $subscriber): static return $this; } + public function getReason(): string + { + return $this->reason; + } + + public function setReason(string $reason): static + { + $this->reason = $reason; + return $this; + } + public function getCreatedAt(): \DateTimeImmutable { return $this->created_at; diff --git a/backend/src/Api/Console/Input/Subscriber/ListSkipResubscribeOn.php b/backend/src/Entity/Type/ListRemovalReason.php similarity index 53% rename from backend/src/Api/Console/Input/Subscriber/ListSkipResubscribeOn.php rename to backend/src/Entity/Type/ListRemovalReason.php index 207d7d4a..e4356887 100644 --- a/backend/src/Api/Console/Input/Subscriber/ListSkipResubscribeOn.php +++ b/backend/src/Entity/Type/ListRemovalReason.php @@ -1,10 +1,11 @@ */ diff --git a/backend/src/Service/Subscriber/SubscriberService.php b/backend/src/Service/Subscriber/SubscriberService.php index b05a4d8a..1eb3b4ec 100644 --- a/backend/src/Service/Subscriber/SubscriberService.php +++ b/backend/src/Service/Subscriber/SubscriberService.php @@ -8,7 +8,7 @@ use App\Entity\Send; use App\Entity\Subscriber; use App\Entity\SubscriberExport; -use App\Entity\SubscriberListUnsubscribed; +use App\Entity\SubscriberListRemoval; use App\Entity\Type\SubscriberExportStatus; use App\Entity\Type\SubscriberSource; use App\Entity\Type\SubscriberStatus; @@ -150,8 +150,10 @@ public function getSubscribers( return new ArrayCollection($results); } - public function updateSubscriber(Subscriber $subscriber, UpdateSubscriberDto $updates): Subscriber - { + public function updateSubscriber( + Subscriber $subscriber, + UpdateSubscriberDto $updates, + ): Subscriber { if ($updates->has('status')) { $subscriber->setStatus($updates->status); } @@ -177,11 +179,11 @@ public function updateSubscriber(Subscriber $subscriber, UpdateSubscriberDto $up } if ($updates->has('metadata')) { - $metadata = $subscriber->getMetadata(); - foreach ($updates->metadata as $key => $value) { - $metadata[$key] = $value; - } - $subscriber->setMetadata($metadata); + $subscriber->setMetadata($updates->metadata); + } + + if ($updates->has('lists')) { + $subscriber->setLists(new ArrayCollection($updates->lists)); } $subscriber->setUpdatedAt($this->now()); @@ -284,35 +286,6 @@ public function getExports(Newsletter $newsletter): array ->findBy(['newsletter' => $newsletter], ['created_at' => 'DESC']); } - /** - * @param Subscriber $subscriber - * @param NewsletterList[] $lists - */ - public function setSubscriberLists( - Subscriber $subscriber, - array $lists, - ): void { - $listIds = array_map(fn(NewsletterList $list) => $list->getId(), $lists); - - // remove lists that are not in the new list - foreach ($subscriber->getLists() as $existingList) { - if (!in_array($existingList->getId(), $listIds)) { - $subscriber->removeList($existingList); - } - } - - // add new lists - foreach ($lists as $list) { - if (!$subscriber->getLists()->contains($list)) { - $subscriber->addList($list); - } - } - - $subscriber->setUpdatedAt($this->now()); - - $this->em->persist($subscriber); - $this->em->flush(); - } public function addSubscriberToList( Subscriber $subscriber, @@ -338,13 +311,13 @@ public function removeSubscriberFromList( if ($recordUnsubscription) { - $existing = $this->em->getRepository(SubscriberListUnsubscribed::class)->findOneBy([ + $existing = $this->em->getRepository(SubscriberListRemoval::class)->findOneBy([ 'list' => $list, 'subscriber' => $subscriber, ]); if ($existing === null) { - $unsubscribed = new SubscriberListUnsubscribed() + $unsubscribed = new SubscriberListRemoval() ->setList($list) ->setSubscriber($subscriber) ->setCreatedAt($this->now()); @@ -358,7 +331,7 @@ public function removeSubscriberFromList( public function hasSubscriberUnsubscribedFromList(Subscriber $subscriber, NewsletterList $list): bool { - $record = $this->em->getRepository(SubscriberListUnsubscribed::class)->findOneBy([ + $record = $this->em->getRepository(SubscriberListRemoval::class)->findOneBy([ 'list' => $list, 'subscriber' => $subscriber, ]); diff --git a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php index 96c391bc..c47a6901 100644 --- a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php +++ b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php @@ -5,7 +5,7 @@ use App\Api\Console\Controller\SubscriberController; use App\Entity\Newsletter; use App\Entity\Subscriber; -use App\Entity\SubscriberListUnsubscribed; +use App\Entity\SubscriberListRemoval; use App\Entity\Type\SubscriberMetadataDefinitionType; use App\Entity\Type\SubscriberSource; use App\Entity\Type\SubscriberStatus; @@ -18,7 +18,7 @@ use App\Tests\Factory\NewsletterFactory; use App\Tests\Factory\NewsletterListFactory; use App\Tests\Factory\SubscriberFactory; -use App\Tests\Factory\SubscriberListUnsubscribedFactory; +use App\Tests\Factory\SubscriberListRemovalFactory; use App\Tests\Factory\SubscriberMetadataDefinitionFactory; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\TestWith; @@ -230,6 +230,10 @@ public function test_updates_subscriber_all(): void $this->assertContains($list2->getId(), $listIds); } + public function test_lists_strategy_overwrite(): void {} + + public function test_lists_strategy_remove(): void {} + public function testCreateSubscriberWithListsById(): void { @@ -379,7 +383,7 @@ public function testListAddStrategyIgnore(): void $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); - SubscriberListUnsubscribedFactory::createOne([ + SubscriberListRemovalFactory::createOne([ 'list' => $list, 'subscriber' => $subscriber, ]); @@ -410,7 +414,7 @@ public function testListAddStrategyForceAdd(): void $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); - SubscriberListUnsubscribedFactory::createOne([ + SubscriberListRemovalFactory::createOne([ 'list' => $list, 'subscriber' => $subscriber, ]); @@ -461,11 +465,11 @@ public function testListRemoveReasonUnsubscribe(): void $this->assertCount(0, $updated->getLists()); // Should record unsubscription - $record = $this->em->getRepository(SubscriberListUnsubscribed::class)->findOneBy([ + $record = $this->em->getRepository(SubscriberListRemoval::class)->findOneBy([ 'list' => $list->_real(), 'subscriber' => $updated, ]); - $this->assertInstanceOf(SubscriberListUnsubscribed::class, $record); + $this->assertInstanceOf(SubscriberListRemoval::class, $record); } public function testListRemoveReasonOther(): void @@ -493,7 +497,7 @@ public function testListRemoveReasonOther(): void $this->assertCount(0, $updated->getLists()); // Should NOT record unsubscription - $record = $this->em->getRepository(SubscriberListUnsubscribed::class)->findOneBy([ + $record = $this->em->getRepository(SubscriberListRemoval::class)->findOneBy([ 'list' => $list->_real(), 'subscriber' => $updated, ]); diff --git a/backend/tests/Factory/SubscriberListUnsubscribedFactory.php b/backend/tests/Factory/SubscriberListRemovalFactory.php similarity index 62% rename from backend/tests/Factory/SubscriberListUnsubscribedFactory.php rename to backend/tests/Factory/SubscriberListRemovalFactory.php index ee3f0660..a18a877d 100644 --- a/backend/tests/Factory/SubscriberListUnsubscribedFactory.php +++ b/backend/tests/Factory/SubscriberListRemovalFactory.php @@ -2,21 +2,19 @@ namespace App\Tests\Factory; -use App\Entity\SubscriberListUnsubscribed; +use App\Entity\SubscriberListRemoval; use Zenstruck\Foundry\Persistence\PersistentProxyObjectFactory; /** - * @extends PersistentProxyObjectFactory + * @extends PersistentProxyObjectFactory */ -final class SubscriberListUnsubscribedFactory extends PersistentProxyObjectFactory +final class SubscriberListRemovalFactory extends PersistentProxyObjectFactory { - public function __construct() - { - } + public function __construct() {} public static function class(): string { - return SubscriberListUnsubscribed::class; + return SubscriberListRemoval::class; } /** @@ -27,6 +25,7 @@ protected function defaults(): array return [ 'list' => NewsletterListFactory::new(), 'subscriber' => SubscriberFactory::new(), + 'reason' => self::faker()->randomElement(['unsubscribe', 'bounce', 'other']), 'created_at' => \DateTimeImmutable::createFromMutable(self::faker()->dateTime()), ]; } diff --git a/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte b/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte index 8ec62e21..a6973e14 100644 --- a/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte +++ b/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte @@ -372,8 +372,8 @@ // if the subscriber was previously removed from a list, // define the reason(s) for ignoring the re-subscription to that list. // see below for more info - // default: ['unsubscribe', 'bounce'] - list_skip_resubscribe_on?: ('unsubscribe' | 'bounce' | 'auto')[]; + // default: ['unsubscribe', 'bounce', 'complaint'] + list_skip_resubscribe_on?: ('unsubscribe' | 'bounce' | 'complaint' | 'auto')[]; // define the reason for removing the subscriber from a list // (only when updating, see below for more info) From 1020efda5e4e86ba23db0bef144f258878c356c2 Mon Sep 17 00:00:00 2001 From: Supun Wimalasena Date: Tue, 3 Mar 2026 15:20:06 +0100 Subject: [PATCH 15/23] lists strategy done, metadata strategy before tests --- .../Controller/SubscriberController.php | 10 +++- .../Subscriber/CreateSubscriberTest.php | 53 ++++++++++++++++++- 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/backend/src/Api/Console/Controller/SubscriberController.php b/backend/src/Api/Console/Controller/SubscriberController.php index 395ab687..f0d9907e 100644 --- a/backend/src/Api/Console/Controller/SubscriberController.php +++ b/backend/src/Api/Console/Controller/SubscriberController.php @@ -7,6 +7,7 @@ use App\Api\Console\Input\Subscriber\BulkActionSubscriberInput; use App\Api\Console\Input\Subscriber\CreateSubscriberInput; use App\Api\Console\Input\Subscriber\ListsStrategy; +use App\Api\Console\Input\Subscriber\MetadataStrategy; use App\Api\Console\Object\SubscriberObject; use App\Entity\Newsletter; use App\Entity\NewsletterList; @@ -126,7 +127,14 @@ public function createSubscriber( } if ($input->metadata) { - $updates->metadata = $input->metadata; + if ($input->metadata_strategy === MetadataStrategy::MERGE) { + $updates->metadata = array_merge( + $subscriber->getMetadata(), + $input->metadata, + ); + } else { + $updates->metadata = $input->metadata; + } } if ($input->lists !== null) { diff --git a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php index c47a6901..bf782b0f 100644 --- a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php +++ b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php @@ -230,10 +230,59 @@ public function test_updates_subscriber_all(): void $this->assertContains($list2->getId(), $listIds); } - public function test_lists_strategy_overwrite(): void {} + public function test_lists_strategy_overwrite(): void + { + $newsletter = NewsletterFactory::createOne(); + $list1 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + $list2 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + + $subscriber = SubscriberFactory::createOne([ + 'newsletter' => $newsletter, + 'lists' => [$list1], + ]); + + $this->consoleApi($newsletter, 'POST', '/subscribers', [ + 'email' => $subscriber->getEmail(), + 'lists' => [$list2->getId()], + 'lists_strategy' => 'overwrite', + ]); + + $this->assertResponseIsSuccessful(); + + refresh($subscriber); + $listIds = $subscriber->getLists()->map(fn($l) => $l->getId())->toArray(); + $this->assertCount(1, $listIds); + $this->assertContains($list2->getId(), $listIds); + } + + public function test_lists_strategy_remove(): void + { + $newsletter = NewsletterFactory::createOne(); + $list1 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + $list2 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + + $subscriber = SubscriberFactory::createOne([ + 'newsletter' => $newsletter, + 'lists' => [$list1, $list2], + ]); + + $this->consoleApi($newsletter, 'POST', '/subscribers', [ + 'email' => $subscriber->getEmail(), + 'lists' => [$list1->getId()], + 'lists_strategy' => 'remove', + ]); + + $this->assertResponseIsSuccessful(); + + refresh($subscriber); + $listIds = $subscriber->getLists()->map(fn($l) => $l->getId())->toArray(); + $this->assertCount(1, $listIds); + $this->assertContains($list2->getId(), $listIds); + } - public function test_lists_strategy_remove(): void {} + public function test_metadata_strategy_merge(): void {} + public function test_metadata_strategy_overwrite(): void {} public function testCreateSubscriberWithListsById(): void { From b010c7e9378f288f23faba86840ce15411b76174 Mon Sep 17 00:00:00 2001 From: Supun Wimalasena Date: Tue, 3 Mar 2026 15:40:06 +0100 Subject: [PATCH 16/23] saving list removals --- .../Controller/SubscriberController.php | 34 ++----- .../Subscriber/CreateSubscriberInput.php | 2 +- .../Input/Subscriber/ListRemoveReason.php | 10 --- .../Event/SubscriberUpdatedEvent.php | 25 ++++++ .../Event/SubscriberUpdatingEvent.php | 32 +++++++ .../ListRemoval/ListRemovalListener.php | 48 ++++++++++ .../Service/Subscriber/SubscriberService.php | 29 +++++- .../Subscriber/CreateSubscriberTest.php | 89 ++++++++++++++++++- .../docs/[...slug]/content/ConsoleApi.svelte | 6 +- 9 files changed, 227 insertions(+), 48 deletions(-) delete mode 100644 backend/src/Api/Console/Input/Subscriber/ListRemoveReason.php create mode 100644 backend/src/Service/Subscriber/Event/SubscriberUpdatedEvent.php create mode 100644 backend/src/Service/Subscriber/Event/SubscriberUpdatingEvent.php create mode 100644 backend/src/Service/Subscriber/ListRemoval/ListRemovalListener.php diff --git a/backend/src/Api/Console/Controller/SubscriberController.php b/backend/src/Api/Console/Controller/SubscriberController.php index f0d9907e..f78ab676 100644 --- a/backend/src/Api/Console/Controller/SubscriberController.php +++ b/backend/src/Api/Console/Controller/SubscriberController.php @@ -158,37 +158,13 @@ public function createSubscriber( $updates->lists = $newLists; } - $subscriber = $this->subscriberService->updateSubscriber($subscriber, $updates); + $subscriber = $this->subscriberService->updateSubscriber( + $subscriber, + $updates, + listRemovalReason: $input->list_removal_reason, + ); } -// // Sync lists -// $resolvedListIds = array_map(fn($l) => $l->getId(), $resolvedLists); -// $currentListIds = $subscriber->getLists()->map(fn($l) => $l->getId())->toArray(); -// -// // Add new lists -// foreach ($resolvedLists as $list) { -// if (!in_array($list->getId(), $currentListIds)) { -// if ( -// $input->list_add_strategy_if_unsubscribed === ListAddStrategyIfUnsubscribed::IGNORE && -// $this->subscriberService->hasSubscriberUnsubscribedFromList($subscriber, $list) -// ) { -// continue; -// } -// $this->subscriberService->addSubscriberToList($subscriber, $list); -// } -// } -// -// // Remove lists no longer in the resolved set -// foreach ($subscriber->getLists()->toArray() as $existingList) { -// if (!in_array($existingList->getId(), $resolvedListIds)) { -// $this->subscriberService->removeSubscriberFromList( -// $subscriber, -// $existingList, -// $input->list_remove_reason === ListRemoveReason::UNSUBSCRIBE, -// ); -// } -// } - return $this->json(new SubscriberObject($subscriber)); } diff --git a/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php b/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php index 34dab027..54fc4bab 100644 --- a/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php +++ b/backend/src/Api/Console/Input/Subscriber/CreateSubscriberInput.php @@ -52,7 +52,7 @@ class CreateSubscriberInput #[Assert\All(new Assert\Choice(callback: 'getListResubscribeOnValues'))] private array $list_skip_resubscribe_on = ['unsubscribe', 'bounce', 'complaint']; - public ListRemoveReason $list_remove_reason = ListRemoveReason::UNSUBSCRIBE; + public ListRemovalReason $list_removal_reason = ListRemovalReason::UNSUBSCRIBE; public MetadataStrategy $metadata_strategy = MetadataStrategy::MERGE; diff --git a/backend/src/Api/Console/Input/Subscriber/ListRemoveReason.php b/backend/src/Api/Console/Input/Subscriber/ListRemoveReason.php deleted file mode 100644 index c92c6633..00000000 --- a/backend/src/Api/Console/Input/Subscriber/ListRemoveReason.php +++ /dev/null @@ -1,10 +0,0 @@ -subscriber; + } + + public function getSubscriberOld(): Subscriber + { + return $this->subscriberOld; + } + +} diff --git a/backend/src/Service/Subscriber/Event/SubscriberUpdatingEvent.php b/backend/src/Service/Subscriber/Event/SubscriberUpdatingEvent.php new file mode 100644 index 00000000..5c3197ed --- /dev/null +++ b/backend/src/Service/Subscriber/Event/SubscriberUpdatingEvent.php @@ -0,0 +1,32 @@ +subscriber; + } + + public function getSubscriberOld(): Subscriber + { + return $this->subscriberOld; + } + + public function getListRemovalReason(): ListRemovalReason + { + return $this->listRemovalReason; + } + +} diff --git a/backend/src/Service/Subscriber/ListRemoval/ListRemovalListener.php b/backend/src/Service/Subscriber/ListRemoval/ListRemovalListener.php new file mode 100644 index 00000000..4585be77 --- /dev/null +++ b/backend/src/Service/Subscriber/ListRemoval/ListRemovalListener.php @@ -0,0 +1,48 @@ +getSubscriberOld()->getLists()->map(fn($list) => $list->getId())->toArray(); + $newListIds = $event->getSubscriber()->getLists()->map(fn($list) => $list->getId())->toArray(); + + $removedListIds = array_diff($oldListIds, $newListIds); + + foreach ($removedListIds as $removedListId) { + // PGSQL query with ON CONFLICT to update subscriber_list_removals + + $query = << $removedListId, + 'subscriber_id' => $event->getSubscriber()->getId(), + 'reason' => $event->getListRemovalReason()->value, + 'created_at' => $this->now()->format('Y-m-d H:i:s'), + ]; + + $this->em->getConnection()->executeQuery($query, $params); + } + } + +} diff --git a/backend/src/Service/Subscriber/SubscriberService.php b/backend/src/Service/Subscriber/SubscriberService.php index 1eb3b4ec..7365db11 100644 --- a/backend/src/Service/Subscriber/SubscriberService.php +++ b/backend/src/Service/Subscriber/SubscriberService.php @@ -9,12 +9,15 @@ use App\Entity\Subscriber; use App\Entity\SubscriberExport; use App\Entity\SubscriberListRemoval; +use App\Entity\Type\ListRemovalReason; use App\Entity\Type\SubscriberExportStatus; use App\Entity\Type\SubscriberSource; use App\Entity\Type\SubscriberStatus; use App\Repository\SubscriberRepository; use App\Service\Subscriber\Dto\UpdateSubscriberDto; use App\Service\Subscriber\Event\SubscriberCreatedEvent; +use App\Service\Subscriber\Event\SubscriberUpdatedEvent; +use App\Service\Subscriber\Event\SubscriberUpdatingEvent; use App\Service\Subscriber\Message\ExportSubscribersMessage; use App\Service\Subscriber\Message\SubscriberCreatedMessage; use Doctrine\Common\Collections\ArrayCollection; @@ -153,7 +156,13 @@ public function getSubscribers( public function updateSubscriber( Subscriber $subscriber, UpdateSubscriberDto $updates, + + // if some lists are being removed, set the reason to correctly record + // it in ListRemovalListener + ListRemovalReason $listRemovalReason = ListRemovalReason::UNSUBSCRIBE, ): Subscriber { + $subscriberOld = clone $subscriber; + if ($updates->has('status')) { $subscriber->setStatus($updates->status); } @@ -188,8 +197,24 @@ public function updateSubscriber( $subscriber->setUpdatedAt($this->now()); - $this->em->persist($subscriber); - $this->em->flush(); + $this->em->wrapInTransaction(function () use ($subscriberOld, $subscriber, $listRemovalReason) { + $this->em->persist($subscriber); + $this->ed->dispatch( + new SubscriberUpdatingEvent( + $subscriberOld, + $subscriber, + listRemovalReason: $listRemovalReason, + ), + ); + $this->em->flush(); + }); + + $this->ed->dispatch( + new SubscriberUpdatedEvent( + $subscriberOld, + $subscriber, + ), + ); return $subscriber; } diff --git a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php index bf782b0f..c862f2e5 100644 --- a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php +++ b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php @@ -6,12 +6,15 @@ use App\Entity\Newsletter; use App\Entity\Subscriber; use App\Entity\SubscriberListRemoval; +use App\Entity\Type\ListRemovalReason; use App\Entity\Type\SubscriberMetadataDefinitionType; use App\Entity\Type\SubscriberSource; use App\Entity\Type\SubscriberStatus; -use App\Repository\SubscriberRepository; use App\Service\NewsletterList\NewsletterListService; use App\Service\Subscriber\Event\SubscriberCreatedEvent; +use App\Service\Subscriber\Event\SubscriberUpdatedEvent; +use App\Service\Subscriber\Event\SubscriberUpdatingEvent; +use App\Service\Subscriber\ListRemoval\ListRemovalListener; use App\Service\Subscriber\Message\SubscriberCreatedMessage; use App\Service\Subscriber\SubscriberService; use App\Tests\Case\WebTestCase; @@ -29,7 +32,10 @@ #[CoversClass(SubscriberController::class)] #[CoversClass(SubscriberService::class)] #[CoversClass(SubscriberCreatedEvent::class)] +#[CoversClass(SubscriberUpdatingEvent::class)] +#[CoversClass(SubscriberUpdatedEvent::class)] #[CoversClass(NewsletterListService::class)] +#[CoversClass(ListRemovalListener::class)] class CreateSubscriberTest extends WebTestCase { @@ -280,9 +286,86 @@ public function test_lists_strategy_remove(): void $this->assertContains($list2->getId(), $listIds); } - public function test_metadata_strategy_merge(): void {} + public function test_metadata_strategy_merge(): void + { + $newsletter = NewsletterFactory::createOne(); + + foreach (['a', 'b', 'c'] as $key) { + SubscriberMetadataDefinitionFactory::createOne(['newsletter' => $newsletter, 'key' => $key]); + } + + $subscriber = SubscriberFactory::createOne([ + 'newsletter' => $newsletter, + 'metadata' => ['a' => '1', 'b' => '2'], + ]); + + $this->consoleApi($newsletter, 'POST', '/subscribers', [ + 'email' => $subscriber->getEmail(), + 'metadata' => ['b' => 'updated', 'c' => '3'], + 'metadata_strategy' => 'merge', + ]); + + $this->assertResponseIsSuccessful(); + + refresh($subscriber); + $this->assertSame(['a' => '1', 'b' => 'updated', 'c' => '3'], $subscriber->getMetadata()); + } + + public function test_metadata_strategy_overwrite(): void + { + $newsletter = NewsletterFactory::createOne(); + + foreach (['a', 'b', 'c'] as $key) { + SubscriberMetadataDefinitionFactory::createOne(['newsletter' => $newsletter, 'key' => $key]); + } + + $subscriber = SubscriberFactory::createOne([ + 'newsletter' => $newsletter, + 'metadata' => ['a' => '1', 'b' => '2'], + ]); + + $this->consoleApi($newsletter, 'POST', '/subscribers', [ + 'email' => $subscriber->getEmail(), + 'metadata' => ['c' => '3'], + 'metadata_strategy' => 'overwrite', + ]); + + $this->assertResponseIsSuccessful(); + + refresh($subscriber); + $this->assertSame(['c' => '3'], $subscriber->getMetadata()); + } + + #[TestWith([ListRemovalReason::UNSUBSCRIBE])] + #[TestWith([ListRemovalReason::BOUNCE])] + public function test_records_list_removal(ListRemovalReason $reason): void + { + $newsletter = NewsletterFactory::createOne(); + $list1 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + $list2 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + + $subscriber = SubscriberFactory::createOne([ + 'newsletter' => $newsletter, + 'lists' => [$list1, $list2], + ]); + + $this->consoleApi($newsletter, 'POST', '/subscribers', [ + 'email' => $subscriber->getEmail(), + 'lists' => [], + 'lists_strategy' => 'overwrite', + 'list_removal_reason' => $reason->value, + ]); + // make sure the records are recorded + } + + public function test_updates_list_removal(): void + { + // test ON CONFLICT + } + + public function test_list_removal_make_sure_adding_lists_is_not_recorded(): void {} - public function test_metadata_strategy_overwrite(): void {} + public function test_list_removal_with_strategy_remove(): void {} public function testCreateSubscriberWithListsById(): void { diff --git a/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte b/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte index a6973e14..a30a6601 100644 --- a/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte +++ b/frontend/src/routes/(marketing)/docs/[...slug]/content/ConsoleApi.svelte @@ -378,7 +378,7 @@ // define the reason for removing the subscriber from a list // (only when updating, see below for more info) // default: 'unsubscribe' - list_remove_reason?: 'unsubscribe' | 'bounce' | 'other'; + list_removal_reason?: 'unsubscribe' | 'bounce' | 'other'; // whether to overwrite or merge the subscriber's metadata // when updating an existing subscriber. @@ -420,7 +420,7 @@

    - list_remove_reason: + list_removal_reason:

      @@ -489,7 +489,7 @@ "lists_strategy": "remove", // unsubscribe, bounce, or other - "list_remove_reason": "unsubscribe" + "list_removal_reason": "unsubscribe" } `} /> From 8564977475b1b70471cb5967ae041abb8eef5a36 Mon Sep 17 00:00:00 2001 From: Supun Wimalasena Date: Tue, 3 Mar 2026 15:58:20 +0100 Subject: [PATCH 17/23] wip --- .../Message/SendConfirmationEmailMessage.php | 19 +++ .../Message/SubscriberCreatedMessage.php | 20 --- .../SendConfirmationEmailMessageHandler.php | 89 ++++++++++++ .../SubscriberCreatedMessageHandler.php | 128 ------------------ .../Service/Subscriber/SubscriberService.php | 2 - .../newsletter/mail/confirm.json.twig | 44 ++++++ .../Subscriber/CreateSubscriberTest.php | 108 ++++++++++++++- .../SubscriberCreatedMessageHandlerTest.php | 10 +- 8 files changed, 259 insertions(+), 161 deletions(-) create mode 100644 backend/src/Service/Subscriber/Message/SendConfirmationEmailMessage.php delete mode 100644 backend/src/Service/Subscriber/Message/SubscriberCreatedMessage.php create mode 100644 backend/src/Service/Subscriber/MessageHandler/SendConfirmationEmailMessageHandler.php delete mode 100644 backend/src/Service/Subscriber/MessageHandler/SubscriberCreatedMessageHandler.php create mode 100644 backend/templates/newsletter/mail/confirm.json.twig diff --git a/backend/src/Service/Subscriber/Message/SendConfirmationEmailMessage.php b/backend/src/Service/Subscriber/Message/SendConfirmationEmailMessage.php new file mode 100644 index 00000000..7759fe50 --- /dev/null +++ b/backend/src/Service/Subscriber/Message/SendConfirmationEmailMessage.php @@ -0,0 +1,19 @@ +subscriberId; + } +} diff --git a/backend/src/Service/Subscriber/Message/SubscriberCreatedMessage.php b/backend/src/Service/Subscriber/Message/SubscriberCreatedMessage.php deleted file mode 100644 index 0e84ed5c..00000000 --- a/backend/src/Service/Subscriber/Message/SubscriberCreatedMessage.php +++ /dev/null @@ -1,20 +0,0 @@ -subscriberExportId; - } -} diff --git a/backend/src/Service/Subscriber/MessageHandler/SendConfirmationEmailMessageHandler.php b/backend/src/Service/Subscriber/MessageHandler/SendConfirmationEmailMessageHandler.php new file mode 100644 index 00000000..3d5e5e4e --- /dev/null +++ b/backend/src/Service/Subscriber/MessageHandler/SendConfirmationEmailMessageHandler.php @@ -0,0 +1,89 @@ +em->getRepository(Subscriber::class)->find($message->getSubscriberId()); + assert($subscriber !== null); + $newsletter = $subscriber->getNewsletter(); + + if ($subscriber->getStatus() !== SubscriberStatus::PENDING) { + // If the subscriber is not pending, we do not send a confirmation email. + return; + } + + $data = [ + 'subscriber_id' => $subscriber->getId(), + 'expires_at' => $this->now()->add(new \DateInterval('P1D'))->format('Y-m-d H:i:s'), + ]; + + $token = $this->encryption->encrypt($data); + + $strings = $this->stringsFactory->create(); + + $heading = $strings->get('mail.subscriberConfirmation.heading'); + + $variables = $this->templateVariableService->variablesFromNewsletter($newsletter); + + $content = $this->twig->render('newsletter/mail/config.json', [ + 'newsletterName' => $newsletter->getName(), + 'buttonUrl' => $this->newsletterService->getArchiveUrl($newsletter) . "/confirm?token=" . $token, + 'buttonText' => $strings->get('mail.subscriberConfirmation.buttonText'), + ]); + + $variables->subject = $heading; + $variables->content = $this->contentService->getHtmlFromJson($content); + + $template = $this->templateService->getTemplateStringFromNewsletter($newsletter); + + $email = new Email(); + $this->sendingProfileService->setSendingProfileToEmail( + $email, + $this->sendingProfileService->getCurrentDefaultSendingProfileOfNewsletter($newsletter), + ); + + $email + ->to($subscriber->getEmail()) + ->html($this->htmlTemplateRenderer->render($template, $variables)) + ->subject($heading . ' to ' . $newsletter->getName()); + $this->relayApiClient->sendEmail($email); + } +} diff --git a/backend/src/Service/Subscriber/MessageHandler/SubscriberCreatedMessageHandler.php b/backend/src/Service/Subscriber/MessageHandler/SubscriberCreatedMessageHandler.php deleted file mode 100644 index b471182e..00000000 --- a/backend/src/Service/Subscriber/MessageHandler/SubscriberCreatedMessageHandler.php +++ /dev/null @@ -1,128 +0,0 @@ -em->getRepository(Subscriber::class)->find($message->getSubscriberId()); - assert($subscriber !== null); - $newsletter = $subscriber->getNewsletter(); - - if ($subscriber->getStatus() !== SubscriberStatus::PENDING) { - // If the subscriber is not pending, we do not send a confirmation email. - return; - } - - $data = [ - 'subscriber_id' => $subscriber->getId(), - 'expires_at' => $this->now()->add(new \DateInterval('P1D'))->format('Y-m-d H:i:s'), - ]; - - $token = $this->encryption->encrypt($data); - - $strings = $this->stringsFactory->create(); - - $heading = $strings->get('mail.subscriberConfirmation.heading'); - - $variables = $this->templateVariableService->variablesFromNewsletter($newsletter); - - $content = (string)json_encode([ - 'type' => 'doc', - 'content' => [ - [ - 'type' => 'paragraph', - 'content' => [ - [ - 'type' => 'text', - 'text' => 'Hey 👋,', - ], - ], - ], - [ - 'type' => 'paragraph', - 'content' => [ - [ - 'type' => 'text', - 'text' => 'Thank you for subscribing to ' . $newsletter->getName() . '! To confirm your subscription and start receiving updates, please click the button below.', - ], - ], - ], - [ - 'type' => 'button', - 'attrs' => [ - 'href' => $this->newsletterService->getArchiveUrl($newsletter) . "/confirm?token=" . $token, - ], - 'content' => [ - [ - 'type' => 'text', - 'text' => $strings->get('mail.subscriberConfirmation.buttonText'), - ], - ], - ], - [ - 'type' => 'paragraph', - 'content' => [ - [ - 'type' => 'text', - 'text' => 'This link will expire in 24 hours. If you did not request or expect this invitation, you can safely ignore this email.', - ], - ], - ], - ], - ]); - - $variables->subject = $heading; - $variables->content = $this->contentService->getHtmlFromJson($content); - - $template = $this->templateService->getTemplateStringFromNewsletter($newsletter); - - $email = new Email(); - $this->sendingProfileService->setSendingProfileToEmail( - $email, - $this->sendingProfileService->getCurrentDefaultSendingProfileOfNewsletter($newsletter) - ); - - $email->to($subscriber->getEmail()) - ->html($this->htmlTemplateRenderer->render($template, $variables)) - ->subject($heading . ' to ' . $newsletter->getName()); - $this->relayApiClient->sendEmail($email); - } -} diff --git a/backend/src/Service/Subscriber/SubscriberService.php b/backend/src/Service/Subscriber/SubscriberService.php index 7365db11..23d05e72 100644 --- a/backend/src/Service/Subscriber/SubscriberService.php +++ b/backend/src/Service/Subscriber/SubscriberService.php @@ -19,12 +19,10 @@ use App\Service\Subscriber\Event\SubscriberUpdatedEvent; use App\Service\Subscriber\Event\SubscriberUpdatingEvent; use App\Service\Subscriber\Message\ExportSubscribersMessage; -use App\Service\Subscriber\Message\SubscriberCreatedMessage; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\ORM\EntityManagerInterface; use Symfony\Component\Clock\ClockAwareTrait; use Symfony\Component\EventDispatcher\EventDispatcherInterface; -use Symfony\Component\Messenger\MessageBusInterface; class SubscriberService { diff --git a/backend/templates/newsletter/mail/confirm.json.twig b/backend/templates/newsletter/mail/confirm.json.twig new file mode 100644 index 00000000..2e32289c --- /dev/null +++ b/backend/templates/newsletter/mail/confirm.json.twig @@ -0,0 +1,44 @@ +{ + "type": "doc", + "content": [ + { + "type": "paragraph", + "content": [ + { + "type": "text", + "text": "Hey 👋," + } + ] + }, + { + "type": "paragraph", + "content": [ + { + "type": "text", + "text": "Thank you for subscribing to {{ newsletterName }}! To confirm your subscription and start receiving updates, please click the button below." + } + ] + }, + { + "type": "button", + "attrs": { + "href": "{{ buttonUrl }}" + }, + "content": [ + { + "type": "text", + "text": "{{ buttonText }}" + } + ] + }, + { + "type": "paragraph", + "content": [ + { + "type": "text", + "text": "This link will expire in 24 hours. If you did not request or expect this invitation, you can safely ignore this email." + } + ] + } + ] +} diff --git a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php index c862f2e5..60c6e17d 100644 --- a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php +++ b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php @@ -15,7 +15,7 @@ use App\Service\Subscriber\Event\SubscriberUpdatedEvent; use App\Service\Subscriber\Event\SubscriberUpdatingEvent; use App\Service\Subscriber\ListRemoval\ListRemovalListener; -use App\Service\Subscriber\Message\SubscriberCreatedMessage; +use App\Service\Subscriber\Message\SendConfirmationEmailMessage; use App\Service\Subscriber\SubscriberService; use App\Tests\Case\WebTestCase; use App\Tests\Factory\NewsletterFactory; @@ -355,17 +355,113 @@ public function test_records_list_removal(ListRemovalReason $reason): void 'lists_strategy' => 'overwrite', 'list_removal_reason' => $reason->value, ]); - // make sure the records are recorded + + $this->assertResponseIsSuccessful(); + + $repo = $this->em->getRepository(SubscriberListRemoval::class); + foreach ([$list1, $list2] as $list) { + $record = $repo->findOneBy(['list' => $list, 'subscriber' => $subscriber]); + $this->assertInstanceOf(SubscriberListRemoval::class, $record); + $this->assertSame($reason->value, $record->getReason()); + } } public function test_updates_list_removal(): void { - // test ON CONFLICT + $newsletter = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + + $subscriber = SubscriberFactory::createOne([ + 'newsletter' => $newsletter, + 'lists' => [$list], + ]); + + // First removal: UNSUBSCRIBE + $this->consoleApi($newsletter, 'POST', '/subscribers', [ + 'email' => $subscriber->getEmail(), + 'lists' => [], + 'lists_strategy' => 'overwrite', + 'list_removal_reason' => 'unsubscribe', + ]); + + // Re-add the list + $this->consoleApi($newsletter, 'POST', '/subscribers', [ + 'email' => $subscriber->getEmail(), + 'lists' => [$list->getId()], + 'lists_strategy' => 'overwrite', + ]); + + // Second removal: BOUNCE + $this->consoleApi($newsletter, 'POST', '/subscribers', [ + 'email' => $subscriber->getEmail(), + 'lists' => [], + 'lists_strategy' => 'overwrite', + 'list_removal_reason' => 'bounce', + ]); + + $this->assertResponseIsSuccessful(); + + $records = $this->em->getRepository(SubscriberListRemoval::class)->findBy([ + 'list' => $list->_real(), + 'subscriber' => $subscriber->_real(), + ]); + + $this->assertCount(1, $records); + $this->assertSame('bounce', $records[0]->getReason()); } - public function test_list_removal_make_sure_adding_lists_is_not_recorded(): void {} + public function test_list_removal_make_sure_adding_lists_is_not_recorded(): void + { + $newsletter = NewsletterFactory::createOne(); + $list = NewsletterListFactory::createOne(['newsletter' => $newsletter]); - public function test_list_removal_with_strategy_remove(): void {} + $subscriber = SubscriberFactory::createOne(['newsletter' => $newsletter]); + + $this->consoleApi($newsletter, 'POST', '/subscribers', [ + 'email' => $subscriber->getEmail(), + 'lists' => [$list->getId()], + 'lists_strategy' => 'overwrite', + ]); + + $this->assertResponseIsSuccessful(); + + $records = $this->em->getRepository(SubscriberListRemoval::class)->findBy([ + 'subscriber' => $subscriber->_real(), + ]); + $this->assertCount(0, $records); + } + + public function test_list_removal_with_strategy_remove(): void + { + $newsletter = NewsletterFactory::createOne(); + $list1 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + $list2 = NewsletterListFactory::createOne(['newsletter' => $newsletter]); + + $subscriber = SubscriberFactory::createOne([ + 'newsletter' => $newsletter, + 'lists' => [$list1, $list2], + ]); + + $this->consoleApi($newsletter, 'POST', '/subscribers', [ + 'email' => $subscriber->getEmail(), + 'lists' => [$list1->getId()], + 'lists_strategy' => 'remove', + 'list_removal_reason' => 'unsubscribe', + ]); + + $this->assertResponseIsSuccessful(); + + refresh($subscriber); + $this->assertCount(1, $subscriber->getLists()); + $this->assertSame($list2->getId(), $subscriber->getLists()->first()->getId()); + + $record = $this->em->getRepository(SubscriberListRemoval::class)->findOneBy([ + 'list' => $list1->_real(), + 'subscriber' => $subscriber->_real(), + ]); + $this->assertInstanceOf(SubscriberListRemoval::class, $record); + $this->assertSame('unsubscribe', $record->getReason()); + } public function testCreateSubscriberWithListsById(): void { @@ -657,7 +753,7 @@ public function testSendPendingConfirmationEmail(): void $transport = $this->transport('async'); $transport->queue()->assertCount(1); $message = $transport->queue()->first()->getMessage(); - $this->assertInstanceOf(SubscriberCreatedMessage::class, $message); + $this->assertInstanceOf(SendConfirmationEmailMessage::class, $message); } public function testNoConfirmationEmailByDefault(): void diff --git a/backend/tests/Api/Console/Subscriber/SubscriberCreatedMessageHandlerTest.php b/backend/tests/Api/Console/Subscriber/SubscriberCreatedMessageHandlerTest.php index 55af18c1..5cd50234 100644 --- a/backend/tests/Api/Console/Subscriber/SubscriberCreatedMessageHandlerTest.php +++ b/backend/tests/Api/Console/Subscriber/SubscriberCreatedMessageHandlerTest.php @@ -4,8 +4,8 @@ use App\Entity\Subscriber; use App\Entity\Type\SubscriberStatus; -use App\Service\Subscriber\Message\SubscriberCreatedMessage; -use App\Service\Subscriber\MessageHandler\SubscriberCreatedMessageHandler; +use App\Service\Subscriber\Message\SendConfirmationEmailMessage; +use App\Service\Subscriber\MessageHandler\SendConfirmationEmailMessageHandler; use App\Tests\Case\KernelTestCase; use App\Tests\Factory\NewsletterFactory; use App\Tests\Factory\NewsletterListFactory; @@ -17,8 +17,8 @@ use Symfony\Component\Clock\Test\ClockSensitiveTrait; use Symfony\Component\HttpClient\Response\JsonMockResponse; -#[CoversClass(SubscriberCreatedMessageHandler::class)] -#[CoversClass(SubscriberCreatedMessage::class)] +#[CoversClass(SendConfirmationEmailMessageHandler::class)] +#[CoversClass(SendConfirmationEmailMessage::class)] class SubscriberCreatedMessageHandlerTest extends KernelTestCase { use ClockSensitiveTrait; @@ -65,7 +65,7 @@ public function test_send_confirmation_email_for_pending_subscriber(): void $this->mockRelayClient($callback); - $message = new SubscriberCreatedMessage($subscriber->getId()); + $message = new SendConfirmationEmailMessage($subscriber->getId()); $this->getMessageBus()->dispatch($message); $transport = $this->transport('async'); From 4cdc90b73a0e51d1ff4ba6796f6f6dfc55627182 Mon Sep 17 00:00:00 2001 From: Supun Wimalasena Date: Tue, 3 Mar 2026 16:07:14 +0100 Subject: [PATCH 18/23] record list removal --- backend/src/Entity/SubscriberListRemoval.php | 9 ++-- .../Subscriber/CreateSubscriberTest.php | 43 ++++++++----------- backend/tests/Factory/ApiKeyFactory.php | 10 ++--- backend/tests/Factory/ApprovalFactory.php | 6 +-- backend/tests/Factory/DomainFactory.php | 10 ++--- backend/tests/Factory/IssueFactory.php | 10 ++--- backend/tests/Factory/MediaFactory.php | 6 +-- backend/tests/Factory/NewsletterFactory.php | 10 ++--- .../tests/Factory/NewsletterListFactory.php | 15 +++---- backend/tests/Factory/SendFactory.php | 10 ++--- .../tests/Factory/SendingProfileFactory.php | 10 ++--- .../tests/Factory/SubscriberExportFactory.php | 15 +++---- backend/tests/Factory/SubscriberFactory.php | 6 +-- .../tests/Factory/SubscriberImportFactory.php | 10 ++--- .../Factory/SubscriberListRemovalFactory.php | 9 ++-- .../SubscriberMetadataDefinitionFactory.php | 8 ++-- backend/tests/Factory/TemplateFactory.php | 15 +++---- backend/tests/Factory/UserFactory.php | 15 +++---- backend/tests/Factory/UserInviteFactory.php | 15 +++---- 19 files changed, 99 insertions(+), 133 deletions(-) diff --git a/backend/src/Entity/SubscriberListRemoval.php b/backend/src/Entity/SubscriberListRemoval.php index e0caa8f8..e00f87ae 100644 --- a/backend/src/Entity/SubscriberListRemoval.php +++ b/backend/src/Entity/SubscriberListRemoval.php @@ -2,6 +2,7 @@ namespace App\Entity; +use App\Entity\Type\ListRemovalReason; use Doctrine\ORM\Mapping as ORM; #[ORM\Entity] @@ -22,8 +23,8 @@ class SubscriberListRemoval #[ORM\JoinColumn(name: 'subscriber_id', nullable: false, onDelete: 'CASCADE')] private Subscriber $subscriber; - #[ORM\Column(type: 'string')] - private string $reason; + #[ORM\Column(enumType: ListRemovalReason::class)] + private ListRemovalReason $reason; #[ORM\Column] private \DateTimeImmutable $created_at; @@ -60,12 +61,12 @@ public function setSubscriber(Subscriber $subscriber): static return $this; } - public function getReason(): string + public function getReason(): ListRemovalReason { return $this->reason; } - public function setReason(string $reason): static + public function setReason(ListRemovalReason $reason): static { $this->reason = $reason; return $this; diff --git a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php index 60c6e17d..a816c9bf 100644 --- a/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php +++ b/backend/tests/Api/Console/Subscriber/CreateSubscriberTest.php @@ -362,7 +362,7 @@ public function test_records_list_removal(ListRemovalReason $reason): void foreach ([$list1, $list2] as $list) { $record = $repo->findOneBy(['list' => $list, 'subscriber' => $subscriber]); $this->assertInstanceOf(SubscriberListRemoval::class, $record); - $this->assertSame($reason->value, $record->getReason()); + $this->assertSame($reason, $record->getReason()); } } @@ -376,22 +376,12 @@ public function test_updates_list_removal(): void 'lists' => [$list], ]); - // First removal: UNSUBSCRIBE - $this->consoleApi($newsletter, 'POST', '/subscribers', [ - 'email' => $subscriber->getEmail(), - 'lists' => [], - 'lists_strategy' => 'overwrite', - 'list_removal_reason' => 'unsubscribe', - ]); - - // Re-add the list - $this->consoleApi($newsletter, 'POST', '/subscribers', [ - 'email' => $subscriber->getEmail(), - 'lists' => [$list->getId()], - 'lists_strategy' => 'overwrite', + $removal = SubscriberListRemovalFactory::createOne([ + 'subscriber' => $subscriber, + 'list' => $list, + 'reason' => ListRemovalReason::OTHER, ]); - // Second removal: BOUNCE $this->consoleApi($newsletter, 'POST', '/subscribers', [ 'email' => $subscriber->getEmail(), 'lists' => [], @@ -401,13 +391,14 @@ public function test_updates_list_removal(): void $this->assertResponseIsSuccessful(); - $records = $this->em->getRepository(SubscriberListRemoval::class)->findBy([ - 'list' => $list->_real(), - 'subscriber' => $subscriber->_real(), - ]); + $this->em->clear(); + $records = $this->em->getRepository(SubscriberListRemoval::class)->findAll(); $this->assertCount(1, $records); - $this->assertSame('bounce', $records[0]->getReason()); + $this->assertSame($removal->getId(), $records[0]->getId()); + $this->assertSame(ListRemovalReason::BOUNCE, $records[0]->getReason()); + $this->assertSame($subscriber->getId(), $records[0]->getSubscriber()->getId()); + $this->assertSame($list->getId(), $records[0]->getList()->getId()); } public function test_list_removal_make_sure_adding_lists_is_not_recorded(): void @@ -426,7 +417,7 @@ public function test_list_removal_make_sure_adding_lists_is_not_recorded(): void $this->assertResponseIsSuccessful(); $records = $this->em->getRepository(SubscriberListRemoval::class)->findBy([ - 'subscriber' => $subscriber->_real(), + 'subscriber' => $subscriber, ]); $this->assertCount(0, $records); } @@ -446,21 +437,21 @@ public function test_list_removal_with_strategy_remove(): void 'email' => $subscriber->getEmail(), 'lists' => [$list1->getId()], 'lists_strategy' => 'remove', - 'list_removal_reason' => 'unsubscribe', + 'list_removal_reason' => 'other', ]); $this->assertResponseIsSuccessful(); refresh($subscriber); $this->assertCount(1, $subscriber->getLists()); - $this->assertSame($list2->getId(), $subscriber->getLists()->first()->getId()); + $this->assertSame($list2->getId(), $subscriber->getLists()[0]?->getId()); $record = $this->em->getRepository(SubscriberListRemoval::class)->findOneBy([ - 'list' => $list1->_real(), - 'subscriber' => $subscriber->_real(), + 'list' => $list1, + 'subscriber' => $subscriber, ]); $this->assertInstanceOf(SubscriberListRemoval::class, $record); - $this->assertSame('unsubscribe', $record->getReason()); + $this->assertSame(ListRemovalReason::OTHER, $record->getReason()); } public function testCreateSubscriberWithListsById(): void diff --git a/backend/tests/Factory/ApiKeyFactory.php b/backend/tests/Factory/ApiKeyFactory.php index 85766d13..70d1405c 100644 --- a/backend/tests/Factory/ApiKeyFactory.php +++ b/backend/tests/Factory/ApiKeyFactory.php @@ -4,12 +4,12 @@ use App\Api\Console\Authorization\Scope; use App\Entity\ApiKey; -use Zenstruck\Foundry\Persistence\PersistentProxyObjectFactory; +use Zenstruck\Foundry\Persistence\PersistentObjectFactory; /** - * @extends PersistentProxyObjectFactory + * @extends PersistentObjectFactory */ -final class ApiKeyFactory extends PersistentProxyObjectFactory +final class ApiKeyFactory extends PersistentObjectFactory { public function __construct() { @@ -34,8 +34,8 @@ protected function defaults(): array 'scopes' => [ ...array_map( fn(Scope $scope) => $scope->value, - Scope::cases() - ) + Scope::cases(), + ), ], 'is_enabled' => true, 'last_accessed_at' => null, diff --git a/backend/tests/Factory/ApprovalFactory.php b/backend/tests/Factory/ApprovalFactory.php index 82c29588..39e8b5b0 100644 --- a/backend/tests/Factory/ApprovalFactory.php +++ b/backend/tests/Factory/ApprovalFactory.php @@ -4,12 +4,12 @@ use App\Entity\Approval; use App\Entity\Type\ApprovalStatus; -use Zenstruck\Foundry\Persistence\PersistentProxyObjectFactory; +use Zenstruck\Foundry\Persistence\PersistentObjectFactory; /** - * @extends PersistentProxyObjectFactory + * @extends PersistentObjectFactory */ -final class ApprovalFactory extends PersistentProxyObjectFactory +final class ApprovalFactory extends PersistentObjectFactory { public function __construct() { diff --git a/backend/tests/Factory/DomainFactory.php b/backend/tests/Factory/DomainFactory.php index dce4535e..1efbe5af 100644 --- a/backend/tests/Factory/DomainFactory.php +++ b/backend/tests/Factory/DomainFactory.php @@ -4,21 +4,19 @@ use App\Entity\Domain; use App\Entity\Type\RelayDomainStatus; -use Zenstruck\Foundry\Persistence\PersistentProxyObjectFactory; +use Zenstruck\Foundry\Persistence\PersistentObjectFactory; /** - * @extends PersistentProxyObjectFactory + * @extends PersistentObjectFactory */ -final class DomainFactory extends PersistentProxyObjectFactory +final class DomainFactory extends PersistentObjectFactory { /** * @see https://symfony.com/bundles/ZenstruckFoundryBundle/current/index.html#factories-as-services * * @todo inject services if required */ - public function __construct() - { - } + public function __construct() {} public static function class(): string { diff --git a/backend/tests/Factory/IssueFactory.php b/backend/tests/Factory/IssueFactory.php index b70e2acd..3da90685 100644 --- a/backend/tests/Factory/IssueFactory.php +++ b/backend/tests/Factory/IssueFactory.php @@ -5,21 +5,19 @@ use App\Entity\Issue; use App\Entity\Type\IssueStatus; use Symfony\Component\Uid\Uuid; -use Zenstruck\Foundry\Persistence\PersistentProxyObjectFactory; +use Zenstruck\Foundry\Persistence\PersistentObjectFactory; /** - * @extends PersistentProxyObjectFactory + * @extends PersistentObjectFactory */ -final class IssueFactory extends PersistentProxyObjectFactory +final class IssueFactory extends PersistentObjectFactory { /** * @see https://symfony.com/bundles/ZenstruckFoundryBundle/current/index.html#factories-as-services * * @todo inject services if required */ - public function __construct() - { - } + public function __construct() {} public static function class(): string { diff --git a/backend/tests/Factory/MediaFactory.php b/backend/tests/Factory/MediaFactory.php index 0abf7bdf..464680a1 100644 --- a/backend/tests/Factory/MediaFactory.php +++ b/backend/tests/Factory/MediaFactory.php @@ -4,12 +4,12 @@ use App\Entity\Media; use App\Entity\Type\MediaFolder; -use Zenstruck\Foundry\Persistence\PersistentProxyObjectFactory; +use Zenstruck\Foundry\Persistence\PersistentObjectFactory; /** - * @extends PersistentProxyObjectFactory + * @extends PersistentObjectFactory */ -final class MediaFactory extends PersistentProxyObjectFactory +final class MediaFactory extends PersistentObjectFactory { public static function class(): string diff --git a/backend/tests/Factory/NewsletterFactory.php b/backend/tests/Factory/NewsletterFactory.php index c2bdebda..dd302db7 100644 --- a/backend/tests/Factory/NewsletterFactory.php +++ b/backend/tests/Factory/NewsletterFactory.php @@ -4,21 +4,19 @@ use App\Entity\Meta\NewsletterMeta; use App\Entity\Newsletter; -use Zenstruck\Foundry\Persistence\PersistentProxyObjectFactory; +use Zenstruck\Foundry\Persistence\PersistentObjectFactory; /** - * @extends PersistentProxyObjectFactory + * @extends PersistentObjectFactory */ -final class NewsletterFactory extends PersistentProxyObjectFactory +final class NewsletterFactory extends PersistentObjectFactory { /** * @see https://symfony.com/bundles/ZenstruckFoundryBundle/current/index.html#factories-as-services * * @todo inject services if required */ - public function __construct() - { - } + public function __construct() {} public static function class(): string { diff --git a/backend/tests/Factory/NewsletterListFactory.php b/backend/tests/Factory/NewsletterListFactory.php index 6d94d596..5304e9b4 100644 --- a/backend/tests/Factory/NewsletterListFactory.php +++ b/backend/tests/Factory/NewsletterListFactory.php @@ -3,21 +3,19 @@ namespace App\Tests\Factory; use App\Entity\NewsletterList; -use Zenstruck\Foundry\Persistence\PersistentProxyObjectFactory; +use Zenstruck\Foundry\Persistence\PersistentObjectFactory; /** - * @extends PersistentProxyObjectFactory + * @extends PersistentObjectFactory */ -final class NewsletterListFactory extends PersistentProxyObjectFactory +final class NewsletterListFactory extends PersistentObjectFactory { /** * @see https://symfony.com/bundles/ZenstruckFoundryBundle/current/index.html#factories-as-services * * @todo inject services if required */ - public function __construct() - { - } + public function __construct() {} public static function class(): string { @@ -45,8 +43,7 @@ protected function defaults(): array */ protected function initialize(): static { - return $this - // ->afterInstantiate(function(NewsletterList $newsletterList): void {}) - ; + return $this// ->afterInstantiate(function(NewsletterList $newsletterList): void {}) + ; } } diff --git a/backend/tests/Factory/SendFactory.php b/backend/tests/Factory/SendFactory.php index 4fb9edfa..0d945449 100644 --- a/backend/tests/Factory/SendFactory.php +++ b/backend/tests/Factory/SendFactory.php @@ -5,21 +5,19 @@ use App\Entity\Send; use App\Entity\Type\IssueStatus; use App\Entity\Type\SendStatus; -use Zenstruck\Foundry\Persistence\PersistentProxyObjectFactory; +use Zenstruck\Foundry\Persistence\PersistentObjectFactory; /** - * @extends PersistentProxyObjectFactory + * @extends PersistentObjectFactory */ -final class SendFactory extends PersistentProxyObjectFactory +final class SendFactory extends PersistentObjectFactory { /** * @see https://symfony.com/bundles/ZenstruckFoundryBundle/current/index.html#factories-as-services * * @todo inject services if required */ - public function __construct() - { - } + public function __construct() {} public static function class(): string { diff --git a/backend/tests/Factory/SendingProfileFactory.php b/backend/tests/Factory/SendingProfileFactory.php index 0c128019..99411e11 100644 --- a/backend/tests/Factory/SendingProfileFactory.php +++ b/backend/tests/Factory/SendingProfileFactory.php @@ -3,21 +3,19 @@ namespace App\Tests\Factory; use App\Entity\SendingProfile; -use Zenstruck\Foundry\Persistence\PersistentProxyObjectFactory; +use Zenstruck\Foundry\Persistence\PersistentObjectFactory; /** - * @extends PersistentProxyObjectFactory + * @extends PersistentObjectFactory */ -final class SendingProfileFactory extends PersistentProxyObjectFactory +final class SendingProfileFactory extends PersistentObjectFactory { /** * @see https://symfony.com/bundles/ZenstruckFoundryBundle/current/index.html#factories-as-services * * @todo inject services if required */ - public function __construct() - { - } + public function __construct() {} public static function class(): string { diff --git a/backend/tests/Factory/SubscriberExportFactory.php b/backend/tests/Factory/SubscriberExportFactory.php index 3317d34b..27326be0 100644 --- a/backend/tests/Factory/SubscriberExportFactory.php +++ b/backend/tests/Factory/SubscriberExportFactory.php @@ -4,21 +4,19 @@ use App\Entity\SubscriberExport; use App\Entity\Type\SubscriberExportStatus; -use Zenstruck\Foundry\Persistence\PersistentProxyObjectFactory; +use Zenstruck\Foundry\Persistence\PersistentObjectFactory; /** - * @extends PersistentProxyObjectFactory + * @extends PersistentObjectFactory */ -final class SubscriberExportFactory extends PersistentProxyObjectFactory +final class SubscriberExportFactory extends PersistentObjectFactory { /** * @see https://symfony.com/bundles/ZenstruckFoundryBundle/current/index.html#factories-as-services * * @todo inject services if required */ - public function __construct() - { - } + public function __construct() {} public static function class(): string { @@ -45,8 +43,7 @@ protected function defaults(): array */ protected function initialize(): static { - return $this - // ->afterInstantiate(function(Domain $domain): void {}) - ; + return $this// ->afterInstantiate(function(Domain $domain): void {}) + ; } } diff --git a/backend/tests/Factory/SubscriberFactory.php b/backend/tests/Factory/SubscriberFactory.php index 45a657e4..809f194a 100644 --- a/backend/tests/Factory/SubscriberFactory.php +++ b/backend/tests/Factory/SubscriberFactory.php @@ -5,12 +5,12 @@ use App\Entity\Subscriber; use App\Entity\Type\SubscriberSource; use App\Entity\Type\SubscriberStatus; -use Zenstruck\Foundry\Persistence\PersistentProxyObjectFactory; +use Zenstruck\Foundry\Persistence\PersistentObjectFactory; /** - * @extends PersistentProxyObjectFactory + * @extends PersistentObjectFactory */ -final class SubscriberFactory extends PersistentProxyObjectFactory +final class SubscriberFactory extends PersistentObjectFactory { /** * @see https://symfony.com/bundles/ZenstruckFoundryBundle/current/index.html#factories-as-services diff --git a/backend/tests/Factory/SubscriberImportFactory.php b/backend/tests/Factory/SubscriberImportFactory.php index 3beabedd..febd31a1 100644 --- a/backend/tests/Factory/SubscriberImportFactory.php +++ b/backend/tests/Factory/SubscriberImportFactory.php @@ -4,21 +4,19 @@ use App\Entity\SubscriberImport; use App\Entity\Type\SubscriberImportStatus; -use Zenstruck\Foundry\Persistence\PersistentProxyObjectFactory; +use Zenstruck\Foundry\Persistence\PersistentObjectFactory; /** - * @extends PersistentProxyObjectFactory + * @extends PersistentObjectFactory */ -final class SubscriberImportFactory extends PersistentProxyObjectFactory +final class SubscriberImportFactory extends PersistentObjectFactory { /** * @see https://symfony.com/bundles/ZenstruckFoundryBundle/current/index.html#factories-as-services * * @todo inject services if required */ - public function __construct() - { - } + public function __construct() {} public static function class(): string { diff --git a/backend/tests/Factory/SubscriberListRemovalFactory.php b/backend/tests/Factory/SubscriberListRemovalFactory.php index a18a877d..f56a9705 100644 --- a/backend/tests/Factory/SubscriberListRemovalFactory.php +++ b/backend/tests/Factory/SubscriberListRemovalFactory.php @@ -3,12 +3,13 @@ namespace App\Tests\Factory; use App\Entity\SubscriberListRemoval; -use Zenstruck\Foundry\Persistence\PersistentProxyObjectFactory; +use App\Entity\Type\ListRemovalReason; +use Zenstruck\Foundry\Persistence\PersistentObjectFactory; /** - * @extends PersistentProxyObjectFactory + * @extends PersistentObjectFactory */ -final class SubscriberListRemovalFactory extends PersistentProxyObjectFactory +final class SubscriberListRemovalFactory extends PersistentObjectFactory { public function __construct() {} @@ -25,7 +26,7 @@ protected function defaults(): array return [ 'list' => NewsletterListFactory::new(), 'subscriber' => SubscriberFactory::new(), - 'reason' => self::faker()->randomElement(['unsubscribe', 'bounce', 'other']), + 'reason' => self::faker()->randomElement(ListRemovalReason::cases()), 'created_at' => \DateTimeImmutable::createFromMutable(self::faker()->dateTime()), ]; } diff --git a/backend/tests/Factory/SubscriberMetadataDefinitionFactory.php b/backend/tests/Factory/SubscriberMetadataDefinitionFactory.php index cdedcc87..86b5271f 100644 --- a/backend/tests/Factory/SubscriberMetadataDefinitionFactory.php +++ b/backend/tests/Factory/SubscriberMetadataDefinitionFactory.php @@ -4,12 +4,12 @@ use App\Entity\SubscriberMetadataDefinition; use App\Entity\Type\SubscriberMetadataDefinitionType; -use Zenstruck\Foundry\Persistence\PersistentProxyObjectFactory; +use Zenstruck\Foundry\Persistence\PersistentObjectFactory; /** - * @extends PersistentProxyObjectFactory + * @extends PersistentObjectFactory */ -final class SubscriberMetadataDefinitionFactory extends PersistentProxyObjectFactory +final class SubscriberMetadataDefinitionFactory extends PersistentObjectFactory { public static function class(): string @@ -34,4 +34,4 @@ protected function defaults(): array ]; } -} \ No newline at end of file +} diff --git a/backend/tests/Factory/TemplateFactory.php b/backend/tests/Factory/TemplateFactory.php index d0258863..538af452 100644 --- a/backend/tests/Factory/TemplateFactory.php +++ b/backend/tests/Factory/TemplateFactory.php @@ -6,21 +6,19 @@ use App\Entity\Template; use App\Entity\Type\IssueStatus; use App\Entity\Type\SendStatus; -use Zenstruck\Foundry\Persistence\PersistentProxyObjectFactory; +use Zenstruck\Foundry\Persistence\PersistentObjectFactory; /** - * @extends PersistentProxyObjectFactory