feat: add bus-only travel mailing lists

This commit is contained in:
fromme committed 2026-11-09 15:53:53 +01:00
1 parent f5894a6bef
commit 2705401393
15 files changed
+335 -60

No files matched your search

+34
View File
@@ -0,0 +1,34 @@
<?php
declare(strict_types=1);
namespace DoctrineMigrations;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;
final class Version20261109074654 extends AbstractMigration
{
public function getDescription(): string
{
return 'Allows a bus-only travel mailing list next to the full one: adds bus_leg and the list name, which replaces travel_code as the unique key.';
}
public function up(Schema $schema): void
{
$this->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');
}
}
@@ -4,6 +4,8 @@ declare(strict_types=1);
namespace App\BpnConnect\Model; namespace App\BpnConnect\Model;
use App\Enum\BusLeg;
readonly class TravelMailingResponse readonly class TravelMailingResponse
{ {
/** /**
@@ -12,6 +14,7 @@ readonly class TravelMailingResponse
public function __construct( public function __construct(
public string $travelCode, public string $travelCode,
public array $data, public array $data,
public ?BusLeg $busLeg = null,
) { ) {
} }
} }
+25 -5
View File
@@ -4,8 +4,10 @@ declare(strict_types=1);
namespace App\BpnConnect; namespace App\BpnConnect;
use App\BpnConnect\Exception\BpnConnectException;
use App\BpnConnect\Model\TravelMailingRecipient; use App\BpnConnect\Model\TravelMailingRecipient;
use App\BpnConnect\Model\TravelMailingResponse; use App\BpnConnect\Model\TravelMailingResponse;
use App\Enum\BusLeg;
class TravelsClient extends AbstractApiClient 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 * An unknown travel code yields an empty list, not an error: bpn-connect cannot tell it
* apart from a travel without confirmed participants. * 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, [ $query = ['travelCode' => $travelCode];
'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( $recipients = array_map(
static fn (array $item) => new TravelMailingRecipient( static fn (array $item) => new TravelMailingRecipient(
@@ -30,6 +50,6 @@ class TravelsClient extends AbstractApiClient
$payload['data'] ?? [], $payload['data'] ?? [],
); );
return new TravelMailingResponse($payload['meta']['travel_code'] ?? $travelCode, $recipients); return new TravelMailingResponse($payload['meta']['travel_code'] ?? $travelCode, $recipients, $busLeg);
} }
} }
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace App\Command; namespace App\Command;
use App\Entity\TravelMailingList;
use App\Exception\NewsletterProviderException; use App\Exception\NewsletterProviderException;
use App\Repository\TravelMailingListRepository; use App\Repository\TravelMailingListRepository;
use App\Service\TravelMailingListManager; use App\Service\TravelMailingListManager;
@@ -39,20 +40,29 @@ class CleanupTravelMailingListsCommand extends Command
return Command::SUCCESS; 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; $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 { try {
$deletedContacts = $this->travelMailingListManager->delete($mailingList); $deletedContacts = $this->travelMailingListManager->delete($travelLists);
$io->writeln(sprintf('Deleted %s (list %d) and %d orphaned contact(s).', $mailingList->getTravelCode(), $mailingList->getMailjetListId(), $deletedContacts)); $deletedLists += count($travelLists);
$io->writeln(sprintf('Deleted %s (lists %s) and %d orphaned contact(s).', $travelCode, $listIds, $deletedContacts));
} catch (NewsletterProviderException $exception) { } 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; ++$failed;
$this->logger->error('Delete travel mailing list failed', [ $this->logger->error('Delete travel mailing lists failed', [
'travel_code' => $mailingList->getTravelCode(), 'travel_code' => $travelCode,
'mailjet_list_id' => $mailingList->getMailjetListId(), 'mailjet_list_ids' => $listIds,
'error' => $exception->getMessage(), '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; 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; return Command::SUCCESS;
} }
@@ -7,7 +7,9 @@ namespace App\Controller\Admin\TravelMailing;
use App\BpnConnect\Exception\BpnConnectException; use App\BpnConnect\Exception\BpnConnectException;
use App\BpnConnect\TravelsClient; use App\BpnConnect\TravelsClient;
use App\BusProNet\Utility\DateCodeUtility; use App\BusProNet\Utility\DateCodeUtility;
use App\Entity\TravelMailingList;
use App\Exception\NewsletterProviderException; use App\Exception\NewsletterProviderException;
use App\Form\Model\TravelMailingLookupDto;
use App\Repository\TravelMailingListRepository; use App\Repository\TravelMailingListRepository;
use App\Service\TravelDataProvider; use App\Service\TravelDataProvider;
use App\Service\TravelMailingListManager; use App\Service\TravelMailingListManager;
@@ -39,11 +41,19 @@ class CreateListController extends AbstractController
throw $this->createAccessDeniedException('Invalid CSRF token.'); throw $this->createAccessDeniedException('Invalid CSRF token.');
} }
$travelCode = (new DateCodeUtility())->sanitize($request->request->getString('travelCode')); $lookup = new TravelMailingLookupDto();
$returnUrl = $this->generateUrl('app_admin_travelmailing', ['travelCode' => $travelCode]); $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)) { $name = TravelMailingList::nameFor($travelCode, $busLeg);
$this->addFlash('error', sprintf('Für %s gibt es bereits eine Mailjet-Liste.', $travelCode)); if (null !== $this->travelMailingListRepository->findOneByName($name)) {
$this->addFlash('error', sprintf('Die Mailjet-Liste %s gibt es bereits.', $name));
return $this->redirect($returnUrl); 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. // Fetched again rather than posted from the page, so the list holds what BusPro reports now.
try { try {
$recipients = $this->travelsClient->getMailingRecipients($travelCode)->data; $recipients = $this->travelsClient->getMailingRecipients($travelCode, $busLeg)->data;
} catch (BpnConnectException) { } catch (BpnConnectException) {
$this->addFlash('error', 'Die Teilnehmerliste konnte nicht von BusPro abgerufen werden. Bitte später erneut versuchen.'); $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 { try {
$mailingList = $this->travelMailingListManager->create($travelCode, $recipients, $travelStartsAt); $mailingList = $this->travelMailingListManager->create($travelCode, $recipients, $busLeg, $travelStartsAt);
} catch (NewsletterProviderException $exception) { } catch (NewsletterProviderException $exception) {
$this->adminLogger->error('Create travel mailing list failed', [ $this->adminLogger->error('Create travel mailing list failed', [
'travel_code' => $travelCode, 'travel_code' => $travelCode,
@@ -85,9 +95,10 @@ class CreateListController extends AbstractController
$this->adminLogger->info('Create travel mailing list', [ $this->adminLogger->info('Create travel mailing list', [
'travel_code' => $travelCode, 'travel_code' => $travelCode,
'mailjet_list_id' => $mailingList->getMailjetListId(), 'mailjet_list_id' => $mailingList->getMailjetListId(),
'bus_leg' => $busLeg?->value,
'recipients' => count($recipients), '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); return $this->redirect($returnUrl);
} }
@@ -7,6 +7,7 @@ namespace App\Controller\Admin\TravelMailing;
use App\BpnConnect\Exception\BpnConnectException; use App\BpnConnect\Exception\BpnConnectException;
use App\BpnConnect\TravelsClient; use App\BpnConnect\TravelsClient;
use App\BusProNet\Utility\DateCodeUtility; use App\BusProNet\Utility\DateCodeUtility;
use App\Entity\TravelMailingList;
use App\Form\Admin\TravelMailingLookupType; use App\Form\Admin\TravelMailingLookupType;
use App\Form\Model\TravelMailingLookupDto; use App\Form\Model\TravelMailingLookupDto;
use App\Repository\TravelMailingListRepository; use App\Repository\TravelMailingListRepository;
@@ -39,7 +40,9 @@ class IndexController extends AbstractController
$result = null; $result = null;
$travelLabel = null; $travelLabel = null;
$travelStartsAt = null; $travelStartsAt = null;
$mailingLists = [];
$mailingList = null; $mailingList = null;
$listName = null;
$failed = false; $failed = false;
if ($form->isSubmitted() && $form->isValid()) { if ($form->isSubmitted() && $form->isValid()) {
// The local index matches codes exactly, so "lpjmp-281126" has to become "LPJMP281126" // 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); $travelCode = (new DateCodeUtility())->sanitize((string) $lookup->travelCode);
try { try {
$result = $this->travelsClient->getMailingRecipients($travelCode); $result = $this->travelsClient->getMailingRecipients($travelCode, $lookup->getBusLeg());
$travelLabel = $this->travelDataProvider->mapDateCodeToLabel($travelCode); $travelLabel = $this->travelDataProvider->mapDateCodeToLabel($travelCode);
$travelStartsAt = $this->travelDataProvider->mapDateCodeToStartDate($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) { } catch (BpnConnectException) {
// The client has already logged the cause; the page only needs to say it failed. // The client has already logged the cause; the page only needs to say it failed.
$failed = true; $failed = true;
@@ -62,7 +72,9 @@ class IndexController extends AbstractController
'result' => $result, 'result' => $result,
'travel_label' => $travelLabel, 'travel_label' => $travelLabel,
'travel_starts_at' => $travelStartsAt, 'travel_starts_at' => $travelStartsAt,
'mailing_lists' => $mailingLists,
'mailing_list' => $mailingList, 'mailing_list' => $mailingList,
'list_name' => $listName,
'failed' => $failed, 'failed' => $failed,
]); ]);
} }
+34 -2
View File
@@ -4,41 +4,63 @@ declare(strict_types=1);
namespace App\Entity; namespace App\Entity;
use App\Enum\BusLeg;
use App\Repository\TravelMailingListRepository; use App\Repository\TravelMailingListRepository;
use Doctrine\ORM\Mapping as ORM; use Doctrine\ORM\Mapping as ORM;
/** /**
* A Mailjet contact list created for one travel's participants. The row exists so the list can * 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. * 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\Entity(repositoryClass: TravelMailingListRepository::class)]
#[ORM\Index(columns: ['travel_code'])]
class TravelMailingList class TravelMailingList
{ {
private const string BUS_ONLY_SUFFIX = ' nur Bus';
#[ORM\Id] #[ORM\Id]
#[ORM\GeneratedValue] #[ORM\GeneratedValue]
#[ORM\Column(type: 'integer')] #[ORM\Column(type: 'integer')]
private ?int $id = null; private ?int $id = null;
#[ORM\Column(type: 'string', length: 32, unique: true)] #[ORM\Column(type: 'string', length: 32)]
private string $travelCode; private string $travelCode;
/** The Mailjet list name. */
#[ORM\Column(type: 'string', length: 64, unique: true)]
private string $name;
#[ORM\Column(type: 'integer')] #[ORM\Column(type: 'integer')]
private int $mailjetListId; 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')] #[ORM\Column(type: 'datetime_immutable')]
private \DateTimeImmutable $deleteAfter; private \DateTimeImmutable $deleteAfter;
#[ORM\Column(type: 'datetime_immutable')] #[ORM\Column(type: 'datetime_immutable')]
private \DateTimeImmutable $createdAt; 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->travelCode = $travelCode;
$this->name = self::nameFor($travelCode, $busLeg);
$this->mailjetListId = $mailjetListId; $this->mailjetListId = $mailjetListId;
$this->busLeg = $busLeg;
$this->deleteAfter = $deleteAfter; $this->deleteAfter = $deleteAfter;
$this->createdAt = new \DateTimeImmutable(); $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 public function getId(): ?int
{ {
return $this->id; return $this->id;
@@ -49,11 +71,21 @@ class TravelMailingList
return $this->travelCode; return $this->travelCode;
} }
public function getName(): string
{
return $this->name;
}
public function getMailjetListId(): int public function getMailjetListId(): int
{ {
return $this->mailjetListId; return $this->mailjetListId;
} }
public function getBusLeg(): ?BusLeg
{
return $this->busLeg;
}
public function getDeleteAfter(): \DateTimeImmutable public function getDeleteAfter(): \DateTimeImmutable
{ {
return $this->deleteAfter; return $this->deleteAfter;
+27
View File
@@ -0,0 +1,27 @@
<?php
declare(strict_types=1);
namespace App\Enum;
/**
* Which bus leg a travel participant must have booked to be selected, as bpn-connect's travel
* mailing endpoint takes it (`busLeg`).
*/
enum BusLeg: string
{
case Outbound = 'outbound';
case Inbound = 'inbound';
case Any = 'any';
case Both = 'both';
public function label(): string
{
return match ($this) {
self::Outbound => 'Busreisende (Hinfahrt)',
self::Inbound => 'Busreisende (Rückfahrt)',
self::Any => 'Busreisende (Hin- oder Rückfahrt)',
self::Both => 'Busreisende (Hin- und Rückfahrt)',
};
}
}
+6 -1
View File
@@ -6,13 +6,14 @@ namespace App\Form\Admin;
use App\Form\Model\TravelMailingLookupDto; use App\Form\Model\TravelMailingLookupDto;
use Symfony\Component\Form\AbstractType; 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\Extension\Core\Type\TextType;
use Symfony\Component\Form\FormBuilderInterface; use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\OptionsResolver\OptionsResolver; use Symfony\Component\OptionsResolver\OptionsResolver;
/** /**
* Submits as GET with an empty block prefix, so a lookup is the plain URL `?travelCode=…` * 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<TravelMailingLookupDto> * @extends AbstractType<TravelMailingLookupDto>
*/ */
@@ -26,6 +27,10 @@ class TravelMailingLookupType extends AbstractType
'placeholder' => 'z. B. LPJMP281126', 'placeholder' => 'z. B. LPJMP281126',
], ],
]); ]);
$builder->add('busOnly', CheckboxType::class, [
'label' => 'nur '.TravelMailingLookupDto::BUS_LEG->label(),
'required' => false,
]);
} }
public function configureOptions(OptionsResolver $resolver): void public function configureOptions(OptionsResolver $resolver): void
+13
View File
@@ -4,10 +4,23 @@ declare(strict_types=1);
namespace App\Form\Model; namespace App\Form\Model;
use App\Enum\BusLeg;
use Symfony\Component\Validator\Constraints as Assert; use Symfony\Component\Validator\Constraints as Assert;
class TravelMailingLookupDto 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.')] #[Assert\NotBlank(message: 'Bitte einen Reisecode angeben.')]
public ?string $travelCode = null; public ?string $travelCode = null;
public bool $busOnly = false;
public function getBusLeg(): ?BusLeg
{
return $this->busOnly ? self::BUS_LEG : null;
}
} }
+10 -2
View File
@@ -18,9 +18,17 @@ class TravelMailingListRepository extends ServiceEntityRepository
parent::__construct($registry, TravelMailingList::class); 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<TravelMailingList>
*/
public function findByTravelCode(string $travelCode): array
{
return $this->findBy(['travelCode' => $travelCode], ['id' => 'ASC']);
} }
/** /**
+42 -19
View File
@@ -6,14 +6,16 @@ namespace App\Service;
use App\BpnConnect\Model\TravelMailingRecipient; use App\BpnConnect\Model\TravelMailingRecipient;
use App\Entity\TravelMailingList; use App\Entity\TravelMailingList;
use App\Enum\BusLeg;
use App\Exception\NewsletterProviderException; use App\Exception\NewsletterProviderException;
use App\Mailjet\ApiClient; use App\Mailjet\ApiClient;
use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityManagerInterface;
use Psr\Log\LoggerInterface; use Psr\Log\LoggerInterface;
/** /**
* Creates one Mailjet contact list per travel and deletes it again 30 days after the travel * Creates Mailjet contact lists for a travel (all participants, bus passengers only) and deletes
* started, together with every contact that is on no other Mailjet list. * them again 30 days after the travel started, together with every contact that is on no other
* Mailjet list.
*/ */
class TravelMailingListManager class TravelMailingListManager
{ {
@@ -28,16 +30,17 @@ class TravelMailingListManager
/** /**
* @param TravelMailingRecipient[] $recipients * @param TravelMailingRecipient[] $recipients
* @param ?BusLeg $busLeg the bus leg the recipients were filtered by, if any
* *
* @throws NewsletterProviderException * @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 // Tracked before the contacts go in, so a list Mailjet already holds is never left without a row
// that the cleanup would find. // 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->persist($mailingList);
$this->entityManager->flush(); $this->entityManager->flush();
@@ -51,8 +54,10 @@ class TravelMailingListManager
$this->logger->info('Created Mailjet travel mailing list', [ $this->logger->info('Created Mailjet travel mailing list', [
'travel_code' => $travelCode, 'travel_code' => $travelCode,
'name' => $mailingList->getName(),
'mailjet_list_id' => $listId, 'mailjet_list_id' => $listId,
'mailjet_job_id' => $jobId, 'mailjet_job_id' => $jobId,
'bus_leg' => $busLeg?->value,
'recipients' => count($recipients), '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 * Deletes all lists of one travel together. Taken one at a time, a contact on both would
* repeated: the list still names whoever is left. * 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 * @return int the number of contacts deleted
* *
* @throws NewsletterProviderException * @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; $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 // Anyone who is, or recently was, on another list is kept: their newsletter consent or
// another travel's mailing still needs the contact. // 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; continue;
} }
@@ -83,17 +98,24 @@ class TravelMailingListManager
++$deletedContacts; ++$deletedContacts;
} }
$this->mailjet->deleteContactsList($listId); $this->logger->info('Deleted orphaned travel mailing contacts', [
'mailjet_list_ids' => $listIds,
$this->entityManager->remove($mailingList);
$this->entityManager->flush();
$this->logger->info('Deleted Mailjet travel mailing list', [
'travel_code' => $mailingList->getTravelCode(),
'mailjet_list_id' => $listId,
'deleted_contacts' => $deletedContacts, '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; return $deletedContacts;
} }
@@ -129,6 +151,7 @@ class TravelMailingListManager
// The row stays, so the scheduled cleanup removes the list later. // 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', [ $this->logger->error('Could not delete Mailjet travel mailing list after a failed import', [
'travel_code' => $mailingList->getTravelCode(), 'travel_code' => $mailingList->getTravelCode(),
'name' => $mailingList->getName(),
'mailjet_list_id' => $mailingList->getMailjetListId(), 'mailjet_list_id' => $mailingList->getMailjetListId(),
'error' => $exception->getMessage(), 'error' => $exception->getMessage(),
]); ]);
+18 -8
View File
@@ -9,6 +9,9 @@
<div class="w-64"> <div class="w-64">
{{ form_row(form.travelCode) }} {{ form_row(form.travelCode) }}
</div> </div>
<div>
{{ form_row(form.busOnly) }}
</div>
<button type="submit" <button type="submit"
class="button button--primary button--small"> class="button button--primary button--small">
abrufen abrufen
@@ -26,7 +29,7 @@
{% if result.data is empty %} {% if result.data is empty %}
{% include '_partials/_alert.html.twig' with { {% include '_partials/_alert.html.twig' with {
level: 'warning', level: 'warning',
messages: ['Keine Empfänger für <code>%s</code> gefunden: Reisecode unbekannt oder keine bestätigten Teilnehmer.'|format(result.travelCode|e)], messages: ['Keine Empfänger für <code>%s</code> gefunden: Reisecode unbekannt oder keine bestätigten Teilnehmer%s.'|format(result.travelCode|e, result.busLeg ? ' unter den ' ~ result.busLeg.label : '')],
} %} } %}
{% else %} {% else %}
<div class="pb-4"> <div class="pb-4">
@@ -34,26 +37,33 @@
{{ travel_label|default('Reise nicht im lokalen Reisebestand') }} {{ travel_label|default('Reise nicht im lokalen Reisebestand') }}
</h2> </h2>
<p class="text-sm text-gray-700"> <p class="text-sm text-gray-700">
{{ result.data|length }} Empfänger für <code>{{ result.travelCode }}</code> {{ result.data|length }} Empfänger für <code>{{ result.travelCode }}</code>{% if result.busLeg %}, nur {{ result.busLeg.label }}{% endif %}
</p> </p>
</div> </div>
<div class="pb-6 text-sm"> <div class="pb-6 text-sm">
{% if mailing_list is not null %} {% for list in mailing_lists %}
<p> <p>
Mailjet-Liste <code>{{ mailing_list.travelCode }}</code> (ID {{ mailing_list.mailjetListId }}) Mailjet-Liste <code>{{ list.name }}</code> (ID {{ list.mailjetListId }},
am {{ mailing_list.createdAt|date('d.m.Y, H:i') }} angelegt, {{ list.busLeg ? 'nur ' ~ list.busLeg.label : 'alle Teilnehmer' }})
wird ab {{ mailing_list.deleteAfter|date('d.m.Y') }} automatisch gelöscht. am {{ list.createdAt|date('d.m.Y, H:i') }} angelegt,
wird ab {{ list.deleteAfter|date('d.m.Y') }} automatisch gelöscht.
</p> </p>
{% endfor %}
{% if mailing_list is not null %}
{# the list for this filter exists and is listed above #}
{% elseif travel_starts_at is null %} {% elseif travel_starts_at is null %}
<p class="text-gray-700"> <p class="text-gray-700">
Kein Reisebeginn bekannt, daher kann keine Mailjet-Liste angelegt werden. Kein Reisebeginn bekannt, daher kann keine Mailjet-Liste angelegt werden.
</p> </p>
{% else %} {% else %}
<form method="post" action="{{ path('app_admin_travelmailing_createlist') }}"> <form method="post" action="{{ path('app_admin_travelmailing_createlist') }}"{% if mailing_lists is not empty %} class="pt-2"{% endif %}>
<input type="hidden" name="travelCode" value="{{ result.travelCode }}"> <input type="hidden" name="travelCode" value="{{ result.travelCode }}">
{% if result.busLeg %}
<input type="hidden" name="busOnly" value="1">
{% endif %}
<input type="hidden" name="_token" value="{{ csrf_token(constant('App\\Controller\\Admin\\TravelMailing\\CreateListController::CSRF_TOKEN_ID')) }}"> <input type="hidden" name="_token" value="{{ csrf_token(constant('App\\Controller\\Admin\\TravelMailing\\CreateListController::CSRF_TOKEN_ID')) }}">
<button type="submit" class="button button--primary button--small"> <button type="submit" class="button button--primary button--small">
Mailjet-Liste anlegen Mailjet-Liste {{ list_name }} anlegen
</button> </button>
<span class="pl-2 text-gray-700"> <span class="pl-2 text-gray-700">
Alle Empfänger werden ohne Opt-in eingetragen; die Liste wird 30 Tage nach Reisebeginn Alle Empfänger werden ohne Opt-in eingetragen; die Liste wird 30 Tage nach Reisebeginn
+38
View File
@@ -7,6 +7,7 @@ namespace App\Tests\BpnConnect;
use App\BpnConnect\Exception\BpnConnectException; use App\BpnConnect\Exception\BpnConnectException;
use App\BpnConnect\Model\TravelMailingRecipient; use App\BpnConnect\Model\TravelMailingRecipient;
use App\BpnConnect\TravelsClient; use App\BpnConnect\TravelsClient;
use App\Enum\BusLeg;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
use Psr\Log\NullLogger; use Psr\Log\NullLogger;
use Symfony\Component\HttpClient\MockHttpClient; 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::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::assertContains('X-API-KEY: secret', $requests[0][2]['headers']);
self::assertSame('LPJMP281126', $response->travelCode); self::assertSame('LPJMP281126', $response->travelCode);
self::assertNull($response->busLeg);
self::assertEquals([ self::assertEquals([
new TravelMailingRecipient('[email protected]', 'Erika', 'Muster'), new TravelMailingRecipient('[email protected]', 'Erika', 'Muster'),
new TravelMailingRecipient('[email protected]', 'Max', 'Muster'), new TravelMailingRecipient('[email protected]', 'Max', 'Muster'),
], $response->data); ], $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' => '[email protected]', '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' => '[email protected]', 'first_name' => 'Erika', 'last_name' => 'Muster'],
],
]));
$this->expectException(BpnConnectException::class);
$this->client($http)->getMailingRecipients('LPJMP281126', BusLeg::Outbound);
}
public function testAnUnknownTravelCodeYieldsAnEmptyList(): void public function testAnUnknownTravelCodeYieldsAnEmptyList(): void
{ {
$http = new MockHttpClient($this->jsonResponse([ $http = new MockHttpClient($this->jsonResponse([
+34 -5
View File
@@ -6,6 +6,7 @@ namespace App\Tests\Service;
use App\BpnConnect\Model\TravelMailingRecipient; use App\BpnConnect\Model\TravelMailingRecipient;
use App\Entity\TravelMailingList; use App\Entity\TravelMailingList;
use App\Enum\BusLeg;
use App\Exception\NewsletterProviderException; use App\Exception\NewsletterProviderException;
use App\Mailjet\ApiClient; use App\Mailjet\ApiClient;
use App\Service\TravelMailingListManager; use App\Service\TravelMailingListManager;
@@ -18,7 +19,7 @@ class TravelMailingListManagerTest extends TestCase
public function testCreateTracksTheListAndAddsDeduplicatedContacts(): void public function testCreateTracksTheListAndAddsDeduplicatedContacts(): void
{ {
$mailjet = $this->createMock(ApiClient::class); $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, [ $mailjet->expects(self::once())->method('addContactsToList')->with(42, [
['email' => '[email protected]', 'firstName' => 'Mia'], ['email' => '[email protected]', 'firstName' => 'Mia'],
['email' => '[email protected]', 'firstName' => null], ['email' => '[email protected]', 'firstName' => null],
@@ -32,9 +33,11 @@ class TravelMailingListManagerTest extends TestCase
new TravelMailingRecipient('[email protected]', 'Mia', 'Muster'), new TravelMailingRecipient('[email protected]', 'Mia', 'Muster'),
new TravelMailingRecipient('[email protected]', 'Lea', 'Muster'), new TravelMailingRecipient('[email protected]', 'Lea', 'Muster'),
new TravelMailingRecipient('[email protected]', '', 'Test'), new TravelMailingRecipient('[email protected]', '', 'Test'),
], new \DateTimeImmutable('2026-11-28')); ], BusLeg::Outbound, new \DateTimeImmutable('2026-11-28'));
self::assertSame(42, $mailingList->getMailjetListId()); 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()); 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 TravelMailingListManager($mailjet, $entityManager, new NullLogger()))->create('LPJMP281126', [
new TravelMailingRecipient('[email protected]', 'Mia', 'Muster'), new TravelMailingRecipient('[email protected]', 'Mia', 'Muster'),
], new \DateTimeImmutable('2026-11-28')); ], null, new \DateTimeImmutable('2026-11-28'));
} }
public function testDeleteRemovesOnlyContactsOnNoOtherList(): void public function testDeleteRemovesOnlyContactsOnNoOtherList(): void
@@ -70,7 +73,33 @@ class TravelMailingListManagerTest extends TestCase
$entityManager->expects(self::once())->method('remove'); $entityManager->expects(self::once())->method('remove');
$deleted = (new TravelMailingListManager($mailjet, $entityManager, new NullLogger())) $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); self::assertSame(1, $deleted);
} }
@@ -87,6 +116,6 @@ class TravelMailingListManagerTest extends TestCase
$this->expectException(NewsletterProviderException::class); $this->expectException(NewsletterProviderException::class);
(new TravelMailingListManager($mailjet, $entityManager, new NullLogger())) (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'))]);
} }
} }