diff --git a/migrations/Version20261109074654.php b/migrations/Version20261109074654.php new file mode 100644 index 00000000..e65e757a --- /dev/null +++ b/migrations/Version20261109074654.php @@ -0,0 +1,34 @@ +addSql('ALTER TABLE travel_mailing_list ADD bus_leg VARCHAR(20) DEFAULT NULL, ADD name VARCHAR(64) DEFAULT NULL'); + // Every existing list holds all participants and is named by its travel code alone. + $this->addSql('UPDATE travel_mailing_list SET name = travel_code'); + $this->addSql('ALTER TABLE travel_mailing_list MODIFY name VARCHAR(64) NOT NULL'); + $this->addSql('CREATE UNIQUE INDEX UNIQ_68DC6E285E237E06 ON travel_mailing_list (name)'); + $this->addSql('ALTER TABLE travel_mailing_list DROP INDEX UNIQ_68DC6E28BC4DD335, ADD INDEX IDX_68DC6E28BC4DD335 (travel_code)'); + } + + public function down(Schema $schema): void + { + // Fails while a travel has two lists; delete the bus-only one first. + $this->addSql('ALTER TABLE travel_mailing_list DROP INDEX IDX_68DC6E28BC4DD335, ADD UNIQUE INDEX UNIQ_68DC6E28BC4DD335 (travel_code)'); + $this->addSql('DROP INDEX UNIQ_68DC6E285E237E06 ON travel_mailing_list'); + $this->addSql('ALTER TABLE travel_mailing_list DROP bus_leg, DROP name'); + } +} diff --git a/src/BpnConnect/Model/TravelMailingResponse.php b/src/BpnConnect/Model/TravelMailingResponse.php index ea9fa591..bc79dce2 100644 --- a/src/BpnConnect/Model/TravelMailingResponse.php +++ b/src/BpnConnect/Model/TravelMailingResponse.php @@ -4,6 +4,8 @@ declare(strict_types=1); namespace App\BpnConnect\Model; +use App\Enum\BusLeg; + readonly class TravelMailingResponse { /** @@ -12,6 +14,7 @@ readonly class TravelMailingResponse public function __construct( public string $travelCode, public array $data, + public ?BusLeg $busLeg = null, ) { } } diff --git a/src/BpnConnect/TravelsClient.php b/src/BpnConnect/TravelsClient.php index 407c0059..a02139b2 100644 --- a/src/BpnConnect/TravelsClient.php +++ b/src/BpnConnect/TravelsClient.php @@ -4,8 +4,10 @@ declare(strict_types=1); namespace App\BpnConnect; +use App\BpnConnect\Exception\BpnConnectException; use App\BpnConnect\Model\TravelMailingRecipient; use App\BpnConnect\Model\TravelMailingResponse; +use App\Enum\BusLeg; class TravelsClient extends AbstractApiClient { @@ -14,12 +16,30 @@ class TravelsClient extends AbstractApiClient /** * An unknown travel code yields an empty list, not an error: bpn-connect cannot tell it * apart from a travel without confirmed participants. + * + * @param ?BusLeg $busLeg only participants who booked this bus leg, plus their bookings' applicants */ - public function getMailingRecipients(string $travelCode): TravelMailingResponse + public function getMailingRecipients(string $travelCode, ?BusLeg $busLeg = null): TravelMailingResponse { - $payload = $this->request(self::MAILING_PATH, [ - 'travelCode' => $travelCode, - ]); + $query = ['travelCode' => $travelCode]; + if (null !== $busLeg) { + $query['busLeg'] = $busLeg->value; + } + + $payload = $this->request(self::MAILING_PATH, $query); + + // A bpn-connect that predates the filter ignores the parameter and returns everyone, which + // must not end up on a list meant for bus passengers only. + $appliedBusLeg = $payload['meta']['bus_leg'] ?? null; + if ($appliedBusLeg !== $busLeg?->value) { + $this->logger->error('BpnConnect ignored the travel mailing bus filter', [ + 'travel_code' => $travelCode, + 'requested_bus_leg' => $busLeg?->value, + 'applied_bus_leg' => $appliedBusLeg, + ]); + + throw new BpnConnectException('BpnConnect did not apply the requested bus leg filter.'); + } $recipients = array_map( static fn (array $item) => new TravelMailingRecipient( @@ -30,6 +50,6 @@ class TravelsClient extends AbstractApiClient $payload['data'] ?? [], ); - return new TravelMailingResponse($payload['meta']['travel_code'] ?? $travelCode, $recipients); + return new TravelMailingResponse($payload['meta']['travel_code'] ?? $travelCode, $recipients, $busLeg); } } diff --git a/src/Command/CleanupTravelMailingListsCommand.php b/src/Command/CleanupTravelMailingListsCommand.php index 4adc377c..2862d414 100644 --- a/src/Command/CleanupTravelMailingListsCommand.php +++ b/src/Command/CleanupTravelMailingListsCommand.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace App\Command; +use App\Entity\TravelMailingList; use App\Exception\NewsletterProviderException; use App\Repository\TravelMailingListRepository; use App\Service\TravelMailingListManager; @@ -39,20 +40,29 @@ class CleanupTravelMailingListsCommand extends Command return Command::SUCCESS; } + // A travel's lists are deleted together (see TravelMailingListManager::delete()), including + // one that is not due yet because the travel's start date moved after it was created. + $travelCodes = array_unique(array_map(static fn (TravelMailingList $mailingList) => $mailingList->getTravelCode(), $mailingLists)); + $failed = 0; - foreach ($mailingLists as $mailingList) { + $deletedLists = 0; + foreach ($travelCodes as $travelCode) { + $travelLists = $this->travelMailingListRepository->findByTravelCode($travelCode); + $listIds = implode(', ', array_map(static fn (TravelMailingList $mailingList) => $mailingList->getMailjetListId(), $travelLists)); + try { - $deletedContacts = $this->travelMailingListManager->delete($mailingList); - $io->writeln(sprintf('Deleted %s (list %d) and %d orphaned contact(s).', $mailingList->getTravelCode(), $mailingList->getMailjetListId(), $deletedContacts)); + $deletedContacts = $this->travelMailingListManager->delete($travelLists); + $deletedLists += count($travelLists); + $io->writeln(sprintf('Deleted %s (lists %s) and %d orphaned contact(s).', $travelCode, $listIds, $deletedContacts)); } catch (NewsletterProviderException $exception) { - // The row stays, so the next run picks the list up again. + // The rows of lists not yet deleted stay, so the next run picks them up again. ++$failed; - $this->logger->error('Delete travel mailing list failed', [ - 'travel_code' => $mailingList->getTravelCode(), - 'mailjet_list_id' => $mailingList->getMailjetListId(), + $this->logger->error('Delete travel mailing lists failed', [ + 'travel_code' => $travelCode, + 'mailjet_list_ids' => $listIds, 'error' => $exception->getMessage(), ]); - $io->error(sprintf('%s (list %d): %s', $mailingList->getTravelCode(), $mailingList->getMailjetListId(), $exception->getMessage())); + $io->error(sprintf('%s (lists %s): %s', $travelCode, $listIds, $exception->getMessage())); } } @@ -60,7 +70,7 @@ class CleanupTravelMailingListsCommand extends Command return Command::FAILURE; } - $io->success(sprintf('Deleted %d travel mailing list(s).', count($mailingLists))); + $io->success(sprintf('Deleted %d travel mailing list(s).', $deletedLists)); return Command::SUCCESS; } diff --git a/src/Controller/Admin/TravelMailing/CreateListController.php b/src/Controller/Admin/TravelMailing/CreateListController.php index 21611d8f..f039c4b1 100644 --- a/src/Controller/Admin/TravelMailing/CreateListController.php +++ b/src/Controller/Admin/TravelMailing/CreateListController.php @@ -7,7 +7,9 @@ namespace App\Controller\Admin\TravelMailing; use App\BpnConnect\Exception\BpnConnectException; use App\BpnConnect\TravelsClient; use App\BusProNet\Utility\DateCodeUtility; +use App\Entity\TravelMailingList; use App\Exception\NewsletterProviderException; +use App\Form\Model\TravelMailingLookupDto; use App\Repository\TravelMailingListRepository; use App\Service\TravelDataProvider; use App\Service\TravelMailingListManager; @@ -39,11 +41,19 @@ class CreateListController extends AbstractController throw $this->createAccessDeniedException('Invalid CSRF token.'); } - $travelCode = (new DateCodeUtility())->sanitize($request->request->getString('travelCode')); - $returnUrl = $this->generateUrl('app_admin_travelmailing', ['travelCode' => $travelCode]); + $lookup = new TravelMailingLookupDto(); + $lookup->travelCode = (new DateCodeUtility())->sanitize($request->request->getString('travelCode')); + $lookup->busOnly = $request->request->getBoolean('busOnly'); + $travelCode = $lookup->travelCode; + $busLeg = $lookup->getBusLeg(); + $returnUrl = $this->generateUrl('app_admin_travelmailing', array_filter([ + 'travelCode' => $travelCode, + 'busOnly' => $lookup->busOnly ? 1 : null, + ])); - if (null !== $this->travelMailingListRepository->findOneByTravelCode($travelCode)) { - $this->addFlash('error', sprintf('Für %s gibt es bereits eine Mailjet-Liste.', $travelCode)); + $name = TravelMailingList::nameFor($travelCode, $busLeg); + if (null !== $this->travelMailingListRepository->findOneByName($name)) { + $this->addFlash('error', sprintf('Die Mailjet-Liste %s gibt es bereits.', $name)); return $this->redirect($returnUrl); } @@ -57,7 +67,7 @@ class CreateListController extends AbstractController // Fetched again rather than posted from the page, so the list holds what BusPro reports now. try { - $recipients = $this->travelsClient->getMailingRecipients($travelCode)->data; + $recipients = $this->travelsClient->getMailingRecipients($travelCode, $busLeg)->data; } catch (BpnConnectException) { $this->addFlash('error', 'Die Teilnehmerliste konnte nicht von BusPro abgerufen werden. Bitte später erneut versuchen.'); @@ -71,7 +81,7 @@ class CreateListController extends AbstractController } try { - $mailingList = $this->travelMailingListManager->create($travelCode, $recipients, $travelStartsAt); + $mailingList = $this->travelMailingListManager->create($travelCode, $recipients, $busLeg, $travelStartsAt); } catch (NewsletterProviderException $exception) { $this->adminLogger->error('Create travel mailing list failed', [ 'travel_code' => $travelCode, @@ -85,9 +95,10 @@ class CreateListController extends AbstractController $this->adminLogger->info('Create travel mailing list', [ 'travel_code' => $travelCode, 'mailjet_list_id' => $mailingList->getMailjetListId(), + 'bus_leg' => $busLeg?->value, 'recipients' => count($recipients), ]); - $this->addFlash('success', sprintf('Die Mailjet-Liste %s wurde mit %d Empfängern angelegt.', $travelCode, count($recipients))); + $this->addFlash('success', sprintf('Die Mailjet-Liste %s wurde mit %d Empfängern angelegt.', $mailingList->getName(), count($recipients))); return $this->redirect($returnUrl); } diff --git a/src/Controller/Admin/TravelMailing/IndexController.php b/src/Controller/Admin/TravelMailing/IndexController.php index eec53f3a..f4cef21e 100644 --- a/src/Controller/Admin/TravelMailing/IndexController.php +++ b/src/Controller/Admin/TravelMailing/IndexController.php @@ -7,6 +7,7 @@ namespace App\Controller\Admin\TravelMailing; use App\BpnConnect\Exception\BpnConnectException; use App\BpnConnect\TravelsClient; use App\BusProNet\Utility\DateCodeUtility; +use App\Entity\TravelMailingList; use App\Form\Admin\TravelMailingLookupType; use App\Form\Model\TravelMailingLookupDto; use App\Repository\TravelMailingListRepository; @@ -39,7 +40,9 @@ class IndexController extends AbstractController $result = null; $travelLabel = null; $travelStartsAt = null; + $mailingLists = []; $mailingList = null; + $listName = null; $failed = false; if ($form->isSubmitted() && $form->isValid()) { // The local index matches codes exactly, so "lpjmp-281126" has to become "LPJMP281126" @@ -47,10 +50,17 @@ class IndexController extends AbstractController $travelCode = (new DateCodeUtility())->sanitize((string) $lookup->travelCode); try { - $result = $this->travelsClient->getMailingRecipients($travelCode); + $result = $this->travelsClient->getMailingRecipients($travelCode, $lookup->getBusLeg()); $travelLabel = $this->travelDataProvider->mapDateCodeToLabel($travelCode); $travelStartsAt = $this->travelDataProvider->mapDateCodeToStartDate($travelCode); - $mailingList = $this->travelMailingListRepository->findOneByTravelCode($travelCode); + $mailingLists = $this->travelMailingListRepository->findByTravelCode($travelCode); + // The list matching the current filter, if one exists, replaces the create button. + $listName = TravelMailingList::nameFor($travelCode, $lookup->getBusLeg()); + foreach ($mailingLists as $existing) { + if ($listName === $existing->getName()) { + $mailingList = $existing; + } + } } catch (BpnConnectException) { // The client has already logged the cause; the page only needs to say it failed. $failed = true; @@ -62,7 +72,9 @@ class IndexController extends AbstractController 'result' => $result, 'travel_label' => $travelLabel, 'travel_starts_at' => $travelStartsAt, + 'mailing_lists' => $mailingLists, 'mailing_list' => $mailingList, + 'list_name' => $listName, 'failed' => $failed, ]); } diff --git a/src/Entity/TravelMailingList.php b/src/Entity/TravelMailingList.php index b8afcb8d..207df9bd 100644 --- a/src/Entity/TravelMailingList.php +++ b/src/Entity/TravelMailingList.php @@ -4,41 +4,63 @@ declare(strict_types=1); namespace App\Entity; +use App\Enum\BusLeg; use App\Repository\TravelMailingListRepository; use Doctrine\ORM\Mapping as ORM; /** * A Mailjet contact list created for one travel's participants. The row exists so the list can * be deleted again once the travel is past; the list itself is never updated after creation. + * + * A travel can have two lists, one for all participants and one for bus passengers only, told + * apart by their name. */ #[ORM\Entity(repositoryClass: TravelMailingListRepository::class)] +#[ORM\Index(columns: ['travel_code'])] class TravelMailingList { + private const string BUS_ONLY_SUFFIX = ' nur Bus'; + #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column(type: 'integer')] private ?int $id = null; - #[ORM\Column(type: 'string', length: 32, unique: true)] + #[ORM\Column(type: 'string', length: 32)] private string $travelCode; + /** The Mailjet list name. */ + #[ORM\Column(type: 'string', length: 64, unique: true)] + private string $name; + #[ORM\Column(type: 'integer')] private int $mailjetListId; + /** The bus leg its participants were filtered by; null when it holds all participants. */ + #[ORM\Column(length: 20, nullable: true, enumType: BusLeg::class)] + private ?BusLeg $busLeg; + #[ORM\Column(type: 'datetime_immutable')] private \DateTimeImmutable $deleteAfter; #[ORM\Column(type: 'datetime_immutable')] private \DateTimeImmutable $createdAt; - public function __construct(string $travelCode, int $mailjetListId, \DateTimeImmutable $deleteAfter) + public function __construct(string $travelCode, int $mailjetListId, ?BusLeg $busLeg, \DateTimeImmutable $deleteAfter) { $this->travelCode = $travelCode; + $this->name = self::nameFor($travelCode, $busLeg); $this->mailjetListId = $mailjetListId; + $this->busLeg = $busLeg; $this->deleteAfter = $deleteAfter; $this->createdAt = new \DateTimeImmutable(); } + public static function nameFor(string $travelCode, ?BusLeg $busLeg): string + { + return null !== $busLeg ? $travelCode.self::BUS_ONLY_SUFFIX : $travelCode; + } + public function getId(): ?int { return $this->id; @@ -49,11 +71,21 @@ class TravelMailingList return $this->travelCode; } + public function getName(): string + { + return $this->name; + } + public function getMailjetListId(): int { return $this->mailjetListId; } + public function getBusLeg(): ?BusLeg + { + return $this->busLeg; + } + public function getDeleteAfter(): \DateTimeImmutable { return $this->deleteAfter; diff --git a/src/Enum/BusLeg.php b/src/Enum/BusLeg.php new file mode 100644 index 00000000..112580f4 --- /dev/null +++ b/src/Enum/BusLeg.php @@ -0,0 +1,27 @@ + 'Busreisende (Hinfahrt)', + self::Inbound => 'Busreisende (Rückfahrt)', + self::Any => 'Busreisende (Hin- oder Rückfahrt)', + self::Both => 'Busreisende (Hin- und Rückfahrt)', + }; + } +} diff --git a/src/Form/Admin/TravelMailingLookupType.php b/src/Form/Admin/TravelMailingLookupType.php index d2dd243e..5a9087c1 100644 --- a/src/Form/Admin/TravelMailingLookupType.php +++ b/src/Form/Admin/TravelMailingLookupType.php @@ -6,13 +6,14 @@ namespace App\Form\Admin; use App\Form\Model\TravelMailingLookupDto; use Symfony\Component\Form\AbstractType; +use Symfony\Component\Form\Extension\Core\Type\CheckboxType; use Symfony\Component\Form\Extension\Core\Type\TextType; use Symfony\Component\Form\FormBuilderInterface; use Symfony\Component\OptionsResolver\OptionsResolver; /** * Submits as GET with an empty block prefix, so a lookup is the plain URL `?travelCode=…` - * and can be bookmarked or shared. + * (plus `&busOnly=1`) and can be bookmarked or shared. * * @extends AbstractType */ @@ -26,6 +27,10 @@ class TravelMailingLookupType extends AbstractType 'placeholder' => 'z. B. LPJMP281126', ], ]); + $builder->add('busOnly', CheckboxType::class, [ + 'label' => 'nur '.TravelMailingLookupDto::BUS_LEG->label(), + 'required' => false, + ]); } public function configureOptions(OptionsResolver $resolver): void diff --git a/src/Form/Model/TravelMailingLookupDto.php b/src/Form/Model/TravelMailingLookupDto.php index 324d4fff..3772a86a 100644 --- a/src/Form/Model/TravelMailingLookupDto.php +++ b/src/Form/Model/TravelMailingLookupDto.php @@ -4,10 +4,23 @@ declare(strict_types=1); namespace App\Form\Model; +use App\Enum\BusLeg; use Symfony\Component\Validator\Constraints as Assert; class TravelMailingLookupDto { + /** + * The leg "bus passengers only" stands for: whoever takes the bus out, whatever their return. + */ + public const BusLeg BUS_LEG = BusLeg::Outbound; + #[Assert\NotBlank(message: 'Bitte einen Reisecode angeben.')] public ?string $travelCode = null; + + public bool $busOnly = false; + + public function getBusLeg(): ?BusLeg + { + return $this->busOnly ? self::BUS_LEG : null; + } } diff --git a/src/Repository/TravelMailingListRepository.php b/src/Repository/TravelMailingListRepository.php index 69f839bd..81500f99 100644 --- a/src/Repository/TravelMailingListRepository.php +++ b/src/Repository/TravelMailingListRepository.php @@ -18,9 +18,17 @@ class TravelMailingListRepository extends ServiceEntityRepository parent::__construct($registry, TravelMailingList::class); } - public function findOneByTravelCode(string $travelCode): ?TravelMailingList + public function findOneByName(string $name): ?TravelMailingList { - return $this->findOneBy(['travelCode' => $travelCode]); + return $this->findOneBy(['name' => $name]); + } + + /** + * @return list + */ + public function findByTravelCode(string $travelCode): array + { + return $this->findBy(['travelCode' => $travelCode], ['id' => 'ASC']); } /** diff --git a/src/Service/TravelMailingListManager.php b/src/Service/TravelMailingListManager.php index 8849f5c4..0609f52e 100644 --- a/src/Service/TravelMailingListManager.php +++ b/src/Service/TravelMailingListManager.php @@ -6,14 +6,16 @@ namespace App\Service; use App\BpnConnect\Model\TravelMailingRecipient; use App\Entity\TravelMailingList; +use App\Enum\BusLeg; use App\Exception\NewsletterProviderException; use App\Mailjet\ApiClient; use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; /** - * Creates one Mailjet contact list per travel and deletes it again 30 days after the travel - * started, together with every contact that is on no other Mailjet list. + * Creates Mailjet contact lists for a travel (all participants, bus passengers only) and deletes + * them again 30 days after the travel started, together with every contact that is on no other + * Mailjet list. */ class TravelMailingListManager { @@ -28,16 +30,17 @@ class TravelMailingListManager /** * @param TravelMailingRecipient[] $recipients + * @param ?BusLeg $busLeg the bus leg the recipients were filtered by, if any * * @throws NewsletterProviderException */ - public function create(string $travelCode, array $recipients, \DateTimeImmutable $travelStartsAt): TravelMailingList + public function create(string $travelCode, array $recipients, ?BusLeg $busLeg, \DateTimeImmutable $travelStartsAt): TravelMailingList { - $listId = $this->mailjet->createContactsList($travelCode); + $listId = $this->mailjet->createContactsList(TravelMailingList::nameFor($travelCode, $busLeg)); // Tracked before the contacts go in, so a list Mailjet already holds is never left without a row // that the cleanup would find. - $mailingList = new TravelMailingList($travelCode, $listId, $travelStartsAt->modify(self::DELETE_AFTER)); + $mailingList = new TravelMailingList($travelCode, $listId, $busLeg, $travelStartsAt->modify(self::DELETE_AFTER)); $this->entityManager->persist($mailingList); $this->entityManager->flush(); @@ -51,8 +54,10 @@ class TravelMailingListManager $this->logger->info('Created Mailjet travel mailing list', [ 'travel_code' => $travelCode, + 'name' => $mailingList->getName(), 'mailjet_list_id' => $listId, 'mailjet_job_id' => $jobId, + 'bus_leg' => $busLeg?->value, 'recipients' => count($recipients), ]); @@ -60,22 +65,32 @@ class TravelMailingListManager } /** - * Deletes the orphaned contacts first and the list last, so a run that fails halfway can be - * repeated: the list still names whoever is left. + * Deletes all lists of one travel together. Taken one at a time, a contact on both would + * never count as orphaned: Mailjet still reports a list for 60 days after its deletion. + * + * The orphaned contacts go first and the lists last, so a run that fails halfway can be + * repeated: the lists still name whoever is left. + * + * @param TravelMailingList[] $mailingLists * * @return int the number of contacts deleted * * @throws NewsletterProviderException */ - public function delete(TravelMailingList $mailingList): int + public function delete(array $mailingLists): int { - $listId = $mailingList->getMailjetListId(); + $listIds = array_map(static fn (TravelMailingList $mailingList) => $mailingList->getMailjetListId(), $mailingLists); + + $contactIds = []; + foreach ($listIds as $listId) { + array_push($contactIds, ...$this->mailjet->getListContactIds($listId)); + } $deletedContacts = 0; - foreach ($this->mailjet->getListContactIds($listId) as $contactId) { + foreach (array_unique($contactIds) as $contactId) { // Anyone who is, or recently was, on another list is kept: their newsletter consent or // another travel's mailing still needs the contact. - if ([$listId] !== array_values(array_unique($this->mailjet->getContactListIds($contactId)))) { + if ([] !== array_diff($this->mailjet->getContactListIds($contactId), $listIds)) { continue; } @@ -83,17 +98,24 @@ class TravelMailingListManager ++$deletedContacts; } - $this->mailjet->deleteContactsList($listId); - - $this->entityManager->remove($mailingList); - $this->entityManager->flush(); - - $this->logger->info('Deleted Mailjet travel mailing list', [ - 'travel_code' => $mailingList->getTravelCode(), - 'mailjet_list_id' => $listId, + $this->logger->info('Deleted orphaned travel mailing contacts', [ + 'mailjet_list_ids' => $listIds, 'deleted_contacts' => $deletedContacts, ]); + foreach ($mailingLists as $mailingList) { + $this->mailjet->deleteContactsList($mailingList->getMailjetListId()); + + $this->entityManager->remove($mailingList); + $this->entityManager->flush(); + + $this->logger->info('Deleted Mailjet travel mailing list', [ + 'travel_code' => $mailingList->getTravelCode(), + 'name' => $mailingList->getName(), + 'mailjet_list_id' => $mailingList->getMailjetListId(), + ]); + } + return $deletedContacts; } @@ -129,6 +151,7 @@ class TravelMailingListManager // The row stays, so the scheduled cleanup removes the list later. $this->logger->error('Could not delete Mailjet travel mailing list after a failed import', [ 'travel_code' => $mailingList->getTravelCode(), + 'name' => $mailingList->getName(), 'mailjet_list_id' => $mailingList->getMailjetListId(), 'error' => $exception->getMessage(), ]); diff --git a/templates/admin/travel_mailing/index.html.twig b/templates/admin/travel_mailing/index.html.twig index d9fc4b72..853fb1c5 100644 --- a/templates/admin/travel_mailing/index.html.twig +++ b/templates/admin/travel_mailing/index.html.twig @@ -9,6 +9,9 @@
{{ form_row(form.travelCode) }}
+
+ {{ form_row(form.busOnly) }} +
Alle Empfänger werden ohne Opt-in eingetragen; die Liste wird 30 Tage nach Reisebeginn diff --git a/tests/BpnConnect/TravelsClientTest.php b/tests/BpnConnect/TravelsClientTest.php index 584eb46c..cc0c5a68 100644 --- a/tests/BpnConnect/TravelsClientTest.php +++ b/tests/BpnConnect/TravelsClientTest.php @@ -7,6 +7,7 @@ namespace App\Tests\BpnConnect; use App\BpnConnect\Exception\BpnConnectException; use App\BpnConnect\Model\TravelMailingRecipient; use App\BpnConnect\TravelsClient; +use App\Enum\BusLeg; use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; use Symfony\Component\HttpClient\MockHttpClient; @@ -35,12 +36,49 @@ class TravelsClientTest extends TestCase self::assertSame('https://bpn-connect.test/api/v1/travels/mailing?travelCode=LPJMP281126', $requests[0][1]); self::assertContains('X-API-KEY: secret', $requests[0][2]['headers']); self::assertSame('LPJMP281126', $response->travelCode); + self::assertNull($response->busLeg); self::assertEquals([ new TravelMailingRecipient('erika.muster@example.com', 'Erika', 'Muster'), new TravelMailingRecipient('max.muster@example.com', 'Max', 'Muster'), ], $response->data); } + public function testGetMailingRecipientsPassesTheBusLegAndAcceptsItsEcho(): void + { + $requests = []; + $http = new MockHttpClient(function (string $method, string $url) use (&$requests): MockResponse { + $requests[] = $url; + + return $this->jsonResponse([ + 'meta' => ['travel_code' => 'LPJMP281126', 'bus_leg' => 'outbound', 'row_count' => 1, 'execution_time_ms' => 12], + 'data' => [ + ['email' => 'erika.muster@example.com', 'first_name' => 'Erika', 'last_name' => 'Muster'], + ], + ]); + }); + + $response = $this->client($http)->getMailingRecipients('LPJMP281126', BusLeg::Outbound); + + self::assertSame(['https://bpn-connect.test/api/v1/travels/mailing?travelCode=LPJMP281126&busLeg=outbound'], $requests); + self::assertSame(BusLeg::Outbound, $response->busLeg); + self::assertCount(1, $response->data); + } + + public function testABusLegTheServerDidNotApplyIsRejected(): void + { + // What a bpn-connect without the filter answers: everyone, and no bus_leg in meta. + $http = new MockHttpClient($this->jsonResponse([ + 'meta' => ['travel_code' => 'LPJMP281126', 'row_count' => 1, 'execution_time_ms' => 12], + 'data' => [ + ['email' => 'erika.muster@example.com', 'first_name' => 'Erika', 'last_name' => 'Muster'], + ], + ])); + + $this->expectException(BpnConnectException::class); + + $this->client($http)->getMailingRecipients('LPJMP281126', BusLeg::Outbound); + } + public function testAnUnknownTravelCodeYieldsAnEmptyList(): void { $http = new MockHttpClient($this->jsonResponse([ diff --git a/tests/Service/TravelMailingListManagerTest.php b/tests/Service/TravelMailingListManagerTest.php index a8daed0d..4b8ba2ee 100644 --- a/tests/Service/TravelMailingListManagerTest.php +++ b/tests/Service/TravelMailingListManagerTest.php @@ -6,6 +6,7 @@ namespace App\Tests\Service; use App\BpnConnect\Model\TravelMailingRecipient; use App\Entity\TravelMailingList; +use App\Enum\BusLeg; use App\Exception\NewsletterProviderException; use App\Mailjet\ApiClient; use App\Service\TravelMailingListManager; @@ -18,7 +19,7 @@ class TravelMailingListManagerTest extends TestCase public function testCreateTracksTheListAndAddsDeduplicatedContacts(): void { $mailjet = $this->createMock(ApiClient::class); - $mailjet->expects(self::once())->method('createContactsList')->with('LPJMP281126')->willReturn(42); + $mailjet->expects(self::once())->method('createContactsList')->with('LPJMP281126 nur Bus')->willReturn(42); $mailjet->expects(self::once())->method('addContactsToList')->with(42, [ ['email' => 'mia@example.com', 'firstName' => 'Mia'], ['email' => 'tom@example.com', 'firstName' => null], @@ -32,9 +33,11 @@ class TravelMailingListManagerTest extends TestCase new TravelMailingRecipient('Mia@Example.com', 'Mia', 'Muster'), new TravelMailingRecipient('mia@example.com', 'Lea', 'Muster'), new TravelMailingRecipient('tom@example.com', '', 'Test'), - ], new \DateTimeImmutable('2026-11-28')); + ], BusLeg::Outbound, new \DateTimeImmutable('2026-11-28')); self::assertSame(42, $mailingList->getMailjetListId()); + self::assertSame(BusLeg::Outbound, $mailingList->getBusLeg()); + self::assertSame('LPJMP281126 nur Bus', $mailingList->getName()); self::assertEquals(new \DateTimeImmutable('2026-12-28'), $mailingList->getDeleteAfter()); } @@ -52,7 +55,7 @@ class TravelMailingListManagerTest extends TestCase (new TravelMailingListManager($mailjet, $entityManager, new NullLogger()))->create('LPJMP281126', [ new TravelMailingRecipient('mia@example.com', 'Mia', 'Muster'), - ], new \DateTimeImmutable('2026-11-28')); + ], null, new \DateTimeImmutable('2026-11-28')); } public function testDeleteRemovesOnlyContactsOnNoOtherList(): void @@ -70,7 +73,33 @@ class TravelMailingListManagerTest extends TestCase $entityManager->expects(self::once())->method('remove'); $deleted = (new TravelMailingListManager($mailjet, $entityManager, new NullLogger())) - ->delete(new TravelMailingList('LPJMP281126', 42, new \DateTimeImmutable('2026-12-28'))); + ->delete([new TravelMailingList('LPJMP281126', 42, null, new \DateTimeImmutable('2026-12-28'))]); + + self::assertSame(1, $deleted); + } + + public function testDeleteTreatsTheTravelsOtherListAsItsOwn(): void + { + // 1 is on both lists of the travel, 2 also on a newsletter list. + $mailjet = $this->createMock(ApiClient::class); + $mailjet->method('getListContactIds')->willReturnMap([ + [42, [1, 2]], + [43, [1]], + ]); + $mailjet->method('getContactListIds')->willReturnMap([ + [1, [42, 43]], + [2, [42, 10321569]], + ]); + $mailjet->expects(self::once())->method('deleteContact')->with(1); + $mailjet->expects(self::exactly(2))->method('deleteContactsList'); + + $entityManager = $this->createMock(EntityManagerInterface::class); + $entityManager->expects(self::exactly(2))->method('remove'); + + $deleted = (new TravelMailingListManager($mailjet, $entityManager, new NullLogger()))->delete([ + new TravelMailingList('LPJMP281126', 42, null, new \DateTimeImmutable('2026-12-28')), + new TravelMailingList('LPJMP281126', 43, BusLeg::Outbound, new \DateTimeImmutable('2026-12-28')), + ]); self::assertSame(1, $deleted); } @@ -87,6 +116,6 @@ class TravelMailingListManagerTest extends TestCase $this->expectException(NewsletterProviderException::class); (new TravelMailingListManager($mailjet, $entityManager, new NullLogger())) - ->delete(new TravelMailingList('LPJMP281126', 42, new \DateTimeImmutable('2026-12-28'))); + ->delete([new TravelMailingList('LPJMP281126', 42, null, new \DateTimeImmutable('2026-12-28'))]); } }